From 267358a7992ed1644f0570269054e3c4efa39c80 Mon Sep 17 00:00:00 2001
From: Massimiliano Culpo <massimiliano.culpo@gmail.com>
Date: Thu, 16 Jun 2022 23:17:40 +0200
Subject: ASP-based solver: fix rules on version weights selection (#31153)

* ASP: sort and deduplicate version weights from installed specs

* Pick version weights according to provenance

* Add unit test
---
 lib/spack/spack/solver/asp.py        |  8 +++++++-
 lib/spack/spack/solver/concretize.lp | 24 +++++++++++++++++++++---
 lib/spack/spack/test/concretize.py   | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+), 4 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index bcdb929fdc..148e896bdb 100644
--- a/lib/spack/spack/solver/asp.py
+++ b/lib/spack/spack/solver/asp.py
@@ -748,7 +748,13 @@ class SpackSolverSetup(object):
 
         pkg = packagize(pkg)
         declared_versions = self.declared_versions[pkg.name]
-        most_to_least_preferred = sorted(declared_versions, key=key_fn)
+        partially_sorted_versions = sorted(set(declared_versions), key=key_fn)
+
+        most_to_least_preferred = []
+        for _, group in itertools.groupby(partially_sorted_versions, key=key_fn):
+            most_to_least_preferred.extend(list(sorted(
+                group, reverse=True, key=lambda x: spack.version.ver(x.version)
+            )))
 
         for weight, declared_version in enumerate(most_to_least_preferred):
             self.gen.fact(fn.version_declared(
diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp
index 70ff5b438d..94a4bedcfe 100644
--- a/lib/spack/spack/solver/concretize.lp
+++ b/lib/spack/spack/solver/concretize.lp
@@ -107,10 +107,28 @@ possible_version_weight(Package, Weight)
  :- version(Package, Version),
     version_declared(Package, Version, Weight).
 
-version_weight(Package, Weight)
+% we can't use the weight for an external version if we don't use the
+% corresponding external spec.
+:- version(Package, Version),
+   version_weight(Package, Weight),
+   version_declared(Package, Version, Weight, "external"),
+   not external(Package).
+
+% we can't use a weight from an installed spec if we are building it
+% and vice-versa
+:- version(Package, Version),
+   version_weight(Package, Weight),
+   version_declared(Package, Version, Weight, "installed"),
+   build(Package).
+
+:- version(Package, Version),
+   version_weight(Package, Weight),
+   not version_declared(Package, Version, Weight, "installed"),
+   not build(Package).
+
+1 { version_weight(Package, Weight) : version_declared(Package, Version, Weight) } 1
   :- version(Package, Version),
-     node(Package),
-     Weight = #min{W : version_declared(Package, Version, W)}.
+     node(Package).
 
 % node_version_satisfies implies that exactly one of the satisfying versions
 % is the package's version, and vice versa.
diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py
index a8f8b86236..ceb72078e6 100644
--- a/lib/spack/spack/test/concretize.py
+++ b/lib/spack/spack/test/concretize.py
@@ -1753,3 +1753,36 @@ class TestConcretize(object):
             with pytest.raises(spack.solver.asp.UnsatisfiableSpecError,
                                match="'dep-with-variants' satisfies '@999'"):
                 solver.driver.solve(setup, [root_spec], reuse=reusable_specs)
+
+    @pytest.mark.regression('31148')
+    def test_version_weight_and_provenance(self):
+        """Test package preferences during coconcretization."""
+        import spack.solver.asp
+        if spack.config.get('config:concretizer') == 'original':
+            pytest.skip('Original concretizer cannot reuse')
+
+        reusable_specs = [
+            spack.spec.Spec(spec_str).concretized()
+            for spec_str in ('b@0.9', 'b@1.0')
+        ]
+        root_spec = spack.spec.Spec('a foobar=bar')
+
+        with spack.config.override("concretizer:reuse", True):
+            solver = spack.solver.asp.Solver()
+            setup = spack.solver.asp.SpackSolverSetup()
+            result = solver.driver.solve(
+                setup, [root_spec], reuse=reusable_specs, out=sys.stdout
+            )
+            # The result here should have a single spec to build ('a')
+            # and it should be using b@1.0 with a version badness of 2
+            # The provenance is:
+            # version_declared("b","1.0",0,"package_py").
+            # version_declared("b","0.9",1,"package_py").
+            # version_declared("b","1.0",2,"installed").
+            # version_declared("b","0.9",3,"installed").
+            for criterion in [
+                (1, None, 'number of packages to build (vs. reuse)'),
+                (2, 0, 'version badness')
+            ]:
+                assert criterion in result.criteria
+            assert result.specs[0].satisfies('^b@1.0')
-- 
cgit v1.2.3-60-g2f50