ASP-based solver: variants set from cli are considered as defaults (#23542)

Variants explicitly set in an abstract root spec are considered
as defaults for the package they refer to, and they override
what is in packages.yaml and in package.py. This is relevant
only for multi-valued variants, where a constraint may extend
an already default value.

3 files changed, 40 insertions, 7 deletions

diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index 311b0c4753..8fb426c325 100644
--- a/lib/spack/spack/solver/asp.py
+++ b/lib/spack/spack/solver/asp.py
@@ -1393,7 +1393,10 @@ class SpackSolverSetup(object):
             )
             for clause in self.spec_clauses(spec):
                 self.gen.fact(clause)
-
+                if clause.name == 'variant_set':
+                    self.gen.fact(fn.variant_default_value_from_cli(
+                        *clause.args
+                    ))
         self.gen.h1("Variant Values defined in specs")
         self.define_variant_values()
 
diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp
index 4fe8f64761..8b5671ee4e 100644
--- a/lib/spack/spack/solver/concretize.lp
+++ b/lib/spack/spack/solver/concretize.lp
@@ -403,14 +403,23 @@ variant_not_default(Package, Variant, Value, 0)
     external_with_variant_set(Package, Variant, Value),
     node(Package).
 
-% The default value for a variant in a package is what is written
-% in the package.py file, unless some preference is set in packages.yaml
+% The default value for a variant in a package is what is prescribed:
+%
+% 1. On the command line
+% 2. In packages.yaml (if there's no command line settings)
+% 3. In the package.py file (if there are no settings in
+%    packages.yaml and the command line)
+%
 variant_default_value(Package, Variant, Value)
  :- variant_default_value_from_package_py(Package, Variant, Value),
-    not variant_default_value_from_packages_yaml(Package, Variant, _).
+    not variant_default_value_from_packages_yaml(Package, Variant, _),
+    not variant_default_value_from_cli(Package, Variant, _).
 
 variant_default_value(Package, Variant, Value)
- :- variant_default_value_from_packages_yaml(Package, Variant, Value).
+ :- variant_default_value_from_packages_yaml(Package, Variant, Value),
+    not variant_default_value_from_cli(Package, Variant, _).
+
+variant_default_value(Package, Variant, Value) :- variant_default_value_from_cli(Package, Variant, Value).
 
 % Treat 'none' in a special way - it cannot be combined with other
 % values even if the variant is multi-valued
@@ -435,6 +444,7 @@ variant_single_value(Package, "dev_path")
 #defined variant_single_value/2.
 #defined variant_default_value/3.
 #defined variant_possible_value/3.
+#defined variant_default_value_from_cli/3.
 #defined variant_default_value_from_packages_yaml/3.
 #defined variant_default_value_from_package_py/3.
 #defined variant_value_from_disjoint_sets/4.
@@ -704,7 +714,7 @@ opt_criterion(14, "number of non-default variants (roots)").
 % If the value is a multivalued variant there could be multiple
 % values set as default. Since a default value has a weight of 0 we
 % need to maximize their number below to ensure they're all set
-opt_criterion(13, "multi-valued variants + preferred providers for roots").
+opt_criterion(13, "multi-valued variants").
 #minimize{ 0@13 : #true }.
 #maximize {
     1@13,Package,Variant,Value
@@ -712,8 +722,10 @@ opt_criterion(13, "multi-valued variants + preferred providers for roots").
     not variant_single_value(Package, Variant),
     root(Package)
 }.
+opt_criterion(12, "preferred providers for roots").
+#minimize{ 0@12 : #true }.
 #minimize{
-    Weight@13,Provider
+    Weight@12,Provider
     : provider_weight(Provider, Weight), root(Provider)
 }.
 
diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py
index db08a8a189..fd97c8684f 100644
--- a/lib/spack/spack/test/concretize.py
+++ b/lib/spack/spack/test/concretize.py
@@ -1210,3 +1210,21 @@ class TestConcretize(object):
 
         for node in s.traverse():
             assert node.satisfies(expected_compiler)
+
+    @pytest.mark.parametrize('spec_str,expected_dict', [
+        # Check the defaults from the package (libs=shared)
+        ('multivalue-variant', {
+            'libs=shared': True,
+            'libs=static': False
+        }),
+        # Check that libs=static doesn't extend the default
+        ('multivalue-variant libs=static', {
+            'libs=shared': False,
+            'libs=static': True
+        }),
+    ])
+    def test_multivalued_variants_from_cli(self, spec_str, expected_dict):
+        s = Spec(spec_str).concretized()
+
+        for constraint, value in expected_dict.items():
+            assert s.satisfies(constraint) == value