From a5faf7d27a3ef1afca28e5d2348130f580be898d Mon Sep 17 00:00:00 2001
From: Greg Becker <becker33@llnl.gov>
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(-)
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-70-g09d2