ASP-based solver: reordered low priority optimization criteria (#24184)

Minimizing compiler mismatches in the DAG and preferring newer 
versions of packages are now higher priority than trying to use as 
many default values as possible in multi-valued variants.

5 files changed, 53 insertions, 16 deletions

diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp
index ec0d4fdf5a..c7d4c1ae44 100644
--- a/lib/spack/spack/solver/concretize.lp
+++ b/lib/spack/spack/solver/concretize.lp
@@ -748,37 +748,37 @@ opt_criterion(11, "number of non-default variants (non-roots)").
 
 % Minimize the weights of the providers, i.e. use as much as
 % possible the most preferred providers
-opt_criterion(9, "number of non-default providers (non-roots)").
+opt_criterion(9, "preferred providers (non-roots)").
 #minimize{ 0@9 : #true }.
 #minimize{
     Weight@9,Provider
     : provider_weight(Provider, Weight), not root(Provider)
 }.
 
+% Try to minimize the number of compiler mismatches in the DAG.
+opt_criterion(8, "compiler mismatches").
+#minimize{ 0@8 : #true }.
+#minimize{ 1@8,Package,Dependency : compiler_mismatch(Package, Dependency) }.
+
+% Choose more recent versions for nodes
+opt_criterion(7, "version badness").
+#minimize{ 0@7 : #true }.
+#minimize{
+    Weight@7,Package : version_weight(Package, Weight)
+}.
+
 % 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(8, "count of non-root multi-valued variants").
-#minimize{ 0@8 : #true }.
+opt_criterion(6, "count of non-root multi-valued variants").
+#minimize{ 0@6 : #true }.
 #maximize {
-    1@8,Package,Variant,Value
+    1@6,Package,Variant,Value
     : variant_not_default(Package, Variant, Value, _),
     not variant_single_value(Package, Variant),
     not root(Package)
 }.
 
-% Try to minimize the number of compiler mismatches in the DAG.
-opt_criterion(7, "compiler mismatches").
-#minimize{ 0@7 : #true }.
-#minimize{ 1@7,Package,Dependency : compiler_mismatch(Package, Dependency) }.
-
-% Choose more recent versions for nodes
-opt_criterion(6, "version badness").
-#minimize{ 0@6 : #true }.
-#minimize{
-    Weight@6,Package : version_weight(Package, Weight)
-}.
-
 % Try to use preferred compilers
 opt_criterion(5, "non-preferred compilers").
 #minimize{ 0@5 : #true }.
diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py
index f392387099..c0f7b4c1f8 100644
--- a/lib/spack/spack/test/concretize.py
+++ b/lib/spack/spack/test/concretize.py
@@ -1244,3 +1244,12 @@ class TestConcretize(object):
 
         for abstract_spec in expected:
             assert abstract_spec in s
+
+    @pytest.mark.regression('24196')
+    def test_version_badness_more_important_than_default_mv_variants(self):
+        # If a dependency had an old version that for some reason pulls in
+        # a transitive dependency with a multi-valued variant, that old
+        # version was preferred because of the order of our optimization
+        # criteria.
+        s = spack.spec.Spec('root').concretized()
+        assert s['gmt'].satisfies('@2.0')
diff --git a/var/spack/repos/builtin.mock/packages/gmt/package.py b/var/spack/repos/builtin.mock/packages/gmt/package.py
new file mode 100644
index 0000000000..444c3d3d95
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/gmt/package.py
@@ -0,0 +1,10 @@
+# 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)
+class Gmt(Package):
+
+    version('2.0', 'abcdef')
+    version('1.0', 'abcdef')
+
+    depends_on('mvdefaults', when='@1.0')
diff --git a/var/spack/repos/builtin.mock/packages/mvdefaults/package.py b/var/spack/repos/builtin.mock/packages/mvdefaults/package.py
new file mode 100644
index 0000000000..d6f7e7e0db
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/mvdefaults/package.py
@@ -0,0 +1,10 @@
+# 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)
+class Mvdefaults(Package):
+
+    version('1.0', 'abcdef')
+
+    variant('foo', values=('a', 'b', 'c'), default=('a', 'b', 'c'),
+            multi=True, description='')
diff --git a/var/spack/repos/builtin.mock/packages/root/package.py b/var/spack/repos/builtin.mock/packages/root/package.py
new file mode 100644
index 0000000000..b0de915b29
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/root/package.py
@@ -0,0 +1,8 @@
+# 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)
+class Root(Package):
+    version('1.0', 'abcdef')
+
+    depends_on('gmt')