From 9224994dad57d701583e190b9bcd87fb97d3a15b Mon Sep 17 00:00:00 2001
From: Philip Sakievich <psakiev@sandia.gov>
Date: Tue, 25 Mar 2025 21:09:27 -0600
Subject: [PATCH] Attempt to add solver constraint

---
 lib/spack/spack/package_base.py      | 8 ++++----
 lib/spack/spack/solver/asp.py        | 8 +++++++-
 lib/spack/spack/solver/concretize.lp | 5 +++++
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/lib/spack/spack/package_base.py b/lib/spack/spack/package_base.py
index 58892eb7ffc..9305e5a5029 100644
--- a/lib/spack/spack/package_base.py
+++ b/lib/spack/spack/package_base.py
@@ -1001,15 +1001,15 @@ def version_or_package_attr(self, attr, version, default=None):
             raise PackageError(f"{attr} attribute not defined on {self.name}")
         return value
 
-    @property
-    def needs_commit(self) -> bool:
+    @classmethod
+    def needs_commit(cls, version) -> bool:
         """
         Method for checking if the package instance needs a commit sha to be found
         """
-        if isinstance(self.version, GitVersion):
+        if isinstance(version, GitVersion):
             return True
 
-        ver_attrs = self.versions.get(self.version)
+        ver_attrs = cls.versions.get(version)
         if ver_attrs:
             return bool(ver_attrs.get("tag") or ver_attrs.get("branch"))
 
diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index cf2222253b4..43643a69c17 100644
--- a/lib/spack/spack/solver/asp.py
+++ b/lib/spack/spack/solver/asp.py
@@ -1522,6 +1522,7 @@ 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(set)
         self.deprecated_versions: Dict[str, Set[GitOrStandardVersion]] = collections.defaultdict(
             set
         )
@@ -1593,6 +1594,8 @@ def key_fn(version):
                     ),
                 )
             )
+            if pkg.needs_commit(declared_version.version):
+                self.git_commit_versions[pkg.name].add(declared_version.version)
 
         # Declare deprecated versions for this package, if any
         deprecated = self.deprecated_versions[pkg.name]
@@ -2897,11 +2900,14 @@ def virtual_providers(self):
 
     def define_version_constraints(self):
         """Define what version_satisfies(...) means in ASP logic."""
+        # TODO(psakiev)
         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)))
+                if v in self.git_commit_versions[pkg_name]:
+                    self.gen.fact(fn.pkg_fact(pkg_name, fn.version_needs_commit(v)))
 
             self.gen.newline()
 
@@ -4199,7 +4205,7 @@ 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"
+    assert spec.package.needs_commit(spec.version), f"{spec.name}@{spec.version} can not have a commit variant"
 
     # Specs with commit variants
     # - variant value satsifies commit regex
diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp
index af92dc1f177..186db84e357 100644
--- a/lib/spack/spack/solver/concretize.lp
+++ b/lib/spack/spack/solver/concretize.lp
@@ -335,6 +335,11 @@ attr("node_version_satisfies", node(ID, Package), Constraint)
   :- attr("version", node(ID, Package), Version),
      pkg_fact(Package, version_satisfies(Constraint, Version)).
 
+% Specs with a commit variant can't use versions that don't need commits
+  :- attr("version", node(ID, Package), Version),
+     not pkg_fact(Package, version_needs_commit(Version)),
+     attr("variant_value", PackageNode, "commit", _).
+
 #defined version_satisfies/3.
 #defined deprecated_versions_not_allowed/0.
 #defined deprecated_version/2.