diff options
| -rw-r--r-- | lib/spack/spack/build_environment.py | 37 | ||||
| -rw-r--r-- | lib/spack/spack/environment.py | 42 | ||||
| -rw-r--r-- | lib/spack/spack/test/build_environment.py | 27 | ||||
| -rw-r--r-- | lib/spack/spack/test/environment.py | 19 |
4 files changed, 110 insertions, 15 deletions
diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py index db484c12c4..4d272b51f3 100644 --- a/lib/spack/spack/build_environment.py +++ b/lib/spack/spack/build_environment.py @@ -71,6 +71,7 @@ import spack.main import spack.paths import spack.store from spack.environment import EnvironmentModifications, validate +from spack.environment import preserve_environment from spack.util.environment import env_flag, filter_system_paths, get_path from spack.util.executable import Executable from spack.util.module_cmd import load_module, get_path_from_module @@ -130,7 +131,7 @@ class MakeExecutable(Executable): def set_compiler_environment_variables(pkg, env): - assert(pkg.spec.concrete) + assert pkg.spec.concrete compiler = pkg.compiler # Set compiler variables used by CMake and autotools @@ -610,20 +611,26 @@ def setup_package(pkg, dirty): validate(spack_env, tty.warn) spack_env.apply_modifications() - # All module loads that otherwise would belong in previous functions - # have to occur after the spack_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. - 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) - - if pkg.architecture.target.module_name: - load_module(pkg.architecture.target.module_name) - - load_external_modules(pkg) + # 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 preserve_environment('CC', 'CXX', 'FC', 'F77'): + # All module loads that otherwise would belong in previous + # functions have to occur after the spack_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. + 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) + + if pkg.architecture.target.module_name: + load_module(pkg.architecture.target.module_name) + + load_external_modules(pkg) def fork(pkg, function, dirty, fake): diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py index 0db7706d9c..fa67cb658b 100644 --- a/lib/spack/spack/environment.py +++ b/lib/spack/spack/environment.py @@ -23,6 +23,7 @@ # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA ############################################################################## import collections +import contextlib import inspect import json import os @@ -30,6 +31,9 @@ import re import sys import os.path import subprocess + +import llnl.util.tty as tty + from llnl.util.lang import dedupe @@ -598,3 +602,41 @@ def inspect_path(root, inspections, exclude=None): env.prepend_path(variable, expected) return env + + +@contextlib.contextmanager +def preserve_environment(*variables): + """Ensures that the value of the environment variables passed as + arguments is the same before entering to the context manager and after + exiting it. + + Variables that are unset before entering the context manager will be + explicitly unset on exit. + + Args: + variables (list of str): list of environment variables to be preserved + """ + cache = {} + for var in variables: + # The environment variable to be preserved might not be there. + # In that case store None as a placeholder. + cache[var] = os.environ.get(var, None) + + yield + + for var in variables: + value = cache[var] + msg = '[PRESERVE_ENVIRONMENT]' + if value is not None: + # Print a debug statement if the value changed + if var not in os.environ: + msg += ' {0} was unset, will be reset to "{1}"' + tty.debug(msg.format(var, value)) + elif os.environ[var] != value: + msg += ' {0} was set to "{1}", will be reset to "{2}"' + tty.debug(msg.format(var, os.environ[var], value)) + os.environ[var] = value + elif var in os.environ: + msg += ' {0} was set to "{1}", will be unset' + tty.debug(msg.format(var, os.environ[var])) + del os.environ[var] diff --git a/lib/spack/spack/test/build_environment.py b/lib/spack/spack/test/build_environment.py index 504febffac..982e7822b3 100644 --- a/lib/spack/spack/test/build_environment.py +++ b/lib/spack/spack/test/build_environment.py @@ -25,6 +25,8 @@ import os import pytest +import spack.build_environment +import spack.spec from spack.paths import build_env_path from spack.build_environment import dso_suffix, _static_to_shared_library from spack.util.executable import Executable @@ -96,3 +98,28 @@ def test_static_to_shared_library(build_environment): assert output == expected[arch].format( static_lib, shared_lib, os.path.basename(shared_lib)) + + +@pytest.mark.regression('8345') +@pytest.mark.usefixtures('config', 'mock_packages') +def test_cc_not_changed_by_modules(monkeypatch): + + s = spack.spec.Spec('cmake') + s.concretize() + pkg = s.package + + def _set_wrong_cc(x): + os.environ['CC'] = 'NOT_THIS_PLEASE' + os.environ['ANOTHER_VAR'] = 'THIS_IS_SET' + + monkeypatch.setattr( + spack.build_environment, 'load_module', _set_wrong_cc + ) + monkeypatch.setattr( + pkg.compiler, 'modules', ['some_module'] + ) + + spack.build_environment.setup_package(pkg, False) + + assert os.environ['CC'] != 'NOT_THIS_PLEASE' + assert os.environ['ANOTHER_VAR'] == 'THIS_IS_SET' diff --git a/lib/spack/spack/test/environment.py b/lib/spack/spack/test/environment.py index bc1b6da3a8..f3272d0e78 100644 --- a/lib/spack/spack/test/environment.py +++ b/lib/spack/spack/test/environment.py @@ -306,3 +306,22 @@ def test_source_files(files_to_be_sourced): assert modifications['PATH_LIST'][1].value == '/path/fourth' assert isinstance(modifications['PATH_LIST'][2], PrependPath) assert modifications['PATH_LIST'][2].value == '/path/first' + + +@pytest.mark.regression('8345') +def test_preserve_environment(prepare_environment_for_tests): + # UNSET_ME is defined, and will be unset in the context manager, + # NOT_SET is not in the environment and will be set within the + # context manager, PATH_LIST is set and will be changed. + with environment.preserve_environment('UNSET_ME', 'NOT_SET', 'PATH_LIST'): + os.environ['NOT_SET'] = 'a' + assert os.environ['NOT_SET'] == 'a' + + del os.environ['UNSET_ME'] + assert 'UNSET_ME' not in os.environ + + os.environ['PATH_LIST'] = 'changed' + + assert 'NOT_SET' not in os.environ + assert os.environ['UNSET_ME'] == 'foo' + assert os.environ['PATH_LIST'] == '/path/second:/path/third' |