Merge pull request #4516 from Flamefire/disallow-template-failure

don't allow unresolved templates in easyconfig parameters by default + add support for `--allow-unresolved-templates` configuration option

Author: boegel

easybuild/framework/easyblock.py

@@ -613,19 +613,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
@@ -662,17 +659,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,
@@ -707,7 +705,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:
@@ -2991,7 +2989,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()
 

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
@@ -1653,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)
@@ -1826,11 +1831,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 +1851,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))
@@ -1859,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
 
@@ -2007,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,
@@ -2063,7 +2074,14 @@ 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}")
+                    if expect_resolved:
+                        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)
+                        else:
+                            raise EasyBuildError(msg)
                     value = raw_value  # Undo "%"-escaping
 
                 for key in tmpl_dict:
@@ -2084,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
 

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)

easybuild/tools/config.py

@@ -280,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',

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!)",

test/framework/easyblock.py

@@ -1225,7 +1225,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")

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"""
@@ -5147,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 = 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
+        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 """