Fix commit check

lib/spack/llnl/util/lock.py

@@ -269,7 +269,7 @@ def __init__(
 
     @staticmethod
     def _poll_interval_generator(
-        _wait_times: Optional[Tuple[float, float, float]] = None,
+        _wait_times: Optional[Tuple[float, float, float]] = None
     ) -> Generator[float, None, None]:
         """This implements a backoff scheme for polling a contended resource
         by suggesting a succession of wait times between polls.

lib/spack/spack/detection/common.py

@@ -310,7 +310,7 @@ def find_windows_kit_roots() -> List[str]:
 
     @staticmethod
     def find_windows_kit_bin_paths(
-        kit_base: Union[Optional[str], Optional[list]] = None,
+        kit_base: Union[Optional[str], Optional[list]] = None
     ) -> List[str]:
         """Returns Windows kit bin directory per version"""
         kit_base = WindowsKitExternalPaths.find_windows_kit_roots() if not kit_base else kit_base
@@ -325,7 +325,7 @@ def find_windows_kit_bin_paths(
 
     @staticmethod
     def find_windows_kit_lib_paths(
-        kit_base: Union[Optional[str], Optional[list]] = None,
+        kit_base: Union[Optional[str], Optional[list]] = None
     ) -> List[str]:
         """Returns Windows kit lib directory per version"""
         kit_base = WindowsKitExternalPaths.find_windows_kit_roots() if not kit_base else kit_base

lib/spack/spack/directives.py

@@ -567,7 +567,7 @@ def patch(
     """
 
     def _execute_patch(
-        pkg_or_dep: Union[Type[spack.package_base.PackageBase], Dependency],
+        pkg_or_dep: Union[Type[spack.package_base.PackageBase], Dependency]
     ) -> None:
         pkg = pkg_or_dep.pkg if isinstance(pkg_or_dep, Dependency) else pkg_or_dep
 

lib/spack/spack/package_base.py

@@ -989,7 +989,21 @@ 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 resolve_binary_provenance(self):
+    @property
+    def needs_commit(self) -> bool:
+        """
+        Method for checking if the package instance needs a commit sha to be found
+        """
+        if isinstance(self.version, GitVersion):
+            return True
+
+        ver_attrs = self.versions.get(self.version)
+        if ver_attrs:
+            return bool(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.

lib/spack/spack/solver/asp.py

@@ -4186,9 +4186,6 @@ def execute_explicit_splices(self):
 
 
 def _specs_with_commits(spec):
-    # StandardVersions paired to git branches or tags and GitVersions
-    if not spec.version.needs_commit:
-        return
     spec.package.resolve_binary_provenance()
 
     # method above is in charge of assigning the commit variant
@@ -4197,6 +4194,8 @@ def _specs_with_commits(spec):
 
     if not (has_commit_var or has_git_version):
         return
+    # StandardVersions paired to git branches or tags and GitVersions
+    assert spec.package.needs_commit, f"{spec.name}@{spec.version} can not have a commit variant"
 
     # Specs with commit variants
     # - variant value satsifies commit regex

lib/spack/spack/spec.py

@@ -2673,7 +2673,7 @@ def name_and_dependency_types(s: str) -> Tuple[str, dt.DepFlag]:
                 return name, depflag
 
             def spec_and_dependency_types(
-                s: Union[Spec, Tuple[Spec, str]],
+                s: Union[Spec, Tuple[Spec, str]]
             ) -> Tuple[Spec, dt.DepFlag]:
                 """Given a non-string key in the literal, extracts the spec
                 and its dependency types.

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

@@ -3316,7 +3316,6 @@ def _ensure_cache_hits(self, problem: str):
         (f"git-ref-package@main commit={'a' * 39}", False, AssertionError),
         (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),
     ],
 )
 def test_spec_containing_commit_variant(spec_str, should_pass, error_type):

lib/spack/spack/version/version_types.py

@@ -391,9 +391,6 @@ def isdevelop(self) -> bool:
             isinstance(p, VersionStrComponent) and isinstance(p.data, int) for p in self.version[0]
         )
 
-    def needs_commit(self) -> bool:
-        return bool(self.get("branch") or self.get("tag"))
-
     def is_prerelease(self) -> bool:
         return self.version[1][0] != FINAL
 
@@ -719,9 +716,6 @@ def __hash__(self):
     def __contains__(self, other: object) -> bool:
         raise NotImplementedError
 
-    def needs_commit(self) -> bool:
-        return True
-
     @property
     def ref_lookup(self):
         if self._ref_lookup: