diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index bb7635f3b0a..a7ad6231a57 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -3107,24 +3107,6 @@ def setup( if node.namespace is not None: self.explicitly_required_namespaces[node.name] = node.namespace - # abstract specs with commit variants are assigend version most likely to have commit sha - for spec in specs: - commit = spec.variants.get("commit") - version = spec.versions.concrete_range_as_version - if not version and commit: - # look for a matching commit in versions otherwise default to max infinity version - versions = spack.repo.PATH.get_pkg_class(spec.fullname).versions - match = None - for v, data in versions.items(): - if data.get("commit") == commit.value[0]: - match = v - break - if match: - spec.versions = spack.version.VersionList([match]) - else: - version = max(versions.keys()) - spec.versions = spack.version.VersionList([version]) - self.gen = ProblemInstanceBuilder() compiler_parser = CompilerParser(configuration=spack.config.CONFIG).with_input_specs(specs) @@ -4229,7 +4211,7 @@ def execute_explicit_splices(self): def _specs_with_commits(spec): if not spec.package.needs_commit(spec.version): - return + return # check integrity of specified commit shas if "commit" in spec.variants: @@ -4336,42 +4318,6 @@ def _ensure_external_path_if_external(spec: spack.spec.Spec) -> None: ) -def _specs_with_commits(spec): - 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 - - # 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: - invalid_commit_msg = ( - f"Internal Error: {spec.name}'s assigned commit {spec.variants['commit'].value}" - " does not meet commit syntax requirements." - ) - - # TODO probably want a more specific function just for sha validation - assert vn.is_git_version(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: - if not spec.version.commit_sha: - # TODO(psakiev) this will be a failure when commit look up is automated - return - - spec.variants["commit"] = vt.SingleValuedVariant("commit", spec.version.commit_sha) - else: - # if we are not a GitVersion then only versions with a branch or tag should be - # allowed to have the commit variant - version_dict = spec.package_class.versions.get(spec.version, {}) - assert version_dict.get("branch") or version_dict.get("tag") - - def _develop_specs_from_env(spec, env): dev_info = env.dev_specs.get(spec.name, {}) if env else {} if not dev_info: diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index ce2542df669..fe7526ae96d 100644 --- a/lib/spack/spack/spec.py +++ b/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. diff --git a/lib/spack/spack/test/concretization/core.py b/lib/spack/spack/test/concretization/core.py index 85cb14179a5..f629c7827a4 100644 --- a/lib/spack/spack/test/concretization/core.py +++ b/lib/spack/spack/test/concretization/core.py @@ -3299,13 +3299,15 @@ def test_phil_spec_containing_commit_variant(spec_str, error_type): ("git-test-commit@{sha} commit=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", None), ], ) -def test_phil_spec_with_commit_interacts_with_lookup(mock_git_version_info, monkeypatch, spec_str, error_type): +def test_phil_spec_with_commit_interacts_with_lookup( + mock_git_version_info, monkeypatch, spec_str, error_type +): # This test will be short lived. Technically we could do further checks with a Lookup # but skipping impl since we are going to deprecate 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) - spec = spack.spec.Spec(spec_str.format(sha = commits[-1])) + spec = spack.spec.Spec(spec_str.format(sha=commits[-1])) if error_type is None: spack.concretize.concretize_one(spec) else: @@ -3377,33 +3379,3 @@ def _ensure_cache_hits(self, problem: str): # object for _ in range(5): assert h == spack.concretize.concretize_one("hdf5") - - -@pytest.mark.usefixtures("mutable_config", "mock_packages", "do_not_check_runtimes_on_reuse") -@pytest.mark.parametrize( - "spec_str, should_pass, error_type", - [ - # TODO write actual Exceptions for these to give good error messages - (f"git-ref-package@main commit={'a' * 40}", True, None), - (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), - ], -) -def test_spec_containing_commit_variant(spec_str, should_pass, error_type): - spec = spack.spec.Spec(spec_str) - if should_pass: - spec.concretize() - else: - with pytest.raises(error_type): - spec.concretize() - - -@pytest.mark.usefixtures("mutable_config", "mock_packages", "do_not_check_runtimes_on_reuse") -@pytest.mark.parametrize("version_str", [f"git.{'a' * 40}=main", "git.2.1.5=main"]) -def test_relationship_git_versions_and_commit_variant(version_str): - spec = spack.spec.Spec(f"git-ref-package@{version_str}").concretized() - if spec.version.commit_sha: - assert spec.version.commit_sha == spec.variants["commit"].value - else: - assert "commit" not in spec.variants diff --git a/lib/spack/spack/test/packages.py b/lib/spack/spack/test/packages.py index 70f23427f2f..9076e0e1874 100644 --- a/lib/spack/spack/test/packages.py +++ b/lib/spack/spack/test/packages.py @@ -348,6 +348,7 @@ def test_phil_package_condtional_variants_may_depend_on_commit(mock_packages, co assert conditional_variant assert conditional_variant.value + """ Issues: - version has commit Version("foo", commit=) diff --git a/lib/spack/spack/version/common.py b/lib/spack/spack/version/common.py index 60f06d6575f..ad13fc53ad1 100644 --- a/lib/spack/spack/version/common.py +++ b/lib/spack/spack/version/common.py @@ -28,11 +28,7 @@ def is_git_commit_sha(string: str) -> bool: def is_git_version(string: str) -> bool: - return ( - string.startswith("git.") - or is_git_commit_sha(string) - or "=" in string[1:] - ) + return string.startswith("git.") or is_git_commit_sha(string) or "=" in string[1:] class VersionError(spack.error.SpackError): 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 6f33571d0b3..6fce9beba37 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 @@ -38,7 +38,12 @@ class GitRefPackage(AutotoolsPackage): variant("opt", default=True, description="Enable optimizations") variant("shared", default=True, description="Build shared library") variant("pic", default=True, description="Enable position-independent code (PIC)") - variant("surgical", default=True, when=f"commit={'b' * 40}", description="Testing conditional on commit") + variant( + "surgical", + default=True, + when=f"commit={'b' * 40}", + description="Testing conditional on commit", + ) conflicts("+shared~pic")