From 4dc9d9f60ec5e53b130ff06848524796eaca4bc3 Mon Sep 17 00:00:00 2001 From: Harmen Stoppels Date: Thu, 23 Mar 2023 12:10:46 +0100 Subject: Revert "Bugfix: package requirements with git commits (#35057)" (#36341) This reverts commit 3d597e29bee1429d2b8d204016df40b575900d66. --- lib/spack/spack/solver/asp.py | 90 +------------------- lib/spack/spack/test/concretize.py | 10 ++- lib/spack/spack/test/concretize_requirements.py | 108 ------------------------ lib/spack/spack/version.py | 18 +--- 4 files changed, 11 insertions(+), 215 deletions(-) diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index 01624cba1b..8b9acd8b5e 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -115,30 +115,8 @@ version_provenance = collections.namedtuple( # type: ignore "VersionProvenance", version_origin_fields )(**{name: i for i, name in enumerate(version_origin_fields)}) - -class DeclaredVersion(object): - def __init__(self, version: spack.version.VersionBase, idx, origin): - if not isinstance(version, spack.version.VersionBase): - raise ValueError("Unexpected type for declared version: {0}".format(type(version))) - self.version = version - self.idx = idx - self.origin = origin - - def __eq__(self, other): - if not isinstance(other, DeclaredVersion): - return False - is_git = lambda v: isinstance(v, spack.version.GitVersion) - if is_git(self.version) != is_git(other.version): - # GitVersion(hash=x) and Version(y) compare equal if x == y, but - # we do not want that to be true for DeclaredVersion, which is - # tracking how much information is provided: in that sense, the - # GitVersion provides more information and is therefore not equal. - return False - return (self.version, self.idx, self.origin) == (other.version, other.idx, other.origin) - - def __hash__(self): - return hash((self.version, self.idx, self.origin)) - +#: Named tuple to contain information on declared versions +DeclaredVersion = collections.namedtuple("DeclaredVersion", ["version", "idx", "origin"]) # Below numbers are used to map names of criteria to the order # they appear in the solution. See concretize.lp @@ -1673,15 +1651,6 @@ class SpackSolverSetup(object): DeclaredVersion(version=dep.version, idx=0, origin=origin) ) self.possible_versions[dep.name].add(dep.version) - if ( - isinstance(dep.version, spack.version.GitVersion) - and dep.version.user_supplied_reference - ): - defined_version = spack.version.Version(dep.version.ref_version_str) - self.declared_versions[dep.name].append( - DeclaredVersion(version=defined_version, idx=0, origin=origin) - ) - self.possible_versions[dep.name].add(defined_version) def _supported_targets(self, compiler_name, compiler_version, targets): """Get a list of which targets are supported by the compiler. @@ -1920,11 +1889,7 @@ class SpackSolverSetup(object): # This is needed to account for a variable number of # numbers e.g. if both 1.0 and 1.0.2 are possible versions - exact_match = [ - v - for v in allowed_versions - if v == versions and not isinstance(v, spack.version.GitVersion) - ] + exact_match = [v for v in allowed_versions if v == versions] if exact_match: allowed_versions = exact_match @@ -2126,9 +2091,6 @@ class SpackSolverSetup(object): self.add_concrete_versions_from_specs(specs, version_provenance.spec) self.add_concrete_versions_from_specs(dev_specs, version_provenance.dev_spec) - req_version_specs = _get_versioned_specs_from_pkg_requirements() - self.add_concrete_versions_from_specs(req_version_specs, version_provenance.packages_yaml) - self.gen.h1("Concrete input spec definitions") self.define_concrete_input_specs(specs, possible) @@ -2203,52 +2165,6 @@ class SpackSolverSetup(object): self.gen.fact(fn.concretize_everything()) -def _get_versioned_specs_from_pkg_requirements(): - """If package requirements mention versions that are not mentioned - elsewhere, then we need to collect those to mark them as possible - versions. - """ - req_version_specs = list() - config = spack.config.get("packages") - for pkg_name, d in config.items(): - if pkg_name == "all": - continue - if "require" in d: - req_version_specs.extend(_specs_from_requires(pkg_name, d["require"])) - return req_version_specs - - -def _specs_from_requires(pkg_name, section): - if isinstance(section, str): - spec = spack.spec.Spec(section) - if not spec.name: - spec.name = pkg_name - extracted_specs = [spec] - else: - spec_strs = [] - # Each of these will be one_of or any_of - for spec_group in section: - (x,) = spec_group.values() - spec_strs.extend(x) - - extracted_specs = [] - for spec_str in spec_strs: - spec = spack.spec.Spec(spec_str) - if not spec.name: - spec.name = pkg_name - extracted_specs.append(spec) - - version_specs = [] - for spec in extracted_specs: - try: - spec.version - version_specs.append(spec) - except spack.error.SpecError: - pass - - return version_specs - - class SpecBuilder(object): """Class with actions to rebuild a spec from ASP results.""" diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py index 531fdf66d0..5c92a2af97 100644 --- a/lib/spack/spack/test/concretize.py +++ b/lib/spack/spack/test/concretize.py @@ -22,6 +22,7 @@ import spack.platforms import spack.repo import spack.variant as vt from spack.concretize import find_spec +from spack.solver.asp import UnsatisfiableSpecError from spack.spec import Spec from spack.version import ver @@ -1844,13 +1845,16 @@ class TestConcretize(object): assert s.satisfies("@0.1:") @pytest.mark.parametrize("git_ref", ("a" * 40, "0.2.15", "fbranch")) - def test_git_ref_version_succeeds_with_unknown_version(self, git_ref): + def test_git_ref_version_errors_if_unknown_version(self, git_ref): if spack.config.get("config:concretizer") == "original": pytest.skip("Original concretizer cannot account for git hashes") # main is not defined in the package.py for this file s = Spec("develop-branch-version@git.%s=main" % git_ref) - s.concretize() - assert s.satisfies("develop-branch-version@main") + with pytest.raises( + UnsatisfiableSpecError, + match="The reference version 'main' for package 'develop-branch-version'", + ): + s.concretized() @pytest.mark.regression("31484") def test_installed_externals_are_reused(self, mutable_database, repo_with_changing_recipe): diff --git a/lib/spack/spack/test/concretize_requirements.py b/lib/spack/spack/test/concretize_requirements.py index a84ae6a3c1..f58cd87694 100644 --- a/lib/spack/spack/test/concretize_requirements.py +++ b/lib/spack/spack/test/concretize_requirements.py @@ -140,114 +140,6 @@ packages: Spec("x@1.1").concretize() -def test_git_user_supplied_reference_satisfaction( - concretize_scope, test_repo, mock_git_version_info, monkeypatch -): - repo_path, filename, commits = mock_git_version_info - - monkeypatch.setattr( - spack.package_base.PackageBase, "git", "file://%s" % repo_path, raising=False - ) - - specs = ["v@{commit0}=2.2", "v@{commit0}", "v@2.2", "v@{commit0}=2.3"] - - format_info = {"commit0": commits[0]} - - hash_eq_ver, just_hash, just_ver, hash_eq_other_ver = [ - Spec(x.format(**format_info)) for x in specs - ] - - assert hash_eq_ver.satisfies(just_hash) - assert not just_hash.satisfies(hash_eq_ver) - assert hash_eq_ver.satisfies(just_ver) - assert not just_ver.satisfies(hash_eq_ver) - assert not hash_eq_ver.satisfies(hash_eq_other_ver) - assert not hash_eq_other_ver.satisfies(hash_eq_ver) - - -def test_requirement_adds_new_version( - concretize_scope, test_repo, mock_git_version_info, monkeypatch -): - if spack.config.get("config:concretizer") == "original": - pytest.skip("Original concretizer does not support configuration" " requirements") - - repo_path, filename, commits = mock_git_version_info - monkeypatch.setattr( - spack.package_base.PackageBase, "git", "file://%s" % repo_path, raising=False - ) - - a_commit_hash = commits[0] - conf_str = """\ -packages: - v: - require: "@{0}=2.2" -""".format( - a_commit_hash - ) - update_packages_config(conf_str) - - s1 = Spec("v").concretized() - assert s1.satisfies("@2.2") - assert s1.satisfies("@{0}".format(a_commit_hash)) - # Make sure the git commit info is retained - assert isinstance(s1.version, spack.version.GitVersion) - assert s1.version.ref == a_commit_hash - - -def test_requirement_adds_git_hash_version( - concretize_scope, test_repo, mock_git_version_info, monkeypatch -): - if spack.config.get("config:concretizer") == "original": - pytest.skip("Original concretizer does not support configuration" " requirements") - - repo_path, filename, commits = mock_git_version_info - monkeypatch.setattr( - spack.package_base.PackageBase, "git", "file://%s" % repo_path, raising=False - ) - - a_commit_hash = commits[0] - conf_str = """\ -packages: - v: - require: "@{0}" -""".format( - a_commit_hash - ) - update_packages_config(conf_str) - - s1 = Spec("v").concretized() - assert s1.satisfies("@{0}".format(a_commit_hash)) - - -def test_requirement_adds_multiple_new_versions( - concretize_scope, test_repo, mock_git_version_info, monkeypatch -): - if spack.config.get("config:concretizer") == "original": - pytest.skip("Original concretizer does not support configuration" " requirements") - - repo_path, filename, commits = mock_git_version_info - monkeypatch.setattr( - spack.package_base.PackageBase, "git", "file://%s" % repo_path, raising=False - ) - - conf_str = """\ -packages: - v: - require: - - one_of: ["@{0}=2.2", "@{1}=2.3"] -""".format( - commits[0], commits[1] - ) - update_packages_config(conf_str) - - s1 = Spec("v").concretized() - assert s1.satisfies("@2.2") - - s2 = Spec("v@{0}".format(commits[1])).concretized() - assert s2.satisfies("@{0}".format(commits[1])) - assert s2.satisfies("@2.3") - - def test_requirement_is_successfully_applied(concretize_scope, test_repo): """If a simple requirement can be satisfied, make sure the concretization succeeds and the requirement spec is applied. diff --git a/lib/spack/spack/version.py b/lib/spack/spack/version.py index a4a6effa23..53093d9459 100644 --- a/lib/spack/spack/version.py +++ b/lib/spack/spack/version.py @@ -506,9 +506,6 @@ class VersionBase(object): return VersionList() -_version_debug = False - - class GitVersion(VersionBase): """Class to represent versions interpreted from git refs. @@ -630,20 +627,7 @@ class GitVersion(VersionBase): self_cmp = self._cmp(other.ref_lookup) other_cmp = other._cmp(self.ref_lookup) - if self.is_ref and other.is_ref: - if self.ref != other.ref: - return False - elif self.user_supplied_reference and other.user_supplied_reference: - return self.ref_version == other.ref_version - elif other.user_supplied_reference: - return False - else: - # In this case, 'other' does not supply a version equivalence - # with "=" and the commit strings are equal. 'self' may specify - # a version equivalence, but that is extra info and will - # satisfy no matter what it is. - return True - elif other.is_ref: + if other.is_ref: # if other is a ref then satisfaction requires an exact version match # i.e. the GitRef must match this.version for satisfaction # this creates an asymmetric comparison: -- cgit v1.2.3-70-g09d2