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>

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
         )

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
     )
 
 

lib/spack/spack/solver/asp.py

@@ -1436,14 +1436,13 @@ def define_variant(
         for value in sorted(values):
             pkg_fact(fn.variant_possible_value(vid, value))
 
-            # when=True means unconditional, so no need for conditional values
+            # we're done here for unconditional values
-            if getattr(value, "when", True) is True:
+            if not isinstance(value, vt.ConditionalValue):
                 continue
 
-            # now we have to handle conditional values
+            # make a spec indicating whether the variant has this conditional value
-            quoted_value = spack.parser.quote_if_needed(str(value))
+            variant_has_value = spack.spec.Spec()
-            vstring = f"{name}={quoted_value}"
+            variant_has_value.variants[name] = spack.variant.AbstractVariant(name, value.value)
-            variant_has_value = spack.spec.Spec(vstring)
 
             if value.when:
                 # the conditional value is always "possible", but it imposes its when condition as
@@ -1454,10 +1453,12 @@ def define_variant(
                     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(

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
 

lib/spack/spack/variant.py

@@ -775,18 +775,21 @@ def disjoint_sets(*sets):
 
 
 @functools.total_ordering
-class Value:
+class ConditionalValue:
-    """Conditional value that might be used in variants."""
+    """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)