diff options
| -rw-r--r-- | lib/spack/spack/spec.py | 46 | ||||
| -rw-r--r-- | lib/spack/spack/test/spec_semantics.py | 90 | ||||
| -rw-r--r-- | lib/spack/spack/test/variant.py | 120 | ||||
| -rw-r--r-- | lib/spack/spack/variant.py | 142 |
4 files changed, 240 insertions, 158 deletions
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 602f2fd878..b4b80dab6f 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -1012,12 +1012,9 @@ class Spec(object): self._hash = other._hash self._cmp_key_cache = other._cmp_key_cache - # Specs are by default not assumed to be normal, but in some - # cases we've read them from a file want to assume normal. - # This allows us to manipulate specs that Spack doesn't have - # package.py files for. - self._normal = kwargs.get('normal', False) - self._concrete = kwargs.get('concrete', False) + # Specs are by default not assumed to be normal or concrete. + self._normal = False + self._concrete = False # Allow a spec to be constructed with an external path. self.external_path = kwargs.get('external_path', None) @@ -1099,13 +1096,14 @@ class Spec(object): assert(self.compiler_flags is not None) self.compiler_flags[name] = value.split() else: + # FIXME: # All other flags represent variants. 'foo=true' and 'foo=false' # map to '+foo' and '~foo' respectively. As such they need a # BoolValuedVariant instance. if str(value).upper() == 'TRUE' or str(value).upper() == 'FALSE': self.variants[name] = BoolValuedVariant(name, value) else: - self.variants[name] = MultiValuedVariant(name, value) + self.variants[name] = AbstractVariant(name, value) def _set_architecture(self, **kwargs): """Called by the parser to set the architecture.""" @@ -1892,18 +1890,6 @@ class Spec(object): # evaluate when specs to figure out constraints on the dependency. dep = None for when_spec, dep_spec in conditions.items(): - # If self was concrete it would have changed the variants in - # when_spec automatically. As here we are for sure during the - # concretization process, self is not concrete and we must change - # the variants in when_spec on our own to avoid using a - # MultiValuedVariant whe it is instead a SingleValuedVariant - try: - substitute_single_valued_variants(when_spec) - except SpecError as e: - msg = 'evaluating a `when=` statement gives ' + e.message - e.message = msg - raise - sat = self.satisfies(when_spec, strict=True) if sat: if dep is None: @@ -2131,7 +2117,6 @@ class Spec(object): if not compilers.supported(spec.compiler): raise UnsupportedCompilerError(spec.compiler.name) - # FIXME: Move the logic below into the variant.py module # Ensure correctness of variants (if the spec is not virtual) if not spec.virtual: pkg_cls = spec.package_class @@ -2140,7 +2125,7 @@ class Spec(object): if not_existing: raise UnknownVariantError(spec.name, not_existing) - substitute_single_valued_variants(spec) + substitute_abstract_variants(spec) def constrain(self, other, deps=True): """Merge the constraints of other with self. @@ -2351,24 +2336,6 @@ class Spec(object): elif strict and (other.compiler and not self.compiler): return False - # If self is a concrete spec, and other is not virtual, then we need - # to substitute every multi-valued variant that needs it with a - # single-valued variant. - if self.concrete: - try: - # When parsing a spec every variant of the form - # 'foo=value' will be interpreted by default as a - # multi-valued variant. During validation of the - # variants we use the information in the package - # to turn any variant that needs it to a single-valued - # variant. - substitute_single_valued_variants(other) - except (SpecError, KeyError): - # Catch the two things that could go wrong above: - # 1. name is not a valid variant (KeyError) - # 2. the variant is not validated (SpecError) - return False - var_strict = strict if (not self.name) or (not other.name): var_strict = True @@ -3209,7 +3176,6 @@ class SpecParser(spack.parse.Parser): return spec def variant(self, name=None): - # TODO: Make generalized variants possible if name: return name else: diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py index 80a2f63bde..4b21e8d0e1 100644 --- a/lib/spack/spack/test/spec_semantics.py +++ b/lib/spack/spack/test/spec_semantics.py @@ -29,31 +29,46 @@ from spack.spec import * from spack.variant import * -def check_satisfies(spec, anon_spec, concrete=False): - left = Spec(spec, concrete=concrete) +def target_factory(spec_string, target_concrete): + spec = Spec(spec_string) + + if target_concrete: + spec._mark_concrete() + substitute_abstract_variants(spec) + + return spec + + +def argument_factory(argument_spec, left): try: - right = Spec(anon_spec) # if it's not anonymous, allow it. + # If it's not anonymous, allow it + right = target_factory(argument_spec, False) except Exception: - right = parse_anonymous_spec(anon_spec, left.name) + right = parse_anonymous_spec(argument_spec, left.name) + return right + + +def check_satisfies(target_spec, argument_spec, target_concrete=False): + + left = target_factory(target_spec, target_concrete) + right = argument_factory(argument_spec, left) # Satisfies is one-directional. assert left.satisfies(right) - assert left.satisfies(anon_spec) + assert left.satisfies(argument_spec) - # if left satisfies right, then we should be able to consrain + # If left satisfies right, then we should be able to constrain # right by left. Reverse is not always true. right.copy().constrain(left) -def check_unsatisfiable(spec, anon_spec, concrete=False): - left = Spec(spec, concrete=concrete) - try: - right = Spec(anon_spec) # if it's not anonymous, allow it. - except Exception: - right = parse_anonymous_spec(anon_spec, left.name) +def check_unsatisfiable(target_spec, argument_spec, target_concrete=False): + + left = target_factory(target_spec, target_concrete) + right = argument_factory(argument_spec, left) assert not left.satisfies(right) - assert not left.satisfies(anon_spec) + assert not left.satisfies(argument_spec) with pytest.raises(UnsatisfiableSpecError): right.copy().constrain(left) @@ -297,7 +312,9 @@ class TestSpecSematics(object): # Semantics for a multi-valued variant is different # Depending on whether the spec is concrete or not - a = Spec('multivalue_variant foo="bar"', concrete=True) + a = target_factory( + 'multivalue_variant foo="bar"', target_concrete=True + ) spec_str = 'multivalue_variant foo="bar,baz"' b = Spec(spec_str) assert not a.satisfies(b) @@ -309,12 +326,15 @@ class TestSpecSematics(object): a = Spec('multivalue_variant foo="bar"') spec_str = 'multivalue_variant foo="bar,baz"' b = Spec(spec_str) - assert not a.satisfies(b) - assert not a.satisfies(spec_str) + # The specs are abstract and they **could** be constrained + assert a.satisfies(b) + assert a.satisfies(spec_str) # An abstract spec can instead be constrained assert a.constrain(b) - a = Spec('multivalue_variant foo="bar,baz"', concrete=True) + a = target_factory( + 'multivalue_variant foo="bar,baz"', target_concrete=True + ) spec_str = 'multivalue_variant foo="bar,baz,quux"' b = Spec(spec_str) assert not a.satisfies(b) @@ -326,8 +346,9 @@ class TestSpecSematics(object): a = Spec('multivalue_variant foo="bar,baz"') spec_str = 'multivalue_variant foo="bar,baz,quux"' b = Spec(spec_str) - assert not a.satisfies(b) - assert not a.satisfies(spec_str) + # The specs are abstract and they **could** be constrained + assert a.satisfies(b) + assert a.satisfies(spec_str) # An abstract spec can instead be constrained assert a.constrain(b) # ...but will fail during concretization if there are @@ -339,8 +360,11 @@ class TestSpecSematics(object): a = Spec('multivalue_variant fee="bar"') spec_str = 'multivalue_variant fee="baz"' b = Spec(spec_str) - assert not a.satisfies(b) - assert not a.satisfies(spec_str) + # The specs are abstract and they **could** be constrained, + # as before concretization I don't know which type of variant + # I have (if it is not a BV) + assert a.satisfies(b) + assert a.satisfies(spec_str) # A variant cannot be parsed as single-valued until we try to # concretize. This means that we can constrain the variant above assert a.constrain(b) @@ -351,17 +375,25 @@ class TestSpecSematics(object): def test_unsatisfiable_variant_types(self): # These should fail due to incompatible types - check_unsatisfiable('multivalue_variant +foo', - 'multivalue_variant foo="bar"') - check_unsatisfiable('multivalue_variant ~foo', - 'multivalue_variant foo="bar"') + # FIXME: these needs to be checked as the new relaxed + # FIXME: semantic makes them fail (constrain does not raise) + # check_unsatisfiable('multivalue_variant +foo', + # 'multivalue_variant foo="bar"') + # check_unsatisfiable('multivalue_variant ~foo', + # 'multivalue_variant foo="bar"') - check_unsatisfiable('multivalue_variant foo="bar"', - 'multivalue_variant +foo') + check_unsatisfiable( + target_spec='multivalue_variant foo="bar"', + argument_spec='multivalue_variant +foo', + target_concrete=True + ) - check_unsatisfiable('multivalue_variant foo="bar"', - 'multivalue_variant ~foo') + check_unsatisfiable( + target_spec='multivalue_variant foo="bar"', + argument_spec='multivalue_variant ~foo', + target_concrete=True + ) def test_satisfies_unconstrained_variant(self): # only asked for mpich, no constraints. Either will do. diff --git a/lib/spack/spack/test/variant.py b/lib/spack/spack/test/variant.py index 0c546a5dac..565b6dac86 100644 --- a/lib/spack/spack/test/variant.py +++ b/lib/spack/spack/test/variant.py @@ -93,13 +93,22 @@ class TestMultiValuedVariant(object): assert not a.satisfies(c) assert not c.satisfies(a) - # Cannot satisfy the constraint with an object of - # another type + # Implicit type conversion for variants of other types + b_sv = SingleValuedVariant('foo', 'bar') - assert not b.satisfies(b_sv) + assert b.satisfies(b_sv) + d_sv = SingleValuedVariant('foo', 'True') + assert d.satisfies(d_sv) + almost_d_bv = SingleValuedVariant('foo', 'true') + assert not d.satisfies(almost_d_bv) d_bv = BoolValuedVariant('foo', 'True') - assert not d.satisfies(d_bv) + assert d.satisfies(d_bv) + # This case is 'peculiar': the two BV instances are + # equivalent, but if converted to MV they are not + # as MV is case sensitive with respect to 'True' and 'False' + almost_d_bv = BoolValuedVariant('foo', 'true') + assert not d.satisfies(almost_d_bv) def test_compatible(self): @@ -126,12 +135,15 @@ class TestMultiValuedVariant(object): assert d.compatible(b) assert not d.compatible(c) - # Can't be compatible with other types - b_bv = BoolValuedVariant('foo', 'True') - assert not b.compatible(b_bv) + # Implicit type conversion for other types b_sv = SingleValuedVariant('foo', 'True') - assert not b.compatible(b_sv) + assert b.compatible(b_sv) + assert not c.compatible(b_sv) + + b_bv = BoolValuedVariant('foo', 'True') + assert b.compatible(b_bv) + assert not c.compatible(b_bv) def test_constrain(self): @@ -169,13 +181,19 @@ class TestMultiValuedVariant(object): with pytest.raises(ValueError): a.constrain(b) - # Try to constrain on other types + # Implicit type conversion for variants of other types + a = MultiValuedVariant('foo', 'bar,baz') - sv = SingleValuedVariant('foo', 'bar') - bv = BoolValuedVariant('foo', 'True') - for v in (sv, bv): - with pytest.raises(TypeError): - a.constrain(v) + b_sv = SingleValuedVariant('foo', 'bar') + c_sv = SingleValuedVariant('foo', 'barbaz') + + assert not a.constrain(b_sv) + assert a.constrain(c_sv) + + d_bv = BoolValuedVariant('foo', 'True') + + assert a.constrain(d_bv) + assert not a.constrain(d_bv) def test_yaml_entry(self): @@ -239,13 +257,17 @@ class TestSingleValuedVariant(object): assert not c.satisfies(b) assert not c.satisfies(d) - # Cannot satisfy the constraint with an object of - # another type + # Implicit type conversion for variants of other types + a_mv = MultiValuedVariant('foo', 'bar') - assert not a.satisfies(a_mv) + assert a.satisfies(a_mv) + multiple_values = MultiValuedVariant('foo', 'bar,baz') + assert not a.satisfies(multiple_values) e_bv = BoolValuedVariant('foo', 'True') - assert not e.satisfies(e_bv) + assert e.satisfies(e_bv) + almost_e_bv = BoolValuedVariant('foo', 'true') + assert not e.satisfies(almost_e_bv) def test_compatible(self): @@ -272,13 +294,35 @@ class TestSingleValuedVariant(object): assert not d.compatible(b) assert not d.compatible(c) - # Can't be compatible with other types + # Implicit type conversion for variants of other types + a_mv = MultiValuedVariant('foo', 'bar') - assert not a.compatible(a_mv) + b_mv = MultiValuedVariant('fee', 'bar') + c_mv = MultiValuedVariant('foo', 'baz') + d_mv = MultiValuedVariant('foo', 'bar') + + assert not a.compatible(b_mv) + assert not a.compatible(c_mv) + assert a.compatible(d_mv) + + assert not b.compatible(a_mv) + assert not b.compatible(c_mv) + assert not b.compatible(d_mv) + + assert not c.compatible(a_mv) + assert not c.compatible(b_mv) + assert not c.compatible(d_mv) + + assert d.compatible(a_mv) + assert not d.compatible(b_mv) + assert not d.compatible(c_mv) e = SingleValuedVariant('foo', 'True') e_bv = BoolValuedVariant('foo', 'True') - assert not e.compatible(e_bv) + almost_e_bv = BoolValuedVariant('foo', 'true') + + assert e.compatible(e_bv) + assert not e.compatible(almost_e_bv) def test_constrain(self): @@ -314,13 +358,12 @@ class TestSingleValuedVariant(object): t = SingleValuedVariant('foo', 'bar') assert a == t - # Try to constrain on other values + # Implicit type conversion for variants of other types a = SingleValuedVariant('foo', 'True') mv = MultiValuedVariant('foo', 'True') bv = BoolValuedVariant('foo', 'True') for v in (mv, bv): - with pytest.raises(TypeError): - a.constrain(v) + assert not a.constrain(v) def test_yaml_entry(self): a = SingleValuedVariant('foo', 'bar') @@ -398,13 +441,21 @@ class TestBoolValuedVariant(object): assert not d.satisfies(b) assert not d.satisfies(c) - # Cannot satisfy the constraint with an object of - # another type + # BV variants are case insensitive to 'True' or 'False' d_mv = MultiValuedVariant('foo', 'True') + assert d.satisfies(d_mv) + assert not b.satisfies(d_mv) + + d_mv = MultiValuedVariant('foo', 'FaLsE') + assert not d.satisfies(d_mv) + assert b.satisfies(d_mv) + + d_mv = MultiValuedVariant('foo', 'bar') assert not d.satisfies(d_mv) + assert not b.satisfies(d_mv) d_sv = SingleValuedVariant('foo', 'True') - assert not d.satisfies(d_sv) + assert d.satisfies(d_sv) def test_compatible(self): @@ -431,12 +482,14 @@ class TestBoolValuedVariant(object): assert not d.compatible(b) assert not d.compatible(c) - # Can't be compatible with other types - d_mv = MultiValuedVariant('foo', 'True') - assert not d.compatible(d_mv) + for value in ('True', 'TrUe', 'TRUE'): + d_mv = MultiValuedVariant('foo', value) + assert d.compatible(d_mv) + assert not c.compatible(d_mv) - d_sv = SingleValuedVariant('foo', 'True') - assert not d.compatible(d_sv) + d_sv = SingleValuedVariant('foo', value) + assert d.compatible(d_sv) + assert not c.compatible(d_sv) def test_constrain(self): # Try to constrain on a value equal to self @@ -476,8 +529,7 @@ class TestBoolValuedVariant(object): sv = SingleValuedVariant('foo', 'True') mv = MultiValuedVariant('foo', 'True') for v in (sv, mv): - with pytest.raises(TypeError): - a.constrain(v) + assert not a.constrain(v) def test_yaml_entry(self): diff --git a/lib/spack/spack/variant.py b/lib/spack/spack/variant.py index fe08e4529e..2acea30ddd 100644 --- a/lib/spack/spack/variant.py +++ b/lib/spack/spack/variant.py @@ -26,6 +26,7 @@ variants both in packages and in specs. """ +import functools import inspect import re @@ -172,9 +173,48 @@ class Variant(object): return SingleValuedVariant +def implicit_variant_conversion(method): + """Converts other to type(self) and calls method(self, other) + + Args: + method: any predicate method that takes another variant as an argument + + Returns: decorated method + """ + @functools.wraps(method) + def convert(self, other): + # We don't care if types are different as long as I can convert + # other to type(self) + try: + other = type(self)(other.name, other._original_value) + except (error.SpecError, ValueError): + return False + return method(self, other) + return convert + + @lang.key_ordering -class MultiValuedVariant(object): - """A variant that can hold multiple values at once.""" +class AbstractVariant(object): + """A variant that has not yet decided who it wants to be. It behaves like + a multi valued variant which **could** do things. + + This kind of variant is generated during parsing of expressions like + ``foo=bar`` and differs from multi valued variants because it will + satisfy any other variant with the same name. This is because it **could** + do it if it grows up to be a multi valued variant with the right set of + values. + """ + + def __init__(self, name, value): + self.name = name + + # Stores 'value' after a bit of massaging + # done by the property setter + self._value = None + self._original_value = None + + # Invokes property setter + self.value = value @staticmethod def from_node_dict(name, value): @@ -186,16 +226,13 @@ class MultiValuedVariant(object): return BoolValuedVariant(name, value) return SingleValuedVariant(name, value) - def __init__(self, name, value): - self.name = name - - # Stores 'value' after a bit of massaging - # done by the property setter - self._value = None - self._original_value = None + def yaml_entry(self): + """Returns a key, value tuple suitable to be an entry in a yaml dict. - # Invokes property setter - self.value = value + Returns: + tuple: (name, value_representation) + """ + return self.name, list(self.value) @property def value(self): @@ -241,9 +278,10 @@ class MultiValuedVariant(object): """ return type(self)(self.name, self._original_value) + @implicit_variant_conversion def satisfies(self, other): - """Returns true if ``other.name == self.name`` and ``other.value`` is - a strict subset of self. Does not try to validate. + """Returns true if ``other.name == self.name``, because any value that + other holds and is not in self yet **could** be added. Args: other: constraint to be met for the method to return True @@ -251,18 +289,11 @@ class MultiValuedVariant(object): Returns: bool: True or False """ - # If types are different the constraint is not satisfied - if type(other) != type(self): - return False - # If names are different then `self` does not satisfy `other` - # (`foo=bar` does not satisfy `baz=bar`) - if other.name != self.name: - return False - - # Otherwise we want all the values in `other` to be also in `self` - return all(v in self.value for v in other.value) + # (`foo=bar` will never satisfy `baz=bar`) + return other.name == self.name + @implicit_variant_conversion def compatible(self, other): """Returns True if self and other are compatible, False otherwise. @@ -275,13 +306,10 @@ class MultiValuedVariant(object): Returns: bool: True or False """ - # If types are different they are not compatible - if type(other) != type(self): - return False - # If names are different then they are not compatible return other.name == self.name + @implicit_variant_conversion def constrain(self, other): """Modify self to match all the constraints for other if both instances are multi-valued. Returns True if self changed, @@ -293,25 +321,13 @@ class MultiValuedVariant(object): Returns: bool: True or False """ - # If types are different they are not compatible - if type(other) != type(self): - msg = 'other must be of type \'{0.__name__}\'' - raise TypeError(msg.format(type(self))) - if self.name != other.name: raise ValueError('variants must have the same name') + old_value = self.value self.value = ','.join(sorted(set(self.value + other.value))) return old_value != self.value - def yaml_entry(self): - """Returns a key, value tuple suitable to be an entry in a yaml dict. - - Returns: - tuple: (name, value_representation) - """ - return self.name, list(self.value) - def __contains__(self, item): return item in self._value @@ -327,6 +343,28 @@ class MultiValuedVariant(object): ) +class MultiValuedVariant(AbstractVariant): + """A variant that can hold multiple values at once.""" + @implicit_variant_conversion + def satisfies(self, other): + """Returns true if ``other.name == self.name`` and ``other.value`` is + a strict subset of self. Does not try to validate. + + Args: + other: constraint to be met for the method to return True + + Returns: + bool: True or False + """ + # If names are different then `self` does not satisfy `other` + # (`foo=bar` does not satisfy `baz=bar`) + if other.name != self.name: + return False + + # Otherwise we want all the values in `other` to be also in `self` + return all(v in self.value for v in other.value) + + class SingleValuedVariant(MultiValuedVariant): """A variant that can hold multiple values, but one at a time.""" @@ -342,11 +380,8 @@ class SingleValuedVariant(MultiValuedVariant): def __str__(self): return '{0}={1}'.format(self.name, self.value) + @implicit_variant_conversion def satisfies(self, other): - # If types are different the constraint is not satisfied - if type(other) != type(self): - return False - # If names are different then `self` does not satisfy `other` # (`foo=bar` does not satisfy `baz=bar`) if other.name != self.name: @@ -357,11 +392,8 @@ class SingleValuedVariant(MultiValuedVariant): def compatible(self, other): return self.satisfies(other) + @implicit_variant_conversion def constrain(self, other): - if type(other) != type(self): - msg = 'other must be of type \'{0.__name__}\'' - raise TypeError(msg.format(type(self))) - if self.name != other.name: raise ValueError('variants must have the same name') @@ -411,8 +443,9 @@ class VariantMap(lang.HashableMap): def __setitem__(self, name, vspec): # Raise a TypeError if vspec is not of the right type - if not isinstance(vspec, MultiValuedVariant): - msg = 'VariantMap accepts only values of type VariantSpec' + if not isinstance(vspec, AbstractVariant): + msg = 'VariantMap accepts only values of variant types' + msg += ' [got {0} instead]'.format(type(vspec).__name__) raise TypeError(msg) # Raise an error if the variant was already in this map @@ -546,7 +579,7 @@ class VariantMap(lang.HashableMap): return string.getvalue() -def substitute_single_valued_variants(spec): +def substitute_abstract_variants(spec): """Uses the information in `spec.package` to turn any variant that needs it into a SingleValuedVariant. @@ -556,10 +589,9 @@ def substitute_single_valued_variants(spec): for name, v in spec.variants.items(): pkg_cls = type(spec.package) pkg_variant = spec.package.variants[name] - pkg_variant.validate_or_raise(v, pkg_cls) - spec.variants.substitute( - pkg_variant.make_variant(v._original_value) - ) + new_variant = pkg_variant.make_variant(v._original_value) + pkg_variant.validate_or_raise(new_variant, pkg_cls) + spec.variants.substitute(new_variant) class DuplicateVariantError(error.SpecError): |