ASP-based solver: decrease the priority of multi-valued variant optimization for root (#26677)

The ASP-based solver maximizes the number of values in multi-valued
variants (if other higher order constraints are met), to avoid cases
where only a subset of the values that have been specified on the
command line or imposed by another constraint are selected.

Here we swap the priority of this optimization target with the
selection of the default providers, to avoid unexpected results
like the one in #26598

4 files changed, 70 insertions, 10 deletions

diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp
index 8006e5f94f..e444faffec 100644
--- a/lib/spack/spack/solver/concretize.lp
+++ b/lib/spack/spack/solver/concretize.lp
@@ -732,23 +732,24 @@ opt_criterion(14, "number of non-default variants (roots)").
     : variant_not_default(Package, Variant, Value, Weight), root(Package)
 }.
 
+opt_criterion(13, "preferred providers for roots").
+#minimize{ 0@13 : #true }.
+#minimize{
+    Weight@13,Provider,Virtual
+    : provider_weight(Provider, Virtual, Weight), root(Provider)
+}.
+
 % 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").
-#minimize{ 0@13 : #true }.
+opt_criterion(12, "number of values in multi valued variants (root)").
+#minimize{ 0@12 : #true }.
 #maximize {
-    1@13,Package,Variant,Value
+    1@12,Package,Variant,Value
     : variant_not_default(Package, Variant, Value, Weight),
     not variant_single_value(Package, Variant),
     root(Package)
 }.
-opt_criterion(12, "preferred providers for roots").
-#minimize{ 0@12 : #true }.
-#minimize{
-    Weight@12,Provider,Virtual
-    : provider_weight(Provider, Virtual, Weight), root(Provider)
-}.
 
 % Try to use default variants or variants that have been set
 opt_criterion(11, "number of non-default variants (non-roots)").
@@ -782,7 +783,7 @@ opt_criterion(7, "version badness").
 % 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(6, "count of non-root multi-valued variants").
+opt_criterion(6, "number of values in multi valued variants (non-root)").
 #minimize{ 0@6 : #true }.
 #maximize {
     1@6,Package,Variant,Value
diff --git a/lib/spack/spack/test/concretize_preferences.py b/lib/spack/spack/test/concretize_preferences.py
index 598e9d08fb..733e3efa77 100644
--- a/lib/spack/spack/test/concretize_preferences.py
+++ b/lib/spack/spack/test/concretize_preferences.py
@@ -396,3 +396,23 @@ mpi:
 
         assert s.satisfies('^version-test-pkg@2.4.6')
         assert 'version-test-dependency-preferred' not in s
+
+    @pytest.mark.regression('26598')
+    def test_multivalued_variants_are_lower_priority_than_providers(self):
+        """Test that the rule to maximize the number of values for multivalued
+        variants is considered at lower priority than selecting the default
+        provider for virtual dependencies.
+
+        This ensures that we don't e.g. select openmpi over mpich even if we
+        specified mpich as the default mpi provider, just because openmpi supports
+        more fabrics by default.
+        """
+        with spack.config.override(
+            'packages:all', {
+                'providers': {
+                    'somevirtual': ['some-virtual-preferred']
+                }
+            }
+        ):
+            s = Spec('somevirtual').concretized()
+            assert s.name == 'some-virtual-preferred'
diff --git a/var/spack/repos/builtin.mock/packages/some-virtual-mv/package.py b/var/spack/repos/builtin.mock/packages/some-virtual-mv/package.py
new file mode 100644
index 0000000000..0aeb6d0c0a
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/some-virtual-mv/package.py
@@ -0,0 +1,22 @@
+# Copyright 2013-2021 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+from spack import *
+
+
+class SomeVirtualMv(Package):
+    """Package providing a virtual dependency and with a multivalued variant."""
+
+    homepage = "http://www.example.com"
+    url = "http://www.example.com/foo-1.0.tar.gz"
+
+    version('1.0', '0123456789abcdef0123456789abcdef')
+
+    provides('somevirtual')
+
+    # This multi valued variant is needed to trigger an optimization
+    # criteria for clingo
+    variant('libs', default='shared,static', values=('shared', 'static'),
+            multi=True, description='Build shared libs, static libs or both')
diff --git a/var/spack/repos/builtin.mock/packages/some-virtual-preferred/package.py b/var/spack/repos/builtin.mock/packages/some-virtual-preferred/package.py
new file mode 100644
index 0000000000..1e8ba80526
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/some-virtual-preferred/package.py
@@ -0,0 +1,17 @@
+# Copyright 2013-2021 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+from spack import *
+
+
+class SomeVirtualPreferred(Package):
+    """Package providing a virtual dependency with a preference in packages.yaml"""
+
+    homepage = "http://www.example.com"
+    url = "http://www.example.com/foo-1.0.tar.gz"
+
+    version('1.0', '0123456789abcdef0123456789abcdef')
+
+    provides('somevirtual')