From 269cc399539b1799544166141077897262536e9c Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 3 Jan 2025 13:39:43 +0100 Subject: [PATCH 1/8] Add context manager to disable the check if templates are resolved. This should only be disabled temporarily so make the state private and expose only the context manager and a property. --- easybuild/framework/easyconfig/easyconfig.py | 34 +++++++++++++++++--- test/framework/easyconfig.py | 14 ++++---- 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/easybuild/framework/easyconfig/easyconfig.py b/easybuild/framework/easyconfig/easyconfig.py index 1d476c9868..85bf3e6051 100644 --- a/easybuild/framework/easyconfig/easyconfig.py +++ b/easybuild/framework/easyconfig/easyconfig.py @@ -428,10 +428,10 @@ def __init__(self, path, extra_options=None, build_specs=None, validate=True, hi :param local_var_naming_check: mode to use when checking if local variables use the recommended naming scheme """ self.template_values = None - # a boolean to control templating, can be (temporarily) disabled in easyblocks + # a boolean to control templating, can be (temporarily) disabled self.enable_templating = True - # boolean to control whether all template values must be resolvable, can be (temporarily) disabled in easyblocks - self.expect_resolved_template_values = True + # boolean to control whether all template values must be resolvable on access, can be (temporarily) disabled + self._expect_resolved_template_values = True self.log = fancylogger.getLogger(self.__class__.__name__, fname=False) @@ -552,6 +552,32 @@ def disable_templating(self): finally: self.enable_templating = old_enable_templating + @contextmanager + def allow_unresolved_templates(self): + """Temporarily allow templates to be not (fully) resolved. + + This should only be used when it is intended to use partially resolved templates. + Otherwise `ec.get(key, resolve=False)` should be used. + See also @ref disable_templating. + + Usage: + with ec.allow_unresolved_templates(): + value = ec.get('key') # This will not raise an error + print(value % {'extra_key': exta_value}) + # Resolving is enforced again if it was before + """ + old_expect_resolved_template_values = self._expect_resolved_template_values + self._expect_resolved_template_values = False + try: + yield old_expect_resolved_template_values + finally: + self._expect_resolved_template_values = old_expect_resolved_template_values + + @property + def expect_resolved_template_values(self): + """Check whether resolving all template values on access is enforced.""" + return self._expect_resolved_template_values + def __str__(self): """Return a string representation of this EasyConfig instance""" if self.path: @@ -1776,7 +1802,7 @@ def resolve_template(self, value): """Resolve all templates in the given value using this easyconfig""" if not self.template_values: self.generate_template_values() - return resolve_template(value, self.template_values, expect_resolved=self.expect_resolved_template_values) + return resolve_template(value, self.template_values, expect_resolved=self._expect_resolved_template_values) @handle_deprecated_or_replaced_easyconfig_parameters def __contains__(self, key): diff --git a/test/framework/easyconfig.py b/test/framework/easyconfig.py index 61a6b183be..a079089dd4 100644 --- a/test/framework/easyconfig.py +++ b/test/framework/easyconfig.py @@ -1314,12 +1314,14 @@ def test_ec_method_resolve_template(self): self.assertErrorRegex(EasyBuildError, error_pattern, ec.resolve_template, val) self.assertErrorRegex(EasyBuildError, error_pattern, ec.get, 'installopts') - # this can be (temporarily) disabled via expect_resolved_template_values in EasyConfig instance - ec.expect_resolved_template_values = False - self.assertEqual(ec.resolve_template(val), val) - self.assertEqual(ec['installopts'], val) - - ec.expect_resolved_template_values = True + # this can be (temporarily) disabled + with ec.allow_unresolved_templates(): + self.assertFalse(ec.expect_resolved_template_values) + self.assertEqual(ec.resolve_template(val), val) + self.assertEqual(ec['installopts'], val) + + # Enforced again + self.assertTrue(ec.expect_resolved_template_values) self.assertErrorRegex(EasyBuildError, error_pattern, ec.resolve_template, val) self.assertErrorRegex(EasyBuildError, error_pattern, ec.get, 'installopts') From 24fac111612ab9472e228a556dd8c4570737296c Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 3 Jan 2025 13:47:39 +0100 Subject: [PATCH 2/8] Make EasyConfig.enable_templating private Only expose the context manager and a property --- easybuild/framework/easyconfig/easyconfig.py | 19 ++++++++++------ test/framework/easyconfig.py | 24 +++++++++++--------- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/easybuild/framework/easyconfig/easyconfig.py b/easybuild/framework/easyconfig/easyconfig.py index 85bf3e6051..2bd81ca556 100644 --- a/easybuild/framework/easyconfig/easyconfig.py +++ b/easybuild/framework/easyconfig/easyconfig.py @@ -429,7 +429,7 @@ def __init__(self, path, extra_options=None, build_specs=None, validate=True, hi """ self.template_values = None # a boolean to control templating, can be (temporarily) disabled - self.enable_templating = True + self._templating_enabled = True # boolean to control whether all template values must be resolvable on access, can be (temporarily) disabled self._expect_resolved_template_values = True @@ -545,12 +545,17 @@ def disable_templating(self): # Do what you want without templating # Templating set to previous value """ - old_enable_templating = self.enable_templating - self.enable_templating = False + old_templating_enabled = self._templating_enabled + self._templating_enabled = False try: - yield old_enable_templating + yield old_templating_enabled finally: - self.enable_templating = old_enable_templating + self._templating_enabled = old_templating_enabled + + @property + def templating_enabled(self): + """Check whether templating is enabled on this EasyConfig""" + return self._templating_enabled @contextmanager def allow_unresolved_templates(self): @@ -1818,7 +1823,7 @@ def __getitem__(self, key): else: raise EasyBuildError("Use of unknown easyconfig parameter '%s' when getting parameter value", key) - if self.enable_templating: + if self._templating_enabled: value = self.resolve_template(value) return value @@ -1898,7 +1903,7 @@ def asdict(self): res = {} for key, tup in self._config.items(): value = tup[0] - if self.enable_templating: + if self._templating_enabled: if not self.template_values: self.generate_template_values() # Not all values can be resolved, e.g. %(installdir)s diff --git a/test/framework/easyconfig.py b/test/framework/easyconfig.py index a079089dd4..f3b5093ded 100644 --- a/test/framework/easyconfig.py +++ b/test/framework/easyconfig.py @@ -1213,9 +1213,11 @@ def test_templating_constants(self): ec.validate() # temporarily disable templating, just so we can check later whether it's *still* disabled + self.assertTrue(ec.templating_enabled) with ec.disable_templating(): ec.generate_template_values() - self.assertFalse(ec.enable_templating) + self.assertFalse(ec.templating_enabled) + self.assertTrue(ec.templating_enabled) self.assertEqual(ec['description'], "test easyconfig PI") self.assertEqual(ec['sources'][0], 'PI-3.04.tar.gz') @@ -2543,11 +2545,11 @@ def test_dump(self): test_ec = os.path.join(self.test_prefix, 'test.eb') ec = EasyConfig(os.path.join(test_ecs_dir, ecfile)) - ec.enable_templating = False - ecdict = ec.asdict() - ec.dump(test_ec) - # dict representation of EasyConfig instance should not change after dump - self.assertEqual(ecdict, ec.asdict()) + with ec.disable_templating(): + ecdict = ec.asdict() + ec.dump(test_ec) + # dict representation of EasyConfig instance should not change after dump + self.assertEqual(ecdict, ec.asdict()) ectxt = read_file(test_ec) patterns = [ @@ -2562,7 +2564,6 @@ def test_dump(self): # parse result again dumped_ec = EasyConfig(test_ec) - dumped_ec.enable_templating = False # check that selected parameters still have the same value params = [ @@ -2571,9 +2572,10 @@ def test_dump(self): 'dependencies', # checking this is important w.r.t. filtered hidden dependencies being restored in dump 'exts_list', # exts_lists (in Python easyconfig) use another layer of templating so shouldn't change ] - for param in params: - if param in ec: - self.assertEqual(ec[param], dumped_ec[param]) + with ec.disable_templating(), dumped_ec.disable_templating(): + for param in params: + if param in ec: + self.assertEqual(ec[param], dumped_ec[param]) ec_txt = textwrap.dedent(""" easyblock = 'EB_toy' @@ -2824,7 +2826,7 @@ def test_dump_template(self): ec.dump(testec) ectxt = read_file(testec) - self.assertTrue(ec.enable_templating) # templating should still be enabled after calling dump() + self.assertTrue(ec.templating_enabled) # templating should still be enabled after calling dump() patterns = [ r"easyblock = 'EB_foo'", From 7854b25b9fb0362ebfc4d011519da818bd981e60 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 3 Jan 2025 13:59:10 +0100 Subject: [PATCH 3/8] Use `allow_unresolved_templates` in EasyConfig.asdict Allows to use the `resolve` member method that checks for generated values already. --- easybuild/framework/easyconfig/easyconfig.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/easybuild/framework/easyconfig/easyconfig.py b/easybuild/framework/easyconfig/easyconfig.py index 2bd81ca556..ebaaf1e36d 100644 --- a/easybuild/framework/easyconfig/easyconfig.py +++ b/easybuild/framework/easyconfig/easyconfig.py @@ -1901,14 +1901,13 @@ def asdict(self): Return dict representation of this EasyConfig instance. """ res = {} - for key, tup in self._config.items(): - value = tup[0] - if self._templating_enabled: - if not self.template_values: - self.generate_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 + # Not all values can be resolved, e.g. %(installdir)s + with self.allow_unresolved_templates(): + for key, tup in self._config.items(): + value = tup[0] + if self._templating_enabled: + value = self.resolve_template(value) + res[key] = value return res def get_cuda_cc_template_value(self, key): From 26bc054e1eefd8629c78a0f8f460a1be4468b070 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 3 Jan 2025 14:04:12 +0100 Subject: [PATCH 4/8] Use try-except for the exist-check in __getitem__ --- easybuild/framework/easyconfig/easyconfig.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/easybuild/framework/easyconfig/easyconfig.py b/easybuild/framework/easyconfig/easyconfig.py index ebaaf1e36d..ffec232ee4 100644 --- a/easybuild/framework/easyconfig/easyconfig.py +++ b/easybuild/framework/easyconfig/easyconfig.py @@ -1817,10 +1817,9 @@ def __contains__(self, key): @handle_deprecated_or_replaced_easyconfig_parameters def __getitem__(self, key): """Return value of specified easyconfig parameter (without help text, etc.)""" - value = None - if key in self._config: + try: value = self._config[key][0] - else: + except KeyError: raise EasyBuildError("Use of unknown easyconfig parameter '%s' when getting parameter value", key) if self._templating_enabled: From 54e312bf29316d1e5b9444afa84b40cf446bed97 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 3 Jan 2025 14:04:20 +0100 Subject: [PATCH 5/8] Simplify `EasyConfig.asdict` to a dict comprehension --- easybuild/framework/easyconfig/easyconfig.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/easybuild/framework/easyconfig/easyconfig.py b/easybuild/framework/easyconfig/easyconfig.py index ffec232ee4..de0a08e5d3 100644 --- a/easybuild/framework/easyconfig/easyconfig.py +++ b/easybuild/framework/easyconfig/easyconfig.py @@ -1899,15 +1899,9 @@ def asdict(self): """ Return dict representation of this EasyConfig instance. """ - res = {} # Not all values can be resolved, e.g. %(installdir)s with self.allow_unresolved_templates(): - for key, tup in self._config.items(): - value = tup[0] - if self._templating_enabled: - value = self.resolve_template(value) - res[key] = value - return res + return {key: self[key] for key in self._config} def get_cuda_cc_template_value(self, key): """ From 3c63cda48ea4c71ca5646a33bd81a550a6b71901 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 3 Jan 2025 14:05:22 +0100 Subject: [PATCH 6/8] Revert "Simplify `EasyConfig.asdict` to a dict comprehension" This reverts commit 2a6386396c7e969d4a0c1e23ab5f880a72dedf23. --- easybuild/framework/easyconfig/easyconfig.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/easybuild/framework/easyconfig/easyconfig.py b/easybuild/framework/easyconfig/easyconfig.py index de0a08e5d3..ffec232ee4 100644 --- a/easybuild/framework/easyconfig/easyconfig.py +++ b/easybuild/framework/easyconfig/easyconfig.py @@ -1899,9 +1899,15 @@ def asdict(self): """ Return dict representation of this EasyConfig instance. """ + res = {} # Not all values can be resolved, e.g. %(installdir)s with self.allow_unresolved_templates(): - return {key: self[key] for key in self._config} + for key, tup in self._config.items(): + value = tup[0] + if self._templating_enabled: + value = self.resolve_template(value) + res[key] = value + return res def get_cuda_cc_template_value(self, key): """ From 45196fbbd90760848f94607da4343e7c11597995 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 3 Jan 2025 14:35:11 +0100 Subject: [PATCH 7/8] Show a meaningful error when using the old ec.enable_templating --- easybuild/framework/easyconfig/easyconfig.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/easybuild/framework/easyconfig/easyconfig.py b/easybuild/framework/easyconfig/easyconfig.py index ffec232ee4..9a6e682568 100644 --- a/easybuild/framework/easyconfig/easyconfig.py +++ b/easybuild/framework/easyconfig/easyconfig.py @@ -557,6 +557,11 @@ def templating_enabled(self): """Check whether templating is enabled on this EasyConfig""" return self._templating_enabled + def _enable_templating(self, *_): + self.log.nosupport("self.enable_templating is replaced by self.templating_enabled. " + "To disable it use the self.disable_templating context manager", '5.0') + enable_templating = property(_enable_templating, _enable_templating) + @contextmanager def allow_unresolved_templates(self): """Temporarily allow templates to be not (fully) resolved. From d243c795f925a197eabc4f61655fda3ff837526d Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Mon, 24 Feb 2025 15:44:37 +0100 Subject: [PATCH 8/8] use expect_resolved_template_values + enable_templating properties rather than accessing private class variables directly --- easybuild/framework/easyconfig/easyconfig.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/easybuild/framework/easyconfig/easyconfig.py b/easybuild/framework/easyconfig/easyconfig.py index d15dde5eae..4665a1d1e8 100644 --- a/easybuild/framework/easyconfig/easyconfig.py +++ b/easybuild/framework/easyconfig/easyconfig.py @@ -442,9 +442,10 @@ def __init__(self, path, extra_options=None, build_specs=None, validate=True, hi :param local_var_naming_check: mode to use when checking if local variables use the recommended naming scheme """ self.template_values = None - # a boolean to control templating, can be (temporarily) disabled + # a boolean to control templating, can be (temporarily) disabled via disable_templating context manager self._templating_enabled = True - # boolean to control whether all template values must be resolvable on access, can be (temporarily) disabled + # boolean to control whether all template values must be resolvable on access, + # can be (temporarily) disabled via allow_unresolved_templates context manager self._expect_resolved_template_values = True self.log = fancylogger.getLogger(self.__class__.__name__, fname=False) @@ -1865,7 +1866,7 @@ def resolve_template(self, value): """Resolve all templates in the given value using this easyconfig""" if not self.template_values: self.generate_template_values() - return resolve_template(value, self.template_values, expect_resolved=self._expect_resolved_template_values) + return resolve_template(value, self.template_values, expect_resolved=self.expect_resolved_template_values) @handle_deprecated_or_replaced_easyconfig_parameters def __contains__(self, key): @@ -1880,7 +1881,7 @@ def __getitem__(self, key): except KeyError: raise EasyBuildError("Use of unknown easyconfig parameter '%s' when getting parameter value", key) - if self._templating_enabled: + if self.templating_enabled: value = self.resolve_template(value) return value @@ -1962,7 +1963,7 @@ def asdict(self): with self.allow_unresolved_templates(): for key, tup in self._config.items(): value = tup[0] - if self._templating_enabled: + if self.templating_enabled: value = self.resolve_template(value) res[key] = value return res