Simplify setup_package in build environment (#26070)

* Remove redundant preserve environment code in build environment

* Remove fix for a bug in a module

See https://github.com/spack/spack/issues/3153#issuecomment-280460041,
this shouldn't be part of core spack.

* Don't module unload cray-libsci on all platforms

1 files changed, 22 insertions, 29 deletions

diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index 918ae02c50..68fa77a532 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -72,7 +72,6 @@ from spack.util.environment import (
     get_path,
     inspect_path,
     is_system_path,
-    preserve_environment,
     system_dirs,
     validate,
 )
@@ -148,6 +147,13 @@ class MakeExecutable(Executable):
         return super(MakeExecutable, self).__call__(*args, **kwargs)
 
 
+def _on_cray():
+    hostarch = arch.Arch(arch.platform(), 'default_os', 'default_target')
+    on_cray = str(hostarch.platform) == 'cray'
+    using_cnl = re.match(r'cnl\d+', str(hostarch.os))
+    return on_cray, using_cnl
+
+
 def clean_environment():
     # Stuff in here sanitizes the build environment to eliminate
     # anything the user has set that may interfere. We apply it immediately
@@ -182,9 +188,7 @@ def clean_environment():
     # interference with Spack dependencies.
     # CNL requires these variables to be set (or at least some of them,
     # depending on the CNL version).
-    hostarch = arch.Arch(arch.platform(), 'default_os', 'default_target')
-    on_cray = str(hostarch.platform) == 'cray'
-    using_cnl = re.match(r'cnl\d+', str(hostarch.os))
+    on_cray, using_cnl = _on_cray()
     if on_cray and not using_cnl:
         env.unset('CRAY_LD_LIBRARY_PATH')
         for varname in os.environ.keys():
@@ -796,34 +800,23 @@ def setup_package(pkg, dirty, context='build'):
         pkg.setup_run_environment(env)
         env.prepend_path('PATH', '.')
 
-    # Loading modules, in particular if they are meant to be used outside
-    # of Spack, can change environment variables that are relevant to the
-    # build of packages. To avoid a polluted environment, preserve the
-    # value of a few, selected, environment variables
-    # With the current ordering of environment modifications, this is strictly
-    # unnecessary. Modules affecting these variables will be overwritten anyway
-    with preserve_environment('CC', 'CXX', 'FC', 'F77'):
-        # All module loads that otherwise would belong in previous
-        # functions have to occur after the env object has its
-        # modifications applied. Otherwise the environment modifications
-        # could undo module changes, such as unsetting LD_LIBRARY_PATH
-        # after a module changes it.
-        if need_compiler:
-            for mod in pkg.compiler.modules:
-                # Fixes issue https://github.com/spack/spack/issues/3153
-                if os.environ.get("CRAY_CPU_TARGET") == "mic-knl":
-                    load_module("cce")
-                load_module(mod)
-
-        # kludge to handle cray libsci being automatically loaded by PrgEnv
-        # modules on cray platform. Module unload does no damage when
-        # unnecessary
+    # Load modules on an already clean environment, just before applying Spack's
+    # own environment modifications. This ensures Spack controls CC/CXX/... variables.
+    if need_compiler:
+        for mod in pkg.compiler.modules:
+            load_module(mod)
+
+    # kludge to handle cray libsci being automatically loaded by PrgEnv
+    # modules on cray platform. Module unload does no damage when
+    # unnecessary
+    on_cray, _ = _on_cray()
+    if on_cray:
         module('unload', 'cray-libsci')
 
-        if pkg.architecture.target.module_name:
-            load_module(pkg.architecture.target.module_name)
+    if pkg.architecture.target.module_name:
+        load_module(pkg.architecture.target.module_name)
 
-        load_external_modules(pkg)
+    load_external_modules(pkg)
 
     implicit_rpaths = pkg.compiler.implicit_rpaths()
     if implicit_rpaths: