From a5faf7d27a3ef1afca28e5d2348130f580be898d Mon Sep 17 00:00:00 2001 From: Greg Becker Date: Tue, 20 Oct 2020 23:05:29 -0700 Subject: Change 'any' to wildcard for variants (#19381) --- lib/spack/spack/concretize.py | 25 ++-------- lib/spack/spack/environment.py | 10 ++-- lib/spack/spack/test/cmd/dev_build.py | 2 +- lib/spack/spack/test/cmd/undevelop.py | 4 +- lib/spack/spack/test/concretize_preferences.py | 15 ++---- lib/spack/spack/test/spec_semantics.py | 30 +++++------- lib/spack/spack/variant.py | 66 +++++++++----------------- 7 files changed, 49 insertions(+), 103 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/concretize.py b/lib/spack/spack/concretize.py index 928d35b956..eb09cce3a7 100644 --- a/lib/spack/spack/concretize.py +++ b/lib/spack/spack/concretize.py @@ -352,18 +352,13 @@ class Concretizer(object): preferred_variants = PackagePrefs.preferred_variants(spec.name) pkg_cls = spec.package_class for name, variant in pkg_cls.variants.items(): - any_set = False var = spec.variants.get(name, None) - if var and 'any' in var: - # remove 'any' variant before concretizing - # 'any' cannot be combined with other variables in a + if var and '*' in var: + # remove variant wildcard before concretizing + # wildcard cannot be combined with other variables in a # multivalue variant, a concrete variant cannot have the value - # 'any', and 'any' does not constrain a variant except to - # preclude the values 'none' and None. We track `any_set` to - # avoid replacing 'any' with None, and remove it to continue - # concretization. + # wildcard, and a wildcard does not constrain a variant spec.variants.pop(name) - any_set = True if name not in spec.variants: changed = True if name in preferred_variants: @@ -371,14 +366,6 @@ class Concretizer(object): else: spec.variants[name] = variant.make_default() - var = spec.variants[name] - if any_set and 'none' in var or None in var: - msg = "Attempted non-deterministic setting of variant" - msg += " '%s' set to 'any' and preference is." % name - msg += "'%s'. Set the variant to a non 'any'" % var.value - msg += " value or set a preference for variant '%s'." % name - raise NonDeterministicVariantError(msg) - return changed def concretize_compiler(self, spec): @@ -805,7 +792,3 @@ class NoBuildError(spack.error.SpackError): msg = ("The spec\n '%s'\n is configured as not buildable, " "and no matching external installs were found") super(NoBuildError, self).__init__(msg % spec) - - -class NonDeterministicVariantError(spack.error.SpecError): - """Raised when a spec variant is set to 'any' and concretizes to 'none'.""" diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py index e656e5e368..42b4cb819a 100644 --- a/lib/spack/spack/environment.py +++ b/lib/spack/spack/environment.py @@ -1336,21 +1336,21 @@ class Environment(object): # if spec and all deps aren't dev builds, we don't need to overwrite it if not any(spec.satisfies(c) - for c in ('dev_path=any', '^dev_path=any')): + for c in ('dev_path=*', '^dev_path=*')): return False # if any dep needs overwrite, or any dep is missing and is a dev build # then overwrite this package if any( self._spec_needs_overwrite(dep) or - ((not dep.package.installed) and dep.satisfies('dev_path=any')) + ((not dep.package.installed) and dep.satisfies('dev_path=*')) for dep in spec.traverse(root=False) ): return True # if it's not a direct dev build and its dependencies haven't # changed, it hasn't changed. - # We don't merely check satisfaction (spec.satisfies('dev_path=any') + # We don't merely check satisfaction (spec.satisfies('dev_path=*') # because we need the value of the variant in the next block of code dev_path_var = spec.variants.get('dev_path', None) if not dev_path_var: @@ -1420,8 +1420,8 @@ class Environment(object): for concretized_hash in self.concretized_order: spec = self.specs_by_hash[concretized_hash] if not spec.package.installed or ( - spec.satisfies('dev_path=any') or - spec.satisfies('^dev_path=any') + spec.satisfies('dev_path=*') or + spec.satisfies('^dev_path=*') ): # If it's a dev build it could need to be reinstalled specs_to_install.append(spec) diff --git a/lib/spack/spack/test/cmd/dev_build.py b/lib/spack/spack/test/cmd/dev_build.py index 2739bf1524..111e500dcd 100644 --- a/lib/spack/spack/test/cmd/dev_build.py +++ b/lib/spack/spack/test/cmd/dev_build.py @@ -333,7 +333,7 @@ env: # Ensure variants set properly for dep in (dep_spec, spec['dev-build-test-install']): assert dep.satisfies('dev_path=%s' % build_dir) - assert spec.satisfies('^dev_path=any') + assert spec.satisfies('^dev_path=*') @pytest.mark.parametrize('test_spec', ['dev-build-test-install', diff --git a/lib/spack/spack/test/cmd/undevelop.py b/lib/spack/spack/test/cmd/undevelop.py index 2ebc554f29..d44330fe76 100644 --- a/lib/spack/spack/test/cmd/undevelop.py +++ b/lib/spack/spack/test/cmd/undevelop.py @@ -35,8 +35,8 @@ env: after = spack.spec.Spec('mpich').concretized() # Removing dev spec from environment changes concretization - assert before.satisfies('dev_path=any') - assert not after.satisfies('dev_path=any') + assert before.satisfies('dev_path=*') + assert not after.satisfies('dev_path=*') def test_undevelop_nonexistent(tmpdir, mock_packages, mutable_mock_env_path): diff --git a/lib/spack/spack/test/concretize_preferences.py b/lib/spack/spack/test/concretize_preferences.py index 3910d528e9..7d2a1bd3fe 100644 --- a/lib/spack/spack/test/concretize_preferences.py +++ b/lib/spack/spack/test/concretize_preferences.py @@ -9,7 +9,6 @@ import stat import spack.package_prefs import spack.repo import spack.util.spack_yaml as syaml -from spack.concretize import NonDeterministicVariantError from spack.config import ConfigScope, ConfigError from spack.spec import Spec from spack.version import Version @@ -85,23 +84,15 @@ class TestConcretizePreferences(object): 'mpileaks', debug=True, opt=True, shared=False, static=False ) - def test_preferred_variants_from_any(self): + def test_preferred_variants_from_wildcard(self): """ - Test that 'foo=any' concretizes to any non-none value - - Test that concretization of variants raises an error attempting - non-deterministic concretization from 'any' when preferred value is - 'none'. + Test that 'foo=*' concretizes to any value """ update_packages('multivalue-variant', 'variants', 'foo=bar') assert_variant_values( - 'multivalue-variant foo=any', foo=('bar',) + 'multivalue-variant foo=*', foo=('bar',) ) - update_packages('multivalue-variant', 'variants', 'foo=none') - with pytest.raises(NonDeterministicVariantError): - concretize('multivalue-variant foo=any') - def test_preferred_compilers(self): """Test preferred compilers are applied correctly """ diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py index 50a1731dfe..4d221378c7 100644 --- a/lib/spack/spack/test/spec_semantics.py +++ b/lib/spack/spack/test/spec_semantics.py @@ -274,8 +274,8 @@ class TestSpecSematics(object): check_satisfies('mpich foo=true', 'mpich+foo') check_satisfies('mpich~foo', 'mpich foo=FALSE') check_satisfies('mpich foo=False', 'mpich~foo') - check_satisfies('mpich foo=any', 'mpich~foo') - check_satisfies('mpich +foo', 'mpich foo=any') + check_satisfies('mpich foo=*', 'mpich~foo') + check_satisfies('mpich +foo', 'mpich foo=*') def test_satisfies_multi_value_variant(self): # Check quoting @@ -288,9 +288,9 @@ class TestSpecSematics(object): # A more constrained spec satisfies a less constrained one check_satisfies('multivalue-variant foo="bar,baz"', - 'multivalue-variant foo=any') + 'multivalue-variant foo=*') - check_satisfies('multivalue-variant foo=any', + check_satisfies('multivalue-variant foo=*', 'multivalue-variant foo="bar,baz"') check_satisfies('multivalue-variant foo="bar,baz"', @@ -317,7 +317,7 @@ class TestSpecSematics(object): a.concretize() assert a.satisfies('foobar=bar') - assert a.satisfies('foobar=any') + assert a.satisfies('foobar=*') # Assert that an autospec generated from a literal # gives the right result for a single valued variant @@ -452,10 +452,6 @@ class TestSpecSematics(object): check_unsatisfiable('mpich', 'mpich~foo', True) check_unsatisfiable('mpich', 'mpich foo=1', True) - # None and any do not satisfy each other - check_unsatisfiable('foo=none', 'foo=any') - check_unsatisfiable('foo=any', 'foo=none') - def test_unsatisfiable_variant_mismatch(self): # No matchi in specs check_unsatisfiable('mpich~foo', 'mpich+foo') @@ -624,9 +620,9 @@ class TestSpecSematics(object): ) check_constrain( - 'libelf foo=bar,baz', 'libelf foo=bar,baz', 'libelf foo=any') + 'libelf foo=bar,baz', 'libelf foo=bar,baz', 'libelf foo=*') check_constrain( - 'libelf foo=bar,baz', 'libelf foo=any', 'libelf foo=bar,baz') + 'libelf foo=bar,baz', 'libelf foo=*', 'libelf foo=bar,baz') def test_constrain_compiler_flags(self): check_constrain( @@ -668,8 +664,6 @@ class TestSpecSematics(object): check_invalid_constraint('libelf+debug', 'libelf~debug') check_invalid_constraint('libelf+debug~foo', 'libelf+debug+foo') check_invalid_constraint('libelf debug=True', 'libelf debug=False') - check_invalid_constraint('libelf foo=none', 'libelf foo=any') - check_invalid_constraint('libelf foo=any', 'libelf foo=none') check_invalid_constraint( 'libelf cppflags="-O3"', 'libelf cppflags="-O2"') @@ -684,7 +678,7 @@ class TestSpecSematics(object): check_constrain_changed('libelf', '%gcc') check_constrain_changed('libelf%gcc', '%gcc@4.5') check_constrain_changed('libelf', '+debug') - check_constrain_changed('libelf', 'debug=any') + check_constrain_changed('libelf', 'debug=*') check_constrain_changed('libelf', '~debug') check_constrain_changed('libelf', 'debug=2') check_constrain_changed('libelf', 'cppflags="-O3"') @@ -704,7 +698,7 @@ class TestSpecSematics(object): check_constrain_not_changed('libelf+debug', '+debug') check_constrain_not_changed('libelf~debug', '~debug') check_constrain_not_changed('libelf debug=2', 'debug=2') - check_constrain_not_changed('libelf debug=2', 'debug=any') + check_constrain_not_changed('libelf debug=2', 'debug=*') check_constrain_not_changed( 'libelf cppflags="-O3"', 'cppflags="-O3"') @@ -918,14 +912,14 @@ class TestSpecSematics(object): for x in ('cflags', 'cxxflags', 'fflags') ) - def test_combination_of_any_or_none(self): + def test_combination_of_wildcard_or_none(self): # Test that using 'none' and another value raises with pytest.raises(spack.variant.InvalidVariantValueCombinationError): Spec('multivalue-variant foo=none,bar') - # Test that using 'any' and another value raises + # Test that using wildcard and another value raises with pytest.raises(spack.variant.InvalidVariantValueCombinationError): - Spec('multivalue-variant foo=any,bar') + Spec('multivalue-variant foo=*,bar') @pytest.mark.skipif( sys.version_info[0] == 2, reason='__wrapped__ requires python 3' diff --git a/lib/spack/spack/variant.py b/lib/spack/spack/variant.py index d5d6acf6ec..280f2346dd 100644 --- a/lib/spack/spack/variant.py +++ b/lib/spack/spack/variant.py @@ -25,7 +25,7 @@ try: except ImportError: from collections import Sequence -special_variant_values = [None, 'none', 'any'] +special_variant_values = [None, 'none', '*'] class Variant(object): @@ -60,8 +60,8 @@ class Variant(object): self.description = str(description) self.values = None - if values is any: - # 'any' is a special case to make it easy to say any value is ok + if values == '*': + # wildcard is a special case to make it easy to say any value is ok self.single_value_validator = lambda x: True elif isinstance(values, type): @@ -122,13 +122,13 @@ class Variant(object): # Check and record the values that are not allowed not_allowed_values = [ x for x in value - if x != 'any' and self.single_value_validator(x) is False + if x != '*' and self.single_value_validator(x) is False ] if not_allowed_values: raise InvalidVariantValueError(self, not_allowed_values, pkg) # Validate the group of values if needed - if self.group_validator is not None and value != ('any',): + if self.group_validator is not None and value != ('*',): self.group_validator(pkg.name, self.name, value) @property @@ -270,8 +270,6 @@ class AbstractVariant(object): # Tuple is necessary here instead of list because the # values need to be hashed value = re.split(r'\s*,\s*', str(value)) - value = list(map(lambda x: 'any' if str(x).upper() == 'ANY' else x, - value)) for val in special_variant_values: if val in value and len(value) > 1: @@ -313,15 +311,7 @@ class AbstractVariant(object): """ # If names are different then `self` does not satisfy `other` # (`foo=bar` will never satisfy `baz=bar`) - if other.name != self.name: - return False - # If the variant is already set to none, it can't satisfy any - if ('none' in self or None in self) and 'any' in other: - return False - # If the variant is set to any, it can't be constrained by none - if 'any' in self and ('none' in other or None in other): - return False - return True + return other.name == self.name @implicit_variant_conversion def compatible(self, other): @@ -338,15 +328,7 @@ class AbstractVariant(object): """ # If names are different then `self` is not compatible with `other` # (`foo=bar` is incompatible with `baz=bar`) - if other.name != self.name: - return False - # If the variant is already set to none, incompatible with any - if ('none' in self or None in self) and 'any' in other: - return False - # If the variant is set to any, it can't be compatible with none - if 'any' in self and ('none' in other or None in other): - return False - return True + return other.name == self.name @implicit_variant_conversion def constrain(self, other): @@ -366,9 +348,9 @@ class AbstractVariant(object): old_value = self.value values = list(sorted(set(self.value + other.value))) - # If we constraint any by another value, just take value - if 'any' in values and len(values) > 1: - values.remove('any') + # If we constraint wildcard by another value, just take value + if '*' in values and len(values) > 1: + values.remove('*') self.value = ','.join(values) return old_value != self.value @@ -401,13 +383,11 @@ class MultiValuedVariant(AbstractVariant): Returns: bool: True or False """ - # If it doesn't satisfy as an AbstractVariant, it doesn't satisfy as a - # MultiValuedVariant this handles conflicts between none and any super_sat = super(MultiValuedVariant, self).satisfies(other) # Otherwise we want all the values in `other` to be also in `self` return super_sat and (all(v in self.value for v in other.value) or - 'any' in other or 'any' in self) + '*' in other or '*' in self) class SingleValuedVariant(AbstractVariant): @@ -427,12 +407,10 @@ class SingleValuedVariant(AbstractVariant): @implicit_variant_conversion def satisfies(self, other): - # If it doesn't satisfy as an AbstractVariant, it doesn't satisfy as a - # SingleValuedVariant this handles conflicts between none and any abstract_sat = super(SingleValuedVariant, self).satisfies(other) return abstract_sat and (self.value == other.value or - other.value == 'any' or self.value == 'any') + other.value == '*' or self.value == '*') def compatible(self, other): return self.satisfies(other) @@ -442,13 +420,13 @@ class SingleValuedVariant(AbstractVariant): if self.name != other.name: raise ValueError('variants must have the same name') - if self.value == 'any': - self.value = other.value - return self.value != other.value - - if other.value == 'any' and self.value not in ('none', None): + if other.value == '*': return False + if self.value == '*': + self.value = other.value + return True + if self.value != other.value: raise UnsatisfiableVariantSpecError(other.value, self.value) return False @@ -463,8 +441,8 @@ class SingleValuedVariant(AbstractVariant): class BoolValuedVariant(SingleValuedVariant): """A variant that can hold either True or False. - BoolValuedVariant can also hold the value 'any', for coerced - comparisons between ``foo=any`` and ``+foo`` or ``~foo``.""" + BoolValuedVariant can also hold the value '*', for coerced + comparisons between ``foo=*`` and ``+foo`` or ``~foo``.""" def _value_setter(self, value): # Check the string representation of the value and turn @@ -475,9 +453,9 @@ class BoolValuedVariant(SingleValuedVariant): elif str(value).upper() == 'FALSE': self._original_value = value self._value = False - elif str(value).upper() == 'ANY': + elif str(value) == '*': self._original_value = value - self._value = 'any' + self._value = '*' else: msg = 'cannot construct a BoolValuedVariant for "{0}" from ' msg += 'a value that does not represent a bool' @@ -874,7 +852,7 @@ class MultipleValuesInExclusiveVariantError(error.SpecError, ValueError): class InvalidVariantValueCombinationError(error.SpecError): - """Raised when a variant has values 'any' or 'none' with other values.""" + """Raised when a variant has values '*' or 'none' with other values.""" class InvalidVariantValueError(error.SpecError): -- cgit v1.2.3-60-g2f50