zhangyiss/spack
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

unify: when_possible and unify: true -- Bugfix for error in 37438 (#37681)

Browse Source 
Two bugs came in from #37438

1. `unify: when_possible` was broken, because of an incorrect assertion. abstract/concrete
   spec pairs were compared against the results that were in the process of being computed,
   rather than against the previous results.
2. `unify: true` had an ordering bug that could mix the association between abstract and
   concrete specs

- [x] 1 is resolved by creating a lookup from old concrete specs to old abstract specs,
      and we use that to associate the "new" concrete specs that happen to be the old
      ones with their abstract specs (since those are stripped out for concretization
- [x] 2 is resolved by combining the new and old abstract as lists instead of combining
      them as sets. This is important because `set() | set()` does not make any ordering
      promises, even though set ordering is otherwise guaranteed in `python@3.7:`
This commit is contained in:
Greg Becker 2023-05-15 22:08:34 -07:00 committed by GitHub
parent 690661eadd
commit 3765a5f7f8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 15 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
lib/spack/spack/environment/environment.py
View File

@ -1402,6 +1402,10 @@ def _concretize_together_where_possible(
if not new_user_specs:
return []
old_concrete_to_abstract = {
concrete: abstract for (abstract, concrete) in self.concretized_specs()
}
self.concretized_user_specs = []
self.concretized_order = []
self.specs_by_hash = {}
@ -1413,11 +1417,13 @@ def _concretize_together_where_possible(
result = []
for abstract, concrete in sorted(result_by_user_spec.items()):
# If the "abstract" spec is a concrete spec from the previous concretization
# translate it back to an abstract spec. Otherwise, keep the abstract spec
abstract = old_concrete_to_abstract.get(abstract, abstract)
if abstract in new_user_specs:
result.append((abstract, concrete))
else:
assert (abstract, concrete) in result
self._add_concrete_spec(abstract, concrete)
return result
def _concretize_together(
@ -1436,7 +1442,7 @@ def _concretize_together(
self.specs_by_hash = {}
try:
concrete_specs = spack.concretize.concretize_specs_together(
concrete_specs: List[spack.spec.Spec] = spack.concretize.concretize_specs_together(
*specs_to_concretize, tests=tests
)
except spack.error.UnsatisfiableSpecError as e:
@ -1455,11 +1461,14 @@ def _concretize_together(
)
raise
# zip truncates the longer list, which is exactly what we want here
concretized_specs = [x for x in zip(new_user_specs | kept_user_specs, concrete_specs)]
# set() | set() does not preserve ordering, even though sets are ordered
ordered_user_specs = list(new_user_specs) + list(kept_user_specs)
concretized_specs = [x for x in zip(ordered_user_specs, concrete_specs)]
for abstract, concrete in concretized_specs:
self._add_concrete_spec(abstract, concrete)
return concretized_specs
# zip truncates the longer list, which is exactly what we want here
return list(zip(new_user_specs, concrete_specs))
def _concretize_separately(self, tests=False):
"""Concretization strategy that concretizes separately one