From 7383bd393eb36b67f308b643d1ce02b53ebb255d Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Fri, 27 Nov 2015 23:06:18 -0800 Subject: Fixed bug #42: problem with satisfies() for virtual dependencies. - _cross_provider_maps() had suffered some bit rot (map returned was ill-formed but still worked for cases with one vdep) - ProviderIndex.satisfies() was only checking whether the result map was non-empty. It should check whether all common vdeps are *in* the result map, as that indicates there is *some* way to satisfy *all* of them. We were checking whether there was some way to satisfy *any one* of them, which is wrong. - Above would cause a problem when there is more than one vdep provider. - Added test that covers this case. - Added `constrained()` method to Spec. Analogous to `normalized()`: `constrain():constrained() :: normalize():normalized()` --- lib/spack/spack/spec.py | 7 +++++++ lib/spack/spack/test/spec_semantics.py | 13 ++++++++++++- lib/spack/spack/virtual.py | 9 +++++---- 3 files changed, 24 insertions(+), 5 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 92880e9cbf..a10edcc6fc 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -1207,6 +1207,13 @@ class Spec(object): return common + def constrained(self, other, deps=True): + """Return a constrained copy without modifying this spec.""" + clone = self.copy(deps=deps) + clone.constrain(other, deps) + return clone + + def dep_difference(self, other): """Returns dependencies in self that are not in other.""" mine = set(s.name for s in self.traverse(root=False)) diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py index 6666dbbb52..64220e5893 100644 --- a/lib/spack/spack/test/spec_semantics.py +++ b/lib/spack/spack/test/spec_semantics.py @@ -190,11 +190,23 @@ class SpecSematicsTest(MockPackagesTest): def test_satisfies_virtual(self): + # Don't use check_satisfies: it checks constrain() too, and + # you can't constrain a non-virtual by a virtual. self.assertTrue(Spec('mpich').satisfies(Spec('mpi'))) self.assertTrue(Spec('mpich2').satisfies(Spec('mpi'))) self.assertTrue(Spec('zmpi').satisfies(Spec('mpi'))) + def test_satisfies_virtual_dep_with_virtual_constraint(self): + """Ensure we can satisfy virtual constraints when there are multiple + vdep providers in the specs.""" + self.assertTrue(Spec('netlib-lapack ^openblas').satisfies('netlib-lapack ^openblas')) + self.assertFalse(Spec('netlib-lapack ^netlib-blas').satisfies('netlib-lapack ^openblas')) + + self.assertFalse(Spec('netlib-lapack ^openblas').satisfies('netlib-lapack ^netlib-blas')) + self.assertTrue(Spec('netlib-lapack ^netlib-blas').satisfies('netlib-lapack ^netlib-blas')) + + # ================================================================================ # Indexing specs # ================================================================================ @@ -327,4 +339,3 @@ class SpecSematicsTest(MockPackagesTest): self.check_constrain_not_changed('libelf^foo+debug', 'libelf^foo+debug') self.check_constrain_not_changed('libelf^foo~debug', 'libelf^foo~debug') self.check_constrain_not_changed('libelf^foo=bgqos_0', 'libelf^foo=bgqos_0') - diff --git a/lib/spack/spack/virtual.py b/lib/spack/spack/virtual.py index c77b259d61..f92cc4509c 100644 --- a/lib/spack/spack/virtual.py +++ b/lib/spack/spack/virtual.py @@ -117,12 +117,13 @@ class ProviderIndex(object): return sorted(providers) - # TODO: this is pretty darned nasty, and inefficient. + # TODO: this is pretty darned nasty, and inefficient, but there + # are not that many vdeps in most specs. def _cross_provider_maps(self, lmap, rmap): result = {} for lspec, rspec in itertools.product(lmap, rmap): try: - constrained = lspec.copy().constrain(rspec) + constrained = lspec.constrained(rspec) except spack.spec.UnsatisfiableSpecError: continue @@ -130,7 +131,7 @@ class ProviderIndex(object): for lp_spec, rp_spec in itertools.product(lmap[lspec], rmap[rspec]): if lp_spec.name == rp_spec.name: try: - const = lp_spec.copy().constrain(rp_spec,deps=False) + const = lp_spec.constrained(rp_spec, deps=False) result.setdefault(constrained, set()).add(const) except spack.spec.UnsatisfiableSpecError: continue @@ -157,4 +158,4 @@ class ProviderIndex(object): if crossed: result[name] = crossed - return bool(result) + return all(c in result for c in common) -- cgit v1.2.3-70-g09d2