Skip to content

Make sure cached easyconfigs don't get modifed #4818

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Flamefire wants to merge 8 commits into
base: develop
Choose a base branch
from
Open

Make sure cached easyconfigs don't get modifed #4818

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8f42740
Add test for modifiying cached ECs
Flamefire Mar 17, 2025
34cf949
Revert "use copy() method rather than copy.deepcopy on EasyConfig ins…
Flamefire Mar 17, 2025
5f04b7b
Implement the deepcopy protocoll for EasyConfig
Flamefire Mar 17, 2025
207dfef
Revert "Implement the deepcopy protocoll for EasyConfig"
Flamefire Mar 17, 2025
fc4b7b5
Implement EC "deep"-copy only for cached EasyConfigs
Flamefire Mar 17, 2025
d098382
Merge branch 'develop' into fix-ec-cache
Flamefire Mar 20, 2025
b4b5a18
Merge branch 'develop' into fix-ec-cache
Flamefire Jul 30, 2025
3511eda
Improve comment
Flamefire Sep 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions easybuild/framework/easyconfig/easyconfig.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2234,6 +2234,23 @@ def resolve_template(value, tmpl_dict, expect_resolved=True):
return value


def _copy_ec_dict(easyconfig):
"""Copy an easyconfig dict as (initially) parsed"""
# deepcopy on the ec doesn't fully copy it, but requires the copy() method so temorarily remove it
ec = easyconfig.pop('ec')
try:
new_easyconfig = copy.deepcopy(easyconfig) # Copy the rest of the dict
Copy link
Member

@boegel boegel Sep 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you clarify why this is in a try?

What if this fails? Then new_easyconfig will be undefined, and we'll get a nasty crash, no?

Copy link
Contributor Author

@Flamefire Flamefire Sep 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a try-finally. So it will still propagate all errors. The only difference is that in all cases ec (removed above) is put back to the original easyconfig

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
Expand All @@ -2253,7 +2270,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_ec_dicts(_easyconfigs_cache[cache_key])

easyconfigs = []
for spec in blocks:
Expand Down Expand Up @@ -2308,7 +2325,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_ec_dicts(easyconfigs)

return easyconfigs

Expand Down
18 changes: 18 additions & 0 deletions test/framework/easyconfig.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5276,6 +5276,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)
Expand Down
11 changes: 7 additions & 4 deletions test/framework/toy_build.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3663,7 +3663,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
Expand Down Expand Up @@ -3762,7 +3762,10 @@ def post_build_and_install_loop_hook(ecs):
# - for devel module file
expected_output = textwrap.dedent(f"""
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
installing 1 easyconfigs: toy/0.0
Expand All @@ -3773,10 +3776,10 @@ def post_build_and_install_loop_hook(ecs):
' gcc toy.c -o toy && copy_toy_file toy copy_of_toy' command failed (exit code 127), but I fixed it!
in post-install hook for toy v0.0
bin, lib
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
in module-write hook hook for {mod_name}
Expand Down