Enable and constrain reuse for GitVersion installations (#43859)

* Preserve higher weight for CLI git ref versions

Currently the concretizer fails if you reuse a git ref version
that has already been installed but modify the spec at all.

See #38484 for futher diagnosis

The issue here is that since there is no established provenance for
these versions the highest weight they are currently assigned is
that of prior install. Re-use checks then fail because the weight of
the version is identical to the solver.

Ironically, these versions are given the highest weights possible when
specified on the CLI for the first time.  They should only appear in a
DAG if they are an exact match or if the user specifies them at the CLI.
Therefore it makes sense to preserve their higher ordering.

Getting this right is critical to moving all branch based versions to a pinned
git-ref in the future.

* [@spackbot] updating style on behalf of psakievich

* Update lib/spack/spack/solver/asp.py

Co-authored-by: Greg Becker <becker33@llnl.gov>

* Add provenance specific to git ref installs

* Sensitvity to name that I could not track down

* Add regression test

* Adjust test

* Add prefer standard unit-test

* Style

* Add required mock

* Format and mark

* Make unit-test case reproduce CLI investigation

* Remove unnecessary mock package

* [@spackbot] updating style on behalf of psakievich

* Use already developed fixture

* Add zlib-ng to mocks again

* Remove accidental adds

* Remove maintainer

* [@spackbot] updating style on behalf of psakievich

* Rename test file

* [@spackbot] updating style on behalf of psakievich

* Remove unused imports

* Update tests

* [@spackbot] updating style on behalf of psakievich

* Style

* Update lib/spack/spack/test/concretize.py

Co-authored-by: Greg Becker <becker33@llnl.gov>

* Update solver rule

* Duplicate installed rules for installed_git_version

* Revert "Duplicate installed rules for installed_git_version"

This reverts commit 17223fc8d1c9461cb87e5a230f397167d1ab6d40.

---------

Co-authored-by: psakievich <psakievich@users.noreply.github.com>
Co-authored-by: Greg Becker <becker33@llnl.gov>

3 files changed, 56 insertions, 4 deletions

diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index dc1aa5fea0..2701ea25fd 100644
--- a/lib/spack/spack/solver/asp.py
+++ b/lib/spack/spack/solver/asp.py
@@ -116,6 +116,8 @@ class Provenance(enum.IntEnum):
     PACKAGE_PY = enum.auto()
     # An installed spec
     INSTALLED = enum.auto()
+    # lower provenance for installed git refs so concretizer prefers StandardVersion installs
+    INSTALLED_GIT_VERSION = enum.auto()
     # A runtime injected from another package (e.g. a compiler)
     RUNTIME = enum.auto()
 
@@ -2072,7 +2074,7 @@ class SpackSolverSetup:
             # best possible, so they're guaranteed to be used preferentially.
             version = s.versions.concrete
 
-            if version is None or any(v == version for v in self.possible_versions[s.name]):
+            if version is None or (any((v == version) for v in self.possible_versions[s.name])):
                 continue
 
             if require_checksum and not _is_checksummed_git_version(version):
@@ -2386,9 +2388,16 @@ class SpackSolverSetup:
             # - Add OS to possible OS's
             for dep in spec.traverse():
                 self.possible_versions[dep.name].add(dep.version)
-                self.declared_versions[dep.name].append(
-                    DeclaredVersion(version=dep.version, idx=0, origin=Provenance.INSTALLED)
-                )
+                if isinstance(dep.version, vn.GitVersion):
+                    self.declared_versions[dep.name].append(
+                        DeclaredVersion(
+                            version=dep.version, idx=0, origin=Provenance.INSTALLED_GIT_VERSION
+                        )
+                    )
+                else:
+                    self.declared_versions[dep.name].append(
+                        DeclaredVersion(version=dep.version, idx=0, origin=Provenance.INSTALLED)
+                    )
                 self.possible_oses.add(dep.os)
 
     def define_concrete_input_specs(self, specs, possible):
diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp
index 20bcd46ad6..8c270e1702 100644
--- a/lib/spack/spack/solver/concretize.lp
+++ b/lib/spack/spack/solver/concretize.lp
@@ -256,6 +256,7 @@ possible_version_weight(node(ID, Package), Weight)
 :- attr("version", node(ID, Package), Version),
    version_weight(node(ID, Package), Weight),
    not pkg_fact(Package, version_declared(Version, Weight, "installed")),
+   not pkg_fact(Package, version_declared(Version, Weight, "installed_git_version")),
    not build(node(ID, Package)),
    internal_error("Build version weight used for reused package").
 
diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py
index b1cd35bff9..547e3209eb 100644
--- a/lib/spack/spack/test/concretize.py
+++ b/lib/spack/spack/test/concretize.py
@@ -3043,3 +3043,45 @@ def test_spec_filters(specs, include, exclude, expected):
         factory=lambda: specs, is_usable=lambda x: True, include=include, exclude=exclude
     )
     assert f.selected_specs() == expected
+
+
+@pytest.mark.only_clingo("clingo only reuse feature being tested")
+@pytest.mark.regression("38484")
+def test_git_ref_version_can_be_reused(
+    install_mockery_mutable_config, do_not_check_runtimes_on_reuse
+):
+    first_spec = spack.spec.Spec("git-ref-package@git.2.1.5=2.1.5~opt").concretized()
+    first_spec.package.do_install(fake=True, explicit=True)
+
+    with spack.config.override("concretizer:reuse", True):
+        # reproducer of the issue is that spack will solve when there is a change to the base spec
+        second_spec = spack.spec.Spec("git-ref-package@git.2.1.5=2.1.5+opt").concretized()
+        assert second_spec.dag_hash() != first_spec.dag_hash()
+        # we also want to confirm that reuse actually works so leave variant off to
+        # let solver reuse
+        third_spec = spack.spec.Spec("git-ref-package@git.2.1.5=2.1.5")
+        assert first_spec.satisfies(third_spec)
+        third_spec.concretize()
+        assert third_spec.dag_hash() == first_spec.dag_hash()
+
+
+@pytest.mark.only_clingo("clingo only reuse feature being tested")
+@pytest.mark.parametrize("standard_version", ["2.0.0", "2.1.5", "2.1.6"])
+def test_reuse_prefers_standard_over_git_versions(
+    standard_version, install_mockery_mutable_config, do_not_check_runtimes_on_reuse
+):
+    """
+    order matters in this test. typically reuse would pick the highest versioned installed match
+    but we want to prefer the standard version over git ref based versions
+    so install git ref last and ensure it is not picked up by reuse
+    """
+    standard_spec = spack.spec.Spec(f"git-ref-package@{standard_version}").concretized()
+    standard_spec.package.do_install(fake=True, explicit=True)
+
+    git_spec = spack.spec.Spec("git-ref-package@git.2.1.5=2.1.5").concretized()
+    git_spec.package.do_install(fake=True, explicit=True)
+
+    with spack.config.override("concretizer:reuse", True):
+        test_spec = spack.spec.Spec("git-ref-package@2").concretized()
+        assert git_spec.dag_hash() != test_spec.dag_hash()
+        assert standard_spec.dag_hash() == test_spec.dag_hash()