From ecc3752ee915291478cd69b9d7a228b205c410bd Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Tue, 29 Apr 2025 10:09:49 +0200 Subject: [PATCH] fix %compiler satisfaction with specs v4 format (#50140) This PR improves compatibility with specs installed before #45189, and with externals specifying a compiler, by using the annotated compiler to "satisfy" a spec query. On top of that, the PR adds a new flag for: ```console $ spack find --specfile-format -I %gcc -- linux-ubuntu20.04-icelake / gcc@10.5.0 ----------------------- [+] [v4] ca-certificates-mozilla@2023-05-30 [e] [v4] cmake@3.31.6 [+] [v4] gcc-runtime@10.5.0 [e] [v4] glibc@2.31 [+] [v4] gmake@4.4.1 [+] [v4] hdf5@1.14.5 [+] [v4] pkgconf@2.2.0 [+] [v4] zlib-ng@2.2.1 ==> 8 installed packages ``` which shows the specfile format of the specs being retrieved. --- lib/spack/spack/cmd/__init__.py | 6 +- lib/spack/spack/cmd/find.py | 15 ++++- lib/spack/spack/solver/asp.py | 15 +++++ lib/spack/spack/solver/concretize.lp | 19 ++++++ lib/spack/spack/spec.py | 38 ++++++++--- lib/spack/spack/test/concretization/core.py | 60 ++++++++++++++++++ .../data/database/index.json.v7_v8.json.gz | Bin 0 -> 1858 bytes lib/spack/spack/test/database.py | 25 ++++++++ lib/spack/spack/test/spec_yaml.py | 4 ++ share/spack/spack-completion.bash | 2 +- share/spack/spack-completion.fish | 4 +- 11 files changed, 174 insertions(+), 14 deletions(-) create mode 100644 lib/spack/spack/test/data/database/index.json.v7_v8.json.gz diff --git a/lib/spack/spack/cmd/__init__.py b/lib/spack/spack/cmd/__init__.py index 0ceee94ea46..030a95c2388 100644 --- a/lib/spack/spack/cmd/__init__.py +++ b/lib/spack/spack/cmd/__init__.py @@ -436,7 +436,7 @@ def display_specs(specs, args=None, **kwargs): all_headers (bool): show headers even when arch/compiler aren't defined status_fn (typing.Callable): if provided, prepend install-status info output (typing.IO): A file object to write to. Default is ``sys.stdout`` - + specfile_format (bool): specfile format of the current spec """ def get_arg(name, default=None): @@ -458,6 +458,7 @@ def get_arg(name, default=None): all_headers = get_arg("all_headers", False) output = get_arg("output", sys.stdout) status_fn = get_arg("status_fn", None) + specfile_format = get_arg("specfile_format", False) decorator = get_arg("decorator", None) if decorator is None: @@ -479,6 +480,9 @@ def get_arg(name, default=None): vfmt = "{variants}" if variants else "" format_string = nfmt + "{@version}" + vfmt + ffmt + if specfile_format: + format_string = "[{specfile_version}] " + format_string + def fmt(s, depth=0): """Formatter function for all output specs""" string = "" diff --git a/lib/spack/spack/cmd/find.py b/lib/spack/spack/cmd/find.py index e17b6c5fe60..8a1100303a8 100644 --- a/lib/spack/spack/cmd/find.py +++ b/lib/spack/spack/cmd/find.py @@ -51,6 +51,12 @@ def setup_parser(subparser): "-I", "--install-status", action="store_true", help="show install status of packages" ) + subparser.add_argument( + "--specfile-format", + action="store_true", + help="show the specfile format for installed deps ", + ) + subparser.add_argument( "-d", "--deps", action="store_true", help="output dependencies along with found specs" ) @@ -280,6 +286,7 @@ def root_decorator(spec, string): show_flags=True, decorator=root_decorator, variants=True, + specfile_format=args.specfile_format, ) print() @@ -301,6 +308,7 @@ def root_decorator(spec, string): namespace=True, show_flags=True, variants=True, + specfile_format=args.specfile_format, ) print() @@ -390,7 +398,12 @@ def find(parser, args): if args.show_concretized: display_results += concretized_but_not_installed cmd.display_specs( - display_results, args, decorator=decorator, all_headers=True, status_fn=status_fn + display_results, + args, + decorator=decorator, + all_headers=True, + status_fn=status_fn, + specfile_format=args.specfile_format, ) # print number of installed packages last (as the list may be long) diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index 01d897c82e4..d3db57f3ea4 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -3862,6 +3862,17 @@ def external_spec_selected(self, node, idx): ) self._specs[node].extra_attributes = spec_info.get("extra_attributes", {}) + # Annotate compiler specs from externals + external_spec = spack.spec.Spec(spec_info["spec"]) + external_spec_deps = external_spec.dependencies() + if len(external_spec_deps) > 1: + raise InvalidExternalError( + f"external spec {spec_info['spec']} cannot have more than one dependency" + ) + elif len(external_spec_deps) == 1: + compiler_str = external_spec_deps[0] + self._specs[node].annotations.with_compiler(spack.spec.Spec(compiler_str)) + # If this is an extension, update the dependencies to include the extendee package = spack.repo.PATH.get_pkg_class(self._specs[node].fullname)(self._specs[node]) extendee_spec = package.extendee_spec @@ -4765,3 +4776,7 @@ class InvalidSpliceError(spack.error.SpackError): class NoCompilerFoundError(spack.error.SpackError): """Raised when there is no possible compiler""" + + +class InvalidExternalError(spack.error.SpackError): + """Raised when there is no possible compiler""" diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp index 3670caf23bc..697a6d4d08d 100644 --- a/lib/spack/spack/solver/concretize.lp +++ b/lib/spack/spack/solver/concretize.lp @@ -184,6 +184,7 @@ literal_node(Root, node(min_dupe_id, Root)) :- mentioned_in_literal(Root, Root) 1 { build_dependency_of_literal_node(LiteralNode, node(0..Y-1, BuildDependency)) : max_dupes(BuildDependency, Y) } 1 :- literal_node(Root, LiteralNode), build(LiteralNode), + not external(LiteralNode), attr("build_requirement", LiteralNode, build_requirement("node", BuildDependency)). condition_set(node(min_dupe_id, Root), LiteralNode) :- literal_node(Root, LiteralNode). @@ -490,6 +491,7 @@ provider(ProviderNode, VirtualNode) :- attr("provider_set", ProviderNode, Virtua build(node(X, Parent)), not external(node(X, Parent)). +% Concrete nodes :- attr("build_requirement", ParentNode, build_requirement("node", BuildDependency)), concrete(ParentNode), not attr("concrete_build_dependency", ParentNode, BuildDependency, _). @@ -503,6 +505,23 @@ provider(ProviderNode, VirtualNode) :- attr("provider_set", ProviderNode, Virtua attr("virtual_on_build_edge", ParentNode, BuildDependency, Virtual), not 1 { pkg_fact(BuildDependency, version_satisfies(Constraint, Version)) : hash_attr(BuildDependencyHash, "version", BuildDependency, Version) } 1. +% External nodes +:- attr("build_requirement", ParentNode, build_requirement("node", BuildDependency)), + external(ParentNode), + not attr("external_build_requirement", ParentNode, build_requirement("node", BuildDependency)). + +candidate_external_version(Constraint, BuildDependency, Version) + :- attr("build_requirement", ParentNode, build_requirement("node_version_satisfies", BuildDependency, Constraint)), + external(ParentNode), + pkg_fact(BuildDependency, version_satisfies(Constraint, Version)). + +error(100, "External {0} cannot satisfy both {1} and {2}", BuildDependency, LiteralConstraint, ExternalConstraint) + :- attr("build_requirement", ParentNode, build_requirement("node_version_satisfies", BuildDependency, LiteralConstraint)), + external(ParentNode), + attr("external_build_requirement", ParentNode, build_requirement("node_version_satisfies", BuildDependency, ExternalConstraint)), + not 1 { pkg_fact(BuildDependency, version_satisfies(ExternalConstraint, Version)) : candidate_external_version(LiteralConstraint, BuildDependency, Version) }. + + % Asking for gcc@10 %gcc@9 shouldn't give us back an external gcc@10, just because of the hack % we have on externals :- attr("build_requirement", node(X, Parent), build_requirement("node", BuildDependency)), diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 06c42a8e2fc..b31f1f1cb51 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -1429,7 +1429,7 @@ def with_compiler(self, compiler: "Spec") -> "SpecAnnotations": def __repr__(self) -> str: result = f"SpecAnnotations().with_spec_format({self.original_spec_format})" if self.compiler_node_attribute: - result += f"with_compiler({str(self.compiler_node_attribute)})" + result += f".with_compiler({str(self.compiler_node_attribute)})" return result @@ -3394,7 +3394,7 @@ def satisfies(self, other: Union[str, "Spec"], deps: bool = True) -> bool: return True # If we have no dependencies, we can't satisfy any constraints. - if not self._dependencies: + if not self._dependencies and self.original_spec_format() >= 5 and not self.external: return False # If we arrived here, the lhs root node satisfies the rhs root node. Now we need to check @@ -3405,6 +3405,7 @@ def satisfies(self, other: Union[str, "Spec"], deps: bool = True) -> bool: # verify the edge properties, cause everything is encoded in the hash of the nodes that # will be verified later. lhs_edges: Dict[str, Set[DependencySpec]] = collections.defaultdict(set) + mock_nodes_from_old_specfiles = set() for rhs_edge in other.traverse_edges(root=False, cover="edges"): # If we are checking for ^mpi we need to verify if there is any edge if spack.repo.PATH.is_virtual(rhs_edge.spec.name): @@ -3426,13 +3427,27 @@ def satisfies(self, other: Union[str, "Spec"], deps: bool = True) -> bool: except KeyError: return False - candidates = current_node.dependencies( - name=rhs_edge.spec.name, - deptype=rhs_edge.depflag, - virtuals=rhs_edge.virtuals or None, - ) - if not candidates or not any(x.satisfies(rhs_edge.spec) for x in candidates): - return False + if current_node.original_spec_format() < 5 or ( + current_node.original_spec_format() >= 5 and current_node.external + ): + compiler_spec = current_node.annotations.compiler_node_attribute + if compiler_spec is None: + return False + + mock_nodes_from_old_specfiles.add(compiler_spec) + # This checks that the single node compiler spec satisfies the request + # of a direct dependency. The check is not perfect, but based on heuristic. + if not compiler_spec.satisfies(rhs_edge.spec): + return False + + else: + candidates = current_node.dependencies( + name=rhs_edge.spec.name, + deptype=rhs_edge.depflag, + virtuals=rhs_edge.virtuals or None, + ) + if not candidates or not any(x.satisfies(rhs_edge.spec) for x in candidates): + return False continue @@ -3472,8 +3487,9 @@ def satisfies(self, other: Union[str, "Spec"], deps: bool = True) -> bool: return False # Edges have been checked above already, hence deps=False + lhs_nodes = [x for x in self.traverse(root=False)] + sorted(mock_nodes_from_old_specfiles) return all( - any(lhs.satisfies(rhs, deps=False) for lhs in self.traverse(root=False)) + any(lhs.satisfies(rhs, deps=False) for lhs in lhs_nodes) for rhs in other.traverse(root=False) ) @@ -3947,6 +3963,8 @@ def format_attribute(match_object: Match) -> str: except AttributeError: if part == "compiler": return "none" + elif part == "specfile_version": + return f"v{current.original_spec_format()}" raise SpecFormatStringError( f"Attempted to format attribute {attribute}. " diff --git a/lib/spack/spack/test/concretization/core.py b/lib/spack/spack/test/concretization/core.py index 4ea9591a24f..6d3b2f57a50 100644 --- a/lib/spack/spack/test/concretization/core.py +++ b/lib/spack/spack/test/concretization/core.py @@ -3380,3 +3380,63 @@ def test_input_analysis_and_conditional_requirements(default_mock_concretization libceed = default_mock_concretization("libceed") assert libceed["libxsmm"].satisfies("@main") assert libceed["libxsmm"].satisfies("platform=test") + + +@pytest.mark.parametrize( + "compiler_str,expected,not_expected", + [ + # Compiler queries are as specific as the constraint on the external + ("gcc@10", ["%gcc", "%gcc@10"], ["%clang", "%gcc@9"]), + ("gcc", ["%gcc"], ["%clang", "%gcc@9", "%gcc@10"]), + ], +) +@pytest.mark.regression("49841") +def test_installing_external_with_compilers_directly( + compiler_str, expected, not_expected, mutable_config, mock_packages, tmp_path +): + """Tests that version constraints are taken into account for compiler annotations + on externals + """ + spec_str = f"libelf@0.8.12 %{compiler_str}" + packages_yaml = syaml.load_config( + f""" +packages: + libelf: + buildable: false + externals: + - spec: {spec_str} + prefix: {tmp_path / 'libelf'} +""" + ) + mutable_config.set("packages", packages_yaml["packages"]) + s = spack.concretize.concretize_one(spec_str) + + assert s.external + assert all(s.satisfies(c) for c in expected) + assert all(not s.satisfies(c) for c in not_expected) + + +@pytest.mark.regression("49841") +def test_using_externals_with_compilers(mutable_config, mock_packages, tmp_path): + """Tests that version constraints are taken into account for compiler annotations + on externals, even imposed as transitive deps. + """ + packages_yaml = syaml.load_config( + f""" +packages: + libelf: + buildable: false + externals: + - spec: libelf@0.8.12 %gcc@10 + prefix: {tmp_path / 'libelf'} +""" + ) + mutable_config.set("packages", packages_yaml["packages"]) + + with pytest.raises(spack.error.SpackError): + spack.concretize.concretize_one("dyninst%gcc@10.2.1 ^libelf@0.8.12 %gcc@:9") + + s = spack.concretize.concretize_one("dyninst%gcc@10.2.1 ^libelf@0.8.12 %gcc@10:") + + libelf = s["libelf"] + assert libelf.external and libelf.satisfies("%gcc") diff --git a/lib/spack/spack/test/data/database/index.json.v7_v8.json.gz b/lib/spack/spack/test/data/database/index.json.v7_v8.json.gz new file mode 100644 index 0000000000000000000000000000000000000000..d4e4ecb56425bc789d29760f279316835b9a71fe GIT binary patch literal 1858 zcmV-I2fg?oiwFpq69#Ai18Ht#Wq2-Xb8l_{?V3rCtTqsU|4W~T?#660BSp$B$~D(0 zY8hj|jj@dvwx{{;v7y(@%wzIqkzUe$Xlw(nvR(C68Thu!poS?_c=h;g)ncg#XRF86 z{pxN-n9`6^<<9gPB{ItQaS{epMsnXGTHqXup-^H7p}R&2jNFmI+V-|85b~s9n%LUi zMc4jz9j&*v2})GD>ji~6=d!lvC}F0zr;bfKZl$kGYP&IDyWWQOoQAT%`a%#vn8Ion z!ZI#x({Gp=!tn1*V`|%T47HKi_VwoqW=;UK75m@z^FNpmkq>q$v4bwo^){`^#p#wY za#0gpAf~hC)RcCoBFaw1iwp8oiR3jrRh*b}#VKtY{Q2O-BbrmAN-p$N=~6c=KSu5_ zbyMV%Vs-6?RP#1<6Cx0;4Q(dzv#PcrGeNTHC?T5*3S(DLCEQKDD23;LFdxF5e=N)` z3I}e!=V@#&#qJ>5;qtpr4F%~M0DCTunJW!tqiFJ}_2J=>{4Y3%H2l1rR6D5tP+5~qH}Wv%Zo zYWYQ(W>*zT(y9F9grW)-cvnJYO?ahnf)i>YT)?E0eb=#4bbA4EMMDFU#4cMl0@@{M zS3)fSP`vrY{;XaZ7S?DnLu4|~v-enOPVa*R4B89=QW-FmeGX`8JFSwuB1uYvHY}e_ zgzZ-kVm5i}Rh661mPll6JTsi0mh_Nq-0s6Ojx{x&icr+ zBQ(Qxxb|<%yfO1DnOWEq_~ZqjN>C}rea_Q1Y099(ttLUpXqo|B>n^I(5w98t#taA9JK_!`}vu@h^T932Or67hdOVi|T4@({lY z>oyN0?LeloaF5wOD$6>ut5?dF47eX(f;EC3!cvo*xbBo!^O0EjSNH~75O~|j3vPMu zjp6^Dyf+S*XuGE4pifAQ>y}lt4C@lHFe-Y`jCLaAkd{#i2eM|Hg{A$WXs1P=HmeRITYqM+nbAr?u&L z-l&*|6-y%eLS!kkJh}=i{@M@M(Jijr;>st)6)1>3o8jE~hMT?XID-!SnVTI-Y~vr8 zVmJ-h28NTPSo5=Chn?z&Ax!3Vq!>NBmEfoH9=7mQsi8QRopOirb*xe&IbWtriD9N- zetrZs$HuP+Y-1zRl#ZFM8XEUz?2ERo|3hKmRDoO%2;NkETySB5-vTDLf6o* z`@S4;i-T?$)gb67a*%}tvLsPf#qLL-fA_Uf^k^KObT(!uvp8DIe?PvSg^8*_oaJY6 zi0h&f?J(peYCD6%(C&_9K;s&;Ar6A?inELM&+#1ukm7of=e^2XBlPkx?_QkXNs>d# zr|e>vV(I7YVqHx3<~Zyvs`1^dVy?fQRM=!dl)Ro)ESk;pq~dIP*W1&DFtfo7WuuTy5VMq|kU|o|xGTn#N`?QwZ-;*FZRR2c zKX03Ppy{;D+g?ncRJ?la@QQE(nJT_@oVYMb?@MmrPy_VhfOq&y3Ry^E`TW*~zs-;=8BanJiA zGO0}CIwXxMi+V`X7VVkHj?v^}S-0rl*Q|^0{xSIXy*~l#`{dyP+`pcYE^bj@cp3ie z%ca2bXC2=NbR*DLzV?3Q$yUbE$u}4>HcvdiOX=85oVnWQ!jMn;_UR>@U1TpVVL7tH zgUYkssW#Jqya}(H^_19nn6;ah;O+-8D9O;I1E`0wPr72j;XotPWt9mCW5J$lq7zZ` wz63=Vg)9x*9)1c27cujel and similar selections. + """ + test_path = pathlib.Path(spack.paths.test_path) + zipfile = test_path / "data" / "database" / "index.json.v7_v8.json.gz" + with gzip.open(str(zipfile), "rt", encoding="utf-8") as f: + data = json.load(f) + + index_json = tmp_path / spack.database._DB_DIRNAME / spack.database.INDEX_JSON_FILE + index_json.parent.mkdir(parents=True) + index_json.write_text(json.dumps(data)) + + db = spack.database.Database(str(tmp_path)) + + specs = db.query("%gcc") + + assert len(specs) == 8 + assert len([x for x in specs if x.external]) == 2 + assert len([x for x in specs if x.original_spec_format() < 5]) == 8 diff --git a/lib/spack/spack/test/spec_yaml.py b/lib/spack/spack/test/spec_yaml.py index 13f7b65a1f8..a3b09a2e66f 100644 --- a/lib/spack/spack/test/spec_yaml.py +++ b/lib/spack/spack/test/spec_yaml.py @@ -433,6 +433,10 @@ def test_load_json_specfiles(specfile, expected_hash, reader_cls): assert s2.format("{compiler.name}") == "gcc" assert s2.format("{compiler.version}") != "none" + # Ensure satisfies still works with compilers + assert s2.satisfies("%gcc") + assert s2.satisfies("%gcc@9.4.0") + def test_anchorify_1(): """Test that anchorify replaces duplicate values with references to a single instance, and diff --git a/share/spack/spack-completion.bash b/share/spack/spack-completion.bash index 910613babdb..94d4457d393 100644 --- a/share/spack/spack-completion.bash +++ b/share/spack/spack-completion.bash @@ -1214,7 +1214,7 @@ _spack_fetch() { _spack_find() { if $list_options then - SPACK_COMPREPLY="-h --help --format -H --hashes --json -I --install-status -d --deps -p --paths --groups --no-groups -l --long -L --very-long -t --tag -N --namespaces -r --only-roots -c --show-concretized -f --show-flags --show-full-compiler -x --explicit -X --implicit -u --unknown -m --missing -v --variants --loaded -M --only-missing --only-deprecated --deprecated --install-tree --start-date --end-date" + SPACK_COMPREPLY="-h --help --format -H --hashes --json -I --install-status --specfile-format -d --deps -p --paths --groups --no-groups -l --long -L --very-long -t --tag -N --namespaces -r --only-roots -c --show-concretized -f --show-flags --show-full-compiler -x --explicit -X --implicit -u --unknown -m --missing -v --variants --loaded -M --only-missing --only-deprecated --deprecated --install-tree --start-date --end-date" else _installed_packages fi diff --git a/share/spack/spack-completion.fish b/share/spack/spack-completion.fish index 6abc1835f3b..560f47193f5 100644 --- a/share/spack/spack-completion.fish +++ b/share/spack/spack-completion.fish @@ -1786,7 +1786,7 @@ complete -c spack -n '__fish_spack_using_command fetch' -l deprecated -f -a conf complete -c spack -n '__fish_spack_using_command fetch' -l deprecated -d 'allow concretizer to select deprecated versions' # spack find -set -g __fish_spack_optspecs_spack_find h/help format= H/hashes json I/install-status d/deps p/paths groups no-groups l/long L/very-long t/tag= N/namespaces r/only-roots c/show-concretized f/show-flags show-full-compiler x/explicit X/implicit u/unknown m/missing v/variants loaded M/only-missing only-deprecated deprecated install-tree= start-date= end-date= +set -g __fish_spack_optspecs_spack_find h/help format= H/hashes json I/install-status specfile-format d/deps p/paths groups no-groups l/long L/very-long t/tag= N/namespaces r/only-roots c/show-concretized f/show-flags show-full-compiler x/explicit X/implicit u/unknown m/missing v/variants loaded M/only-missing only-deprecated deprecated install-tree= start-date= end-date= complete -c spack -n '__fish_spack_using_command_pos_remainder 0 find' -f -a '(__fish_spack_installed_specs)' complete -c spack -n '__fish_spack_using_command find' -s h -l help -f -a help complete -c spack -n '__fish_spack_using_command find' -s h -l help -d 'show this help message and exit' @@ -1798,6 +1798,8 @@ complete -c spack -n '__fish_spack_using_command find' -l json -f -a json complete -c spack -n '__fish_spack_using_command find' -l json -d 'output specs as machine-readable json records' complete -c spack -n '__fish_spack_using_command find' -s I -l install-status -f -a install_status complete -c spack -n '__fish_spack_using_command find' -s I -l install-status -d 'show install status of packages' +complete -c spack -n '__fish_spack_using_command find' -l specfile-format -f -a specfile_format +complete -c spack -n '__fish_spack_using_command find' -l specfile-format -d 'show the specfile format for installed deps ' complete -c spack -n '__fish_spack_using_command find' -s d -l deps -f -a deps complete -c spack -n '__fish_spack_using_command find' -s d -l deps -d 'output dependencies along with found specs' complete -c spack -n '__fish_spack_using_command find' -s p -l paths -f -a paths