From 0fc3b58890ddb18c4a2384c6f015c8f9417a1c01 Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Sun, 7 Jun 2015 15:21:31 -0700 Subject: [PATCH] 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__` --- lib/spack/spack/package.py | 37 ++++++++++------ lib/spack/spack/spec.py | 33 ++++++++++++-- lib/spack/spack/test/concretize.py | 9 +++- lib/spack/spack/test/spec_semantics.py | 61 ++++++++++++++++++++++++++ 4 files changed, 122 insertions(+), 18 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 @@ def extendee_spec(self): """Spec of the extendee of this package, or None if it is not an extension.""" if not self.extendees: return None - 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] + # TODO: allow more than one extendee. + name = next(iter(self.extendees)) + + # 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 @@ def preorder_traversal(self, visited=None, **kwargs): 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 @@ def installed_dependents(self): 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 @@ def do_deactivate(self, **kwargs): 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 @@ def virtual(self): 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 @@ def satisfies(self, other, deps=True, strict=False): """ 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 @@ def version(self): 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 @@ def __contains__(self, spec): 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 @@ def test_virtual_is_fully_expanded_for_callpath(self): 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 @@ def test_virtual_is_fully_expanded_for_mpileaks(self): 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 @@ def test_unsatisfiable_variant_mismatch(self): 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