diff --git a/lib/spack/docs/basic_usage.rst b/lib/spack/docs/basic_usage.rst index bf038478964..defafad5d82 100644 --- a/lib/spack/docs/basic_usage.rst +++ b/lib/spack/docs/basic_usage.rst @@ -1291,55 +1291,61 @@ based on site policies. Variants ^^^^^^^^ -Variants are named options associated with a particular package. They are -optional, as each package must provide default values for each variant it -makes available. Variants can be specified using -a flexible parameter syntax ``name=``. For example, -``spack install mercury debug=True`` will install mercury built with debug -flags. The names of particular variants available for a package depend on +Variants are named options associated with a particular package and are +typically used to enable or disable certain features at build time. They +are optional, as each package must provide default values for each variant +it makes available. + +The names of variants available for a particular package depend on what was provided by the package author. ``spack info `` will provide information on what build variants are available. -For compatibility with earlier versions, variants which happen to be -boolean in nature can be specified by a syntax that represents turning -options on and off. For example, in the previous spec we could have -supplied ``mercury +debug`` with the same effect of enabling the debug -compile time option for the libelf package. +There are different types of variants: -Depending on the package a variant may have any default value. For -``mercury`` here, ``debug`` is ``False`` by default, and we turned it on -with ``debug=True`` or ``+debug``. If a variant is ``True`` by default -you can turn it off by either adding ``-name`` or ``~name`` to the spec. +1. Boolean variants. Typically used to enable or disable a feature at + compile time. For example, a package might have a ``debug`` variant that + can be explicitly enabled with ``+debug`` and disabled with ``~debug``. +2. Single-valued variants. Often used to set defaults. For example, a package + might have a ``compression`` variant that determines the default + compression algorithm, which users could set to ``compression=gzip`` or + ``compression=zstd``. +3. Multi-valued variants. A package might have a ``fabrics`` variant that + determines which network fabrics to support. Users could set this to + ``fabrics=verbs,ofi`` to enable both InfiniBand verbs and OpenFabrics + interfaces. The values are separated by commas. -There are two syntaxes here because, depending on context, ``~`` and -``-`` may mean different things. In most shells, the following will -result in the shell performing home directory substitution: + The meaning of ``fabrics=verbs,ofi`` is to enable *at least* the specified + fabrics, but other fabrics may be enabled as well. If the intent is to + enable *only* the specified fabrics, then the ``fabrics:=verbs,ofi`` + syntax should be used with the ``:=`` operator. -.. code-block:: sh +.. note:: - mpileaks ~debug # shell may try to substitute this! - mpileaks~debug # use this instead + In certain shells, the the ``~`` character is expanded to the home + directory. To avoid these issues, avoid whitespace between the package + name and the variant: -If there is a user called ``debug``, the ``~`` will be incorrectly -expanded. In this situation, you would want to write ``libelf --debug``. However, ``-`` can be ambiguous when included after a -package name without spaces: + .. code-block:: sh -.. code-block:: sh + mpileaks ~debug # shell may try to substitute this! + mpileaks~debug # use this instead - mpileaks-debug # wrong! - mpileaks -debug # right + Alternatively, you can use the ``-`` character to disable a variant, + but be aware that this requires a space between the package name and + the variant: -Spack allows the ``-`` character to be part of package names, so the -above will be interpreted as a request for the ``mpileaks-debug`` -package, not a request for ``mpileaks`` built without ``debug`` -options. In this scenario, you should write ``mpileaks~debug`` to -avoid ambiguity. + .. code-block:: sh -When spack normalizes specs, it prints them out with no spaces boolean -variants using the backwards compatibility syntax and uses only ``~`` -for disabled boolean variants. The ``-`` and spaces on the command -line are provided for convenience and legibility. + mpileaks-debug # wrong: refers to a package named "mpileaks-debug" + mpileaks -debug # right: refers to a package named mpileaks with debug disabled + + As a last resort, ``debug=False`` can also be used to disable a boolean variant. + + + +""""""""""""""""""""""""""""""""""" +Variant propagation to dependencies +""""""""""""""""""""""""""""""""""" Spack allows variants to propagate their value to the package's dependency by using ``++``, ``--``, and ``~~`` for boolean variants. diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index ec45dc47e2c..26440475ef9 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -1771,7 +1771,7 @@ def define_variant( # make a spec indicating whether the variant has this conditional value variant_has_value = spack.spec.Spec() - variant_has_value.variants[name] = spack.variant.AbstractVariant(name, value.value) + variant_has_value.variants[name] = vt.VariantBase(name, value.value) if value.when: # the conditional value is always "possible", but it imposes its when condition as diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 17fa574e07e..2581ea053a4 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -1706,10 +1706,8 @@ def _dependencies_dict(self, depflag: dt.DepFlag = dt.ALL): result[key] = list(group) return result - def _add_flag(self, name, value, propagate): - """Called by the parser to add a known flag. - Known flags currently include "arch" - """ + def _add_flag(self, name: str, value: str, propagate: bool, concrete: bool) -> None: + """Called by the parser to add a known flag""" if propagate and name in vt.RESERVED_NAMES: raise UnsupportedPropagationError( @@ -1736,14 +1734,12 @@ def _add_flag(self, name, value, propagate): for flag, propagation in flags_and_propagation: self.compiler_flags.add_flag(name, flag, propagation, flag_group) 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] = vt.BoolValuedVariant(name, value, propagate) + elif concrete: + self.variants[name] = vt.MultiValuedVariant(name, value, propagate) else: - self.variants[name] = vt.AbstractVariant(name, value, propagate) + self.variants[name] = vt.VariantBase(name, value, propagate) def _set_architecture(self, **kwargs): """Called by the parser to set the architecture.""" @@ -2351,6 +2347,7 @@ def to_node_dict(self, hash=ht.dag_hash): [v.name for v in self.variants.values() if v.propagate], flag_names ) ) + d["abstract"] = sorted(v.name for v in self.variants.values() if not v.concrete) if self.external: d["external"] = { @@ -3077,7 +3074,7 @@ def constrain(self, other, deps=True): raise UnsatisfiableVersionSpecError(self.versions, other.versions) for v in [x for x in other.variants if x in self.variants]: - if not self.variants[v].compatible(other.variants[v]): + if not self.variants[v].intersects(other.variants[v]): raise vt.UnsatisfiableVariantSpecError(self.variants[v], other.variants[v]) sarch, oarch = self.architecture, other.architecture @@ -4492,7 +4489,7 @@ def __init__(self, spec: Spec): def __setitem__(self, name, vspec): # Raise a TypeError if vspec is not of the right type - if not isinstance(vspec, vt.AbstractVariant): + if not isinstance(vspec, vt.VariantBase): raise TypeError( "VariantMap accepts only values of variant types " f"[got {type(vspec).__name__} instead]" @@ -4602,8 +4599,7 @@ def constrain(self, other: "VariantMap") -> bool: changed = False for k in other: if k in self: - # If they are not compatible raise an error - if not self[k].compatible(other[k]): + if not self[k].intersects(other[k]): raise vt.UnsatisfiableVariantSpecError(self[k], other[k]) # If they are compatible merge them changed |= self[k].constrain(other[k]) @@ -4807,6 +4803,7 @@ def from_node_dict(cls, node): spec.architecture = ArchSpec.from_dict(node) propagated_names = node.get("propagate", []) + abstract_variants = set(node.get("abstract", ())) for name, values in node.get("parameters", {}).items(): propagate = name in propagated_names if name in _valid_compiler_flags: @@ -4815,7 +4812,7 @@ def from_node_dict(cls, node): spec.compiler_flags.add_flag(name, val, propagate) else: spec.variants[name] = vt.MultiValuedVariant.from_node_dict( - name, values, propagate=propagate + name, values, propagate=propagate, abstract=name in abstract_variants ) spec.external_path = None diff --git a/lib/spack/spack/spec_parser.py b/lib/spack/spack/spec_parser.py index 6739dc9aef1..1d89cd93888 100644 --- a/lib/spack/spack/spec_parser.py +++ b/lib/spack/spack/spec_parser.py @@ -99,8 +99,7 @@ VERSION_RANGE = rf"(?:(?:{VERSION})?:(?:{VERSION}(?!\s*=))?)" VERSION_LIST = rf"(?:{VERSION_RANGE}|{VERSION})(?:\s*,\s*(?:{VERSION_RANGE}|{VERSION}))*" -#: Regex with groups to use for splitting (optionally propagated) key-value pairs -SPLIT_KVP = re.compile(rf"^({NAME})(==?)(.*)$") +SPLIT_KVP = re.compile(rf"^({NAME})(:?==?)(.*)$") #: Regex with groups to use for splitting %[virtuals=...] tokens SPLIT_COMPILER_TOKEN = re.compile(rf"^%\[virtuals=({VALUE}|{QUOTED_VALUE})]\s*(.*)$") @@ -135,8 +134,8 @@ class SpecTokens(TokenBase): # Variants PROPAGATED_BOOL_VARIANT = rf"(?:(?:\+\+|~~|--)\s*{NAME})" BOOL_VARIANT = rf"(?:[~+-]\s*{NAME})" - PROPAGATED_KEY_VALUE_PAIR = rf"(?:{NAME}==(?:{VALUE}|{QUOTED_VALUE}))" - KEY_VALUE_PAIR = rf"(?:{NAME}=(?:{VALUE}|{QUOTED_VALUE}))" + PROPAGATED_KEY_VALUE_PAIR = rf"(?:{NAME}:?==(?:{VALUE}|{QUOTED_VALUE}))" + KEY_VALUE_PAIR = rf"(?:{NAME}:?=(?:{VALUE}|{QUOTED_VALUE}))" # Compilers COMPILER_AND_VERSION = rf"(?:%\s*(?:{NAME})(?:[\s]*)@\s*(?:{VERSION_LIST}))" COMPILER = rf"(?:%\s*(?:{NAME}))" @@ -370,10 +369,10 @@ def raise_parsing_error(string: str, cause: Optional[Exception] = None): """Raise a spec parsing error with token context.""" raise SpecParsingError(string, self.ctx.current_token, self.literal_str) from cause - def add_flag(name: str, value: str, propagate: bool): + def add_flag(name: str, value: str, propagate: bool, concrete: bool): """Wrapper around ``Spec._add_flag()`` that adds parser context to errors raised.""" try: - initial_spec._add_flag(name, value, propagate) + initial_spec._add_flag(name, value, propagate, concrete) except Exception as e: raise_parsing_error(str(e), e) @@ -428,29 +427,34 @@ def warn_if_after_compiler(token: str): warn_if_after_compiler(self.ctx.current_token.value) elif self.ctx.accept(SpecTokens.BOOL_VARIANT): + name = self.ctx.current_token.value[1:].strip() variant_value = self.ctx.current_token.value[0] == "+" - add_flag(self.ctx.current_token.value[1:].strip(), variant_value, propagate=False) + add_flag(name, variant_value, propagate=False, concrete=True) warn_if_after_compiler(self.ctx.current_token.value) elif self.ctx.accept(SpecTokens.PROPAGATED_BOOL_VARIANT): + name = self.ctx.current_token.value[2:].strip() variant_value = self.ctx.current_token.value[0:2] == "++" - add_flag(self.ctx.current_token.value[2:].strip(), variant_value, propagate=True) + add_flag(name, variant_value, propagate=True, concrete=True) warn_if_after_compiler(self.ctx.current_token.value) elif self.ctx.accept(SpecTokens.KEY_VALUE_PAIR): - match = SPLIT_KVP.match(self.ctx.current_token.value) - assert match, "SPLIT_KVP and KEY_VALUE_PAIR do not agree." + name, value = self.ctx.current_token.value.split("=", maxsplit=1) + concrete = name.endswith(":") + if concrete: + name = name[:-1] - name, _, value = match.groups() - add_flag(name, strip_quotes_and_unescape(value), propagate=False) + add_flag( + name, strip_quotes_and_unescape(value), propagate=False, concrete=concrete + ) warn_if_after_compiler(self.ctx.current_token.value) elif self.ctx.accept(SpecTokens.PROPAGATED_KEY_VALUE_PAIR): - match = SPLIT_KVP.match(self.ctx.current_token.value) - assert match, "SPLIT_KVP and PROPAGATED_KEY_VALUE_PAIR do not agree." - - name, _, value = match.groups() - add_flag(name, strip_quotes_and_unescape(value), propagate=True) + name, value = self.ctx.current_token.value.split("==", maxsplit=1) + concrete = name.endswith(":") + if concrete: + name = name[:-1] + add_flag(name, strip_quotes_and_unescape(value), propagate=True, concrete=concrete) warn_if_after_compiler(self.ctx.current_token.value) elif self.ctx.expect(SpecTokens.DAG_HASH): @@ -509,7 +513,8 @@ def parse(self): while True: if self.ctx.accept(SpecTokens.KEY_VALUE_PAIR): name, value = self.ctx.current_token.value.split("=", maxsplit=1) - name = name.strip("'\" ") + if name.endswith(":"): + name = name[:-1] value = value.strip("'\" ").split(",") attributes[name] = value if name not in ("deptypes", "virtuals"): diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py index eee7791baa4..9232fcb0292 100644 --- a/lib/spack/spack/test/spec_semantics.py +++ b/lib/spack/spack/test/spec_semantics.py @@ -638,7 +638,7 @@ def test_multivalued_variant_2(self): a = Spec("multivalue-variant foo=bar") b = Spec("multivalue-variant foo=bar,baz") # The specs are abstract and they **could** be constrained - assert a.satisfies(b) + assert b.satisfies(a) and not a.satisfies(b) # An abstract spec can instead be constrained assert a.constrain(b) diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index 1a80540c042..d3ac7344cce 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -633,6 +633,23 @@ def _specfile_for(spec_str, filename): ], "zlib %[virtuals=fortran] gcc@14.1 %[virtuals=c,cxx] clang", ), + # test := and :== syntax for key value pairs + ( + "gcc languages:=c,c++", + [ + Token(SpecTokens.UNQUALIFIED_PACKAGE_NAME, "gcc"), + Token(SpecTokens.KEY_VALUE_PAIR, "languages:=c,c++"), + ], + "gcc languages:='c,c++'", + ), + ( + "gcc languages:==c,c++", + [ + Token(SpecTokens.UNQUALIFIED_PACKAGE_NAME, "gcc"), + Token(SpecTokens.PROPAGATED_KEY_VALUE_PAIR, "languages:==c,c++"), + ], + "gcc languages:=='c,c++'", + ), ], ) def test_parse_single_spec(spec_str, tokens, expected_roundtrip, mock_git_test_package): diff --git a/lib/spack/spack/test/variant.py b/lib/spack/spack/test/variant.py index 1fb6a6ef973..0af893b35e7 100644 --- a/lib/spack/spack/test/variant.py +++ b/lib/spack/spack/test/variant.py @@ -12,7 +12,6 @@ import spack.variant from spack.spec import Spec, VariantMap from spack.variant import ( - AbstractVariant, BoolValuedVariant, DuplicateVariantError, InconsistentValidationError, @@ -22,6 +21,7 @@ SingleValuedVariant, UnsatisfiableVariantSpecError, Variant, + VariantBase, disjoint_sets, ) @@ -31,7 +31,7 @@ def test_initialization(self): # Basic properties a = MultiValuedVariant("foo", "bar,baz") assert repr(a) == "MultiValuedVariant('foo', 'bar,baz')" - assert str(a) == "foo=bar,baz" + assert str(a) == "foo:=bar,baz" assert a.value == ("bar", "baz") assert "bar" in a assert "baz" in a @@ -40,7 +40,7 @@ def test_initialization(self): # Spaces are trimmed b = MultiValuedVariant("foo", "bar, baz") assert repr(b) == "MultiValuedVariant('foo', 'bar, baz')" - assert str(b) == "foo=bar,baz" + assert str(b) == "foo:=bar,baz" assert b.value == ("bar", "baz") assert "bar" in b assert "baz" in b @@ -51,7 +51,7 @@ def test_initialization(self): # Order is not important c = MultiValuedVariant("foo", "baz, bar") assert repr(c) == "MultiValuedVariant('foo', 'baz, bar')" - assert str(c) == "foo=bar,baz" + assert str(c) == "foo:=bar,baz" assert c.value == ("bar", "baz") assert "bar" in c assert "baz" in c @@ -77,116 +77,71 @@ def test_satisfies(self): c = MultiValuedVariant("fee", "bar,baz") d = MultiValuedVariant("foo", "True") - # 'foo=bar,baz' satisfies 'foo=bar' - assert a.satisfies(b) - - # 'foo=bar' does not satisfy 'foo=bar,baz' - assert not b.satisfies(a) - - # 'foo=bar,baz' does not satisfy 'foo=bar,baz' and vice-versa - assert not a.satisfies(c) - assert not c.satisfies(a) - - # Implicit type conversion for variants of other types + # concrete, different values do not satisfy each other + assert not a.satisfies(b) and not b.satisfies(a) + assert not a.satisfies(c) and not c.satisfies(a) + # SingleValuedVariant and MultiValuedVariant with the same single concrete value do satisfy + # eachother b_sv = SingleValuedVariant("foo", "bar") - assert b.satisfies(b_sv) + assert b.satisfies(b_sv) and b_sv.satisfies(b) d_sv = SingleValuedVariant("foo", "True") - assert d.satisfies(d_sv) + assert d.satisfies(d_sv) and d_sv.satisfies(d) almost_d_bv = SingleValuedVariant("foo", "true") assert not d.satisfies(almost_d_bv) + # BoolValuedVariant actually stores the value as a boolean, whereas with MV and SV the + # value is string "True". d_bv = BoolValuedVariant("foo", "True") - 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) + assert not d.satisfies(d_bv) and not d_bv.satisfies(d) - def test_compatible(self): + def test_intersects(self): a = MultiValuedVariant("foo", "bar,baz") b = MultiValuedVariant("foo", "True") c = MultiValuedVariant("fee", "bar,baz") d = MultiValuedVariant("foo", "bar,barbaz") - # If the name of two multi-valued variants is the same, - # they are compatible - assert a.compatible(b) - assert not a.compatible(c) - assert a.compatible(d) - - assert b.compatible(a) - assert not b.compatible(c) - assert b.compatible(d) - - assert not c.compatible(a) - assert not c.compatible(b) - assert not c.compatible(d) - - assert d.compatible(a) - assert d.compatible(b) - assert not d.compatible(c) - - # Implicit type conversion for other types + # concrete, different values do not intersect. + assert not a.intersects(b) and not b.intersects(a) + assert not a.intersects(c) and not c.intersects(a) + assert not a.intersects(d) and not d.intersects(a) + assert not b.intersects(c) and not c.intersects(b) + assert not b.intersects(d) and not d.intersects(b) + assert not c.intersects(d) and not d.intersects(c) + # SV and MV intersect if they have the same concrete value. b_sv = SingleValuedVariant("foo", "True") - assert b.compatible(b_sv) - assert not c.compatible(b_sv) + assert b.intersects(b_sv) + assert not c.intersects(b_sv) + # BoolValuedVariant stores a bool, which is not the same as the string "True" in MV. b_bv = BoolValuedVariant("foo", "True") - assert b.compatible(b_bv) - assert not c.compatible(b_bv) + assert not b.intersects(b_bv) + assert not c.intersects(b_bv) def test_constrain(self): - # Try to constrain on a value with less constraints than self + # Concrete values cannot be constrained a = MultiValuedVariant("foo", "bar,baz") b = MultiValuedVariant("foo", "bar") - - changed = a.constrain(b) - assert not changed - t = MultiValuedVariant("foo", "bar,baz") - assert a == t - - # Try to constrain on a value with more constraints than self - a = MultiValuedVariant("foo", "bar,baz") - b = MultiValuedVariant("foo", "bar") - - changed = b.constrain(a) - assert changed - t = MultiValuedVariant("foo", "bar,baz") - assert a == t + with pytest.raises(UnsatisfiableVariantSpecError): + a.constrain(b) + with pytest.raises(UnsatisfiableVariantSpecError): + b.constrain(a) # Try to constrain on the same value a = MultiValuedVariant("foo", "bar,baz") b = a.copy() - changed = a.constrain(b) - assert not changed - t = MultiValuedVariant("foo", "bar,baz") - assert a == t + assert not a.constrain(b) + assert a == b == MultiValuedVariant("foo", "bar,baz") # Try to constrain on a different name a = MultiValuedVariant("foo", "bar,baz") b = MultiValuedVariant("fee", "bar") - with pytest.raises(ValueError): + with pytest.raises(UnsatisfiableVariantSpecError): a.constrain(b) - # Implicit type conversion for variants of other types - - a = MultiValuedVariant("foo", "bar,baz") - 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): a = MultiValuedVariant("foo", "bar,baz,barbaz") b = MultiValuedVariant("foo", "bar, baz, barbaz") @@ -231,126 +186,56 @@ def test_satisfies(self): b = SingleValuedVariant("foo", "bar") c = SingleValuedVariant("foo", "baz") d = SingleValuedVariant("fee", "bar") - e = SingleValuedVariant("foo", "True") - # 'foo=bar' can only satisfy 'foo=bar' - assert a.satisfies(b) - assert not a.satisfies(c) - assert not a.satisfies(d) + # concrete, different values do not satisfy each other + assert not a.satisfies(c) and not c.satisfies(a) + assert not a.satisfies(d) and not d.satisfies(a) + assert not b.satisfies(c) and not c.satisfies(b) + assert not b.satisfies(d) and not d.satisfies(b) + assert not c.satisfies(d) and not d.satisfies(c) - assert b.satisfies(a) - assert not b.satisfies(c) - assert not b.satisfies(d) + assert a.satisfies(b) and b.satisfies(a) - assert not c.satisfies(a) - assert not c.satisfies(b) - assert not c.satisfies(d) - - # Implicit type conversion for variants of other types - - a_mv = MultiValuedVariant("foo", "bar") - assert a.satisfies(a_mv) - multiple_values = MultiValuedVariant("foo", "bar,baz") - assert not a.satisfies(multiple_values) - - e_bv = BoolValuedVariant("foo", "True") - assert e.satisfies(e_bv) - almost_e_bv = BoolValuedVariant("foo", "true") - assert not e.satisfies(almost_e_bv) - - def test_compatible(self): + def test_intersects(self): a = SingleValuedVariant("foo", "bar") b = SingleValuedVariant("fee", "bar") c = SingleValuedVariant("foo", "baz") d = SingleValuedVariant("foo", "bar") - # If the name of two multi-valued variants is the same, - # they are compatible - assert not a.compatible(b) - assert not a.compatible(c) - assert a.compatible(d) + # concrete, different values do not intersect + assert not a.intersects(b) and not b.intersects(a) + assert not a.intersects(c) and not c.intersects(a) + assert not b.intersects(c) and not c.intersects(b) + assert not b.intersects(d) and not d.intersects(b) + assert not c.intersects(d) and not d.intersects(c) - assert not b.compatible(a) - assert not b.compatible(c) - assert not b.compatible(d) - - assert not c.compatible(a) - assert not c.compatible(b) - assert not c.compatible(d) - - assert d.compatible(a) - assert not d.compatible(b) - assert not d.compatible(c) - - # Implicit type conversion for variants of other types - - a_mv = MultiValuedVariant("foo", "bar") - 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") - almost_e_bv = BoolValuedVariant("foo", "true") - - assert e.compatible(e_bv) - assert not e.compatible(almost_e_bv) + assert a.intersects(d) and d.intersects(a) def test_constrain(self): # Try to constrain on a value equal to self a = SingleValuedVariant("foo", "bar") b = SingleValuedVariant("foo", "bar") - changed = a.constrain(b) - assert not changed - t = SingleValuedVariant("foo", "bar") - assert a == t + assert not a.constrain(b) + assert a == SingleValuedVariant("foo", "bar") # Try to constrain on a value with a different value a = SingleValuedVariant("foo", "bar") b = SingleValuedVariant("foo", "baz") - with pytest.raises(UnsatisfiableVariantSpecError): - b.constrain(a) - # Try to constrain on a value with a different value a = SingleValuedVariant("foo", "bar") b = SingleValuedVariant("fee", "bar") - with pytest.raises(ValueError): + with pytest.raises(UnsatisfiableVariantSpecError): b.constrain(a) # Try to constrain on the same value a = SingleValuedVariant("foo", "bar") b = a.copy() - changed = a.constrain(b) - assert not changed - t = SingleValuedVariant("foo", "bar") - assert a == t - - # 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): - assert not a.constrain(v) + assert not a.constrain(b) + assert a == SingleValuedVariant("foo", "bar") def test_yaml_entry(self): a = SingleValuedVariant("foo", "bar") @@ -411,80 +296,62 @@ def test_satisfies(self): c = BoolValuedVariant("fee", False) d = BoolValuedVariant("foo", "True") - assert not a.satisfies(b) - assert not a.satisfies(c) - assert a.satisfies(d) + # concrete, different values do not satisfy each other + assert not a.satisfies(b) and not b.satisfies(a) + assert not a.satisfies(c) and not c.satisfies(a) + assert not b.satisfies(c) and not c.satisfies(b) + assert not b.satisfies(d) and not d.satisfies(b) + assert not c.satisfies(d) and not d.satisfies(c) - assert not b.satisfies(a) - assert not b.satisfies(c) - assert not b.satisfies(d) + assert a.satisfies(d) and d.satisfies(a) - assert not c.satisfies(a) - assert not c.satisfies(b) - assert not c.satisfies(d) + # # 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) - assert d.satisfies(a) - assert not d.satisfies(b) - assert not d.satisfies(c) + # d_mv = MultiValuedVariant("foo", "FaLsE") + # assert not d.satisfies(d_mv) + # assert b.satisfies(d_mv) - # 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", "bar") + # assert not 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_sv = SingleValuedVariant("foo", "True") + # assert d.satisfies(d_sv) - d_mv = MultiValuedVariant("foo", "bar") - assert not d.satisfies(d_mv) - assert not b.satisfies(d_mv) - - d_sv = SingleValuedVariant("foo", "True") - assert d.satisfies(d_sv) - - def test_compatible(self): + def test_intersects(self): a = BoolValuedVariant("foo", True) b = BoolValuedVariant("fee", True) c = BoolValuedVariant("foo", False) d = BoolValuedVariant("foo", "True") - # If the name of two multi-valued variants is the same, - # they are compatible - assert not a.compatible(b) - assert not a.compatible(c) - assert a.compatible(d) + # concrete, different values do not intersect each other + assert not a.intersects(b) and not b.intersects(a) + assert not a.intersects(c) and not c.intersects(a) + assert not b.intersects(c) and not c.intersects(b) + assert not b.intersects(d) and not d.intersects(b) + assert not c.intersects(d) and not d.intersects(c) - assert not b.compatible(a) - assert not b.compatible(c) - assert not b.compatible(d) + assert a.intersects(d) and d.intersects(a) - assert not c.compatible(a) - assert not c.compatible(b) - assert not c.compatible(d) + # for value in ("True", "TrUe", "TRUE"): + # d_mv = MultiValuedVariant("foo", value) + # assert d.intersects(d_mv) + # assert not c.intersects(d_mv) - assert d.compatible(a) - assert not d.compatible(b) - assert not d.compatible(c) - - 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", value) - assert d.compatible(d_sv) - assert not c.compatible(d_sv) + # d_sv = SingleValuedVariant("foo", value) + # assert d.intersects(d_sv) + # assert not c.intersects(d_sv) def test_constrain(self): # Try to constrain on a value equal to self a = BoolValuedVariant("foo", "True") b = BoolValuedVariant("foo", True) - changed = a.constrain(b) - assert not changed - t = BoolValuedVariant("foo", True) - assert a == t + assert not a.constrain(b) + assert a == BoolValuedVariant("foo", True) # Try to constrain on a value with a different value a = BoolValuedVariant("foo", True) @@ -497,24 +364,15 @@ def test_constrain(self): a = BoolValuedVariant("foo", True) b = BoolValuedVariant("fee", True) - with pytest.raises(ValueError): + with pytest.raises(UnsatisfiableVariantSpecError): b.constrain(a) # Try to constrain on the same value a = BoolValuedVariant("foo", True) b = a.copy() - changed = a.constrain(b) - assert not changed - t = BoolValuedVariant("foo", True) - assert a == t - - # Try to constrain on other values - a = BoolValuedVariant("foo", "True") - sv = SingleValuedVariant("foo", "True") - mv = MultiValuedVariant("foo", "True") - for v in (sv, mv): - assert not a.constrain(v) + assert not a.constrain(b) + assert a == BoolValuedVariant("foo", True) def test_yaml_entry(self): a = BoolValuedVariant("foo", "True") @@ -652,11 +510,9 @@ def test_satisfies_and_constrain(self) -> None: b["foobar"] = SingleValuedVariant("foobar", "fee") b["shared"] = BoolValuedVariant("shared", True) - assert a.intersects(b) - assert b.intersects(a) - - assert not a.satisfies(b) - assert not b.satisfies(a) + # concrete, different values do not intersect / satisfy each other + assert not a.intersects(b) and not b.intersects(a) + assert not a.satisfies(b) and not b.satisfies(a) # foo=bar,baz foobar=fee feebar=foo shared=True c = VariantMap(Spec()) @@ -665,8 +521,9 @@ def test_satisfies_and_constrain(self) -> None: c["feebar"] = SingleValuedVariant("feebar", "foo") c["shared"] = BoolValuedVariant("shared", True) - assert a.constrain(b) - assert a == c + # concrete values cannot be constrained + with pytest.raises(spack.variant.UnsatisfiableVariantSpecError): + a.constrain(b) def test_copy(self) -> None: a = VariantMap(Spec()) @@ -683,7 +540,7 @@ def test_str(self) -> None: c["foobar"] = SingleValuedVariant("foobar", "fee") c["feebar"] = SingleValuedVariant("feebar", "foo") c["shared"] = BoolValuedVariant("shared", True) - assert str(c) == "+shared feebar=foo foo=bar,baz foobar=fee" + assert str(c) == "+shared feebar=foo foo:=bar,baz foobar=fee" def test_disjoint_set_initialization_errors(): @@ -905,7 +762,7 @@ def test_concretize_variant_default_with_multiple_defs( # dev_path is a special case ("foo dev_path=/path/to/source", "dev_path", SingleValuedVariant), # reserved name: won't be touched - ("foo patches=2349dc44", "patches", AbstractVariant), + ("foo patches=2349dc44", "patches", VariantBase), # simple case -- one definition applies ("variant-values@1.0 v=foo", "v", SingleValuedVariant), # simple, but with bool valued variant @@ -913,14 +770,14 @@ def test_concretize_variant_default_with_multiple_defs( # variant doesn't exist at version ("variant-values@4.0 v=bar", "v", spack.spec.InvalidVariantForSpecError), # multiple definitions, so not yet knowable - ("variant-values@2.0 v=bar", "v", AbstractVariant), + ("variant-values@2.0 v=bar", "v", VariantBase), ], ) def test_substitute_abstract_variants(mock_packages, spec, variant_name, after): spec = Spec(spec) - # all variants start out as AbstractVariant - assert isinstance(spec.variants[variant_name], AbstractVariant) + # all variants start out as VariantBase + assert isinstance(spec.variants[variant_name], VariantBase) if issubclass(after, Exception): # if we're checking for an error, use pytest.raises @@ -930,3 +787,142 @@ def test_substitute_abstract_variants(mock_packages, spec, variant_name, after): # ensure that the type of the variant on the spec has been narrowed (or not) spack.spec.substitute_abstract_variants(spec) assert isinstance(spec.variants[variant_name], after) + + +def test_abstract_variant_satisfies_abstract_abstract(): + # rhs should be a subset of lhs + assert Spec("foo=bar").satisfies("foo=bar") + assert Spec("foo=bar,baz").satisfies("foo=bar") + assert Spec("foo=bar,baz").satisfies("foo=bar,baz") + assert not Spec("foo=bar").satisfies("foo=baz") + assert not Spec("foo=bar").satisfies("foo=bar,baz") + assert Spec("foo=bar").satisfies("foo=*") # rhs empty set + assert Spec("foo=*").satisfies("foo=*") # lhs and rhs empty set + assert not Spec("foo=*").satisfies("foo=bar") # lhs empty set, rhs not + + +def test_abstract_variant_satisfies_concrete_abstract(): + # rhs should be a subset of lhs + assert Spec("foo:=bar").satisfies("foo=bar") + assert Spec("foo:=bar,baz").satisfies("foo=bar") + assert Spec("foo:=bar,baz").satisfies("foo=bar,baz") + assert not Spec("foo:=bar").satisfies("foo=baz") + assert not Spec("foo:=bar").satisfies("foo=bar,baz") + assert Spec("foo:=bar").satisfies("foo=*") # rhs empty set + + +def test_abstract_variant_satisfies_abstract_concrete(): + # always false since values can be added to the lhs + assert not Spec("foo=bar").satisfies("foo:=bar") + assert not Spec("foo=bar,baz").satisfies("foo:=bar") + assert not Spec("foo=bar,baz").satisfies("foo:=bar,baz") + assert not Spec("foo=bar").satisfies("foo:=baz") + assert not Spec("foo=bar").satisfies("foo:=bar,baz") + assert not Spec("foo=*").satisfies("foo:=bar") # lhs empty set + + +def test_abstract_variant_satisfies_concrete_concrete(): + # concrete values only satisfy each other when equal + assert Spec("foo:=bar").satisfies("foo:=bar") + assert not Spec("foo:=bar,baz").satisfies("foo:=bar") + assert not Spec("foo:=bar").satisfies("foo:=bar,baz") + assert Spec("foo:=bar,baz").satisfies("foo:=bar,baz") + + +def test_abstract_variant_intersects_abstract_abstract(): + # always true since the union of values satisfies both + assert Spec("foo=bar").intersects("foo=bar") + assert Spec("foo=bar,baz").intersects("foo=bar") + assert Spec("foo=bar,baz").intersects("foo=bar,baz") + assert Spec("foo=bar").intersects("foo=baz") + assert Spec("foo=bar").intersects("foo=bar,baz") + assert Spec("foo=bar").intersects("foo=*") # rhs empty set + assert Spec("foo=*").intersects("foo=*") # lhs and rhs empty set + assert Spec("foo=*").intersects("foo=bar") # lhs empty set, rhs not + + +def test_abstract_variant_intersects_concrete_abstract(): + assert Spec("foo:=bar").intersects("foo=bar") + assert Spec("foo:=bar,baz").intersects("foo=bar") + assert Spec("foo:=bar,baz").intersects("foo=bar,baz") + assert not Spec("foo:=bar").intersects("foo=baz") # rhs has at least baz, lhs has not + assert not Spec("foo:=bar").intersects("foo=bar,baz") # rhs has at least baz, lhs has not + assert Spec("foo:=bar").intersects("foo=*") # rhs empty set + + +def test_abstract_variant_intersects_abstract_concrete(): + assert Spec("foo=bar").intersects("foo:=bar") + assert not Spec("foo=bar,baz").intersects("foo:=bar") # lhs has at least baz, rhs has not + assert Spec("foo=bar,baz").intersects("foo:=bar,baz") + assert not Spec("foo=bar").intersects("foo:=baz") # lhs has at least bar, rhs has not + assert Spec("foo=bar").intersects("foo:=bar,baz") + assert Spec("foo=*").intersects("foo:=bar") # lhs empty set + + +def test_abstract_variant_intersects_concrete_concrete(): + # concrete values only intersect each other when equal + assert Spec("foo:=bar").intersects("foo:=bar") + assert not Spec("foo:=bar,baz").intersects("foo:=bar") + assert not Spec("foo:=bar").intersects("foo:=bar,baz") + assert Spec("foo:=bar,baz").intersects("foo:=bar,baz") + + +def test_abstract_variant_constrain_abstract_abstract(): + s1 = Spec("foo=bar") + s2 = Spec("foo=*") + assert s1.constrain("foo=baz") + assert s1 == Spec("foo=bar,baz") + assert s2.constrain("foo=baz") + assert s2 == Spec("foo=baz") + + +def test_abstract_variant_constrain_abstract_concrete_fail(): + with pytest.raises(UnsatisfiableVariantSpecError): + Spec("foo=bar").constrain("foo:=baz") + + +def test_abstract_variant_constrain_abstract_concrete_ok(): + s1 = Spec("foo=bar") + s2 = Spec("foo=*") + assert s1.constrain("foo:=bar") # the change is concreteness + assert s1 == Spec("foo:=bar") + assert s2.constrain("foo:=bar") + assert s2 == Spec("foo:=bar") + + +def test_abstract_variant_constrain_concrete_concrete_fail(): + with pytest.raises(UnsatisfiableVariantSpecError): + Spec("foo:=bar").constrain("foo:=bar,baz") + + +def test_abstract_variant_constrain_concrete_concrete_ok(): + s = Spec("foo:=bar") + assert not s.constrain("foo:=bar") # no change + + +def test_abstract_variant_constrain_concrete_abstract_fail(): + s = Spec("foo:=bar") + with pytest.raises(UnsatisfiableVariantSpecError): + s.constrain("foo=baz") + + +def test_abstract_variant_constrain_concrete_abstract_ok(): + s = Spec("foo:=bar,baz") + assert not s.constrain("foo=bar") # no change in value or concreteness + assert not s.constrain("foo=*") + + +def test_patches_variant(): + """patches=x,y,z is a variant with special satisfies behavior when the rhs is abstract; it + allows string prefix matching of the lhs.""" + assert Spec("patches:=abcdef").satisfies("patches=ab") + assert Spec("patches:=abcdef").satisfies("patches=abcdef") + assert not Spec("patches:=abcdef").satisfies("patches=xyz") + assert Spec("patches:=abcdef,xyz").satisfies("patches=xyz") + assert not Spec("patches:=abcdef").satisfies("patches=abcdefghi") + + # but when the rhs is concrete, it must match exactly + assert Spec("patches:=abcdef").satisfies("patches:=abcdef") + assert not Spec("patches:=abcdef").satisfies("patches:=ab") + assert not Spec("patches:=abcdef,xyz").satisfies("patches:=abc,xyz") + assert not Spec("patches:=abcdef").satisfies("patches:=abcdefghi") diff --git a/lib/spack/spack/variant.py b/lib/spack/spack/variant.py index 5fa2251f127..e9700ed9466 100644 --- a/lib/spack/spack/variant.py +++ b/lib/spack/spack/variant.py @@ -134,7 +134,7 @@ def isa_type(v): self.sticky = sticky self.precedence = precedence - def validate_or_raise(self, vspec: "AbstractVariant", pkg_name: str): + def validate_or_raise(self, vspec: "VariantBase", pkg_name: str): """Validate a variant spec against this package variant. Raises an exception if any error is found. @@ -200,7 +200,7 @@ def make_default(self): """ return self.make_variant(self.default) - def make_variant(self, value: Union[str, bool]) -> "AbstractVariant": + def make_variant(self, value: Union[str, bool]) -> "VariantBase": """Factory that creates a variant holding the value passed as a parameter. @@ -237,27 +237,6 @@ def __str__(self): ) -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, propagate=other.propagate) - except (spack.error.SpecError, ValueError): - return False - return method(self, other) - - return convert - - def _flatten(values) -> Collection: """Flatten instances of _ConditionalVariantValues for internal representation""" if isinstance(values, DisjointSetsOfValues): @@ -282,16 +261,10 @@ def _flatten(values) -> Collection: @lang.lazy_lexicographic_ordering -class AbstractVariant: - """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. - """ +class VariantBase: + """A BaseVariant corresponds to a spec string of the form ``foo=bar`` or ``foo=bar,baz``. + It is a constraint on the spec and abstract in the sense that it must have **at least** these + values -- concretization may add more values.""" name: str propagate: bool @@ -301,18 +274,19 @@ class AbstractVariant: def __init__(self, name: str, value: ValueType, propagate: bool = False) -> None: self.name = name self.propagate = propagate + self.concrete = False # Invokes property setter self.value = value @staticmethod def from_node_dict( - name: str, value: Union[str, List[str]], *, propagate: bool = False - ) -> "AbstractVariant": + name: str, value: Union[str, List[str]], *, propagate: bool = False, abstract: bool = False + ) -> "VariantBase": """Reconstruct a variant from a node dict.""" if isinstance(value, list): - # read multi-value variants in and be faithful to the YAML - mvar = MultiValuedVariant(name, (), propagate=propagate) + constructor = VariantBase if abstract else MultiValuedVariant + mvar = constructor(name, (), propagate=propagate) mvar._value = tuple(value) mvar._original_value = mvar._value return mvar @@ -358,6 +332,10 @@ def _value_setter(self, value: ValueType) -> None: # Store the original value self._original_value = value + if value == "*": + self._value = () + return + if not isinstance(value, (tuple, list)): # Store a tuple of CSV string representations # Tuple is necessary here instead of list because the @@ -380,81 +358,61 @@ def _cmp_iter(self) -> Iterable: yield self.propagate yield from (str(v) for v in self.value_as_tuple) - def copy(self) -> "AbstractVariant": - """Returns an instance of a variant equivalent to self + def copy(self) -> "VariantBase": + variant = type(self)(self.name, self._original_value, self.propagate) + variant.concrete = self.concrete + return variant - Returns: - AbstractVariant: a copy of self - - >>> a = MultiValuedVariant('foo', True) - >>> b = a.copy() - >>> assert a == b - >>> assert a is not b - """ - return type(self)(self.name, self._original_value, self.propagate) - - @implicit_variant_conversion - def satisfies(self, other: "AbstractVariant") -> bool: - """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 - - Returns: - bool: True or False - """ - # If names are different then `self` does not satisfy `other` - # (`foo=bar` will never satisfy `baz=bar`) - return other.name == self.name - - def intersects(self, other: "AbstractVariant") -> bool: - """Returns True if there are variant matching both self and other, False otherwise.""" - if isinstance(other, (SingleValuedVariant, BoolValuedVariant)): - return other.intersects(self) - return other.name == self.name - - def compatible(self, other: "AbstractVariant") -> bool: - """Returns True if self and other are compatible, False otherwise. - - As there is no semantic check, two VariantSpec are compatible if - either they contain the same value or they are both multi-valued. - - Args: - other: instance against which we test compatibility - - Returns: - bool: True or False - """ - # If names are different then `self` is not compatible with `other` - # (`foo=bar` is incompatible with `baz=bar`) - return self.intersects(other) - - @implicit_variant_conversion - def constrain(self, other: "AbstractVariant") -> bool: - """Modify self to match all the constraints for other if both - instances are multi-valued. Returns True if self changed, - False otherwise. - - Args: - other: instance against which we constrain self - - Returns: - bool: True or False - """ + def satisfies(self, other: "VariantBase") -> bool: + """The lhs satisfies the rhs if all possible concretizations of lhs are also + possible concretizations of rhs.""" if self.name != other.name: - raise ValueError("variants must have the same name") + return False + if not other.concrete: + # rhs abstract means the lhs must at least contain its values. + # special-case patches with rhs abstract: their values may be prefixes of the lhs + # values. + if self.name == "patches": + return all( + isinstance(v, str) + and any(isinstance(w, str) and w.startswith(v) for w in self.value_as_tuple) + for v in other.value_as_tuple + ) + return all(v in self for v in other.value_as_tuple) + if self.concrete: + # both concrete: they must be equal + return self.value_as_tuple == other.value_as_tuple + return False + + def intersects(self, other: "VariantBase") -> bool: + """True iff there exists a concretization that satisfies both lhs and rhs.""" + if self.name != other.name: + return False + if self.concrete: + if other.concrete: + return self.value_as_tuple == other.value_as_tuple + return all(v in self for v in other.value_as_tuple) + if other.concrete: + return all(v in other for v in self.value_as_tuple) + # both abstract: the union is a valid concretization of both + return True + + def constrain(self, other: "VariantBase") -> bool: + """Constrain self with other if they intersect. Returns true iff self was changed.""" + if not self.intersects(other): + raise UnsatisfiableVariantSpecError(self, other) old_value = self.value - - values = list(sorted(set(self.value_as_tuple + other.value_as_tuple))) - # If we constraint wildcard by another value, just take value - if "*" in values and len(values) > 1: - values.remove("*") - + values = list(sorted({*self.value_as_tuple, *other.value_as_tuple})) self._value_setter(",".join(str(v) for v in values)) - self.propagate = self.propagate and other.propagate - return old_value != self.value + changed = old_value != self.value + if self.propagate and not other.propagate: + self.propagate = False + changed = True + if not self.concrete and other.concrete: + self.concrete = True + changed = True + return changed def __contains__(self, item: Union[str, bool]) -> bool: return item in self.value_as_tuple @@ -463,42 +421,20 @@ def __repr__(self) -> str: return f"{type(self).__name__}({repr(self.name)}, {repr(self._original_value)})" def __str__(self) -> str: + concrete = ":" if self.concrete else "" delim = "==" if self.propagate else "=" - values = spack.spec_parser.quote_if_needed(",".join(str(v) for v in self.value_as_tuple)) - return f"{self.name}{delim}{values}" + values_tuple = self.value_as_tuple + if values_tuple: + value_str = ",".join(str(v) for v in values_tuple) + else: + value_str = "*" + return f"{self.name}{concrete}{delim}{spack.spec_parser.quote_if_needed(value_str)}" -class MultiValuedVariant(AbstractVariant): - """A variant that can hold multiple values at once.""" - - @implicit_variant_conversion - def satisfies(self, other: AbstractVariant) -> bool: - """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 - """ - super_sat = super().satisfies(other) - - if not super_sat: - return False - - if "*" in other or "*" in self: - return True - - # allow prefix find on patches - if self.name == "patches": - return all( - any(str(w).startswith(str(v)) for w in self.value_as_tuple) - for v in other.value_as_tuple - ) - - # Otherwise we want all the values in `other` to be also in `self` - return all(v in self for v in other.value_as_tuple) +class MultiValuedVariant(VariantBase): + def __init__(self, name, value, propagate=False): + super().__init__(name, value, propagate) + self.concrete = True def append(self, value: Union[str, bool]) -> None: """Add another value to this multi-valued variant.""" @@ -513,11 +449,13 @@ def __str__(self) -> str: values_str = ",".join(str(x) for x in self.value_as_tuple) delim = "==" if self.propagate else "=" - return f"{self.name}{delim}{spack.spec_parser.quote_if_needed(values_str)}" + return f"{self.name}:{delim}{spack.spec_parser.quote_if_needed(values_str)}" -class SingleValuedVariant(AbstractVariant): - """A variant that can hold multiple values, but one at a time.""" +class SingleValuedVariant(VariantBase): + def __init__(self, name, value, propagate=False): + super().__init__(name, value, propagate) + self.concrete = True def _value_setter(self, value: ValueType) -> None: # Treat the value as a multi-valued variant @@ -530,37 +468,6 @@ def _value_setter(self, value: ValueType) -> None: self._value = values[0] - @implicit_variant_conversion - def satisfies(self, other: "AbstractVariant") -> bool: - abstract_sat = super().satisfies(other) - - return abstract_sat and ( - self.value == other.value or other.value == "*" or self.value == "*" - ) - - def intersects(self, other: "AbstractVariant") -> bool: - return self.satisfies(other) - - def compatible(self, other: "AbstractVariant") -> bool: - return self.satisfies(other) - - @implicit_variant_conversion - def constrain(self, other: "AbstractVariant") -> bool: - if self.name != other.name: - raise ValueError("variants must have the same name") - - 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) - self.propagate = self.propagate and other.propagate - return False - def __contains__(self, item: ValueType) -> bool: return item == self.value @@ -574,10 +481,9 @@ def __str__(self) -> str: class BoolValuedVariant(SingleValuedVariant): - """A variant that can hold either True or False. - - BoolValuedVariant can also hold the value '*', for coerced - comparisons between ``foo=*`` and ``+foo`` or ``~foo``.""" + def __init__(self, name, value, propagate=False): + super().__init__(name, value, propagate) + self.concrete = True def _value_setter(self, value: ValueType) -> None: # Check the string representation of the value and turn @@ -588,13 +494,11 @@ def _value_setter(self, value: ValueType) -> None: elif str(value).upper() == "FALSE": self._original_value = value self._value = False - elif str(value) == "*": - self._original_value = value - self._value = "*" else: - msg = 'cannot construct a BoolValuedVariant for "{0}" from ' - msg += "a value that does not represent a bool" - raise ValueError(msg.format(self.name)) + raise ValueError( + f'cannot construct a BoolValuedVariant for "{self.name}" from ' + "a value that does not represent a bool" + ) def __contains__(self, item: ValueType) -> bool: return item is self.value @@ -810,7 +714,7 @@ def __lt__(self, other): def prevalidate_variant_value( pkg_cls: "Type[spack.package_base.PackageBase]", - variant: AbstractVariant, + variant: VariantBase, spec: Optional["spack.spec.Spec"] = None, strict: bool = False, ) -> List[Variant]: @@ -915,7 +819,7 @@ class MultipleValuesInExclusiveVariantError(spack.error.SpecError, ValueError): only one. """ - def __init__(self, variant: AbstractVariant, pkg_name: Optional[str] = None): + def __init__(self, variant: VariantBase, pkg_name: Optional[str] = None): pkg_info = "" if pkg_name is None else f" in package '{pkg_name}'" msg = f"multiple values are not allowed for variant '{variant.name}'{pkg_info}"