From 60cd97bda85f6970e72d585628c7c128b623b527 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 17 Apr 2020 14:19:03 +0200 Subject: [PATCH 1/6] Deprecate 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. To not break builds completely deprecate continuing for now and promote to a hard error later. --- easybuild/framework/easyconfig/easyconfig.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/easybuild/framework/easyconfig/easyconfig.py b/easybuild/framework/easyconfig/easyconfig.py index 2b6aaa751a..9ee29f9d9e 100644 --- a/easybuild/framework/easyconfig/easyconfig.py +++ b/easybuild/framework/easyconfig/easyconfig.py @@ -2032,7 +2032,9 @@ def resolve_template(value, tmpl_dict): try: value = value % tmpl_dict except KeyError: - _log.warning("Unable to resolve template value %s with dict %s", value, tmpl_dict) + depr_msg = ('Ignoring failure to resolve template value %s with dict %s.' % (value, tmpl_dict) + + '\n\tThis is deprecated and will lead to build failure. Check for correct escaping.') + _log.deprecated(depr_msg, '5.0') value = orig_value # Undo "%"-escaping else: # this block deals with references to objects and returns other references From 2729a786013fae50859a3eb0c273e6fbe3888486 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 17 Apr 2020 16:13:05 +0200 Subject: [PATCH 2/6] 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 80cddf8819..cd0a760e37 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -599,19 +599,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 @@ -648,17 +645,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, string_type): 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, @@ -689,7 +687,7 @@ def collect_exts_file_info(self, fetch_files=True, verify_checksums=True): raise EasyBuildError('Checksum verification for extension source %s failed', src_fn) # 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: @@ -2879,7 +2877,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 9ee29f9d9e..a7832ae1c6 100644 --- a/easybuild/framework/easyconfig/easyconfig.py +++ b/easybuild/framework/easyconfig/easyconfig.py @@ -779,9 +779,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.fetch_extension_sources @@ -1814,11 +1818,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.""" @@ -1831,8 +1838,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 19218eca6a..a502452b39 100644 --- a/test/framework/easyblock.py +++ b/test/framework/easyblock.py @@ -1086,7 +1086,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 6a488be1201407896b42680e2ef113f7094f29ef Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Tue, 5 Jul 2022 10:36:07 +0200 Subject: [PATCH 3/6] 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 | 25 ++++++++++++-------- easybuild/framework/extension.py | 5 +++- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/easybuild/framework/easyconfig/easyconfig.py b/easybuild/framework/easyconfig/easyconfig.py index a7832ae1c6..efc1ee8b6b 100644 --- a/easybuild/framework/easyconfig/easyconfig.py +++ b/easybuild/framework/easyconfig/easyconfig.py @@ -1645,8 +1645,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) @@ -1855,7 +1856,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 @@ -2005,10 +2007,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, string_type): # simple escaping, making all '%foo', '%%foo', '%%%foo' post-templates values available, @@ -2040,9 +2043,10 @@ def resolve_template(value, tmpl_dict): try: value = value % tmpl_dict except KeyError: - depr_msg = ('Ignoring failure to resolve template value %s with dict %s.' % (value, tmpl_dict) + - '\n\tThis is deprecated and will lead to build failure. Check for correct escaping.') - _log.deprecated(depr_msg, '5.0') + if expect_resolved: + depr_msg = ('Ignoring failure to resolve template value %s with dict %s.' % (value, tmpl_dict) + + '\n\tThis is deprecated and will lead to build failure. Check for correct escaping.') + _log.deprecated(depr_msg, '5.0') value = orig_value # Undo "%"-escaping else: # this block deals with references to objects and returns other references @@ -2057,11 +2061,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 56f242ffce..125c68ba4b 100644 --- a/easybuild/framework/extension.py +++ b/easybuild/framework/extension.py @@ -129,7 +129,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 45fbad924da436db5da1a1184cef9fb9736cbe29 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Thu, 7 Dec 2023 14:20:10 +0100 Subject: [PATCH 4/6] Add option to silence the deprecation about failure to resolve a template Useful workaround if there are still some missing fixes. --- easybuild/framework/easyconfig/easyconfig.py | 5 ++++- easybuild/tools/options.py | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/easybuild/framework/easyconfig/easyconfig.py b/easybuild/framework/easyconfig/easyconfig.py index efc1ee8b6b..7271f07a29 100644 --- a/easybuild/framework/easyconfig/easyconfig.py +++ b/easybuild/framework/easyconfig/easyconfig.py @@ -2046,7 +2046,10 @@ def resolve_template(value, tmpl_dict, expect_resolved=True): if expect_resolved: depr_msg = ('Ignoring failure to resolve template value %s with dict %s.' % (value, tmpl_dict) + '\n\tThis is deprecated and will lead to build failure. Check for correct escaping.') - _log.deprecated(depr_msg, '5.0') + if 'resolve-templates' in build_option('silence_deprecation_warnings'): + _log.warning(depr_msg, '5.0') + else: + _log.deprecated(depr_msg, '5.0') value = orig_value # Undo "%"-escaping else: # this block deals with references to objects and returns other references diff --git a/easybuild/tools/options.py b/easybuild/tools/options.py index 68ddba632a..ae6fffd6b7 100644 --- a/easybuild/tools/options.py +++ b/easybuild/tools/options.py @@ -340,7 +340,7 @@ def override_options(self): # override options descr = ("Override options", "Override default EasyBuild behavior.") - all_deprecations = ('python2', 'Lmod6', 'easyconfig', 'toolchain') + all_deprecations = ('python2', 'Lmod6', 'easyconfig', 'toolchain', 'resolve-templates') opts = OrderedDict({ 'accept-eula': ("Accept EULA for specified software [DEPRECATED, use --accept-eula-for instead!]", From e29bf488a3999b736c0ac5e82a0e02606b34865d Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Tue, 5 Jul 2022 11:05:25 +0200 Subject: [PATCH 5/6] Fix test comparing YEB and EB files Use the same templates in the compared EC and YEB files. --- test/framework/easyconfigs/yeb/Python-2.7.10-intel-2018a.yeb | 2 +- test/framework/easyconfigs/yeb/SQLite-3.8.10.2-foss-2018a.yeb | 2 +- test/framework/yeb.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/framework/easyconfigs/yeb/Python-2.7.10-intel-2018a.yeb b/test/framework/easyconfigs/yeb/Python-2.7.10-intel-2018a.yeb index 527f7bf8a5..e6cb14adcb 100644 --- a/test/framework/easyconfigs/yeb/Python-2.7.10-intel-2018a.yeb +++ b/test/framework/easyconfigs/yeb/Python-2.7.10-intel-2018a.yeb @@ -15,7 +15,7 @@ description: | toolchain: {name: intel, version: 2018a} toolchainopts: {pic: True, opt: True, optarch: True} -source_urls: ['http://www.python.org/ftp/python/%(version)s/'] +source_urls: ['http://www.python.org/ftp/%(namelower)s/%(version)s/'] sources: [*SOURCE_TGZ] # python needs bzip2 to build the bz2 package diff --git a/test/framework/easyconfigs/yeb/SQLite-3.8.10.2-foss-2018a.yeb b/test/framework/easyconfigs/yeb/SQLite-3.8.10.2-foss-2018a.yeb index a9d0afea39..acdfe5fa16 100644 --- a/test/framework/easyconfigs/yeb/SQLite-3.8.10.2-foss-2018a.yeb +++ b/test/framework/easyconfigs/yeb/SQLite-3.8.10.2-foss-2018a.yeb @@ -1,5 +1,5 @@ _internal_variables_: - - &versionstr 3081002 + - &versionstr '%(version_major)s081002' easyblock: ConfigureMake diff --git a/test/framework/yeb.py b/test/framework/yeb.py index 346ffa057b..caeeb73752 100644 --- a/test/framework/yeb.py +++ b/test/framework/yeb.py @@ -102,8 +102,8 @@ def test_parse_yeb(self): ec_eb = EasyConfig(ec_file) for key in sorted(ec_yeb.asdict()): - eb_val = ec_eb[key] - yeb_val = ec_yeb[key] + eb_val = ec_eb.get_ref(key) + yeb_val = ec_yeb.get_ref(key) if key == 'description': # multi-line string is always terminated with '\n' in YAML, so strip it off yeb_val = yeb_val.strip() From e60c6c6a50a22ae46488a666797c9c1b7dc97463 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Thu, 6 Jun 2024 16:36:16 +0200 Subject: [PATCH 6/6] Fix test --- 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 4b63dc605b..00acb3bd24 100644 --- a/test/framework/easyconfig.py +++ b/test/framework/easyconfig.py @@ -3676,7 +3676,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"""