diff options
author | Todd Gamblin <tgamblin@llnl.gov> | 2024-11-10 17:37:36 -0800 |
---|---|---|
committer | Todd Gamblin <tgamblin@llnl.gov> | 2024-11-11 01:54:57 -0800 |
commit | c4a5a996a54b0e9385c9311aea0efbe6df00de28 (patch) | |
tree | c5da5a1468792ebf3b0af29f4488da77a3e25eef /lib | |
parent | 6961514122e04f3509c5e4a095060b056c38f5e9 (diff) | |
download | spack-c4a5a996a54b0e9385c9311aea0efbe6df00de28.tar.gz spack-c4a5a996a54b0e9385c9311aea0efbe6df00de28.tar.bz2 spack-c4a5a996a54b0e9385c9311aea0efbe6df00de28.tar.xz spack-c4a5a996a54b0e9385c9311aea0efbe6df00de28.zip |
solver: avoid parsing specs in setup
- [x] Get rid of a call to `parser.quote_if_needed()` during solver setup, which
introduces a circular import and also isn't necessary.
- [x] Rename `spack.variant.Value` to `spack.variant.ConditionalValue`, as it is *only*
used for conditional values. This makes it much easier to understand some of the
logic for variant definitions.
Co-authored-by: Harmen Stoppels <me@harmenstoppels.nl>
Signed-off-by: Todd Gamblin <tgamblin@llnl.gov>
Diffstat (limited to 'lib')
-rw-r--r-- | lib/spack/spack/audit.py | 4 | ||||
-rw-r--r-- | lib/spack/spack/directives.py | 2 | ||||
-rw-r--r-- | lib/spack/spack/solver/asp.py | 17 | ||||
-rw-r--r-- | lib/spack/spack/test/variant.py | 2 | ||||
-rw-r--r-- | lib/spack/spack/variant.py | 11 |
5 files changed, 20 insertions, 16 deletions
diff --git a/lib/spack/spack/audit.py b/lib/spack/spack/audit.py index dc988ac90e..273e335ade 100644 --- a/lib/spack/spack/audit.py +++ b/lib/spack/spack/audit.py @@ -714,9 +714,9 @@ def _ensure_env_methods_are_ported_to_builders(pkgs, error_cls): for pkg_name in pkgs: pkg_cls = spack.repo.PATH.get_pkg_class(pkg_name) - # values are either Value objects (for conditional values) or the values themselves + # values are either ConditionalValue objects or the values themselves build_system_names = set( - v.value if isinstance(v, spack.variant.Value) else v + v.value if isinstance(v, spack.variant.ConditionalValue) else v for _, variant in pkg_cls.variant_definitions("build_system") for v in variant.values ) diff --git a/lib/spack/spack/directives.py b/lib/spack/spack/directives.py index 7a3657e222..0d6b66780c 100644 --- a/lib/spack/spack/directives.py +++ b/lib/spack/spack/directives.py @@ -583,7 +583,7 @@ def conditional(*values: List[Any], when: Optional[WhenType] = None): # _make_when_spec returns None when the condition is statically false. when = _make_when_spec(when) return spack.variant.ConditionalVariantValues( - spack.variant.Value(x, when=when) for x in values + spack.variant.ConditionalValue(x, when=when) for x in values ) diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index b723b6bbb2..24b7aeb4ff 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -1436,14 +1436,13 @@ class SpackSolverSetup: for value in sorted(values): pkg_fact(fn.variant_possible_value(vid, value)) - # when=True means unconditional, so no need for conditional values - if getattr(value, "when", True) is True: + # we're done here for unconditional values + if not isinstance(value, vt.ConditionalValue): continue - # now we have to handle conditional values - quoted_value = spack.parser.quote_if_needed(str(value)) - vstring = f"{name}={quoted_value}" - variant_has_value = spack.spec.Spec(vstring) + # 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) if value.when: # the conditional value is always "possible", but it imposes its when condition as @@ -1454,10 +1453,12 @@ class SpackSolverSetup: imposed_spec=value.when, required_name=pkg.name, imposed_name=pkg.name, - msg=f"{pkg.name} variant {name} has value '{quoted_value}' when {value.when}", + msg=f"{pkg.name} variant {name} has value '{value.value}' when {value.when}", ) else: - # We know the value is never allowed statically (when was false), but we can't just + vstring = f"{name}='{value.value}'" + + # We know the value is never allowed statically (when was None), but we can't just # ignore it b/c it could come in as a possible value and we need a good error msg. # So, it's a conflict -- if the value is somehow used, it'll trigger an error. trigger_id = self.condition( diff --git a/lib/spack/spack/test/variant.py b/lib/spack/spack/test/variant.py index c4c439b86f..518110d525 100644 --- a/lib/spack/spack/test/variant.py +++ b/lib/spack/spack/test/variant.py @@ -762,7 +762,7 @@ def test_disjoint_set_fluent_methods(): @pytest.mark.regression("32694") @pytest.mark.parametrize("other", [True, False]) def test_conditional_value_comparable_to_bool(other): - value = spack.variant.Value("98", when="@1.0") + value = spack.variant.ConditionalValue("98", when=Spec("@1.0")) comparison = value == other assert comparison is False diff --git a/lib/spack/spack/variant.py b/lib/spack/spack/variant.py index e5dcf72f36..0dc82b2ff7 100644 --- a/lib/spack/spack/variant.py +++ b/lib/spack/spack/variant.py @@ -775,18 +775,21 @@ def disjoint_sets(*sets): @functools.total_ordering -class Value: - """Conditional value that might be used in variants.""" +class ConditionalValue: + """Conditional value for a variant.""" value: Any - when: Optional["spack.spec.Spec"] # optional b/c we need to know about disabled values + + # optional because statically disabled values (when=False) are set to None + # when=True results in spack.spec.Spec() + when: Optional["spack.spec.Spec"] def __init__(self, value: Any, when: Optional["spack.spec.Spec"]): self.value = value self.when = when def __repr__(self): - return f"Value({self.value}, when={self.when})" + return f"ConditionalValue({self.value}, when={self.when})" def __str__(self): return str(self.value) |