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()`

3 files changed, 24 insertions, 5 deletions

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)