SPACK-38: Allow specs to be indexed by virtual dependencies.

- The following now work differently:

      spec['mpi']
      spec['blas']

  This can return a spec for openmpi, mpich, mvapich, etc., EVEN if
  the spec is already concretized.  This means that in a package that
  `depends_on('mpi')`, you can do `spec['mpi']` to see what it was
  concretized to.  This should simplify MPI and BLAS packages.

      'mpi' in spec
      'blas' in spec

  Previously, if the spec had been concretized, these would be `False`
  because there was not a dependency in the DAG with either of these
  names. These will now be `True` even if the spec has been
  concretized.  So, e.g., this will print "YES"

      s = Spec('callpath ^mpich')
      if 'mpi' in spec:
          print "YES"

- Similarly, this will be True:

      Spec('mpich').satisfies('mpi')

- Because of the way virtual dependencies are currently implemented,
  the above required some fiddling around with `package.py` so that it
  would never call `Spec.__contains__` (and result in endless
  recursion).

- This should be fixed by allowing virutal dependnecies to have their
  own package class.
  - This would allow a quicker check for vdeps, without a call to
    `all_packages`.
  - For the time being, `package.py` shouldn't call `__contains__`

4 files changed, 121 insertions, 17 deletions

diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index e3d766f582..5abf2a6bb3 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -471,13 +471,18 @@ class Package(object):
         """Spec of the extendee of this package, or None if it is not an extension."""
         if not self.extendees:
             return None
+
+        # TODO: allow more than one extendee.
         name = next(iter(self.extendees))
-        if not name in self.spec:
-            spec, kwargs = self.extendees[name]
-            return spec
 
-        # Need to do this to get the concrete version of the spec
-        return self.spec[name]
+        # If the extendee is in the spec's deps already, return that.
+        for dep in self.spec.traverse():
+            if name == dep.name:
+                return dep
+
+        # Otherwise return the spec from the extends() directive
+        spec, kwargs = self.extendees[name]
+        return spec
 
 
     @property
@@ -542,7 +547,7 @@ class Package(object):
 
     def provides(self, vpkg_name):
         """True if this package provides a virtual package with the specified name."""
-        return vpkg_name in self.provided
+        return any(s.name == vpkg_name for s in self.provided)
 
 
     def virtual_dependencies(self, visited=None):
@@ -561,8 +566,11 @@ class Package(object):
            on this one."""
         dependents = []
         for spec in spack.db.installed_package_specs():
-            if self.name != spec.name and self.spec in spec:
-                dependents.append(spec)
+            if self.name == spec.name:
+                continue
+            for dep in spec.traverse():
+                if spec == dep:
+                    dependents.append(spec)
         return dependents
 
 
@@ -985,10 +993,13 @@ class Package(object):
 
             activated = spack.install_layout.extension_map(self.extendee_spec)
             for name, aspec in activated.items():
-                if aspec != self.spec and self.spec in aspec:
-                    raise ActivationError(
-                        "Cannot deactivate %s beacuse %s is activated and depends on it."
-                        % (self.spec.short_spec, aspec.short_spec))
+                if aspec == self.spec:
+                    continue
+                for dep in aspec.traverse():
+                    if self.spec == dep:
+                        raise ActivationError(
+                            "Cannot deactivate %s beacuse %s is activated and depends on it."
+                            % (self.spec.short_spec, aspec.short_spec))
 
         self.extendee_spec.package.deactivate(self, **self.extendee_args)
 
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index aa13f0422c..5876fc6cf8 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -498,7 +498,13 @@ class Spec(object):
            Possible idea: just use conventin and make virtual deps all
            caps, e.g., MPI vs mpi.
         """
-        return not spack.db.exists(self.name)
+        return Spec.is_virtual(self.name)
+
+
+    @staticmethod
+    def is_virtual(name):
+        """Test if a name is virtual without requiring a Spec."""
+        return not spack.db.exists(name)
 
 
     @property
@@ -1224,7 +1230,17 @@ class Spec(object):
         """
         other = self._autospec(other)
 
-        # First thing we care about is whether the name matches
+        # A concrete provider can satisfy a virtual dependency.
+        if not self.virtual and other.virtual:
+            pkg = spack.db.get(self.name)
+            if pkg.provides(other.name):
+                for provided, when_spec in pkg.provided.items():
+                    if self.satisfies(when_spec, deps=False, strict=strict):
+                        if provided.satisfies(other):
+                            return True
+            return False
+
+        # Otherwise, first thing we care about is whether the name matches
         if self.name != other.name:
             return False
 
@@ -1364,11 +1380,21 @@ class Spec(object):
 
 
     def __getitem__(self, name):
-        """TODO: reconcile __getitem__, _add_dependency, __contains__"""
+        """Get a dependency from the spec by its name."""
         for spec in self.traverse():
             if spec.name == name:
                 return spec
 
+        if Spec.is_virtual(name):
+            # TODO: this is a kind of kludgy way to find providers
+            # TODO: should we just keep virtual deps in the DAG instead of
+            # TODO: removing them on concretize?
+            for spec in self.traverse():
+                if spec.virtual:
+                    continue
+                if spec.package.provides(name):
+                    return spec
+
         raise KeyError("No spec with name %s in %s" % (name, self))
 
 
@@ -1380,6 +1406,7 @@ class Spec(object):
         for s in self.traverse():
             if s.satisfies(spec, strict=True):
                 return True
+
         return False
 
 
diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py
index cc839a2340..b3a77d076a 100644
--- a/lib/spack/spack/test/concretize.py
+++ b/lib/spack/spack/test/concretize.py
@@ -152,7 +152,10 @@ class ConcretizeTest(MockPackagesTest):
         spec.concretize()
 
         self.assertTrue('zmpi' in spec.dependencies)
-        self.assertFalse('mpi' in spec)
+        self.assertTrue(all(not 'mpi' in d.dependencies for d in spec.traverse()))
+        self.assertTrue('zmpi' in spec)
+        self.assertTrue('mpi' in spec)
+
         self.assertTrue('fake' in spec.dependencies['zmpi'])
 
 
@@ -168,7 +171,9 @@ class ConcretizeTest(MockPackagesTest):
         self.assertTrue('zmpi' in spec.dependencies['callpath'].dependencies)
         self.assertTrue('fake' in spec.dependencies['callpath'].dependencies['zmpi'].dependencies)
 
-        self.assertFalse('mpi' in spec)
+        self.assertTrue(all(not 'mpi' in d.dependencies for d in spec.traverse()))
+        self.assertTrue('zmpi' in spec)
+        self.assertTrue('mpi' in spec)
 
 
     def test_my_dep_depends_on_provider_of_my_virtual_dep(self):
diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py
index 20df2603f5..6666dbbb52 100644
--- a/lib/spack/spack/test/spec_semantics.py
+++ b/lib/spack/spack/test/spec_semantics.py
@@ -189,6 +189,67 @@ class SpecSematicsTest(MockPackagesTest):
         self.check_unsatisfiable('mpich+foo', 'mpich~foo')
 
 
+    def test_satisfies_virtual(self):
+        self.assertTrue(Spec('mpich').satisfies(Spec('mpi')))
+        self.assertTrue(Spec('mpich2').satisfies(Spec('mpi')))
+        self.assertTrue(Spec('zmpi').satisfies(Spec('mpi')))
+
+
+    # ================================================================================
+    # Indexing specs
+    # ================================================================================
+    def test_self_index(self):
+        s = Spec('callpath')
+        self.assertTrue(s['callpath'] == s)
+
+
+    def test_dep_index(self):
+        s = Spec('callpath')
+        s.normalize()
+
+        self.assertTrue(s['callpath'] == s)
+        self.assertTrue(type(s['dyninst']) == Spec)
+        self.assertTrue(type(s['libdwarf']) == Spec)
+        self.assertTrue(type(s['libelf']) == Spec)
+        self.assertTrue(type(s['mpi']) == Spec)
+
+        self.assertTrue(s['dyninst'].name  == 'dyninst')
+        self.assertTrue(s['libdwarf'].name == 'libdwarf')
+        self.assertTrue(s['libelf'].name   == 'libelf')
+        self.assertTrue(s['mpi'].name      == 'mpi')
+
+
+    def test_spec_contains_deps(self):
+        s = Spec('callpath')
+        s.normalize()
+        self.assertTrue('dyninst' in s)
+        self.assertTrue('libdwarf' in s)
+        self.assertTrue('libelf' in s)
+        self.assertTrue('mpi' in s)
+
+
+    def test_virtual_index(self):
+        s = Spec('callpath')
+        s.concretize()
+
+        s_mpich = Spec('callpath ^mpich')
+        s_mpich.concretize()
+
+        s_mpich2 = Spec('callpath ^mpich2')
+        s_mpich2.concretize()
+
+        s_zmpi = Spec('callpath ^zmpi')
+        s_zmpi.concretize()
+
+
+        self.assertTrue(s['mpi'].name != 'mpi')
+        self.assertTrue(s_mpich['mpi'].name == 'mpich')
+        self.assertTrue(s_mpich2['mpi'].name == 'mpich2')
+        self.assertTrue(s_zmpi['zmpi'].name == 'zmpi')
+
+        for spec in [s, s_mpich, s_mpich2, s_zmpi]:
+            self.assertTrue('mpi' in spec)
+
 
     # ================================================================================
     # Constraints