From b4269ff8f1464a6fa106568af6644788f7e4c181 Mon Sep 17 00:00:00 2001 From: Philip Sakievich Date: Fri, 28 Mar 2025 21:08:34 -0600 Subject: [PATCH] WIP: refactor test --- lib/spack/spack/git_ref_operators.py | 85 +++++++++++++++++++ lib/spack/spack/solver/asp.py | 34 +++----- lib/spack/spack/test/concretization/core.py | 23 +---- .../packages/git-ref-package/package.py | 2 + 4 files changed, 100 insertions(+), 44 deletions(-) create mode 100644 lib/spack/spack/git_ref_operators.py diff --git a/lib/spack/spack/git_ref_operators.py b/lib/spack/spack/git_ref_operators.py new file mode 100644 index 00000000000..689037dfc5c --- /dev/null +++ b/lib/spack/spack/git_ref_operators.py @@ -0,0 +1,85 @@ +# 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 diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index 44254be15af..49d32ab0317 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -2902,7 +2902,8 @@ def virtual_providers(self): def define_version_constraints(self): """Define what version_satisfies(...) means in ASP logic.""" - # TODO(psakiev) + + # 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]: @@ -4200,36 +4201,21 @@ def execute_explicit_splices(self): def _specs_with_commits(spec): - spec.package.resolve_binary_provenance() - # TODO(psakiev) assert commit is associated with ref + if not spec.package.needs_commit(spec.version): + return - # method above is in charge of assigning the commit variant - has_commit_var = "commit" in spec.variants - has_git_version = isinstance(spec.version, vn.GitVersion) - - if not (has_commit_var or has_git_version): - return - # StandardVersions paired to git branches or tags and GitVersions - 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 - # - paired to a GitVersion or version that is associated with a branch/tag - # - variant value should match GitVersion's commit value - if has_commit_var: + # 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 - # Specs with GitVersions - # - must have a commit variant, or add it here - # - must have a commit on the GitVersion (enforce after look up implemented) - if has_git_version: + 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 diff --git a/lib/spack/spack/test/concretization/core.py b/lib/spack/spack/test/concretization/core.py index 541f07f824c..6dbe52a04c8 100644 --- a/lib/spack/spack/test/concretization/core.py +++ b/lib/spack/spack/test/concretization/core.py @@ -2730,31 +2730,14 @@ def test_correct_external_is_selected_from_packages_yaml(self, mutable_config): assert s.prefix == "/tmp/prefix2" def test_phil_add_git_based_version_must_exist_to_use_ref(self): - s = spack.concretize.concretize_one(f"git-ref-package commit={'a' * 40}") - assert s.satisfies("@main") - # gmake should fail, only has sha256 with pytest.raises(spack.error.UnsatisfiableSpecError) as e: s = spack.concretize.concretize_one(f"gmake commit={'a' * 40}") assert "Cannot use commit variant with" in e.value.message - -@pytest.mark.usefixtures("mutable_config", "mock_packages", "do_not_check_runtimes_on_reuse") -def test_phil_add_git_based_version_commit_must_be_valid(mock_git_version_info, monkeypatch): - """Test installing a git package from a commit. - - This ensures Spack associates commit versions with their packages in time to do - version lookups. Details of version lookup tested elsewhere. - - """ - 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) - - spack.concretize.concretize_one(f"git-test-commit@main commit={commits[0]}") - with pytest.raises(AssertionError): - spack.concretize.concretize_one(f"git-test-commit@main commit={'a' * 40}") + def test_phil_add_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() diff --git a/var/spack/repos/builtin.mock/packages/git-ref-package/package.py b/var/spack/repos/builtin.mock/packages/git-ref-package/package.py index 70df52156dd..f6beebd69b2 100644 --- a/var/spack/repos/builtin.mock/packages/git-ref-package/package.py +++ b/var/spack/repos/builtin.mock/packages/git-ref-package/package.py @@ -14,7 +14,9 @@ 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("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")