Update packages.py

Ensure that if a commit variant is on the spec fetch operations
use the commit

lib/spack/spack/fetch_strategy.py

@@ -1591,13 +1591,15 @@ def for_package_version(pkg, version=None):
         version = pkg.version
 
     # if it's a commit, we must use a GitFetchStrategy
-    if isinstance(version, spack.version.GitVersion):
+    commit_sha = pkg.spec.variants.get("commit", None)
+    if isinstance(version, spack.version.GitVersion) or commit_sha:
         if not hasattr(pkg, "git"):
             raise spack.error.FetchError(
                 f"Cannot fetch git version for {pkg.name}. Package has no 'git' attribute"
             )
         # Populate the version with comparisons to other commits
-        version.attach_lookup(spack.version.git_ref_lookup.GitRefLookup(pkg.name))
+        if isinstance(version, spack.version.GitVersion):
+            version.attach_lookup(spack.version.git_ref_lookup.GitRefLookup(pkg.name))
 
         # For GitVersion, we have no way to determine whether a ref is a branch or tag
         # Fortunately, we handle branches and tags identically, except tags are
@@ -1605,16 +1607,27 @@ def for_package_version(pkg, version=None):
         # We call all non-commit refs tags in this context, at the cost of a slight
         # performance hit for branches on older versions of git.
         # Branches cannot be cached, so we tell the fetcher not to cache tags/branches
-        ref_type = "commit" if version.is_commit else "tag"
-        kwargs = {"git": pkg.git, ref_type: version.ref, "no_cache": True}
 
-        kwargs["submodules"] = getattr(pkg, "submodules", False)
+        # TODO(psakiev) eventually we should  only need to clone based on the commit
+        ref_type = None
+        ref_value = None
+        if commit_sha:
+            ref_type = "commit"
+            ref_value = commit_sha.value
+        else:
+            ref_type = "commit" if version.is_commit else "tag"
+            ref_value = version.ref
+
+        kwargs = {ref_type: ref_value, "no_cache": ref_type != "commit"}
+        kwargs["git"] = pkg.version_or_package_attr("git", version)
+        kwargs["submodules"] = pkg.version_or_package_attr("submodules", version, False)
 
         # if the ref_version is a known version from the package, use that version's
-        # submodule specifications
+        # attributes
-        ref_version_attributes = pkg.versions.get(pkg.version.ref_version)
+        ref_version = getattr(pkg.version, "ref_version", None)
-        if ref_version_attributes:
+        if ref_version:
-            kwargs["submodules"] = ref_version_attributes.get("submodules", kwargs["submodules"])
+            kwargs["git"] = pkg.version_or_package_attr("git", ref_version)
+            kwargs["submodules"] = pkg.version_or_package_attr("submodules", ref_version, False)
 
         fetcher = GitFetchStrategy(**kwargs)
         return fetcher

lib/spack/spack/package_base.py

@@ -989,6 +989,42 @@ def detect_dev_src_change(self) -> bool:
         assert dev_path_var and record, "dev_path variant and record must be present"
         return fsys.recursive_mtime_greater_than(dev_path_var.value, record.installation_time)
 
+    @classmethod
+    def version_or_package_attr(cls, attr, version, default=None):
+        """
+        Get an attribute that could be on the version or package with preference to the version
+        """
+        version_attrs = cls.versions.get(version)
+        if version_attrs and attr in version_attrs:
+            return version_attrs.get(attr)
+        value = getattr(cls, attr, default)
+        if value is None:
+            raise PackageError(f"{attr} attribute not defined on {cls.name}")
+        return value
+
+    @classmethod
+    def needs_commit(cls, version) -> bool:
+        """
+        Method for checking if the package instance needs a commit sha to be found
+        """
+        if isinstance(version, GitVersion):
+            return True
+
+        ver_attrs = cls.versions.get(version)
+        if ver_attrs:
+            return bool(ver_attrs.get("commit") or ver_attrs.get("tag") or ver_attrs.get("branch"))
+
+        return False
+
+    def resolve_binary_provenance(self) -> None:
+        """
+        Method to ensure concrete spec has binary provenance.
+        Base implementation will look up git commits when appropriate.
+        Packages may override this implementation for custom implementations
+        """
+        # TODO in follow on PR adding here so SNL team can begin work ahead of spack core
+        pass
+
     def all_urls_for_version(self, version: StandardVersion) -> List[str]:
         """Return all URLs derived from version_urls(), url, urls, and
         list_url (if it contains a version) in a package in that order.

lib/spack/spack/solver/asp.py

@@ -1522,6 +1522,9 @@ def __init__(self, tests: bool = False):
         self.assumptions: List[Tuple["clingo.Symbol", bool]] = []  # type: ignore[name-defined]
         self.declared_versions: Dict[str, List[DeclaredVersion]] = collections.defaultdict(list)
         self.possible_versions: Dict[str, Set[GitOrStandardVersion]] = collections.defaultdict(set)
+        self.git_commit_versions: Dict[str, Dict[GitOrStandardVersion, str]] = (
+            collections.defaultdict(dict)
+        )
         self.deprecated_versions: Dict[str, Set[GitOrStandardVersion]] = collections.defaultdict(
             set
         )
@@ -1594,6 +1597,11 @@ def key_fn(version):
                 )
             )
 
+        for v in self.possible_versions[pkg.name]:
+            if pkg.needs_commit(v):
+                commit = pkg.version_or_package_attr("commit", v, "")
+                self.git_commit_versions[pkg.name][v] = commit
+
         # Declare deprecated versions for this package, if any
         deprecated = self.deprecated_versions[pkg.name]
         for v in sorted(deprecated):
@@ -2688,6 +2696,8 @@ def define_package_versions_and_validate_preferences(
             if pkg_name not in packages_yaml or "version" not in packages_yaml[pkg_name]:
                 continue
 
+            # TODO(psakiev) Need facts about versions
+            # - requires_commit (associated with tag or branch)
             version_defs: List[GitOrStandardVersion] = []
 
             for vstr in packages_yaml[pkg_name]["version"]:
@@ -2895,12 +2905,22 @@ def virtual_providers(self):
 
     def define_version_constraints(self):
         """Define what version_satisfies(...) means in ASP logic."""
+
+        for pkg_name, versions in sorted(self.possible_versions.items()):
+            for v in versions:
+                if v in self.git_commit_versions[pkg_name]:
+                    sha = self.git_commit_versions[pkg_name].get(v)
+                    if sha:
+                        self.gen.fact(fn.pkg_fact(pkg_name, fn.version_has_commit(v, sha)))
+                    else:
+                        self.gen.fact(fn.pkg_fact(pkg_name, fn.version_needs_commit(v)))
+        self.gen.newline()
+
         for pkg_name, versions in sorted(self.version_constraints):
             # generate facts for each package constraint and the version
             # that satisfies it
             for v in sorted(v for v in self.possible_versions[pkg_name] if v.satisfies(versions)):
                 self.gen.fact(fn.pkg_fact(pkg_name, fn.version_satisfies(versions, v)))
-
             self.gen.newline()
 
     def collect_virtual_constraints(self):
@@ -3164,6 +3184,10 @@ def setup(
             allow_deprecated=allow_deprecated, require_checksum=checksummed
         )
 
+        self.gen.h1("Infinity Versions")
+        for i, v in enumerate(spack.version.infinity_versions):
+            self.gen.fact(fn.infinity_version(v, i))
+
         self.gen.h1("Package Constraints")
         for pkg in sorted(self.pkgs):
             self.gen.h2("Package rules: %s" % pkg)
@@ -3173,6 +3197,7 @@ def setup(
 
         self.gen.h1("Special variants")
         self.define_auto_variant("dev_path", multi=False)
+        self.define_auto_variant("commit", multi=False)
         self.define_auto_variant("patches", multi=True)
 
         self.gen.h1("Develop specs")
@@ -4140,6 +4165,10 @@ def build_specs(self, function_tuples):
                         spack.version.git_ref_lookup.GitRefLookup(spec.fullname)
                     )
 
+        # check for commits must happen after all version adaptations are complete
+        for s in self._specs.values():
+            _specs_with_commits(s)
+
         specs = self.execute_explicit_splices()
         return specs
 
@@ -4180,6 +4209,29 @@ def execute_explicit_splices(self):
         return specs
 
 
+def _specs_with_commits(spec):
+    if not spec.package.needs_commit(spec.version):
+        return
+
+    # check integrity of specified commit shas
+    if "commit" in spec.variants:
+        invalid_commit_msg = (
+            f"Internal Error: {spec.name}'s assigned commit {spec.variants['commit'].value}"
+            " does not meet commit syntax requirements."
+        )
+        assert vn.is_git_commit_sha(spec.variants["commit"].value), invalid_commit_msg
+
+    spec.package.resolve_binary_provenance()
+    # TODO(psakiev) assert commit is associated with ref
+
+    if isinstance(spec.version, spack.version.GitVersion):
+        if not spec.version.commit_sha:
+            # TODO(psakiev) this will be a failure when commit look up is automated
+            return
+        if "commit" not in spec.variants:
+            spec.variants["commit"] = vt.SingleValuedVariant("commit", spec.version.commit_sha)
+
+
 def _inject_patches_variant(root: spack.spec.Spec) -> None:
     # This dictionary will store object IDs rather than Specs as keys
     # since the Spec __hash__ will change as patches are added to them

lib/spack/spack/solver/concretize.lp

@@ -333,9 +333,37 @@ attr("node_version_satisfies", node(ID, Package), Constraint)
   :- attr("version", node(ID, Package), Version),
      pkg_fact(Package, version_satisfies(Constraint, Version)).
 
+% if a version needs a commit or has one it can use the commit variant
+can_accept_commit(Package, Version) :- pkg_fact(Package, version_needs_commit(Version)).
+can_accept_commit(Package, Version) :- pkg_fact(Package, version_has_commit(Version, _)).
+
+% Specs with a commit variant can't use versions that don't need commits
+error(10, "Cannot use commit variant with '{0}@={1}'", Package, Version)
+  :- attr("version", node(ID, Package), Version),
+     not can_accept_commit(Package, Version),
+     attr("variant_value", node(ID, Package), "commit", _).
+
+error(10, "Commit '{0}' must match package.py value '{1}' for '{2}@={3}'", Vsha, Psha, Package, Version)
+  :- attr("version", node(ID, Package), Version),
+     attr("variant_value", node(ID, Package), "commit", Vsha),
+     pkg_fact(Package, version_has_commit(Version, Psha)),
+     Vsha != Psha.
+
+% need a rule for above, can't select a version that needs a commit if variant matches a version that has a commit
+  :- attr("version", node(ID, Package), VersionA),
+     attr("variant_value", node(ID, Package), "commit", Vsha),
+     pkg_fact(Package, version_has_commit(VersionB, Psha)),
+     Vsha == Psha,
+     VersionA != VersionB.
+
+% rule that says if a spec has a commit choose the max version that can accept a commit
+
+% rule that says if a constraint is on a commit and a version matches that commit then the constraint is on the version
+
 #defined version_satisfies/3.
 #defined deprecated_versions_not_allowed/0.
 #defined deprecated_version/2.
+#defined can_accept_commit/2.
 
 %-----------------------------------------------------------------------------
 % Spec conditions and imposed constraints

lib/spack/spack/spec.py

@@ -2866,7 +2866,7 @@ def _validate_version(self):
             v.ref_version
         except vn.VersionLookupError:
             before = self.cformat("{name}{@version}{/hash:7}")
-            v._ref_version = vn.StandardVersion.from_string("develop")
+            v.std_version = vn.StandardVersion.from_string("develop")
             tty.debug(
                 f"the git sha of {before} could not be resolved to spack version; "
                 f"it has been replaced by {self.cformat('{name}{@version}{/hash:7}')}."
@@ -4428,7 +4428,7 @@ def attach_git_version_lookup(self):
         if not self.name:
             return
         for v in self.versions:
-            if isinstance(v, vn.GitVersion) and v._ref_version is None:
+            if isinstance(v, vn.GitVersion) and v.std_version is None:
                 v.attach_lookup(spack.version.git_ref_lookup.GitRefLookup(self.fullname))
 
 
@@ -4615,7 +4615,7 @@ def substitute_abstract_variants(spec: Spec):
     # in $spack/lib/spack/spack/spec_list.py
     unknown = []
     for name, v in spec.variants.items():
-        if name == "dev_path":
+        if name in ("dev_path", "commit"):
             spec.variants.substitute(vt.SingleValuedVariant(name, v._original_value))
             continue
         elif name in vt.reserved_names:

lib/spack/spack/test/cmd/find.py

@@ -5,6 +5,7 @@
 import argparse
 import json
 import os
+import pathlib
 import sys
 from textwrap import dedent
 
@@ -14,6 +15,7 @@
 import spack.cmd.find
 import spack.concretize
 import spack.environment as ev
+import spack.package_base
 import spack.repo
 import spack.store
 import spack.user_environment as uenv
@@ -612,3 +614,18 @@ def _nresults(_qresult):
         assert _nresults(_query(e, "--tag=tag0")) == (1, 0)
         assert _nresults(_query(e, "--tag=tag1")) == (1, 1)
         assert _nresults(_query(e, "--tag=tag2")) == (0, 1)
+
+
+@pytest.mark.usefixtures("install_mockery", "mock_fetch", "mutable_mock_env_path")
+def test_phil_find_based_on_commit_sha(mock_git_version_info, monkeypatch):
+    repo_path, filename, commits = mock_git_version_info
+    file_url = pathlib.Path(repo_path).as_uri()
+
+    monkeypatch.setattr(spack.package_base.PackageBase, "git", file_url, raising=False)
+
+    env("create", "test")
+    with ev.read("test"):
+        install("--fake", "--add", f"git-test-commit commit={commits[0]}")
+
+        output = find(f"commit={commits[0]}")
+        assert "git-test-commit" in output

lib/spack/spack/test/concretization/core.py

@@ -3,6 +3,7 @@
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 import copy
 import os
+import pathlib
 import sys
 
 import jinja2
@@ -22,6 +23,7 @@
 import spack.detection
 import spack.error
 import spack.hash_types as ht
+import spack.package_base
 import spack.paths
 import spack.platforms
 import spack.platforms.test
@@ -2727,6 +2729,17 @@ def test_correct_external_is_selected_from_packages_yaml(self, mutable_config):
         assert s.satisfies("%clang")
         assert s.prefix == "/tmp/prefix2"
 
+    def test_phil_git_based_version_must_exist_to_use_ref(self):
+        # gmake should fail, only has sha256
+        with pytest.raises(spack.error.UnsatisfiableSpecError) as e:
+            spack.concretize.concretize_one(f"gmake commit={'a' * 40}")
+            assert "Cannot use commit variant with" in e.value.message
+
+    @pytest.mark.skip("not supporting on this branch")
+    def test_phil_commit_variant_in_absence_of_version_selects_max_infinity_version(self):
+        spec = spack.concretize.concretize_one(f"git-ref-package commit={'a' * 40}")
+        assert spec.satisfies("@develop")
+
 
 @pytest.fixture()
 def duplicates_test_repository():
@@ -3256,6 +3269,68 @@ def test_spec_unification(unify, mutable_config, mock_packages):
         _ = spack.cmd.parse_specs([a_restricted, b], concretize=True)
 
 
+@pytest.mark.usefixtures("mutable_config", "mock_packages", "do_not_check_runtimes_on_reuse")
+@pytest.mark.parametrize(
+    "spec_str, error_type",
+    [
+        # TODO write actual Exceptions for these to give good error messages
+        (f"git-ref-package@main commit={'a' * 40}", None),
+        (f"git-ref-package@main commit={'a' * 39}", AssertionError),
+        (f"git-ref-package@2.1.6 commit={'a' * 40}", spack.error.UnsatisfiableSpecError),
+        (f"git-ref-package@git.2.1.6=2.1.6 commit={'a' * 40}", None),
+        (f"git-ref-package@git.{'a' * 40}=2.1.6 commit={'a' * 40}", None),
+    ],
+)
+def test_phil_spec_containing_commit_variant(spec_str, error_type):
+    spec = spack.spec.Spec(spec_str)
+    if error_type is None:
+        spack.concretize.concretize_one(spec)
+    else:
+        with pytest.raises(error_type):
+            spack.concretize.concretize_one(spec)
+
+
+@pytest.mark.usefixtures("mutable_config", "mock_packages", "do_not_check_runtimes_on_reuse")
+@pytest.mark.parametrize(
+    "spec_str, error_type",
+    [
+        (f"git-test-commit@git.main commit={'a' * 40}", None),
+        (f"git-test-commit@git.v1.0 commit={'a' * 40}", None),
+        ("git-test-commit@{sha} commit={sha}", None),
+        ("git-test-commit@{sha} commit=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", None),
+    ],
+)
+def test_phil_spec_with_commit_interacts_with_lookup(
+    mock_git_version_info, monkeypatch, spec_str, error_type
+):
+    # This test will be short lived. Technically we could do further checks with a Lookup
+    # but skipping impl since we are going to deprecate
+    repo_path, filename, commits = mock_git_version_info
+    file_url = pathlib.Path(repo_path).as_uri()
+    monkeypatch.setattr(spack.package_base.PackageBase, "git", file_url, raising=False)
+    spec = spack.spec.Spec(spec_str.format(sha=commits[-1]))
+    if error_type is None:
+        spack.concretize.concretize_one(spec)
+    else:
+        with pytest.raises(error_type):
+            spack.concretize.concretize_one(spec)
+
+
+@pytest.mark.usefixtures("mutable_config", "mock_packages", "do_not_check_runtimes_on_reuse")
+@pytest.mark.parametrize("version_str", [f"git.{'a' * 40}=main", "git.2.1.5=main"])
+def test_phil_relationship_git_versions_and_commit_variant(version_str):
+    """
+    Confirm that GitVersions auto assign and poopulate the commit variant correctly
+    """
+    # This should be a short lived test and can be deleted when we remove GitVersions
+    spec = spack.spec.Spec(f"git-ref-package@{version_str}")
+    spec = spack.concretize.concretize_one(spec)
+    if spec.version.commit_sha:
+        assert spec.version.commit_sha == spec.variants["commit"].value
+    else:
+        assert "commit" not in spec.variants
+
+
 def test_concretization_cache_roundtrip(use_concretization_cache, monkeypatch, mutable_config):
     """Tests whether we can write the results of a clingo solve to the cache
     and load the same spec request from the cache to produce identical specs"""

lib/spack/spack/test/conftest.py

@@ -154,7 +154,7 @@ def mock_git_version_info(git, tmpdir, override_git_repos_cache_path):
        o second commit (v1.0)
        o first commit
 
-    The repo consists of a single file, in which the GitVersion._ref_version representation
+    The repo consists of a single file, in which the GitVersion.std_version representation
     of each commit is expressed as a string.
 
     Important attributes of the repo for test coverage are: multiple branches,
@@ -197,6 +197,11 @@ def latest_commit():
 
         # Get name of default branch (differs by git version)
         main = git("rev-parse", "--abbrev-ref", "HEAD", output=str, error=str).strip()
+        if main != "main":
+            # assure the default branch name is consistent for tests
+            git("branch", "-m", "main")
+            main = git("rev-parse", "--abbrev-ref", "HEAD", output=str, error=str).strip()
+        assert "main" == main
 
         # Tag second commit as v1.0
         write_file(filename, "[1, 0]")

lib/spack/spack/test/git_fetch.py

@@ -4,6 +4,7 @@
 
 import copy
 import os
+import pathlib
 import shutil
 
 import pytest
@@ -19,6 +20,7 @@
 from spack.fetch_strategy import GitFetchStrategy
 from spack.spec import Spec
 from spack.stage import Stage
+from spack.variant import SingleValuedVariant
 from spack.version import Version
 
 _mock_transport_error = "Mock HTTP transport error"
@@ -429,3 +431,19 @@ def test_git_sparse_paths_partial_clone(
 
         # fixture file is in the sparse-path expansion tree
         assert os.path.isfile(t.file)
+
+
+@pytest.mark.disable_clean_stage_check
+def test_commit_variant_clone(
+    git, default_mock_concretization, mutable_mock_repo, mock_git_version_info, monkeypatch
+):
+
+    repo_path, filename, commits = mock_git_version_info
+    test_commit = commits[-1]
+    s = default_mock_concretization("git-test")
+    args = {"git": pathlib.Path(repo_path).as_uri()}
+    monkeypatch.setitem(s.package.versions, Version("git"), args)
+    s.variants["commit"] = SingleValuedVariant("commit", test_commit)
+    s.package.do_stage()
+    with working_dir(s.package.stage.source_path):
+        assert git("rev-parse", "HEAD", output=str, error=str).strip() == test_commit

lib/spack/spack/test/packages.py

@@ -333,3 +333,28 @@ def test_package_can_have_sparse_checkout_properties(mock_packages, mock_fetch,
     assert isinstance(fetcher, spack.fetch_strategy.GitFetchStrategy)
     assert hasattr(fetcher, "git_sparse_paths")
     assert fetcher.git_sparse_paths == pkg_cls.git_sparse_paths
+
+
+def test_phil_package_can_depend_on_commit_of_dependency(mock_packages, config):
+    spec = spack.concretize.concretize_one(Spec("git-ref-commit-dep@1.0.0"))
+    assert spec.satisfies(f"^git-ref-package commit={'a' * 40}")
+    assert "surgical" not in spec["git-ref-package"].variants
+
+
+def test_phil_package_condtional_variants_may_depend_on_commit(mock_packages, config):
+    spec = spack.concretize.concretize_one(Spec("git-ref-commit-dep@develop"))
+    assert spec.satisfies(f"^git-ref-package commit={'b' * 40}")
+    conditional_variant = spec["git-ref-package"].variants.get("surgical", None)
+    assert conditional_variant
+    assert conditional_variant.value
+
+
+@pytest.mark.skip("not supporting on this branch")
+def test_phil_commit_variant_finds_matches_for_commit_versions(mock_packages, config):
+    """
+    test conditional dependence on `when='commit=<sha>'`
+    git-ref-commit-dep variant commit-selector depends on a specific commit of git-ref-package
+    that commit is associated with the stable version of git-ref-package
+    """
+    spec = spack.concretize.concretize_one(Spec("git-ref-commit-dep+commit-selector"))
+    assert spec.satisfies("^git-ref-package@stable")

lib/spack/spack/test/versions.py

@@ -811,6 +811,33 @@ def test_version_list_with_range_and_concrete_version_is_not_concrete():
     assert not v.concrete
 
 
+@pytest.mark.parametrize(
+    "git_ref, std_version",
+    (("foo", "develop"), ("a" * 40, "develop"), ("a" * 40, None), ("v1.2.0", "1.2.0")),
+)
+def test_phil_git_versions_store_ref_requests(git_ref, std_version):
+    """
+    User requested ref's should be known on creation
+    Commit and standard version may not be known until concretization
+    To be concrete a GitVersion must have a commit and standard version
+    """
+    if std_version:
+        vstring = f"git.{git_ref}={std_version}"
+    else:
+        vstring = git_ref
+
+    v = Version(vstring)
+
+    assert isinstance(v, GitVersion)
+    assert v.ref == git_ref
+
+    if std_version:
+        assert v.std_version == Version(std_version)
+
+    if v.is_commit:
+        assert v.ref == v.commit_sha
+
+
 @pytest.mark.parametrize(
     "vstring, eq_vstring, is_commit",
     (

lib/spack/spack/variant.py

@@ -24,12 +24,16 @@
 reserved_names = [
     "arch",
     "architecture",
+    "branch",
+    "commit",
     "dev_path",
     "namespace",
     "operating_system",
     "os",
     "patches",
     "platform",
+    "ref",
+    "tag",
     "target",
 ]
 

lib/spack/spack/version/__init__.py

@@ -20,6 +20,7 @@
     VersionError,
     VersionLookupError,
     infinity_versions,
+    is_git_commit_sha,
     is_git_version,
 )
 from .version_types import (
@@ -58,6 +59,7 @@
     "any_version",
     "from_string",
     "infinity_versions",
+    "is_git_commit_sha",
     "is_git_version",
     "ver",
 ]

lib/spack/spack/version/common.py

@@ -23,13 +23,12 @@
 STRING_TO_PRERELEASE = {"alpha": ALPHA, "beta": BETA, "rc": RC, "final": FINAL}
 
 
+def is_git_commit_sha(string: str) -> bool:
+    return len(string) == 40 and bool(COMMIT_VERSION.match(string))
+
+
 def is_git_version(string: str) -> bool:
-    return (
+    return string.startswith("git.") or is_git_commit_sha(string) or "=" in string[1:]
-        string.startswith("git.")
-        or len(string) == 40
-        and bool(COMMIT_VERSION.match(string))
-        or "=" in string[1:]
-    )
 
 
 class VersionError(spack.error.SpackError):

lib/spack/spack/version/version_types.py

@@ -548,15 +548,19 @@ class GitVersion(ConcreteVersion):
     sufficient.
     """
 
-    __slots__ = ["ref", "has_git_prefix", "is_commit", "_ref_lookup", "_ref_version"]
+    __slots__ = ["has_git_prefix", "commit_sha", "ref", "std_version", "_ref_lookup"]
 
     def __init__(self, string: str):
+        # TODO will be required for concrete specs when commit lookup added
+        self.commit_sha: Optional[str] = None
+        self.std_version: Optional[StandardVersion] = None
+
+        # optional user supplied git ref
+        self.ref: Optional[str] = None
+
         # An object that can lookup git refs to compare them to versions
         self._ref_lookup: Optional[AbstractRefLookup] = None
 
-        # This is the effective version.
-        self._ref_version: Optional[StandardVersion]
-
         self.has_git_prefix = string.startswith("git.")
 
         # Drop `git.` prefix
@@ -565,23 +569,27 @@ def __init__(self, string: str):
         if "=" in normalized_string:
             # Store the git reference, and parse the user provided version.
             self.ref, spack_version = normalized_string.split("=")
-            self._ref_version = StandardVersion(
+            self.std_version = StandardVersion(
                 spack_version, *parse_string_components(spack_version)
             )
         else:
             # The ref_version is lazily attached after parsing, since we don't know what
             # package it applies to here.
-            self._ref_version = None
+            self.std_version = None
             self.ref = normalized_string
 
         # Used by fetcher
         self.is_commit: bool = len(self.ref) == 40 and bool(COMMIT_VERSION.match(self.ref))
 
+        # translations
+        if self.is_commit:
+            self.commit_sha = self.ref
+
     @property
     def ref_version(self) -> StandardVersion:
         # Return cached version if we have it
-        if self._ref_version is not None:
+        if self.std_version is not None:
-            return self._ref_version
+            return self.std_version
 
         if self.ref_lookup is None:
             raise VersionLookupError(
@@ -594,10 +602,10 @@ def ref_version(self) -> StandardVersion:
         # Add a -git.<distance> suffix when we're not exactly on a tag
         if distance > 0:
             version_string += f"-git.{distance}"
-        self._ref_version = StandardVersion(
+        self.std_version = StandardVersion(
             version_string, *parse_string_components(version_string)
         )
-        return self._ref_version
+        return self.std_version
 
     def intersects(self, other: VersionType) -> bool:
         # For concrete things intersects = satisfies = equality
@@ -629,7 +637,9 @@ def satisfies(self, other: VersionType) -> bool:
         raise TypeError(f"'satisfies()' not supported for instances of {type(other)}")
 
     def __str__(self) -> str:
-        s = f"git.{self.ref}" if self.has_git_prefix else self.ref
+        s = ""
+        if self.ref:
+            s += f"git.{self.ref}" if self.has_git_prefix else self.ref
         # Note: the solver actually depends on str(...) to produce the effective version.
         # So when a lookup is attached, we require the resolved version to be printed.
         # But for standalone git versions that don't have a repo attached, it would still
@@ -651,6 +661,7 @@ def __eq__(self, other: object) -> bool:
         return (
             isinstance(other, GitVersion)
             and self.ref == other.ref
+            # TODO(psakiev) this needs to chamge to commits when we turn on lookups
             and self.ref_version == other.ref_version
         )
 

var/spack/repos/builtin.mock/packages/git-ref-commit-dep/package.py

@@ -0,0 +1,24 @@
+# Copyright Spack Project Developers. See COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+from spack.package import *
+
+
+class GitRefCommitDep(AutotoolsPackage):
+    """
+    tests dependency using commit
+    """
+
+    homepage = "https://github.com/dummy/dummy"
+    git = "https://github.com/dummy/dummy.git"
+    url = git
+
+    version("develop", branch="develop")
+    version("1.0.0", sha256="a5d504c0d52e2e2721e7e7d86988dec2e290d723ced2307145dedd06aeb6fef2")
+
+    variant("commit-selector", default=False, description="test grabbing a specific commit")
+
+    depends_on(f"git-ref-package commit={'a' * 40}", when="@1.0.0")
+    depends_on(f"git-ref-package commit={'b' * 40}", when="@develop")
+    depends_on(f"git-ref-package commit={'c' * 40}", when="+commit-selector")

var/spack/repos/builtin.mock/packages/git-ref-package/package.py

@@ -14,6 +14,10 @@ class GitRefPackage(AutotoolsPackage):
     url = "https://github.com/dummy/dummy/archive/2.0.0.tar.gz"
     git = "https://github.com/dummy/dummy.git"
 
+    version("develop", branch="develop")
+    version("main", branch="main")
+    version("stable", tag="stable", commit="c" * 40)
+    version("3.0.1", tag="v3.0.1")
     version("2.1.6", sha256="a5d504c0d52e2e2721e7e7d86988dec2e290d723ced2307145dedd06aeb6fef2")
     version("2.1.5", sha256="3f6576971397b379d4205ae5451ff5a68edf6c103b2f03c4188ed7075fbb5f04")
     version("2.1.4", sha256="a0293475e6a44a3f6c045229fe50f69dc0eebc62a42405a51f19d46a5541e77a")
@@ -34,6 +38,12 @@ class GitRefPackage(AutotoolsPackage):
     variant("opt", default=True, description="Enable optimizations")
     variant("shared", default=True, description="Build shared library")
     variant("pic", default=True, description="Enable position-independent code (PIC)")
+    variant(
+        "surgical",
+        default=True,
+        when=f"commit={'b' * 40}",
+        description="Testing conditional on commit",
+    )
 
     conflicts("+shared~pic")
 

var/spack/repos/builtin.mock/packages/git-test-commit/package.py

@@ -11,6 +11,7 @@ class GitTestCommit(Package):
     homepage = "http://www.git-fetch-example.com"
     # git='to-be-filled-in-by-test'
 
+    version("main", branch="main")
     version("1.0", tag="v1.0")
     version("1.1", tag="v1.1")
     version("1.2", tag="1.2")  # not a typo