Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

raise error when unknown 'install_type' is used in Tarball easyblock + fix log messages #2049

Merged
merged 2 commits into from
Apr 30, 2020

Conversation

boegel
Copy link
Member

@boegel boegel commented Apr 30, 2020

@lexming follow-up for #1989

  • we shouldn't silently ignore the use of an unknown 'install_type' value, it should raise an error instead

  • order of name/dir in log message is fixed, was:

    == 2020-04-30 10:26:00,206 tarball.py:114 INFO Wiping KNIME and copying tarball contents of /work/maintainers/CO7/broadwell/software/KNIME/3.6.2 into it...
    

    now:

    == 2020-04-30 10:28:36,328 tarball.py:111 INFO Copying tarball contents of KNIME into /work/maintainers/CO7/broadwell/software/KNIME/3.6.2 after wiping it...
    

@boegel boegel added this to the next release (4.2.1?) milestone Apr 30, 2020
@boegel boegel requested a review from lexming April 30, 2020 08:31
@lexming
Copy link
Contributor

lexming commented Apr 30, 2020

Testing with easybuilders/easybuild-framework#3310

Successful test using tarball and install_type=subdir with easyconfig https://gist.github.com/lexming/61ea21f0add35b4d15c00e0deba34311

Log of the build: https://gist.github.com/lexming/8d2037338c4b365dcbae58b83c3484de

@lexming
Copy link
Contributor

lexming commented Apr 30, 2020

Testing with easybuilders/easybuild-framework#3310

FAILED test using tarball and install_type=merge with easyconfig https://gist.github.com/lexming/dd7a20aaa63ab24751d0f83c770d0559

Log of the build: https://gist.github.com/lexming/156e9ab54a373612a5142d49c66f1480

The reason for the failure is that there is a broken symlink at %(builddir)s/Alpha/20190607/foss-2019b-Python-2.7.16/thekswenson-alpha-3139d8186403/pyinclude
This broken symlink is not an issue with current implementation in develop.

@boegel
Copy link
Member Author

boegel commented Apr 30, 2020

@lexming That use case should be fixed now in easybuilders/easybuild-framework#3310 (also covered by the enhanced tests)

@lexming
Copy link
Contributor

lexming commented Apr 30, 2020

Testing with easybuilders/easybuild-framework#3310 (easybuilders/easybuild-framework@3c80fef)

SUCCESSFUL test with install_type=subdir using easyconfig https://gist.github.com/lexming/61ea21f0add35b4d15c00e0deba34311
Log: https://gist.github.com/lexming/ff33bb00a043728bc94385967d4a86b8
Result: each component is installed in its own subfolder

SUCCESSFUL test with install_type=merge using easyconfig https://gist.github.com/lexming/1d6a1eed0de99a77d8aecd0ad6aa8e92
Log: https://gist.github.com/lexming/e688fd147cdceded708c7881f9ba51d1
Result: the contents of all three components are properly merge in the installation directory

SUCCESSFUL test with install_type=potato using easyconfig https://gist.github.com/lexming/6fe5df2dcf43db787939c945a4874369
Log: https://gist.github.com/lexming/0e3361a0301a0d55a88140df35f81e51
Result: the build is stopped with an error message due to the unsupported install_type

SUCCESSFUL test without install_type using easyconfig https://gist.github.com/lexming/c646484ee0f3c858011bbdd1d9695d50
Log: https://gist.github.com/lexming/60b1f9d5d92e416319cf319c8ce66e31
Result: the installation directory only has the contents of the last component.
This test covers all existing easyconfig as this is the default behaviour of tarball.

Copy link
Contributor

@lexming lexming left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I would just remove the Ignoring part from the error message as it is no longer the case.

easybuild/easyblocks/generic/tarball.py Outdated Show resolved Hide resolved
Co-authored-by: Alex Domingo <alex.domingo.toro@vub.be>
Copy link
Contributor

@lexming lexming left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lexming
Copy link
Contributor

lexming commented Apr 30, 2020

Going in, thanks for the fixes @boegel !

@lexming lexming merged commit 32a5d0e into easybuilders:develop Apr 30, 2020
@boegel boegel deleted the tarball_strict_install_type branch April 30, 2020 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants