From d4b45605c875c5c6a3578cd8dab2a5c54e4779c5 Mon Sep 17 00:00:00 2001 From: Greg Becker Date: Sun, 6 Nov 2022 11:30:37 -0800 Subject: [PATCH] allow multiple compatible deps from CLI (#21262) Currently, Spack can fail for a valid spec if the spec is constructed from overlapping, but not conflicting, concrete specs via the hash. For example, if abcdef and ghijkl are the hashes of specs that both depend on zlib/mnopqr, then foo ^/abcdef ^/ghijkl will fail to construct a spec, with the error message "Cannot depend on zlib... twice". This PR changes this behavior to check whether the specs are compatible before failing. With this PR, foo ^/abcdef ^/ghijkl will concretize. As a side-effect, so will foo ^zlib ^zlib and other specs that are redundant on their dependencies. --- lib/spack/spack/spec.py | 18 +++++++++++++++++- lib/spack/spack/test/spec_syntax.py | 8 +++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 3b8ed07a83a..141f6c4b628 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -1559,8 +1559,24 @@ def _set_compiler(self, compiler): def _add_dependency(self, spec, deptypes): """Called by the parser to add another spec as a dependency.""" if spec.name in self._dependencies: - raise DuplicateDependencyError("Cannot depend on '%s' twice" % spec) + # allow redundant compatible dependency specifications + # depspec equality checks by name, so we need to check components + # separately to test whether the specs are identical + orig = self._dependencies[spec.name] + for dspec in orig: + if deptypes == dspec.deptypes: + try: + dspec.spec.constrain(spec) + return + except spack.error.UnsatisfiableSpecError: + raise DuplicateDependencyError( + "Cannot depend on incompatible specs '%s' and '%s'" + % (dspec.spec, spec) + ) + else: + raise DuplicateDependencyError("Cannot depend on '%s' twice" % spec) + # create an edge and add to parent and child self.add_dependency_edge(spec, deptypes) def add_dependency_edge(self, dependency_spec, deptype): diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index ab560bed084..cf1ce971d01 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -293,6 +293,12 @@ def test_canonicalize(self): self.check_parse("x ^y", "x@: ^y@:") + def test_parse_redundant_deps(self): + self.check_parse("x ^y@foo", "x ^y@foo ^y@foo") + self.check_parse("x ^y@foo+bar", "x ^y@foo ^y+bar") + self.check_parse("x ^y@foo+bar", "x ^y@foo+bar ^y") + self.check_parse("x ^y@foo+bar", "x ^y ^y@foo+bar") + def test_parse_errors(self): errors = ["x@@1.2", "x ^y@@1.2", "x@1.2::", "x::"] self._check_raises(SpecParseError, errors) @@ -481,7 +487,7 @@ def test_multiple_versions(self): self._check_raises(MultipleVersionError, multiples) def test_duplicate_dependency(self): - self._check_raises(DuplicateDependencyError, ["x ^y ^y"]) + self._check_raises(DuplicateDependencyError, ["x ^y@1 ^y@2"]) def test_duplicate_compiler(self): duplicates = [