diff --git a/lib/spack/spack/modules/lmod.py b/lib/spack/spack/modules/lmod.py index f8970cd1d2a..0f9475a14b1 100644 --- a/lib/spack/spack/modules/lmod.py +++ b/lib/spack/spack/modules/lmod.py @@ -6,6 +6,7 @@ import itertools import os import pathlib +import warnings from typing import Dict, List, Optional, Tuple import llnl.util.filesystem as fs @@ -102,15 +103,19 @@ class LmodConfiguration(BaseConfiguration): def __init__(self, spec: spack.spec.Spec, module_set_name: str, explicit: bool) -> None: super().__init__(spec, module_set_name, explicit) - # FIXME (compiler as nodes): make this a bit more robust candidates = collections.defaultdict(list) for node in spec.traverse(deptype=("link", "run")): candidates["c"].extend(node.dependencies(virtuals=("c",))) candidates["cxx"].extend(node.dependencies(virtuals=("c",))) - # FIXME (compiler as nodes): decide what to do when we have more than one C compiler - if candidates["c"] and len(set(candidates["c"])) == 1: + if candidates["c"]: self.compiler = candidates["c"][0] + if len(set(candidates["c"])) > 1: + warnings.warn( + f"{spec.short_spec} uses more than one compiler, and might not fit the " + f"LMod hierarchy. Using {self.compiler.short_spec} as the LMod compiler." + ) + elif not candidates["c"]: self.compiler = None diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp index 3ee427c35c7..b9439724c72 100644 --- a/lib/spack/spack/solver/concretize.lp +++ b/lib/spack/spack/solver/concretize.lp @@ -1246,10 +1246,6 @@ error(100, "Cannot propagate the variant '{0}' from the package: {1} because pac % 1. The same flag type is not set on this node % 2. This node has the same compilers as the propagation source -compiler_penalty(PackageNode, C-1) :- - C = #count { CompilerNode : node_compiler(PackageNode, CompilerNode) }, - node_compiler(PackageNode, _). - node_compiler(node(X, Package), node(Y, Compiler)) :- attr("virtual_on_edge", node(X, Package), node(Y, Compiler), Language), attr("version", node(Y, Compiler), Version), @@ -1295,6 +1291,12 @@ compiler_used_as_a_library(node(X, Child), Hash) :- compiler_package(Child), % Used to restrict grounding for this rule attr("depends_on", _, node(X, Child), "link"). +% If a compiler is used for C on a package, it must provide C++ too, if need be, and vice-versa +:- attr("virtual_on_edge", PackageNode, CompilerNode1, "c"), + attr("virtual_on_edge", PackageNode, CompilerNode2, "cxx"), + CompilerNode1 != CompilerNode2. + + %----------------------------------------------------------------------------- % Runtimes %----------------------------------------------------------------------------- @@ -1660,9 +1662,9 @@ opt_criterion(60, "preferred providers for roots"). #minimize{ 0@260: #true }. #minimize{ 0@60: #true }. #minimize{ - Weight@60+Priority,ProviderNode,Virtual - : provider_weight(ProviderNode, Virtual, Weight), - attr("root", ProviderNode), + Weight@60+Priority,ProviderNode,X,Virtual + : provider_weight(ProviderNode, node(X, Virtual), Weight), + attr("root", ProviderNode), not language(Virtual), build_priority(ProviderNode, Priority) }. @@ -1687,36 +1689,50 @@ opt_criterion(50, "number of non-default variants (non-roots)"). build_priority(PackageNode, Priority) }. -% Minimize the ids of the providers, i.e. use as much as -% possible the first providers -opt_criterion(48, "number of duplicate virtuals needed"). +% Minimize the weights of the providers, i.e. use as much as +% possible the most preferred providers +opt_criterion(48, "preferred providers (non-roots)"). #minimize{ 0@248: #true }. #minimize{ 0@48: #true }. #minimize{ - Weight@48+Priority,ProviderNode,Virtual - : provider(ProviderNode, node(Weight, Virtual)), + Weight@48+Priority,ProviderNode,X,Virtual + : provider_weight(ProviderNode, node(X, Virtual), Weight), + not attr("root", ProviderNode), not language(Virtual), build_priority(ProviderNode, Priority) }. % Minimize the number of compilers used on nodes -opt_criterion(49, "minimize the number of compilers used on the same node"). -#minimize{ 0@249: #true }. -#minimize{ 0@49: #true }. + +compiler_penalty(PackageNode, C-1) :- + C = #count { CompilerNode : node_compiler(PackageNode, CompilerNode) }, + node_compiler(PackageNode, _). + +opt_criterion(46, "number of compilers used on the same node"). +#minimize{ 0@246: #true }. +#minimize{ 0@46: #true }. #minimize{ - Penalty@49+Priority,PackageNode - : compiler_penalty(PackageNode, Penalty), - build_priority(PackageNode, Priority) + Penalty@46+Priority,PackageNode + : compiler_penalty(PackageNode, Penalty), build_priority(PackageNode, Priority) }. -% Minimize the weights of the providers, i.e. use as much as -% possible the most preferred providers -opt_criterion(45, "preferred providers (non-roots)"). +% Minimize the ids of the providers, i.e. use as much as +% possible the first providers +opt_criterion(45, "number of duplicate virtuals needed"). #minimize{ 0@245: #true }. #minimize{ 0@45: #true }. #minimize{ Weight@45+Priority,ProviderNode,Virtual - : provider_weight(ProviderNode, Virtual, Weight), - not attr("root", ProviderNode), + : provider(ProviderNode, node(Weight, Virtual)), + build_priority(ProviderNode, Priority) +}. + +opt_criterion(40, "preferred compilers"). +#minimize{ 0@240: #true }. +#minimize{ 0@40: #true }. +#minimize{ + Weight@40+Priority,ProviderNode,X,Virtual + : provider_weight(ProviderNode, node(X, Virtual), Weight), + language(Virtual), build_priority(ProviderNode, Priority) }. diff --git a/lib/spack/spack/test/concretization/core.py b/lib/spack/spack/test/concretization/core.py index c2246131800..e88ba082093 100644 --- a/lib/spack/spack/test/concretization/core.py +++ b/lib/spack/spack/test/concretization/core.py @@ -741,12 +741,51 @@ def test_virtual_is_fully_expanded_for_mpileaks(self): assert all(not d.dependencies(name="mpi") for d in spec.traverse()) assert all(x in spec for x in ("zmpi", "mpi")) - @pytest.mark.parametrize("compiler_str", ["%clang", "%gcc", "%gcc@10.2.1", "%clang@:15.0.0"]) - def test_compiler_inheritance(self, compiler_str): - spec_str = f"mpileaks {compiler_str}" + @pytest.mark.parametrize( + "spec_str,expected,not_expected", + [ + # clang only provides C, and C++ compilers, while gcc has also fortran + # + # If we ask mpileaks%clang, then %gcc must be used for fortran, and since + # %gcc is preferred to clang in config, it will be used for most nodes + ( + "mpileaks %clang", + {"mpileaks": "%clang", "libdwarf": "%gcc", "libelf": "%gcc"}, + {"libdwarf": "%clang", "libelf": "%clang"}, + ), + ( + "mpileaks %clang@:15.0.0", + {"mpileaks": "%clang", "libdwarf": "%gcc", "libelf": "%gcc"}, + {"libdwarf": "%clang", "libelf": "%clang"}, + ), + ( + "mpileaks %gcc", + {"mpileaks": "%gcc", "libdwarf": "%gcc", "libelf": "%gcc"}, + {"mpileaks": "%clang", "libdwarf": "%clang", "libelf": "%clang"}, + ), + ( + "mpileaks %gcc@10.2.1", + {"mpileaks": "%gcc", "libdwarf": "%gcc", "libelf": "%gcc"}, + {"mpileaks": "%clang", "libdwarf": "%clang", "libelf": "%clang"}, + ), + # dyninst doesn't require fortran, so %clang is propagated + ( + "dyninst %clang", + {"dyninst": "%clang", "libdwarf": "%clang", "libelf": "%clang"}, + {"libdwarf": "%gcc", "libelf": "%gcc"}, + ), + ], + ) + def test_compiler_inheritance(self, spec_str, expected, not_expected): + """Spack tries to propagate compilers as much as possible, but prefers using a single + toolchain on a node, rather than mixing them. + """ spec = spack.concretize.concretize_one(spec_str) - assert spec["libdwarf"].satisfies(compiler_str) - assert spec["libelf"].satisfies(compiler_str) + for name, constraint in expected.items(): + assert spec[name].satisfies(constraint) + + for name, constraint in not_expected.items(): + assert not spec[name].satisfies(constraint) def test_external_package(self): """Tests that an external is preferred, if present, and that it does not diff --git a/lib/spack/spack/test/env.py b/lib/spack/spack/test/env.py index 84c6046ae77..d56cd9e1fb1 100644 --- a/lib/spack/spack/test/env.py +++ b/lib/spack/spack/test/env.py @@ -921,3 +921,31 @@ def test_environment_from_name_or_dir(mock_packages, mutable_mock_env_path, tmp_ with pytest.raises(ev.SpackEnvironmentError, match="no such environment"): _ = ev.environment_from_name_or_dir("fake-env") + + +def test_using_multiple_compilers_on_a_node_is_discouraged( + tmp_path, mutable_config, mock_packages +): + """Tests that when we specify % Spack tries to use that compiler for all the + languages needed by that node. + """ + manifest = tmp_path / "spack.yaml" + manifest.write_text( + """\ +spack: + specs: + - mpileaks%clang ^mpich%gcc + concretizer: + unify: true +""" + ) + with ev.Environment(tmp_path) as e: + e.concretize() + mpileaks = e.concrete_roots()[0] + + assert not mpileaks.satisfies("%gcc") and mpileaks.satisfies("%clang") + assert len(mpileaks.dependencies(virtuals=("c", "cxx"))) == 1 + + mpich = mpileaks["mpich"] + assert mpich.satisfies("%gcc") and not mpich.satisfies("%clang") + assert len(mpich.dependencies(virtuals=("c", "cxx"))) == 1