From 4502912805fbed02f7f7d08c8310c71bc9c6bc14 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 17 Apr 2020 14:19:03 +0200 Subject: [PATCH 1/7] Error on failure to resolve a template value A failure in template resolving is currently hidden in the log although it is likely due to a bug in the code (either EasyBuild, EasyBlock or EasyConfig) This may lead to cryptic error messages later on or misbehave silently which is even worse. --- easybuild/framework/easyconfig/easyconfig.py | 9 +++++++-- easybuild/tools/config.py | 1 + easybuild/tools/options.py | 2 ++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/easybuild/framework/easyconfig/easyconfig.py b/easybuild/framework/easyconfig/easyconfig.py index 3f6181df18..929731870b 100644 --- a/easybuild/framework/easyconfig/easyconfig.py +++ b/easybuild/framework/easyconfig/easyconfig.py @@ -2063,8 +2063,13 @@ def resolve_template(value, tmpl_dict): _log.deprecated(f"Easyconfig template '{old_tmpl}' is deprecated, use '{new_tmpl}' instead", ver) except KeyError: - _log.warning(f"Unable to resolve template value {value} with dict {tmpl_dict}") - value = raw_value # Undo "%"-escaping + msg = ('Failed to resolve template value %s with dict %s. ' % (value, tmpl_dict) + + 'This might cause failures or unexpected behavior, check for correct escaping if this is intended!') + if build_option('allow_unresolved_templates'): + value = raw_value # Undo "%"-escaping + print_warning(msg) + else: + raise EasyBuildError(msg) for key in tmpl_dict: if key in DEPRECATED_EASYCONFIG_TEMPLATES: diff --git a/easybuild/tools/config.py b/easybuild/tools/config.py index e02d9b923b..023f63dd4c 100644 --- a/easybuild/tools/config.py +++ b/easybuild/tools/config.py @@ -206,6 +206,7 @@ def mk_full_default_path(name, prefix=DEFAULT_PREFIX): # build options that have a perfectly matching command line option, listed by default value BUILD_OPTIONS_CMDLINE = { None: [ + 'allow_unresolved_templates', 'aggregate_regtest', 'backup_modules', 'banned_linked_shared_libs', diff --git a/easybuild/tools/options.py b/easybuild/tools/options.py index 154ba74ebd..c40c3ddb6b 100644 --- a/easybuild/tools/options.py +++ b/easybuild/tools/options.py @@ -363,6 +363,8 @@ def override_options(self): None, 'store_true', False), 'allow-loaded-modules': ("List of software names for which to allow loaded modules in initial environment", 'strlist', 'store', DEFAULT_ALLOW_LOADED_MODULES), + 'allow-unresolved-templates': ("Don't error out when templates such as %(name)s in EasyConfigs " + "could not be resolved", None, 'store_true', False), 'allow-modules-tool-mismatch': ("Allow mismatch of modules tool and definition of 'module' function", None, 'store_true', False), 'allow-use-as-root-and-accept-consequences': ("Allow using of EasyBuild as root (NOT RECOMMENDED!)", From d58c3e8301c0807f5b6704ba518df19ddaa07035 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 17 Apr 2020 16:13:05 +0200 Subject: [PATCH 2/7] Avoid uneccessary template resolving If we only want the `len` of some list we don't need to resolve the templates. We cannot resolve templates in collect_exts_file_info like `installdir` present in e.g. `installcmd` of the extension options. --- easybuild/framework/easyblock.py | 20 +++++++++----------- easybuild/framework/easyconfig/easyconfig.py | 20 ++++++++++++++------ test/framework/easyblock.py | 2 +- 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index b6baf5d55e..831d02cf99 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -597,19 +597,16 @@ def collect_exts_file_info(self, fetch_files=True, verify_checksums=True): template_values = copy.deepcopy(self.cfg.template_values) template_values.update(template_constant_dict(ext_src)) - # resolve templates in extension options - ext_options = resolve_template(ext_options, template_values) - - source_urls = ext_options.get('source_urls', []) + source_urls = resolve_template(ext_options.get('source_urls', []), template_values) checksums = ext_options.get('checksums', []) - download_instructions = ext_options.get('download_instructions') + download_instructions = resolve_template(ext_options.get('download_instructions'), template_values) if ext_options.get('nosource', None): self.log.debug("No sources for extension %s, as indicated by 'nosource'", ext_name) elif ext_options.get('sources', None): - sources = ext_options['sources'] + sources = resolve_template(ext_options['sources'], template_values) # only a single source file is supported for extensions currently, # see https://github.com/easybuilders/easybuild-framework/issues/3463 @@ -646,17 +643,18 @@ def collect_exts_file_info(self, fetch_files=True, verify_checksums=True): }) else: - # use default template for name of source file if none is specified - default_source_tmpl = resolve_template('%(name)s-%(version)s.tar.gz', template_values) # if no sources are specified via 'sources', fall back to 'source_tmpl' src_fn = ext_options.get('source_tmpl') if src_fn is None: - src_fn = default_source_tmpl + # use default template for name of source file if none is specified + src_fn = '%(name)s-%(version)s.tar.gz' elif not isinstance(src_fn, str): error_msg = "source_tmpl value must be a string! (found value of type '%s'): %s" raise EasyBuildError(error_msg, type(src_fn).__name__, src_fn) + src_fn = resolve_template(src_fn, template_values) + if fetch_files: src_path = self.obtain_file(src_fn, extension=True, urls=source_urls, force_download=force_download, @@ -691,7 +689,7 @@ def collect_exts_file_info(self, fetch_files=True, verify_checksums=True): ) # locate extension patches (if any), and verify checksums - ext_patches = ext_options.get('patches', []) + ext_patches = resolve_template(ext_options.get('patches', []), template_values) if fetch_files: ext_patches = self.fetch_patches(patch_specs=ext_patches, extension=True) else: @@ -2975,7 +2973,7 @@ def extensions_step(self, fetch=False, install=True): fake_mod_data = self.load_fake_module(purge=True, extra_modules=build_dep_mods) - start_progress_bar(PROGRESS_BAR_EXTENSIONS, len(self.cfg['exts_list'])) + start_progress_bar(PROGRESS_BAR_EXTENSIONS, len(self.cfg.get_ref('exts_list'))) self.prepare_for_extensions() diff --git a/easybuild/framework/easyconfig/easyconfig.py b/easybuild/framework/easyconfig/easyconfig.py index 929731870b..e1c7a14b79 100644 --- a/easybuild/framework/easyconfig/easyconfig.py +++ b/easybuild/framework/easyconfig/easyconfig.py @@ -765,9 +765,13 @@ def count_files(self): """ Determine number of files (sources + patches) required for this easyconfig. """ - cnt = len(self['sources']) + len(self['patches']) - for ext in self['exts_list']: + # No need to resolve templates as we only need a count not the names + with self.disable_templating(): + cnt = len(self['sources']) + len(self['patches']) + exts = self['exts_list'] + + for ext in exts: if isinstance(ext, tuple) and len(ext) >= 3: ext_opts = ext[2] # check for 'sources' first, since that's also considered first by EasyBlock.collect_exts_file_info @@ -1826,11 +1830,14 @@ def get(self, key, default=None, resolve=True): # see also https://docs.python.org/2/reference/datamodel.html#object.__eq__ def __eq__(self, ec): """Is this EasyConfig instance equivalent to the provided one?""" - return self.asdict() == ec.asdict() + # Compare raw values to check that templates used are the same + with self.disable_templating(): + with ec.disable_templating(): + return self.asdict() == ec.asdict() def __ne__(self, ec): """Is this EasyConfig instance equivalent to the provided one?""" - return self.asdict() != ec.asdict() + return not self == ec def __hash__(self): """Return hash value for a hashable representation of this EasyConfig instance.""" @@ -1843,8 +1850,9 @@ def make_hashable(val): return val lst = [] - for (key, val) in sorted(self.asdict().items()): - lst.append((key, make_hashable(val))) + with self.disable_templating(): + for (key, val) in sorted(self.asdict().items()): + lst.append((key, make_hashable(val))) # a list is not hashable, but a tuple is return hash(tuple(lst)) diff --git a/test/framework/easyblock.py b/test/framework/easyblock.py index 2eb654ecae..c9931ad94f 100644 --- a/test/framework/easyblock.py +++ b/test/framework/easyblock.py @@ -1169,7 +1169,7 @@ def test_extension_source_tmpl(self): eb = EasyBlock(EasyConfig(self.eb_file)) error_pattern = r"source_tmpl value must be a string! " - error_pattern += r"\(found value of type 'list'\): \['bar-0\.0\.tar\.gz'\]" + error_pattern += r"\(found value of type 'list'\): \['%\(name\)s-%\(version\)s.tar.gz'\]" self.assertErrorRegex(EasyBuildError, error_pattern, eb.fetch_step) self.contents = self.contents.replace("'source_tmpl': [SOURCE_TAR_GZ]", "'source_tmpl': SOURCE_TAR_GZ") From 9fe0b35d001ce441fe410ec0784a35104db1f38e Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Tue, 5 Jul 2022 10:36:07 +0200 Subject: [PATCH 3/7] Allow unresolved templates in `_finalize_dependencies`, extension options & `asdict` Some dependencies may use templates referring to other dependencies which cannot yet be resolved. Hence add an opt-out to the enforcment of resolved templates via `expect_resolved=False` and use that. This allows use of `%(installdir)s` or `%(ext_name)s` and similar in the options. --- easybuild/framework/easyconfig/easyconfig.py | 34 ++++++++++++-------- easybuild/framework/extension.py | 5 ++- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/easybuild/framework/easyconfig/easyconfig.py b/easybuild/framework/easyconfig/easyconfig.py index e1c7a14b79..4916d776ff 100644 --- a/easybuild/framework/easyconfig/easyconfig.py +++ b/easybuild/framework/easyconfig/easyconfig.py @@ -1657,8 +1657,9 @@ def _finalize_dependencies(self): filter_deps_specs = self.parse_filter_deps() for key in DEPENDENCY_PARAMETERS: - # loop over a *copy* of dependency dicts (with resolved templates); - deps = self[key] + # loop over a *copy* of dependency dicts with resolved templates, + # although some templates may not resolve yet (e.g. those relying on dependencies like %(pyver)s) + deps = resolve_template(self.get_ref(key), self.template_values, expect_resolved=False) # to update the original dep dict, we need to get a reference with templating disabled... deps_ref = self.get_ref(key) @@ -1867,7 +1868,8 @@ def asdict(self): if self.enable_templating: if not self.template_values: self.generate_template_values() - value = resolve_template(value, self.template_values) + # Not all values can be resolved, e.g. %(installdir)s + value = resolve_template(value, self.template_values, expect_resolved=False) res[key] = value return res @@ -2015,10 +2017,11 @@ def get_module_path(name, generic=None, decode=True): return '.'.join(modpath + [module_name]) -def resolve_template(value, tmpl_dict): +def resolve_template(value, tmpl_dict, expect_resolved=True): """Given a value, try to susbstitute the templated strings with actual values. - value: some python object (supported are string, tuple/list, dict or some mix thereof) - tmpl_dict: template dictionary + - expect_resolved: Expects that all templates get resolved """ if isinstance(value, str): # simple escaping, making all '%foo', '%%foo', '%%%foo' post-templates values available, @@ -2071,13 +2074,15 @@ def resolve_template(value, tmpl_dict): _log.deprecated(f"Easyconfig template '{old_tmpl}' is deprecated, use '{new_tmpl}' instead", ver) except KeyError: - msg = ('Failed to resolve template value %s with dict %s. ' % (value, tmpl_dict) + - 'This might cause failures or unexpected behavior, check for correct escaping if this is intended!') - if build_option('allow_unresolved_templates'): - value = raw_value # Undo "%"-escaping - print_warning(msg) - else: - raise EasyBuildError(msg) + if expect_resolved: + msg = ('Failed to resolve template value %s with dict %s. ' % (value, tmpl_dict) + + 'This might cause failures or unexpected behavior, ' + + 'check for correct escaping if this is intended!') + if build_option('allow_unresolved_templates'): + print_warning(msg) + else: + raise EasyBuildError(msg) + value = raw_value # Undo "%"-escaping for key in tmpl_dict: if key in DEPRECATED_EASYCONFIG_TEMPLATES: @@ -2097,11 +2102,12 @@ def resolve_template(value, tmpl_dict): # self._config['x']['y'] = z # it can not be intercepted with __setitem__ because the set is done at a deeper level if isinstance(value, list): - value = [resolve_template(val, tmpl_dict) for val in value] + value = [resolve_template(val, tmpl_dict, expect_resolved) for val in value] elif isinstance(value, tuple): - value = tuple(resolve_template(list(value), tmpl_dict)) + value = tuple(resolve_template(list(value), tmpl_dict, expect_resolved)) elif isinstance(value, dict): - value = {resolve_template(k, tmpl_dict): resolve_template(v, tmpl_dict) for k, v in value.items()} + value = {resolve_template(k, tmpl_dict, expect_resolved): resolve_template(v, tmpl_dict, expect_resolved) + for k, v in value.items()} return value diff --git a/easybuild/framework/extension.py b/easybuild/framework/extension.py index 0005f24428..0ff0bf59fb 100644 --- a/easybuild/framework/extension.py +++ b/easybuild/framework/extension.py @@ -128,7 +128,10 @@ def __init__(self, mself, ext, extra_params=None): self.src = resolve_template(self.ext.get('src', []), self.cfg.template_values) self.src_extract_cmd = self.ext.get('extract_cmd', None) self.patches = resolve_template(self.ext.get('patches', []), self.cfg.template_values) - self.options = resolve_template(copy.deepcopy(self.ext.get('options', {})), self.cfg.template_values) + # Some options may not be resolvable yet + self.options = resolve_template(copy.deepcopy(self.ext.get('options', {})), + self.cfg.template_values, + expect_resolved=False) if extra_params: self.cfg.extend_params(extra_params, overwrite=False) From e9ce14b1907703672325d4c3772fcf7d581c83b0 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Thu, 6 Jun 2024 16:36:16 +0200 Subject: [PATCH 4/7] Fix test --- test/framework/easyconfig.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/framework/easyconfig.py b/test/framework/easyconfig.py index 1e630a5627..d5cb550b0c 100644 --- a/test/framework/easyconfig.py +++ b/test/framework/easyconfig.py @@ -1911,8 +1911,9 @@ def test_alternative_easyconfig_parameters(self): self.assertEqual(ec['env_mod_class'], expected) expected = ['echo TOY > %(installdir)s/README'] - self.assertEqual(ec['postinstallcmds'], expected) - self.assertEqual(ec['post_install_cmds'], expected) + with ec.disable_templating(): + self.assertEqual(ec['postinstallcmds'], expected) + self.assertEqual(ec['post_install_cmds'], expected) # test setting of easyconfig parameter with original & alternative name ec['moduleclass'] = 'test1' @@ -3865,7 +3866,7 @@ def test_resolve_template(self): # On unknown values the value is returned unchanged for value in ('%(invalid)s', '%(name)s %(invalid)s', '%%%(invalid)s', '% %(invalid)s', '%s %(invalid)s'): - self.assertEqual(resolve_template(value, tmpl_dict), value) + self.assertEqual(resolve_template(value, tmpl_dict, expect_resolved=False), value) def test_det_subtoolchain_version(self): """Test det_subtoolchain_version function""" From 53b4912ca4c6d9028091565997b96bfc3b54ef28 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Wed, 18 Dec 2024 09:57:15 +0100 Subject: [PATCH 5/7] move allow_unresolved_templates build options to block with options that use False as default value --- easybuild/tools/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/easybuild/tools/config.py b/easybuild/tools/config.py index 023f63dd4c..b633573a32 100644 --- a/easybuild/tools/config.py +++ b/easybuild/tools/config.py @@ -206,7 +206,6 @@ def mk_full_default_path(name, prefix=DEFAULT_PREFIX): # build options that have a perfectly matching command line option, listed by default value BUILD_OPTIONS_CMDLINE = { None: [ - 'allow_unresolved_templates', 'aggregate_regtest', 'backup_modules', 'banned_linked_shared_libs', @@ -281,6 +280,7 @@ def mk_full_default_path(name, prefix=DEFAULT_PREFIX): False: [ 'add_system_to_minimal_toolchains', 'allow_modules_tool_mismatch', + 'allow_unresolved_templates', 'backup_patched_files', 'consider_archived_easyconfigs', 'container_build_image', From b426aaa43a426ed532d62e96ac57addc3b78edd2 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Wed, 18 Dec 2024 10:22:10 +0100 Subject: [PATCH 6/7] reword error message for unresolved template values + add dedicated test to check on resolving of template values --- easybuild/framework/easyconfig/easyconfig.py | 4 +-- test/framework/easyconfig.py | 37 ++++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/easybuild/framework/easyconfig/easyconfig.py b/easybuild/framework/easyconfig/easyconfig.py index 4916d776ff..c95ab7dafe 100644 --- a/easybuild/framework/easyconfig/easyconfig.py +++ b/easybuild/framework/easyconfig/easyconfig.py @@ -2075,8 +2075,8 @@ def resolve_template(value, tmpl_dict, expect_resolved=True): ver) except KeyError: if expect_resolved: - msg = ('Failed to resolve template value %s with dict %s. ' % (value, tmpl_dict) + - 'This might cause failures or unexpected behavior, ' + + msg = (f'Failed to resolve all templates in "{value}" using template dictionary: {tmpl_dict}. ' + 'This might cause failures or unexpected behavior, ' 'check for correct escaping if this is intended!') if build_option('allow_unresolved_templates'): print_warning(msg) diff --git a/test/framework/easyconfig.py b/test/framework/easyconfig.py index d5cb550b0c..a8bf9d2a88 100644 --- a/test/framework/easyconfig.py +++ b/test/framework/easyconfig.py @@ -5148,6 +5148,43 @@ def test_easyconfigs_caches(self): regex = re.compile(r"libtoy/0\.0 is already installed", re.M) self.assertTrue(regex.search(stdout), "Pattern '%s' should be found in: %s" % (regex.pattern, stdout)) + def test_templates(self): + """ + Test use of template values like %(version)s + """ + test_ecs_dir = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'easyconfigs', 'test_ecs') + toy_ec = os.path.join(test_ecs_dir, 't', 'toy', 'toy-0.0.eb') + + test_ec_txt = read_file(toy_ec) + test_ec_txt += '\ndescription = "name: %(name)s, version: %(version)s"' + + test_ec = os.path.join(self.test_prefix, 'test.eb') + write_file(test_ec, test_ec_txt) + ec = EasyConfig(test_ec) + + # get_ref provides access to non-templated raw value + self.assertEqual(ec.get_ref('description'), "name: %(name)s, version: %(version)s") + self.assertEqual(ec['description'], "name: toy, version: 0.0") + + # error when using wrong template value or using template value that can not be resolved yet too early + test_ec_txt += '\ndescription = "name: %(name)s, version: %(version)s, pyshortver: %(pyshortver)s"' + write_file(test_ec, test_ec_txt) + ec = EasyConfig(test_ec) + + self.assertEqual(ec.get_ref('description'), "name: %(name)s, version: %(version)s, pyshortver: %(pyshortver)s") + error_pattern = "Failed to resolve all templates in.* %\(pyshortver\)s.* using template dictionary:" + self.assertErrorRegex(EasyBuildError, error_pattern, ec.__getitem__, 'description') + + # EasyBuild can be configured to allow unresolved templates + update_build_option('allow_unresolved_templates', True) + self.assertEqual(ec.get_ref('description'), "name: %(name)s, version: %(version)s, pyshortver: %(pyshortver)s") + with self.mocked_stdout_stderr() as (stdout, stderr): + self.assertEqual(ec['description'], "name: %(name)s, version: %(version)s, pyshortver: %(pyshortver)s") + + self.assertFalse(stdout.getvalue()) + regex = re.compile(r"WARNING: Failed to resolve all templates.* %\(pyshortver\)s", re.M) + self.assertRegex(stderr.getvalue(), regex) + def suite(): """ returns all the testcases in this module """ From 43b3835603838cce0b50aad46040ead26b301f59 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Wed, 18 Dec 2024 11:49:53 +0100 Subject: [PATCH 7/7] use raw string for error pattern in test_templates --- test/framework/easyconfig.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/framework/easyconfig.py b/test/framework/easyconfig.py index a8bf9d2a88..4661344d8d 100644 --- a/test/framework/easyconfig.py +++ b/test/framework/easyconfig.py @@ -5172,7 +5172,7 @@ def test_templates(self): ec = EasyConfig(test_ec) self.assertEqual(ec.get_ref('description'), "name: %(name)s, version: %(version)s, pyshortver: %(pyshortver)s") - error_pattern = "Failed to resolve all templates in.* %\(pyshortver\)s.* using template dictionary:" + error_pattern = r"Failed to resolve all templates in.* %\(pyshortver\)s.* using template dictionary:" self.assertErrorRegex(EasyBuildError, error_pattern, ec.__getitem__, 'description') # EasyBuild can be configured to allow unresolved templates