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

eb --try-toolchain doesn't always work with files_to_copy parameter #3049

Closed
ekouts opened this issue Oct 9, 2019 · 3 comments
Closed

eb --try-toolchain doesn't always work with files_to_copy parameter #3049

ekouts opened this issue Oct 9, 2019 · 3 comments
Milestone

Comments

@ekouts
Copy link
Contributor

ekouts commented Oct 9, 2019

eb -f easyconfigs/a/Amber/Amber-18-14-14-CrayGNU-19.06.eb -Dr --try-toolchain=CrayGNU,19.08 gives the error
ERROR: Failed to process easyconfig /run/user/25948/easybuild/tmp/eb-mCVQrJ/tweaked_easyconfigs/Amber-18-14-14-CrayGNU-19.08.eb: mandatory parameters not provided in /run/user/25948/easybuild/tmp/eb-mCVQrJ/tweaked_easyconfigs/Amber-18-14-14-CrayGNU-19.08.eb: ['files_to_copy'] although in the original ec the parameter is there.
Original easyconfig file is https://github.com/eth-cscs/production/blob/master/easybuild/easyconfigs/a/Amber/Amber-18-14-14-CrayGNU-19.06.eb

@boegel boegel changed the title eb try-toolchain doesn't always work with copy_to_files parameter eb --try-toolchain doesn't always work with files_to_copy parameter Nov 13, 2019
@boegel boegel modified the milestones: 4.x, next release (4.1.0) Nov 13, 2019
@boegel
Copy link
Member

boegel commented Nov 13, 2019

Sorry for not picking up on this earlier @ekouts, the timing was a bit unfortunate for me...

It took a quick look at this, and the problem is pretty obvious (to me at least).

In the easyconfig file you mentioned, you were basically setting files_to_copy to its default value:

files_to_copy = []

This passes the check for mandatory easyconfig files (which totally ignores the value being used).

But when the easyconfig file gets dumped again after tweaking the toolchain (due to --try-toolchain being used), it strips out the files_to_copy = [] because it's deemed meaningless since it's set to the same value as the default value.

That's a bug in the easyconfig dumping logic in framework, it should correctly handle mandatory easyconfig parameters (and ignore the value), which we'll look into fixing that.

For now, you can easily work around this by changing the default value for files_to_copy from [] to None (and then take that into account where the files_to_copy value is picked up), by tweaking the MakeCp easyblock as follows:

diff --git a/easybuild/easyblocks/generic/makecp.py b/easybuild/easyblocks/generic/makecp.py
index e5d83a17e..02af510ad 100644
--- a/easybuild/easyblocks/generic/makecp.py
+++ b/easybuild/easyblocks/generic/makecp.py
@@ -47,7 +47,7 @@ class MakeCp(ConfigureMake):
         Define list of files or directories to be copied after make
         """
         extra = {
-            'files_to_copy': [[], "List of files or dirs to copy", MANDATORY],
+            'files_to_copy': [None, "List of files or dirs to copy", MANDATORY],
             'with_configure': [False, "Run configure script before building", BUILD],
         }
         if extra_vars is None:
@@ -68,7 +68,7 @@ class MakeCp(ConfigureMake):
             # make sure we're (still) in the start dir
             os.chdir(self.cfg['start_dir'])
 
-            files_to_copy = self.cfg.get('files_to_copy', [])
+            files_to_copy = self.cfg.get('files_to_copy') or []
             self.log.debug("Starting install_step with files_to_copy: %s" % files_to_copy)
             for fil in files_to_copy:
                 if isinstance(fil, tuple):

@boegel
Copy link
Member

boegel commented Nov 13, 2019

Fix & cleanup in MakeCp easyblock done in easybuilders/easybuild-easyblocks#1848

General fix in framework to always dump mandatory easyconfig parameters, regardless of which value is used: #3081

@boegel
Copy link
Member

boegel commented Nov 14, 2019

Fixed now both PRs are merged, so closing... Fixes will be included in next EasyBuild release (v4.1.0)

@boegel boegel closed this as completed Nov 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants