From 9de61c019738c0165b41c6d2ba4dc519b16f8fd5 Mon Sep 17 00:00:00 2001 From: Scott Wittenburg Date: Thu, 3 Mar 2022 16:23:52 -0700 Subject: env: enforce predictable ordering when reading lockfile Without some enforcement of spec ordering, python 2 produced different results in the affected test than did python 3. This change makes the arbitrary but reproducible decision to sort the specs by their lockfile key alphabetically. --- lib/spack/spack/environment/environment.py | 6 ++++-- lib/spack/spack/test/cmd/env.py | 22 +++++++++++++++------- 2 files changed, 19 insertions(+), 9 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index 35e2956fdd..18a39dc1ad 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -1817,9 +1817,11 @@ class Environment(object): roots = d['roots'] self.concretized_user_specs = [Spec(r['spec']) for r in roots] self.concretized_order = [r['hash'] for r in roots] - json_specs_by_hash = d['concrete_specs'] + json_specs_by_hash = collections.OrderedDict() + for lockfile_key in sorted(d['concrete_specs']): + json_specs_by_hash[lockfile_key] = d['concrete_specs'][lockfile_key] - specs_by_hash = {} + specs_by_hash = collections.OrderedDict() for lockfile_key, node_dict in json_specs_by_hash.items(): specs_by_hash[lockfile_key] = Spec.from_node_dict(node_dict) diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index a0608fecea..48ef699f33 100644 --- a/lib/spack/spack/test/cmd/env.py +++ b/lib/spack/spack/test/cmd/env.py @@ -2868,16 +2868,24 @@ def test_environment_query_spec_by_hash(mock_stage, mock_fetch, install_mockery) def test_read_legacy_lockfile_and_reconcretize(mock_stage, mock_fetch, install_mockery): """Make sure that when we read a legacy environment lock file with a hash conflict (one from before we switched to full hash), the behavior as to - which of the conflicting specs we pick is deterministic. When we read - the lockfile, we process root specs in the order they appear in 'roots', - so we expect the dependencies of the last root in that list to be the - ones that appear in the environment before we forcefully re-concretize - the environment. After we force reconcretization, we should see all - the dependencies again.""" + which of the conflicting specs we pick is deterministic and reproducible. + When we read the lockfile, we (somewhat arbitrarily) process specs in + alphabetical order of their lockfile key. Consequently, when reading an + old lockfile where two specs have a dag hash conflict we expect to keep the + second one we encounter. After we force reconcretization, we should both of + the specs that originally conflicted present in the environment again.""" legacy_lockfile_path = os.path.join( spack.paths.test_path, 'data', 'legacy_env', 'spack.lock' ) + # In the legacy lockfile, we have two conflicting specs that differ only + # in a build-only dependency. The lockfile keys and conflicting specs + # are: + # wci7a3a -> dttop ^dtbuild1@0.5 + # 5zg6wxw -> dttop ^dtbuild1@1.0 + # So when we initially read the legacy lockfile, we expect to have kept + # the version of dttop that depends on dtbuild1@0.5 + env('create', 'test', legacy_lockfile_path) test = ev.read('test') @@ -2888,7 +2896,7 @@ def test_read_legacy_lockfile_and_reconcretize(mock_stage, mock_fetch, install_m single_root = next(iter(test.specs_by_hash.values())) - assert single_root['dtbuild1'].version == Version('1.0') + assert single_root['dtbuild1'].version == Version('0.5') # Now forcefully reconcretize with ev.read('test'): -- cgit v1.2.3-70-g09d2