diff options
author | Greg Becker <becker33@llnl.gov> | 2022-11-06 16:49:35 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-11-06 16:49:35 -0800 |
commit | 4b84cd8af5195c44341c4cecd1dc061ed1790909 (patch) | |
tree | 610f1643fa8255832210076f888867b6e7a25fd1 | |
parent | fce7bf179f03466e348075ae27f109187af6f08e (diff) | |
download | spack-4b84cd8af5195c44341c4cecd1dc061ed1790909.tar.gz spack-4b84cd8af5195c44341c4cecd1dc061ed1790909.tar.bz2 spack-4b84cd8af5195c44341c4cecd1dc061ed1790909.tar.xz spack-4b84cd8af5195c44341c4cecd1dc061ed1790909.zip |
bugfix for matrices with dependencies by hash (#22991)
Dependencies specified by hash are unique in Spack in that the abstract
specs are created with internal structure. In this case, the constraint
generation for spec matrices fails due to flattening the structure.
It turns out that the dep_difference method for Spec.constrain does not
need to operate on transitive deps to ensure correctness. Removing transitive
deps from this method resolves the bug.
- [x] Includes regression test
-rw-r--r-- | lib/spack/spack/spec.py | 12 | ||||
-rw-r--r-- | lib/spack/spack/test/spec_list.py | 19 |
2 files changed, 26 insertions, 5 deletions
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 141f6c4b62..30a67a55a8 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -3568,7 +3568,9 @@ class Spec(object): ) # Update with additional constraints from other spec - for name in other.dep_difference(self): + # operate on direct dependencies only, because a concrete dep + # represented by hash may have structure that needs to be preserved + for name in other.direct_dep_difference(self): dep_spec_copy = other._get_dependency(name) dep_copy = dep_spec_copy.spec deptypes = dep_spec_copy.deptypes @@ -3589,10 +3591,10 @@ class Spec(object): clone.constrain(other, deps) return clone - def dep_difference(self, other): + def direct_dep_difference(self, other): """Returns dependencies in self that are not in other.""" - mine = set(s.name for s in self.traverse(root=False)) - mine.difference_update(s.name for s in other.traverse(root=False)) + mine = set(dname for dname in self._dependencies) + mine.difference_update(dname for dname in other._dependencies) return mine def _autospec(self, spec_like): @@ -4871,7 +4873,7 @@ def merge_abstract_anonymous_specs(*abstract_specs): merged_spec[name].constrain(current_spec_constraint[name], deps=False) # Update with additional constraints from other spec - for name in current_spec_constraint.dep_difference(merged_spec): + for name in current_spec_constraint.direct_dep_difference(merged_spec): edge = next(iter(current_spec_constraint.edges_to_dependencies(name))) merged_spec._add_dependency(edge.spec.copy(), edge.deptypes) diff --git a/lib/spack/spack/test/spec_list.py b/lib/spack/spack/test/spec_list.py index 487417dff1..8923f6e1f6 100644 --- a/lib/spack/spack/test/spec_list.py +++ b/lib/spack/spack/test/spec_list.py @@ -200,3 +200,22 @@ class TestSpecList(object): ] speclist = SpecList("specs", matrix) assert len(speclist.specs) == 1 + + @pytest.mark.regression("22991") + def test_spec_list_constraints_with_structure( + self, mock_packages, mock_fetch, install_mockery + ): + # Setup by getting hash and installing package with dep + libdwarf_spec = Spec("libdwarf").concretized() + libdwarf_spec.package.do_install() + + # Create matrix + matrix = { + "matrix": [["mpileaks"], ["^callpath"], ["^libdwarf/%s" % libdwarf_spec.dag_hash()]] + } + + # ensure the concrete spec was retained in the matrix entry of which + # it is a dependency + speclist = SpecList("specs", [matrix]) + assert len(speclist.specs) == 1 + assert libdwarf_spec in speclist.specs[0] |