From d883883be0f9de86d5c7613ffa69f71aae7026f7 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Thu, 27 Jun 2024 14:58:42 +0200 Subject: [PATCH] Ensure parent runtime version >= child (#44834) Fixes a bug where old gcc-runtime libraries would be loaded at runtime, but newer are required by dependencies, breaking the binaries. --- lib/spack/spack/solver/asp.py | 33 ++++++++++++++ lib/spack/spack/solver/concretize.lp | 43 ++++++++++++++++--- lib/spack/spack/solver/libc_compatibility.lp | 3 -- lib/spack/spack/test/concretize.py | 26 +++++------ .../test/concretize_compiler_runtimes.py | 13 +++--- .../repos/builtin/packages/gcc/package.py | 4 ++ .../packages/gcc/package.py | 3 ++ 7 files changed, 97 insertions(+), 28 deletions(-) diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index b875c9e986..f26a83f9e2 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -3225,6 +3225,39 @@ def requires(self, impose: str, *, when: str): self.runtime_conditions.add((imposed_spec, when_spec)) self.reset() + def propagate(self, constraint_str: str, *, when: str): + msg = "the 'propagate' method can be called only with pkg('*')" + assert self.current_package == "*", msg + + when_spec = spack.spec.Spec(when) + assert when_spec.name is None, "only anonymous when specs are accepted" + + placeholder = "XXX" + node_variable = "node(ID, Package)" + when_spec.name = placeholder + + body_clauses = self._setup.spec_clauses(when_spec, body=True) + body_str = ( + f" {f',{os.linesep} '.join(str(x) for x in body_clauses)},\n" + f" not external({node_variable}),\n" + f" not runtime(Package)" + ).replace(f'"{placeholder}"', f"{node_variable}") + + constraint_spec = spack.spec.Spec(constraint_str) + assert constraint_spec.name is None, "only anonymous constraint specs are accepted" + + constraint_spec.name = placeholder + constraint_clauses = self._setup.spec_clauses(constraint_spec, body=False) + for clause in constraint_clauses: + if clause.args[0] == "node_compiler_version_satisfies": + self._setup.compiler_version_constraints.add(constraint_spec.compiler) + args = f'"{constraint_spec.compiler.name}", "{constraint_spec.compiler.versions}"' + head_str = f"propagate({node_variable}, node_compiler_version_satisfies({args}))" + rule = f"{head_str} :-\n{body_str}.\n\n" + self.rules.append(rule) + + self.reset() + def consume_facts(self): """Consume the facts collected by this object, and emits rules and facts for the runtimes. diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp index f437725bd7..b7acbdce21 100644 --- a/lib/spack/spack/solver/concretize.lp +++ b/lib/spack/spack/solver/concretize.lp @@ -995,6 +995,30 @@ variant_is_propagated(PackageNode, Variant) :- attr("variant_value", PackageNode, Variant, Value), not propagate(PackageNode, variant_value(Variant, Value)). +%---- +% Compiler constraints +%---- + +attr("node_compiler_version_satisfies", node(ID, Package), Compiler, Version) :- + propagate(node(ID, Package), node_compiler_version_satisfies(Compiler, Version)), + node_compiler(node(ID, Package), CompilerID), + compiler_name(CompilerID, Compiler), + not runtime(Package), + not external(Package). + +%----------------------------------------------------------------------------- +% Runtimes +%----------------------------------------------------------------------------- + +% Check whether the DAG has any built package +has_built_packages() :- build(X), not external(X). + +% If we build packages, the runtime nodes must use an available compiler +1 { node_compiler(PackageNode, CompilerID) : build(PackageNode), not external(PackageNode) } :- + has_built_packages(), + runtime(RuntimePackage), + node_compiler(node(_, RuntimePackage), CompilerID). + %----------------------------------------------------------------------------- % Platform semantics %----------------------------------------------------------------------------- @@ -1096,10 +1120,15 @@ attr("node_target", PackageNode, Target) :- attr("node", PackageNode), attr("node_target_set", PackageNode, Target). % each node has the weight of its assigned target -node_target_weight(node(ID, Package), Weight) - :- attr("node", node(ID, Package)), - attr("node_target", node(ID, Package), Target), - target_weight(Target, Weight). +target_weight(Target, 0) + :- attr("node", PackageNode), + attr("node_target", PackageNode, Target), + attr("node_target_set", PackageNode, Target). + +node_target_weight(PackageNode, MinWeight) + :- attr("node", PackageNode), + attr("node_target", PackageNode, Target), + MinWeight = #min { Weight : target_weight(Target, Weight) }. % compatibility rules for targets among nodes node_target_match(ParentNode, DependencyNode) @@ -1161,12 +1190,12 @@ error(10, "No valid compiler for {0} satisfies '%{1}'", Package, Compiler) % If the compiler of a node must satisfy a constraint, then its version % must be chosen among the ones that satisfy said constraint -error(100, "No valid version for '{0}' compiler '{1}' satisfies '@{2}'", Package, Compiler, Constraint) +error(100, "Package {0} cannot satisfy '%{1}@{2}'", Package, Compiler, Constraint) :- attr("node", node(X, Package)), attr("node_compiler_version_satisfies", node(X, Package), Compiler, Constraint), - not compiler_version_satisfies(Compiler, Constraint, _). + not compiler_version_satisfies(Compiler, Constraint, _). -error(100, "No valid version for '{0}' compiler '{1}' satisfies '@{2}'", Package, Compiler, Constraint) +error(100, "Package {0} cannot satisfy '%{1}@{2}'", Package, Compiler, Constraint) :- attr("node", node(X, Package)), attr("node_compiler_version_satisfies", node(X, Package), Compiler, Constraint), not compiler_version_satisfies(Compiler, Constraint, ID), diff --git a/lib/spack/spack/solver/libc_compatibility.lp b/lib/spack/spack/solver/libc_compatibility.lp index 3e2c00e372..6e032515b5 100644 --- a/lib/spack/spack/solver/libc_compatibility.lp +++ b/lib/spack/spack/solver/libc_compatibility.lp @@ -18,9 +18,6 @@ error(100, "Cannot reuse {0} since we cannot determine libc compatibility", Reus ReusedPackage != LibcPackage, not attr("compatible_libc", node(R, ReusedPackage), LibcPackage, LibcVersion). -% Check whether the DAG has any built package -has_built_packages() :- build(X), not external(X). - % A libc is needed in the DAG :- has_built_packages(), not provider(_, node(0, "libc")). diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py index 0b4ce5b42a..5c5b0a73d9 100644 --- a/lib/spack/spack/test/concretize.py +++ b/lib/spack/spack/test/concretize.py @@ -1767,21 +1767,21 @@ def test_reuse_with_unknown_package_dont_raise(self, tmpdir, temporary_store, mo assert s.namespace == "builtin.mock" @pytest.mark.parametrize( - "specs,expected", + "specs,expected,libc_offset", [ - (["libelf", "libelf@0.8.10"], 1), - (["libdwarf%gcc", "libelf%clang"], 2), - (["libdwarf%gcc", "libdwarf%clang"], 3), - (["libdwarf^libelf@0.8.12", "libdwarf^libelf@0.8.13"], 4), - (["hdf5", "zmpi"], 3), - (["hdf5", "mpich"], 2), - (["hdf5^zmpi", "mpich"], 4), - (["mpi", "zmpi"], 2), - (["mpi", "mpich"], 1), + (["libelf", "libelf@0.8.10"], 1, 1), + (["libdwarf%gcc", "libelf%clang"], 2, 1), + (["libdwarf%gcc", "libdwarf%clang"], 3, 2), + (["libdwarf^libelf@0.8.12", "libdwarf^libelf@0.8.13"], 4, 1), + (["hdf5", "zmpi"], 3, 1), + (["hdf5", "mpich"], 2, 1), + (["hdf5^zmpi", "mpich"], 4, 1), + (["mpi", "zmpi"], 2, 1), + (["mpi", "mpich"], 1, 1), ], ) @pytest.mark.only_clingo("Original concretizer cannot concretize in rounds") - def test_best_effort_coconcretize(self, specs, expected): + def test_best_effort_coconcretize(self, specs, expected, libc_offset): specs = [Spec(s) for s in specs] solver = spack.solver.asp.Solver() solver.reuse = False @@ -1790,7 +1790,9 @@ def test_best_effort_coconcretize(self, specs, expected): for s in result.specs: concrete_specs.update(s.traverse()) - libc_offset = 1 if spack.solver.asp.using_libc_compatibility() else 0 + if not spack.solver.asp.using_libc_compatibility(): + libc_offset = 0 + assert len(concrete_specs) == expected + libc_offset @pytest.mark.parametrize( diff --git a/lib/spack/spack/test/concretize_compiler_runtimes.py b/lib/spack/spack/test/concretize_compiler_runtimes.py index 3e13fd8e56..44395b5255 100644 --- a/lib/spack/spack/test/concretize_compiler_runtimes.py +++ b/lib/spack/spack/test/concretize_compiler_runtimes.py @@ -79,13 +79,13 @@ def test_external_nodes_do_not_have_runtimes(runtime_repo, mutable_config, tmp_p [ # The reused runtime is older than we need, thus we'll add a more recent one for a ("a%gcc@10.2.1", "b%gcc@9.4.0", {"a": "gcc-runtime@10.2.1", "b": "gcc-runtime@9.4.0"}, 2), - # The root is compiled with an older compiler, thus we'll reuse the runtime from b - ("a%gcc@9.4.0", "b%gcc@10.2.1", {"a": "gcc-runtime@10.2.1", "b": "gcc-runtime@10.2.1"}, 1), + # The root is compiled with an older compiler, thus we'll NOT reuse the runtime from b + ("a%gcc@9.4.0", "b%gcc@10.2.1", {"a": "gcc-runtime@9.4.0", "b": "gcc-runtime@9.4.0"}, 1), # Same as before, but tests that we can reuse from a more generic target pytest.param( "a%gcc@9.4.0", "b%gcc@10.2.1 target=x86_64", - {"a": "gcc-runtime@10.2.1 target=x86_64", "b": "gcc-runtime@10.2.1 target=x86_64"}, + {"a": "gcc-runtime@9.4.0", "b": "gcc-runtime@9.4.0"}, 1, marks=pytest.mark.skipif( str(archspec.cpu.host().family) != "x86_64", reason="test data is x86_64 specific" @@ -102,13 +102,15 @@ def test_external_nodes_do_not_have_runtimes(runtime_repo, mutable_config, tmp_p ), ], ) +@pytest.mark.regression("44444") def test_reusing_specs_with_gcc_runtime(root_str, reused_str, expected, nruntime, runtime_repo): """Tests that we can reuse specs with a "gcc-runtime" leaf node. In particular, checks that the semantic for gcc-runtimes versions accounts for reused packages too. + + Reusable runtime versions should be lower, or equal, to that of parent nodes. """ root, reused_spec = _concretize_with_reuse(root_str=root_str, reused_str=reused_str) - assert f"{expected['b']}" in reused_spec runtime_a = root.dependencies("gcc-runtime")[0] assert runtime_a.satisfies(expected["a"]) runtime_b = root["b"].dependencies("gcc-runtime")[0] @@ -123,8 +125,7 @@ def test_reusing_specs_with_gcc_runtime(root_str, reused_str, expected, nruntime [ # Ensure that, whether we have multiple runtimes in the DAG or not, # we always link only the latest version - ("a%gcc@10.2.1", "b%gcc@9.4.0", ["gcc-runtime@10.2.1"], ["gcc-runtime@9.4.0"]), - ("a%gcc@9.4.0", "b%gcc@10.2.1", ["gcc-runtime@10.2.1"], ["gcc-runtime@9.4.0"]), + ("a%gcc@10.2.1", "b%gcc@9.4.0", ["gcc-runtime@10.2.1"], ["gcc-runtime@9.4.0"]) ], ) def test_views_can_handle_duplicate_runtime_nodes( diff --git a/var/spack/repos/builtin/packages/gcc/package.py b/var/spack/repos/builtin/packages/gcc/package.py index ce9fe7f702..ebb0b3c95b 100644 --- a/var/spack/repos/builtin/packages/gcc/package.py +++ b/var/spack/repos/builtin/packages/gcc/package.py @@ -1134,6 +1134,10 @@ def runtime_constraints(cls, *, spec, pkg): # The version of gcc-runtime is the same as the %gcc used to "compile" it pkg("gcc-runtime").requires(f"@={str(spec.version)}", when=f"%{str(spec)}") + # If a node used %gcc@X.Y its dependencies must use gcc-runtime@:X.Y + # (technically @:X is broader than ... <= @=X but this should work in practice) + pkg("*").propagate(f"%gcc@:{str(spec.version)}", when=f"%{str(spec)}") + def _post_buildcache_install_hook(self): if not self.spec.satisfies("platform=linux"): return diff --git a/var/spack/repos/compiler_runtime.test/packages/gcc/package.py b/var/spack/repos/compiler_runtime.test/packages/gcc/package.py index ef28e411d3..6d8decc26b 100644 --- a/var/spack/repos/compiler_runtime.test/packages/gcc/package.py +++ b/var/spack/repos/compiler_runtime.test/packages/gcc/package.py @@ -30,3 +30,6 @@ def runtime_constraints(cls, *, spec, pkg): ) # The version of gcc-runtime is the same as the %gcc used to "compile" it pkg("gcc-runtime").requires(f"@={str(spec.version)}", when=f"%{str(spec)}") + + # If a node used %gcc@X.Y its dependencies must use gcc-runtime@:X.Y + pkg("*").propagate(f"%gcc@:{str(spec.version)}", when=f"%{str(spec)}")