New test on version satisfaction

lib/spack/spack/git_ref_operators.py

@@ -1,85 +0,0 @@
-# Copyright Spack Project Developers. See COPYRIGHT file for details.
-#
-# SPDX-License-Identifier: (Apache-2.0 OR MIT)
-import os
-
-import spack.error
-import spack.repo
-import spack.util.git
-from spack.util.executable import ProcessError
-from spack.version import GitVersion, StandardVersion, Version
-
-
-class GitRefFetchError(spack.error.SpackError):
-    pass
-
-
-class GitBranch:
-    def __init__(self, git_address, name):
-        self.address = git_address
-        self.name = name
-        self.flag = "-h"
-
-
-class GitTag:
-    def __init__(self, git_address, name):
-        self.address = git_address
-        self.name = name
-        self.flag = "-t"
-
-
-def associated_git_ref(version, package_class):
-    """
-    Retrieve the branch or tag associated with the version
-    and return the relevant git information for querying the git repo/remote
-    """
-    version_dict = package_class.versions.get(version, {})
-
-    git_address = version_dict.get("git", None) or getattr(package_class, "git", None)
-
-    if git_address:
-        branch = version_dict.get("branch", None)
-        tag = version_dict.get("tag", None)
-        if branch:
-            return GitBranch(git_address, branch)
-        elif tag:
-            return GitTag(git_address, tag)
-    return None
-
-
-def retrieve_latest_git_hash(git_ref):
-    """Get the git hash associated with a tag or branch"""
-    # remote git operations can sometimes have banners so we must parse the output for a sha
-    query = spack.util.git.git(required=True)(
-        "ls-remote", git_ref.flag, git_ref.address, git_ref.name, output=str, error=os.devnull
-    )
-    sha, ref = query.strip().split()
-    return sha
-
-
-def convert_standard_to_git_version(version, package_class_name):
-    """
-    Converts a StandardVersion to a GitVersion
-
-    This function will assign the Git commit sha to a version if it has a branch or tag
-    """
-    pkg_class = spack.repo.PATH.get_pkg_class(package_class_name)
-    git_ref = associated_git_ref(version, pkg_class)
-    if git_ref:
-        try:
-            hash = retrieve_latest_git_hash(git_ref)
-        except (ProcessError, ValueError, AssertionError):
-            raise GitRefFetchError(
-                (
-                    "Failure to fetch git sha when running"
-                    f" `git ls-remote {git_ref.address} {git_ref.name}`\n"
-                    "Confirm network connectivty by running this command followed by:\n"
-                    f"\t`spack fetch {git_ref.address}@{str(version)}`"
-                    "Post a bug report if both of these operations succeed."
-                )
-            )
-
-        new_version_str = f"git.{hash}={str(version)}"
-        return GitVersion(new_version_str)
-    else:
-        return None

lib/spack/spack/package_base.py

@@ -989,16 +989,17 @@ 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)
 
-    def version_or_package_attr(self, attr, version, default=None):
+    @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 = self.versions.get(version)
+        version_attrs = cls.versions.get(version)
         if version_attrs and attr in version_attrs:
             return version_attrs.get(attr)
-        value = getattr(self, attr, default)
+        value = getattr(cls, attr, default)
         if value is None:
-            raise PackageError(f"{attr} attribute not defined on {self.name}")
+            raise PackageError(f"{attr} attribute not defined on {cls.name}")
         return value
 
     @classmethod

lib/spack/spack/solver/asp.py

@@ -1522,8 +1522,8 @@ 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, Set[GitOrStandardVersion]] = collections.defaultdict(
+        self.git_commit_versions: Dict[str, Dict[GitOrStandardVersion, str]] = (
-            set
+            collections.defaultdict(dict)
         )
         self.deprecated_versions: Dict[str, Set[GitOrStandardVersion]] = collections.defaultdict(
             set
@@ -1597,7 +1597,8 @@ def key_fn(version):
                 )
             )
             if pkg.needs_commit(declared_version.version):
-                self.git_commit_versions[pkg.name].add(declared_version.version)
+                commit = pkg.version_or_package_attr("commit", declared_version.version, "")
+                self.git_commit_versions[pkg.name][declared_version.version] = commit
 
         # Declare deprecated versions for this package, if any
         deprecated = self.deprecated_versions[pkg.name]
@@ -2903,11 +2904,13 @@ def virtual_providers(self):
     def define_version_constraints(self):
         """Define what version_satisfies(...) means in ASP logic."""
 
-        # TODO(psakiev) could probably consolidate with loop below
         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)
                     self.gen.fact(fn.pkg_fact(pkg_name, fn.version_needs_commit(v)))
+                    if sha:
+                        self.gen.fact(fn.pkg_fact(pkg_name, fn.version_has_commit(v, sha)))
             self.gen.newline()
 
         for pkg_name, versions in sorted(self.version_constraints):
@@ -4226,7 +4229,7 @@ def _specs_with_commits(spec):
         if not spec.version.commit_sha:
             # TODO(psakiev) this will be a failure when commit look up is automated
             return
-        if not "commit" in spec.variants:
+        if "commit" not in spec.variants:
             spec.variants["commit"] = vt.SingleValuedVariant("commit", spec.version.commit_sha)
 
 

lib/spack/spack/solver/concretize.lp

@@ -257,8 +257,6 @@ pkg_fact(Package, version_declared(Version)) :- pkg_fact(Package, version_declar
 { attr("version", node(ID, Package), Version) : pkg_fact(Package, version_declared(Version)) }
  :- attr("node", node(ID, Package)).
 
-% TODO(psakiev) need rule if spec has commit version must need commit
-
 % A virtual package may or may not have a version, but never has more than one
 error(100, "Cannot select a single version for virtual '{0}'", Virtual)
   :- attr("virtual_node", node(ID, Virtual)),
@@ -341,6 +339,11 @@ error(10, "Cannot use commit variant with '{0}@{1}'", Package, Version)
      not pkg_fact(Package, version_needs_commit(Version)),
      attr("variant_value", node(ID, Package), "commit", _).
 
+% Versions with commits require a matching commit variant
+  :- attr("version", node(ID, Packate), Version),
+     pkg_fact(Package, version_has_commit(Version, Sha)),
+     not attr("variant_value", node(ID, Package), "commit", Sha).
+
 #defined version_satisfies/3.
 #defined deprecated_versions_not_allowed/0.
 #defined deprecated_version/2.

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

@@ -2732,7 +2732,7 @@ def test_correct_external_is_selected_from_packages_yaml(self, mutable_config):
     def test_git_based_version_must_exist_to_use_ref(self):
         # gmake should fail, only has sha256
         with pytest.raises(spack.error.UnsatisfiableSpecError) as e:
-            s = spack.concretize.concretize_one(f"gmake commit={'a' * 40}")
+            spack.concretize.concretize_one(f"gmake commit={'a' * 40}")
             assert "Cannot use commit variant with" in e.value.message
 
     def test_commit_variant_in_absence_of_version_selects_max_infinity_version(self):

lib/spack/spack/test/conftest.py

@@ -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/packages.py

@@ -347,3 +347,8 @@ def test_phil_package_condtional_variants_may_depend_on_commit(mock_packages, co
     conditional_variant = spec["git-ref-package"].variants.get("surgical", None)
     assert conditional_variant
     assert conditional_variant.value
+
+
+def test_phil_commit_variant_finds_matches_for_commit_versions(mock_packages, config):
+    spec = spack.concretize.concretize_one(Spec(f"git-ref-commit-dep commit={'c' * 40}"))
+    assert spec.satisfies("^git-ref-package@stable")

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

@@ -16,6 +16,7 @@ class GitRefPackage(AutotoolsPackage):
 
     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")