diff options
author | Todd Gamblin <tgamblin@llnl.gov> | 2015-04-27 00:10:40 -0700 |
---|---|---|
committer | Todd Gamblin <tgamblin@llnl.gov> | 2015-05-10 12:25:07 -0700 |
commit | 535c1fac87cc2323d2ac6ed6db35bfac78ad6a35 (patch) | |
tree | 0be34b7e1de80beaf1b9afd10984bf33b2f2608d | |
parent | 3b1898b8e479fc1e7d9b71a57f625f36485b1ac0 (diff) | |
download | spack-535c1fac87cc2323d2ac6ed6db35bfac78ad6a35.tar.gz spack-535c1fac87cc2323d2ac6ed6db35bfac78ad6a35.tar.bz2 spack-535c1fac87cc2323d2ac6ed6db35bfac78ad6a35.tar.xz spack-535c1fac87cc2323d2ac6ed6db35bfac78ad6a35.zip |
SPACK-56: fix Variant concretization.
- Variant concretization is tricky:
- During concretization, a spec without variants (e.g., mpich) means
"don't care". So, Spec('mpich').satisfies('mpich+debug') is true
because it *could* still be built that way.
- After concretization, a spec without a particular variant means
"don't know", as that wasn't part of the spec, so the opposite
relationship is true. Assume 'spec' is already installed:
spec.satisfies('mpich+debug')
this is false beacuse the `debug` variant didn't exist when spec
was built, so we can't satisfy the explicit request for +debug.
-rw-r--r-- | lib/spack/spack/concretize.py | 10 | ||||
-rw-r--r-- | lib/spack/spack/spec.py | 40 | ||||
-rw-r--r-- | lib/spack/spack/test/spec_dag.py | 6 | ||||
-rw-r--r-- | lib/spack/spack/test/spec_semantics.py | 38 | ||||
-rw-r--r-- | lib/spack/spack/variant.py | 36 |
5 files changed, 107 insertions, 23 deletions
diff --git a/lib/spack/spack/concretize.py b/lib/spack/spack/concretize.py index 3f569f9dce..15e886ad3c 100644 --- a/lib/spack/spack/concretize.py +++ b/lib/spack/spack/concretize.py @@ -101,6 +101,16 @@ class DefaultConcretizer(object): spec.architecture = spack.architecture.sys_type() + def concretize_variants(self, spec): + """If the spec already has variants filled in, return. Otherwise, add + the default variants from the package specification. + """ + for name, variant in spec.package.variants.items(): + if name not in spec.variants: + spec.variants[name] = spack.spec.VariantSpec( + name, variant.default) + + def concretize_compiler(self, spec): """If the spec already has a compiler, we're done. If not, then take the compiler used for the nearest ancestor with a compiler diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index fca14f97db..4639aea452 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -270,7 +270,7 @@ class CompilerSpec(object): @key_ordering -class Variant(object): +class VariantSpec(object): """Variants are named, build-time options for a package. Names depend on the particular package being built, and each named variant can be enabled or disabled. @@ -285,7 +285,7 @@ class Variant(object): def copy(self): - return Variant(self.name, self.enabled) + return VariantSpec(self.name, self.enabled) def __str__(self): @@ -294,9 +294,27 @@ class Variant(object): class VariantMap(HashableMap): - def satisfies(self, other): - return all(self[key].enabled == other[key].enabled - for key in other if key in self) + def satisfies(self, other, self_is_concrete): + if self_is_concrete: + return all(k in self and self[k].enabled == other[k].enabled + for k in other) + else: + return all(self[k].enabled == other[k].enabled + for k in other if k in self) + + + def constrain(self, other, other_is_concrete): + if other_is_concrete: + for k in self: + if k not in other: + raise UnsatisfiableVariantSpecError(self[k], '<absent>') + + for k in other: + if k in self: + if self[k].enabled != other[k].enabled: + raise UnsatisfiableVariantSpecError(self[k], other[k]) + else: + self[k] = other[k].copy() def __str__(self): @@ -375,7 +393,7 @@ class Spec(object): """Called by the parser to add a variant.""" if name in self.variants: raise DuplicateVariantError( "Cannot specify variant '%s' twice" % name) - self.variants[name] = Variant(name, enabled) + self.variants[name] = VariantSpec(name, enabled) def _set_compiler(self, compiler): @@ -607,6 +625,7 @@ class Spec(object): spack.concretizer.concretize_architecture(self) spack.concretizer.concretize_compiler(self) spack.concretizer.concretize_version(self) + spack.concretizer.concretize_variants(self) presets[self.name] = self visited.add(self.name) @@ -789,8 +808,7 @@ class Spec(object): else: required = index.providers_for(vspec.name) if required: - raise UnsatisfiableProviderSpecError( - required[0], pkg_dep) + raise UnsatisfiableProviderSpecError(required[0], pkg_dep) provider_index.update(pkg_dep) if name not in spec_deps: @@ -929,7 +947,7 @@ class Spec(object): self.compiler = other.compiler self.versions.intersect(other.versions) - self.variants.update(other.variants) + self.variants.constrain(other.variants, other._concrete) self.architecture = self.architecture or other.architecture if constrain_deps: @@ -998,11 +1016,13 @@ class Spec(object): # All these attrs have satisfies criteria of their own, # but can be None to indicate no constraints. for s, o in ((self.versions, other.versions), - (self.variants, other.variants), (self.compiler, other.compiler)): if s and o and not s.satisfies(o): return False + if not self.variants.satisfies(other.variants, self._concrete): + return False + # Architecture satisfaction is currently just string equality. # Can be None for unconstrained, though. if (self.architecture and other.architecture and diff --git a/lib/spack/spack/test/spec_dag.py b/lib/spack/spack/test/spec_dag.py index fb67aa8a8d..ecbc46981c 100644 --- a/lib/spack/spack/test/spec_dag.py +++ b/lib/spack/spack/test/spec_dag.py @@ -242,12 +242,6 @@ class SpecDagTest(MockPackagesTest): self.assertRaises(spack.spec.UnsatisfiableCompilerSpecError, spec.normalize) - def test_unsatisfiable_variant(self): - set_pkg_dep('mpileaks', 'mpich+debug') - spec = Spec('mpileaks ^mpich~debug ^callpath ^dyninst ^libelf ^libdwarf') - self.assertRaises(spack.spec.UnsatisfiableVariantSpecError, spec.normalize) - - def test_unsatisfiable_architecture(self): set_pkg_dep('mpileaks', 'mpich=bgqos_0') spec = Spec('mpileaks ^mpich=sles_10_ppc64 ^callpath ^dyninst ^libelf ^libdwarf') diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py index 1db7956f04..8614b74c7a 100644 --- a/lib/spack/spack/test/spec_semantics.py +++ b/lib/spack/spack/test/spec_semantics.py @@ -33,8 +33,8 @@ class SpecSematicsTest(MockPackagesTest): # ================================================================================ # Utility functions to set everything up. # ================================================================================ - def check_satisfies(self, spec, anon_spec): - left = Spec(spec) + def check_satisfies(self, spec, anon_spec, concrete=False): + left = Spec(spec, concrete=concrete) right = parse_anonymous_spec(anon_spec, left.name) # Satisfies is one-directional. @@ -46,8 +46,8 @@ class SpecSematicsTest(MockPackagesTest): right.copy().constrain(left) - def check_unsatisfiable(self, spec, anon_spec): - left = Spec(spec) + def check_unsatisfiable(self, spec, anon_spec, concrete=False): + left = Spec(spec, concrete=concrete) right = parse_anonymous_spec(anon_spec, left.name) self.assertFalse(left.satisfies(right)) @@ -150,9 +150,33 @@ class SpecSematicsTest(MockPackagesTest): self.check_unsatisfiable('mpileaks^mpi@3:', '^mpich@1.0') - def test_satisfies_variant(self): - self.check_satisfies('foo %gcc@4.7.3', '%gcc@4.7') - self.check_unsatisfiable('foo %gcc@4.7', '%gcc@4.7.3') + def test_satisfies_matching_variant(self): + self.check_satisfies('mpich+foo', 'mpich+foo') + self.check_satisfies('mpich~foo', 'mpich~foo') + + + def test_satisfies_unconstrained_variant(self): + # only asked for mpich, no constraints. Either will do. + self.check_satisfies('mpich+foo', 'mpich') + self.check_satisfies('mpich~foo', 'mpich') + + + def test_unsatisfiable_variants(self): + # This case is different depending on whether the specs are concrete. + + # 'mpich' is not concrete: + self.check_satisfies('mpich', 'mpich+foo', False) + self.check_satisfies('mpich', 'mpich~foo', False) + + # 'mpich' is concrete: + self.check_unsatisfiable('mpich', 'mpich+foo', True) + self.check_unsatisfiable('mpich', 'mpich~foo', True) + + + def test_unsatisfiable_variant_mismatch(self): + # No matchi in specs + self.check_unsatisfiable('mpich~foo', 'mpich+foo') + self.check_unsatisfiable('mpich+foo', 'mpich~foo') diff --git a/lib/spack/spack/variant.py b/lib/spack/spack/variant.py new file mode 100644 index 0000000000..3d3e2b0f6d --- /dev/null +++ b/lib/spack/spack/variant.py @@ -0,0 +1,36 @@ +############################################################################## +# Copyright (c) 2013, Lawrence Livermore National Security, LLC. +# Produced at the Lawrence Livermore National Laboratory. +# +# This file is part of Spack. +# Written by Todd Gamblin, tgamblin@llnl.gov, All rights reserved. +# LLNL-CODE-647188 +# +# For details, see https://scalability-llnl.github.io/spack +# Please also see the LICENSE file for our notice and the LGPL. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License (as published by +# the Free Software Foundation) version 2.1 dated February 1999. +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the IMPLIED WARRANTY OF +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the terms and +# conditions of the GNU General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with this program; if not, write to the Free Software Foundation, +# Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +############################################################################## +"""Variant is a class describing flags on builds, or "variants". + +Could be generalized later to describe aribitrary parameters, but +currently variants are just flags. + +""" + +class Variant(object): + """Represents a variant on a build. Can be either on or off.""" + def __init__(self, default, description): + self.default = bool(default) + self.description = str(description) |