Git Ref versions can be paired to defined versions in the spec (#30998)

The current use of git ref's as a version requires a search algorithm to pick the right matching version based on the tags in the git history of the package.

This is less than ideal for the use case where users already know the specific version they want the git ref to be associated with. This PR makes a new version syntax [package]@[ref]=[version] to allow the users to specify the exact hash they wish to use.

11 files changed, 179 insertions, 20 deletions

diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index 43a3d341f0..c28e6f459a 100644
--- a/lib/spack/spack/solver/asp.py
+++ b/lib/spack/spack/solver/asp.py
@@ -101,14 +101,7 @@ ast_type = ast_getter("ast_type", "type")
 ast_sym = ast_getter("symbol", "term")
 
 #: Order of precedence for version origins. Topmost types are preferred.
-version_origin_fields = [
-    "spec",
-    "external",
-    "packages_yaml",
-    "package_py",
-    "installed",
-]
-
+version_origin_fields = ["spec", "external", "packages_yaml", "package_py", "installed"]
 #: Look up version precedence strings by enum id
 version_origin_str = {i: name for i, name in enumerate(version_origin_fields)}
 
@@ -511,7 +504,10 @@ def _normalize_packages_yaml(packages_yaml):
                 entry["buildable"] = False
 
         externals = data.get("externals", [])
-        keyfn = lambda x: spack.spec.Spec(x["spec"]).name
+
+        def keyfn(x):
+            return spack.spec.Spec(x["spec"]).name
+
         for provider, specs in itertools.groupby(externals, key=keyfn):
             entry = normalized_yaml.setdefault(provider, {})
             entry.setdefault("externals", []).extend(specs)
@@ -793,6 +789,30 @@ class SpackSolverSetup(object):
                 )
             )
 
+        for v in most_to_least_preferred:
+            # There are two paths for creating the ref_version in GitVersions.
+            # The first uses a lookup to supply a tag and distance as a version.
+            # The second is user specified and can be resolved as a standard version.
+            # This second option is constrained such that the user version must be known to Spack
+            if (
+                isinstance(v.version, spack.version.GitVersion)
+                and v.version.user_supplied_reference
+            ):
+                ref_version = spack.version.Version(v.version.ref_version_str)
+                self.gen.fact(fn.version_equivalent(pkg.name, v.version, ref_version))
+                # disqualify any git supplied version from user if they weren't already known
+                # versions in spack
+                if not any(ref_version == dv.version for dv in most_to_least_preferred if v != dv):
+                    msg = (
+                        "The reference version '{version}' for package '{package}' is not defined."
+                        " Either choose another reference version or define '{version}' in your"
+                        " version preferences or package.py file for {package}.".format(
+                            package=pkg.name, version=str(ref_version)
+                        )
+                    )
+
+                    raise UnsatisfiableSpecError(msg)
+
         # Declare deprecated versions for this package, if any
         deprecated = self.deprecated_versions[pkg.name]
         for v in sorted(deprecated):
diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp
index 3b264d7bfd..8ed6b1b6bb 100644
--- a/lib/spack/spack/solver/concretize.lp
+++ b/lib/spack/spack/solver/concretize.lp
@@ -76,6 +76,11 @@ version_declared(Package, Version, Weight) :- version_declared(Package, Version,
 % versions are declared w/priority -- declared with priority implies declared
 version_declared(Package, Version) :- version_declared(Package, Version, _).
 
+% a spec with a git hash version is equivalent to one with the same matched version
+version_satisfies(Package, Constraint, HashVersion) :- version_satisfies(Package, Constraint, EquivalentVersion),
+  version_equivalent(Package, HashVersion, EquivalentVersion).
+#defined version_equivalent/3.
+
 % If something is a package, it has only one version and that must be a
 % declared version.
 % We allow clingo to choose any version(s), and infer an error if there
diff --git a/lib/spack/spack/solver/display.lp b/lib/spack/spack/solver/display.lp
index f8ec006eeb..e8f5eeba16 100644
--- a/lib/spack/spack/solver/display.lp
+++ b/lib/spack/spack/solver/display.lp
@@ -26,6 +26,7 @@
 #show node_flag_source/2.
 #show no_flags/2.
 #show external_spec_selected/2.
+#show version_equivalent/3.
 
 #show build/1.
 
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index 8263c25876..3bad05c612 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -5265,6 +5265,12 @@ class SpecParser(spack.parse.Parser):
         end = None
         if self.accept(ID):
             start = self.token.value
+            if self.accept(EQ):
+                # This is for versions that are associated with a hash
+                # i.e. @[40 char hash]=version
+                start += self.token.value
+                self.expect(VAL)
+                start += self.token.value
 
         if self.accept(COLON):
             if self.accept(ID):
diff --git a/lib/spack/spack/test/cmd/spec.py b/lib/spack/spack/test/cmd/spec.py
index b6a6281bf7..d604bc4515 100644
--- a/lib/spack/spack/test/cmd/spec.py
+++ b/lib/spack/spack/test/cmd/spec.py
@@ -12,6 +12,7 @@ import spack.environment as ev
 import spack.error
 import spack.spec
 import spack.store
+from spack.fetch_strategy import FetchError
 from spack.main import SpackCommand, SpackCommandError
 
 pytestmark = pytest.mark.usefixtures("config", "mutable_mock_repo")
@@ -202,3 +203,21 @@ def test_env_aware_spec(mutable_mock_env_path):
         assert "libdwarf@20130729" in output
         assert "libelf@0.8.1" in output
         assert "mpich@3.0.4" in output
+
+
+@pytest.mark.parametrize(
+    "name, version, error",
+    [
+        ("develop-branch-version", "f3c7206350ac8ee364af687deaae5c574dcfca2c=develop", None),
+        ("develop-branch-version", "git." + "a" * 40 + "=develop", None),
+        ("callpath", "f3c7206350ac8ee364af687deaae5c574dcfca2c=1.0", FetchError),
+        ("develop-branch-version", "git.foo=0.2.15", None),
+    ],
+)
+def test_spec_version_assigned_git_ref_as_version(name, version, error):
+    if error:
+        with pytest.raises(error):
+            output = spec(name + "@" + version)
+    else:
+        output = spec(name + "@" + version)
+        assert version in output
diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py
index 1560a085dd..dd6d0e8ab6 100644
--- a/lib/spack/spack/test/concretize.py
+++ b/lib/spack/spack/test/concretize.py
@@ -21,6 +21,7 @@ import spack.platforms
 import spack.repo
 import spack.variant as vt
 from spack.concretize import find_spec
+from spack.solver.asp import UnsatisfiableSpecError
 from spack.spec import Spec
 from spack.version import ver
 
@@ -1735,3 +1736,34 @@ class TestConcretize(object):
         concrete_spec = result.specs[0]
         assert concrete_spec.satisfies("%gcc@4.5.0")
         assert concrete_spec.satisfies("os=debian6")
+
+    def test_git_hash_assigned_version_is_preferred(self):
+        hash = "a" * 40
+        s = Spec("develop-branch-version@%s=develop" % hash)
+        c = s.concretized()
+        assert hash in str(c)
+
+    @pytest.mark.skipif(sys.platform == "win32", reason="Not supported on Windows (yet)")
+    @pytest.mark.parametrize("git_ref", ("a" * 40, "0.2.15", "main"))
+    def test_git_ref_version_is_equivalent_to_specified_version(self, git_ref):
+        if spack.config.get("config:concretizer") == "original":
+            pytest.skip("Original concretizer cannot account for git hashes")
+        s = Spec("develop-branch-version@git.%s=develop" % git_ref)
+        c = s.concretized()
+        assert git_ref in str(c)
+        print(str(c))
+        assert s.satisfies("@develop")
+        assert s.satisfies("@0.1:")
+
+    @pytest.mark.skipif(sys.platform == "win32", reason="Not supported on Windows (yet)")
+    @pytest.mark.parametrize("git_ref", ("a" * 40, "0.2.15", "fbranch"))
+    def test_git_ref_version_errors_if_unknown_version(self, git_ref):
+        if spack.config.get("config:concretizer") == "original":
+            pytest.skip("Original concretizer cannot account for git hashes")
+        # main is not defined in the package.py for this file
+        s = Spec("develop-branch-version@git.%s=main" % git_ref)
+        with pytest.raises(
+            UnsatisfiableSpecError,
+            match="The reference version 'main' for package 'develop-branch-version'",
+        ):
+            s.concretized()
diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py
index 956593ceb8..e58cf4214d 100644
--- a/lib/spack/spack/test/spec_syntax.py
+++ b/lib/spack/spack/test/spec_syntax.py
@@ -185,6 +185,10 @@ class TestSpecSyntax(object):
             'mvapich cflags="-O3 -fPIC" emacs^ncurses%intel',
         )
 
+    def test_spec_with_version_hash_pair(self):
+        hash = "abc12" * 8
+        self.check_parse("develop-branch-version@%s=develop" % hash)
+
     def test_full_specs(self):
         self.check_parse(
             "mvapich_foo" " ^_openmpi@1.2:1.4,1.6%intel@12.1+debug~qt_4" " ^stackwalker@8.1_1e"
diff --git a/lib/spack/spack/test/versions.py b/lib/spack/spack/test/versions.py
index d73a48cc1b..a6d44027f4 100644
--- a/lib/spack/spack/test/versions.py
+++ b/lib/spack/spack/test/versions.py
@@ -585,7 +585,7 @@ def test_list_highest():
     assert vl2.lowest() == Version("master")
 
 
-@pytest.mark.parametrize("version_str", ["foo 1.2.0", "!", "1!2"])
+@pytest.mark.parametrize("version_str", ["foo 1.2.0", "!", "1!2", "=1.2.0"])
 def test_invalid_versions(version_str):
     """Ensure invalid versions are rejected with a ValueError"""
     with pytest.raises(ValueError):
@@ -727,3 +727,22 @@ def test_version_list_with_range_included_in_concrete_version_interpreted_as_ran
 def test_version_list_with_range_and_concrete_version_is_not_concrete():
     v = VersionList([Version("3.1"), VersionRange("3.1.1", "3.1.2")])
     assert v.concrete
+
+
+@pytest.mark.parametrize(
+    "vstring, eq_vstring, is_commit",
+    (
+        ("abc12" * 8 + "=develop", "develop", True),
+        ("git." + "abc12" * 8 + "=main", "main", True),
+        ("a" * 40 + "=develop", "develop", True),
+        ("b" * 40 + "=3.2", "3.2", True),
+        ("git.foo=3.2", "3.2", False),
+    ),
+)
+def test_git_ref_can_be_assigned_a_version(vstring, eq_vstring, is_commit):
+    v = Version(vstring)
+    v_equivalent = Version(eq_vstring)
+    assert v.is_commit == is_commit
+    assert v.is_ref
+    assert not v._ref_lookup
+    assert v_equivalent.version == v.ref_version
diff --git a/lib/spack/spack/version.py b/lib/spack/spack/version.py
index efcee74afe..89c7414abd 100644
--- a/lib/spack/spack/version.py
+++ b/lib/spack/spack/version.py
@@ -45,7 +45,7 @@ from spack.util.spack_yaml import syaml_dict
 __all__ = ["Version", "VersionRange", "VersionList", "ver"]
 
 # Valid version characters
-VALID_VERSION = re.compile(r"^[A-Za-z0-9_.-]+$")
+VALID_VERSION = re.compile(r"^[A-Za-z0-9_.-][=A-Za-z0-9_.-]*$")
 
 # regex for a commit version
 COMMIT_VERSION = re.compile(r"^[a-f0-9]{40}$")
@@ -178,6 +178,8 @@ def is_git_version(string):
         return True
     elif len(string) == 40 and COMMIT_VERSION.match(string):
         return True
+    elif "=" in string:
+        return True
     return False
 
 
@@ -211,9 +213,13 @@ class VersionBase(object):
         if string and not VALID_VERSION.match(string):
             raise ValueError("Bad characters in version string: %s" % string)
 
+        self.separators, self.version = self._generate_seperators_and_components(string)
+
+    def _generate_seperators_and_components(self, string):
         segments = SEGMENT_REGEX.findall(string)
-        self.version = tuple(int(m[0]) if m[0] else VersionStrComponent(m[1]) for m in segments)
-        self.separators = tuple(m[2] for m in segments)
+        components = tuple(int(m[0]) if m[0] else VersionStrComponent(m[1]) for m in segments)
+        separators = tuple(m[2] for m in segments)
+        return separators, components
 
     @property
     def dotted(self):
@@ -464,21 +470,30 @@ class GitVersion(VersionBase):
         if not isinstance(string, str):
             string = str(string)  # In case we got a VersionBase or GitVersion object
 
+        # An object that can lookup git refs to compare them to versions
+        self.user_supplied_reference = False
+        self._ref_lookup = None
+        self.ref_version = None
+
         git_prefix = string.startswith("git.")
-        self.ref = string[4:] if git_prefix else string
+        pruned_string = string[4:] if git_prefix else string
+
+        if "=" in pruned_string:
+            self.ref, self.ref_version_str = pruned_string.split("=")
+            _, self.ref_version = self._generate_seperators_and_components(self.ref_version_str)
+            self.user_supplied_reference = True
+        else:
+            self.ref = pruned_string
 
-        self.is_commit = len(self.ref) == 40 and COMMIT_VERSION.match(self.ref)
+        self.is_commit = bool(len(self.ref) == 40 and COMMIT_VERSION.match(self.ref))
         self.is_ref = git_prefix  # is_ref False only for comparing to VersionBase
         self.is_ref |= bool(self.is_commit)
 
         # ensure git.<hash> and <hash> are treated the same by dropping 'git.'
-        canonical_string = self.ref if self.is_commit else string
+        # unless we are assigning a version with =
+        canonical_string = self.ref if (self.is_commit and not self.ref_version) else string
         super(GitVersion, self).__init__(canonical_string)
 
-        # An object that can lookup git refs to compare them to versions
-        self._ref_lookup = None
-        self.ref_version = None
-
     def _cmp(self, other_lookups=None):
         # No need to rely on git comparisons for develop-like refs
         if len(self.version) == 2 and self.isdevelop():
@@ -603,6 +618,10 @@ class GitVersion(VersionBase):
         if not self.is_ref:
             tty.die("%s is not a git version." % self)
 
+        # don't need a lookup if we already have a version assigned
+        if self.ref_version:
+            return
+
         # Generate a commit looker-upper
         self._ref_lookup = CommitLookup(pkg_name)
 
diff --git a/var/spack/repos/builtin.mock/packages/depends-on-develop/package.py b/var/spack/repos/builtin.mock/packages/depends-on-develop/package.py
new file mode 100644
index 0000000000..94d0c091d8
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/depends-on-develop/package.py
@@ -0,0 +1,17 @@
+# Copyright 2013-2022 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.package import *
+
+
+class DependsOnDevelop(Package):
+    homepage = "example.com"
+    url = "fake.com"
+
+    version("main", branch="main")
+    version("0.0.0", sha256="0123456789abcdef0123456789abcdef")
+
+    depends_on("develop-branch-version@develop")
diff --git a/var/spack/repos/builtin.mock/packages/develop-branch-version/package.py b/var/spack/repos/builtin.mock/packages/develop-branch-version/package.py
new file mode 100644
index 0000000000..2f8c96352e
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/develop-branch-version/package.py
@@ -0,0 +1,17 @@
+# Copyright 2013-2022 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.package import *
+
+
+class DevelopBranchVersion(Package):
+    """Dummy package with develop version"""
+
+    homepage = "http://www.openblas.net"
+    url = "http://github.com/xianyi/OpenBLAS/archive/v0.2.15.tar.gz"
+    git = "https://github.com/dummy/repo.git"
+
+    version("develop", branch="develop")
+    version("0.2.15", "b1190f3d3471685f17cfd1ec1d252ac9")