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 <scogland1@llnl.gov>

5 files changed, 66 insertions, 2 deletions

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 @@ class SetupContext:
                 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 @@ class EnvironmentModifications:
             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")