diff --git a/lib/spack/spack/fetch_strategy.py b/lib/spack/spack/fetch_strategy.py index a7cfaaf2930..61f4608af91 100644 --- a/lib/spack/spack/fetch_strategy.py +++ b/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,34 @@ 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} + + # 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": True} + + kwargs["git"] = pkg.git + version_attrs = pkg.versions.get(version) + if version_attrs and version_attrs.get("git"): + kwargs["git"] = version_attrs["git"] kwargs["submodules"] = getattr(pkg, "submodules", False) # if the ref_version is a known version from the package, use that version's # submodule specifications - ref_version_attributes = pkg.versions.get(pkg.version.ref_version) - if ref_version_attributes: - kwargs["submodules"] = ref_version_attributes.get("submodules", kwargs["submodules"]) + if hasattr(pkg.version, "ref_version"): + ref_version_attributes = pkg.versions.get(pkg.version.ref_version) + if ref_version_attributes: + kwargs["submodules"] = ref_version_attributes.get( + "submodules", kwargs["submodules"] + ) + kwargs["git"] = ref_version_attributes.get("git", kwargs["git"]) fetcher = GitFetchStrategy(**kwargs) return fetcher diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index 9ed0b8361d9..262e4b2e18a 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -4141,7 +4141,7 @@ def build_specs(self, function_tuples): spack.version.git_ref_lookup.GitRefLookup(spec.fullname) ) - # check for commits mush happen after all version adaptations are complete + # check for commits must happen after all version adaptations are complete for s in self._specs.values(): _specs_with_commits(s) @@ -4278,9 +4278,6 @@ def _specs_with_commits(spec): if not (has_commit_var or has_git_version): return - if "dev_path" in spec.variants: - assert False - # Specs with commit variants # - variant value satsifies commit regex # - paired to a GitVersion or version that is associated with a branch/tag diff --git a/lib/spack/spack/test/concretization/core.py b/lib/spack/spack/test/concretization/core.py index b10016572b1..4588d4c439c 100644 --- a/lib/spack/spack/test/concretization/core.py +++ b/lib/spack/spack/test/concretization/core.py @@ -3317,7 +3317,6 @@ def _ensure_cache_hits(self, problem: str): (f"git-ref-package@2.1.6 commit={'a' * 40}", False, AssertionError), (f"git-ref-package@git.2.1.6=2.1.6 commit={'a' * 40}", True, None), (f"git-ref-package@2.1.6 commit={'a' * 40}", False, AssertionError), - (f"git-ref-package@main commit={'a' * 40} dev_path=/foo/bar/", False, AssertionError), ], ) def test_spec_containing_commit_variant(spec_str, should_pass, error_type): diff --git a/lib/spack/spack/test/git_fetch.py b/lib/spack/spack/test/git_fetch.py index 8ee447aca3e..a416a409883 100644 --- a/lib/spack/spack/test/git_fetch.py +++ b/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