From 461eb944bdff103b8e347c272afb2bcbd31f9723 Mon Sep 17 00:00:00 2001 From: Harmen Stoppels Date: Mon, 6 Nov 2023 23:30:27 +0100 Subject: [PATCH] Don't let runtime env variables of compiler like deps leak into the build environment (#40916) * Test that setup_run_environment changes to CC/CXX/FC/F77 are dropped in build env * compilers set in run env shouldn't impact build Adds `drop` to EnvironmentModifications courtesy of @haampie, and uses it to clear modifications of CC, CXX, F77 and FC made by `setup_{,dependent_}run_environment` routines when producing an environment in BUILD context. * comment / style * comment --------- Co-authored-by: Tom Scogland --- lib/spack/spack/build_environment.py | 11 ++++++++-- lib/spack/spack/test/build_environment.py | 15 ++++++++++++++ lib/spack/spack/util/environment.py | 8 ++++++++ .../build-env-compiler-var-a/package.py | 14 +++++++++++++ .../build-env-compiler-var-b/package.py | 20 +++++++++++++++++++ 5 files changed, 66 insertions(+), 2 deletions(-) create mode 100644 var/spack/repos/builtin.mock/packages/build-env-compiler-var-a/package.py create mode 100644 var/spack/repos/builtin.mock/packages/build-env-compiler-var-b/package.py diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py index 3f6830ad33..4c4eca6567 100644 --- a/lib/spack/spack/build_environment.py +++ b/lib/spack/spack/build_environment.py @@ -1016,10 +1016,17 @@ def get_env_modifications(self) -> EnvironmentModifications: self._make_runnable(dspec, env) if self.should_setup_run_env & flag: + run_env_mods = EnvironmentModifications() for spec in dspec.dependents(deptype=dt.LINK | dt.RUN): if id(spec) in self.nodes_in_subdag: - pkg.setup_dependent_run_environment(env, spec) - pkg.setup_run_environment(env) + pkg.setup_dependent_run_environment(run_env_mods, spec) + pkg.setup_run_environment(run_env_mods) + if self.context == Context.BUILD: + # Don't let the runtime environment of comiler like dependencies leak into the + # build env + run_env_mods.drop("CC", "CXX", "F77", "FC") + env.extend(run_env_mods) + return env def _make_buildtime_detectable(self, dep: spack.spec.Spec, env: EnvironmentModifications): diff --git a/lib/spack/spack/test/build_environment.py b/lib/spack/spack/test/build_environment.py index f2bf740272..cbccbc429e 100644 --- a/lib/spack/spack/test/build_environment.py +++ b/lib/spack/spack/test/build_environment.py @@ -652,3 +652,18 @@ def test_monkey_patching_works_across_virtual(default_mock_concretization): s["mpich"].foo = "foo" assert s["mpich"].foo == "foo" assert s["mpi"].foo == "foo" + + +def test_clear_compiler_related_runtime_variables_of_build_deps(default_mock_concretization): + """Verify that Spack drops CC, CXX, FC and F77 from the dependencies related build environment + variable changes if they are set in setup_run_environment. Spack manages those variables + elsewhere.""" + s = default_mock_concretization("build-env-compiler-var-a") + ctx = spack.build_environment.SetupContext(s, context=Context.BUILD) + result = {} + ctx.get_env_modifications().apply_modifications(result) + assert "CC" not in result + assert "CXX" not in result + assert "FC" not in result + assert "F77" not in result + assert result["ANOTHER_VAR"] == "this-should-be-present" diff --git a/lib/spack/spack/util/environment.py b/lib/spack/spack/util/environment.py index 246df65cb8..d4c352b993 100644 --- a/lib/spack/spack/util/environment.py +++ b/lib/spack/spack/util/environment.py @@ -596,6 +596,14 @@ def group_by_name(self) -> Dict[str, ModificationList]: modifications[item.name].append(item) return modifications + def drop(self, *name) -> bool: + """Drop all modifications to the variable with the given name.""" + old_mods = self.env_modifications + new_mods = [x for x in self.env_modifications if x.name not in name] + self.env_modifications = new_mods + + return len(old_mods) != len(new_mods) + def is_unset(self, variable_name: str) -> bool: """Returns True if the last modification to a variable is to unset it, False otherwise.""" modifications = self.group_by_name() diff --git a/var/spack/repos/builtin.mock/packages/build-env-compiler-var-a/package.py b/var/spack/repos/builtin.mock/packages/build-env-compiler-var-a/package.py new file mode 100644 index 0000000000..ea6f0f34e8 --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/build-env-compiler-var-a/package.py @@ -0,0 +1,14 @@ +# Copyright 2013-2023 Lawrence Livermore National Security, LLC and other +# Spack Project Developers. See the top-level COPYRIGHT file for details. +# +# SPDX-License-Identifier: (Apache-2.0 OR MIT) + +from spack.package import * + + +class BuildEnvCompilerVarA(Package): + """Package with runtime variable that should be dropped in the parent's build environment.""" + + url = "https://www.example.com" + version("1.0", md5="0123456789abcdef0123456789abcdef") + depends_on("build-env-compiler-var-b", type="build") diff --git a/var/spack/repos/builtin.mock/packages/build-env-compiler-var-b/package.py b/var/spack/repos/builtin.mock/packages/build-env-compiler-var-b/package.py new file mode 100644 index 0000000000..7905869b34 --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/build-env-compiler-var-b/package.py @@ -0,0 +1,20 @@ +# Copyright 2013-2023 Lawrence Livermore National Security, LLC and other +# Spack Project Developers. See the top-level COPYRIGHT file for details. +# +# SPDX-License-Identifier: (Apache-2.0 OR MIT) + +from spack.package import * + + +class BuildEnvCompilerVarB(Package): + """Package with runtime variable that should be dropped in the parent's build environment.""" + + url = "https://www.example.com" + version("1.0", md5="0123456789abcdef0123456789abcdef") + + def setup_run_environment(self, env): + env.set("CC", "this-should-be-dropped") + env.set("CXX", "this-should-be-dropped") + env.set("FC", "this-should-be-dropped") + env.set("F77", "this-should-be-dropped") + env.set("ANOTHER_VAR", "this-should-be-present")