From 8f427401734fed6ecc0555d1b62eb7be0bbc2b9c Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 17 Mar 2025 09:11:08 +0100 Subject: [PATCH 1/5] Add test for modifiying cached ECs --- test/framework/easyconfig.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/framework/easyconfig.py b/test/framework/easyconfig.py index 2b165a10ba..668dbcae0a 100644 --- a/test/framework/easyconfig.py +++ b/test/framework/easyconfig.py @@ -5117,6 +5117,24 @@ def test_easyconfigs_caches(self): self.assertIsInstance(ec2['ec'].toolchain, SystemToolchain) self.assertTrue(os.path.samefile(ec2['ec'].path, toy_ec)) + # Returned easyconfigs are independent as-if there was no caching + ec3 = process_easyconfig(toy_ec)[0] + ec3['ec']['name'] = 'newname' + ec3['ec']['version'] = '99.1234' + ec3['spec'] = 'non-existing.eb' + ec3['dependencies'].append('Dummy') + self.assertEqual(ec3['ec'].name, 'newname') + self.assertEqual(ec3['ec'].version, '99.1234') + self.assertEqual(ec3['spec'], 'non-existing.eb') + self.assertEqual(ec3['dependencies'], ['Dummy']) + # Neither the previously returned nor newly requested ECs are modified by the above + ec2_2 = process_easyconfig(toy_ec)[0] + for orig_ec in (ec2, ec2_2): + self.assertEqual(orig_ec['ec'].name, 'toy') + self.assertEqual(orig_ec['ec'].version, '0.0') + self.assertEqual(orig_ec['spec'], toy_ec) + self.assertEqual(orig_ec['dependencies'], []) + # also check whether easyconfigs cache works with end-to-end test args = [libtoy_ec, '--trace'] self.mock_stdout(True) From 34cf949a82a38d2d20f1f3c6f69004d61e777cfd Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 17 Mar 2025 08:59:25 +0100 Subject: [PATCH 2/5] Revert "use copy() method rather than copy.deepcopy on EasyConfig instances" This reverts commit 38361401dd36985c2045f62e19df1dd93319c36e. --- easybuild/framework/easyconfig/easyconfig.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/easybuild/framework/easyconfig/easyconfig.py b/easybuild/framework/easyconfig/easyconfig.py index 57c910c746..5a6d205de4 100644 --- a/easybuild/framework/easyconfig/easyconfig.py +++ b/easybuild/framework/easyconfig/easyconfig.py @@ -2082,7 +2082,7 @@ def process_easyconfig(path, build_specs=None, validate=True, parse_only=False, if not build_specs: cache_key = (path, validate, hidden, parse_only) if cache_key in _easyconfigs_cache: - return [e.copy() for e in _easyconfigs_cache[cache_key]] + return copy.deepcopy(_easyconfigs_cache[cache_key]) easyconfigs = [] for spec in blocks: @@ -2133,7 +2133,7 @@ def process_easyconfig(path, build_specs=None, validate=True, parse_only=False, easyconfig['dependencies'].append(tc) if cache_key is not None: - _easyconfigs_cache[cache_key] = [e.copy() for e in easyconfigs] + _easyconfigs_cache[cache_key] = copy.deepcopy(easyconfigs) return easyconfigs From 5f04b7b79516b93997ad259190269ea8435e63d5 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 17 Mar 2025 09:28:58 +0100 Subject: [PATCH 3/5] Implement the deepcopy protocoll for EasyConfig --- easybuild/framework/easyconfig/easyconfig.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/easybuild/framework/easyconfig/easyconfig.py b/easybuild/framework/easyconfig/easyconfig.py index 5a6d205de4..2074b9fe3f 100644 --- a/easybuild/framework/easyconfig/easyconfig.py +++ b/easybuild/framework/easyconfig/easyconfig.py @@ -606,20 +606,30 @@ def copy(self, validate=None): """ Return a copy of this EasyConfig instance. """ + return self._copy(validate=validate) + + def _copy(self, validate=None, memo=None): + """Implement the deep copy""" if validate is None: validate = self.validation # create a new EasyConfig instance ec = EasyConfig(self.path, validate=validate, hidden=self.hidden, rawtxt=self.rawtxt) # take a copy of the actual config dictionary (which already contains the extra options) - ec._config = copy.deepcopy(self._config) + ec._config = copy.deepcopy(self._config, memo) # since rawtxt is defined, self.path may not get inherited, make sure it does if self.path: ec.path = self.path # also copy template values, since re-generating them may not give the same set of template values straight away - ec.template_values = copy.deepcopy(self.template_values) + ec.template_values = copy.deepcopy(self.template_values, memo) + + return ec + def __deepcopy__(self, memo): + """Implement a deep copy""" + ec = self._copy(validate=False, memo=memo) + ec.validation = self.validation return ec def update(self, key, value, allow_duplicate=True): From 207dfefcf4ebfc95e937112169d0ddec2ec408a1 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 17 Mar 2025 10:41:00 +0100 Subject: [PATCH 4/5] Revert "Implement the deepcopy protocoll for EasyConfig" This reverts commit 5f04b7b79516b93997ad259190269ea8435e63d5. --- easybuild/framework/easyconfig/easyconfig.py | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/easybuild/framework/easyconfig/easyconfig.py b/easybuild/framework/easyconfig/easyconfig.py index 2074b9fe3f..5a6d205de4 100644 --- a/easybuild/framework/easyconfig/easyconfig.py +++ b/easybuild/framework/easyconfig/easyconfig.py @@ -606,30 +606,20 @@ def copy(self, validate=None): """ Return a copy of this EasyConfig instance. """ - return self._copy(validate=validate) - - def _copy(self, validate=None, memo=None): - """Implement the deep copy""" if validate is None: validate = self.validation # create a new EasyConfig instance ec = EasyConfig(self.path, validate=validate, hidden=self.hidden, rawtxt=self.rawtxt) # take a copy of the actual config dictionary (which already contains the extra options) - ec._config = copy.deepcopy(self._config, memo) + ec._config = copy.deepcopy(self._config) # since rawtxt is defined, self.path may not get inherited, make sure it does if self.path: ec.path = self.path # also copy template values, since re-generating them may not give the same set of template values straight away - ec.template_values = copy.deepcopy(self.template_values, memo) - - return ec + ec.template_values = copy.deepcopy(self.template_values) - def __deepcopy__(self, memo): - """Implement a deep copy""" - ec = self._copy(validate=False, memo=memo) - ec.validation = self.validation return ec def update(self, key, value, allow_duplicate=True): From fc4b7b5fc034c7f361f876eba295efb52980ab55 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 17 Mar 2025 10:56:21 +0100 Subject: [PATCH 5/5] Implement EC "deep"-copy only for cached EasyConfigs The `__deepcopy__` needs to be very generic to avoid misleading results by not copying all (possibly changed) attributes. So leave that alone and use `ec.copy()` on the `'ec'` key directly in the cache code. --- easybuild/framework/easyconfig/easyconfig.py | 20 ++++++++++++++++++-- test/framework/toy_build.py | 11 +++++++---- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/easybuild/framework/easyconfig/easyconfig.py b/easybuild/framework/easyconfig/easyconfig.py index 5a6d205de4..c0d320d347 100644 --- a/easybuild/framework/easyconfig/easyconfig.py +++ b/easybuild/framework/easyconfig/easyconfig.py @@ -2063,6 +2063,22 @@ def resolve_template(value, tmpl_dict): return value +def _copy_ec_dict(easyconfig): + """Copy an easyconfig dict as (initially) parsed""" + ec = easyconfig.pop('ec') # Don't copy this via deepcopy + try: + new_easyconfig = copy.deepcopy(easyconfig) # Copy the rest of the dict + finally: + easyconfig['ec'] = ec # Put back + new_easyconfig['ec'] = ec.copy() + return new_easyconfig + + +def _copy_ec_dicts(easyconfigs): + """Copy list of easyconfig dicts as (initially) parsed""" + return [_copy_ec_dict(ec) for ec in easyconfigs] + + def process_easyconfig(path, build_specs=None, validate=True, parse_only=False, hidden=None): """ Process easyconfig, returning some information for each block @@ -2082,7 +2098,7 @@ def process_easyconfig(path, build_specs=None, validate=True, parse_only=False, if not build_specs: cache_key = (path, validate, hidden, parse_only) if cache_key in _easyconfigs_cache: - return copy.deepcopy(_easyconfigs_cache[cache_key]) + return _copy_ec_dicts(_easyconfigs_cache[cache_key]) easyconfigs = [] for spec in blocks: @@ -2133,7 +2149,7 @@ def process_easyconfig(path, build_specs=None, validate=True, parse_only=False, easyconfig['dependencies'].append(tc) if cache_key is not None: - _easyconfigs_cache[cache_key] = copy.deepcopy(easyconfigs) + _easyconfigs_cache[cache_key] = _copy_ec_dicts(easyconfigs) return easyconfigs diff --git a/test/framework/toy_build.py b/test/framework/toy_build.py index 5219ee5aa6..9a1c3d9531 100644 --- a/test/framework/toy_build.py +++ b/test/framework/toy_build.py @@ -3018,7 +3018,7 @@ def start_hook(): print('start hook triggered') def parse_hook(ec): - print('%s %s' % (ec.name, ec.version)) + print('Parse Hook %s %s' % (ec.name, ec.version)) # print sources value to check that raw untemplated strings are exposed in parse_hook print(ec['sources']) # try appending to postinstallcmd to see whether the modification is actually picked up @@ -3096,7 +3096,10 @@ def post_run_shell_cmd_hook(cmd, *args, **kwargs): # - for devel module file expected_output = textwrap.dedent(""" start hook triggered - toy 0.0 + Parse Hook toy 0.0 + ['%(name)s-%(version)s.tar.gz'] + echo toy + Parse Hook toy 0.0 ['%(name)s-%(version)s.tar.gz'] echo toy pre-configure: toy.source: True @@ -3106,10 +3109,10 @@ def post_run_shell_cmd_hook(cmd, *args, **kwargs): in post-install hook for toy v0.0 bin, lib in module-write hook hook for {mod_name} - toy 0.0 + Parse Hook toy 0.0 ['%(name)s-%(version)s.tar.gz'] echo toy - toy 0.0 + Parse Hook toy 0.0 ['%(name)s-%(version)s.tar.gz'] echo toy installing of extension bar is done!