Only install env modifications in <prefix>/.spack (#24081)
- Do not store the full list of environment variables in <prefix>/.spack/spack-build-env.txt because it may contain user secrets. - Only store environment variable modifications upon installation. - Variables like PATH may still contain user and system paths to make spack-build-env.txt sourceable. Variables containing paths are modified through prepending/appending, and if we don't apply these to the current environment variable, we end up with statements like `export PATH=/path/to/spack/bin` with system paths missing, meaning no system binaries are in the path, which is a bad user experience. - Do write the full environment to spack-build-env.txt in the staging dir, but ensure it is readonly for the current user, to make it a bit safer on shared systems.
This commit is contained in:
Harmen Stoppels
GitHub
parent
c0c9ab113e
commit
89220bc0e1
@ -10,6 +10,8 @@
|
||||
|
||||
import os
|
||||
|
||||
import llnl.util.tty as tty
|
||||
|
||||
from spack.util.environment import EnvironmentModifications
|
||||
|
||||
from .analyzer_base import AnalyzerBase
|
||||
@ -43,6 +45,7 @@ def _read_environment_file(self, filename):
|
||||
to remove path prefixes specific to user systems.
|
||||
"""
|
||||
if not os.path.exists(filename):
|
||||
tty.warn("No environment file available")
|
||||
return
|
||||
|
||||
mods = EnvironmentModifications.from_sourcing_file(filename)
|
||||
|
||||
@ -232,7 +232,7 @@ def clean_environment():
|
||||
if '/macports/' in p:
|
||||
env.remove_path('PATH', p)
|
||||
|
||||
env.apply_modifications()
|
||||
return env
|
||||
|
||||
|
||||
def set_compiler_environment_variables(pkg, env):
|
||||
@ -765,43 +765,46 @@ def setup_package(pkg, dirty, context='build'):
|
||||
|
||||
set_module_variables_for_package(pkg)
|
||||
|
||||
env = EnvironmentModifications()
|
||||
|
||||
if not dirty:
|
||||
clean_environment()
|
||||
# Keep track of env changes from packages separately, since we want to
|
||||
# issue warnings when packages make "suspicious" modifications.
|
||||
env_base = EnvironmentModifications() if dirty else clean_environment()
|
||||
env_mods = EnvironmentModifications()
|
||||
|
||||
# setup compilers for build contexts
|
||||
need_compiler = context == 'build' or (context == 'test' and
|
||||
pkg.test_requires_compiler)
|
||||
if need_compiler:
|
||||
set_compiler_environment_variables(pkg, env)
|
||||
set_wrapper_variables(pkg, env)
|
||||
set_compiler_environment_variables(pkg, env_mods)
|
||||
set_wrapper_variables(pkg, env_mods)
|
||||
|
||||
env.extend(modifications_from_dependencies(
|
||||
env_mods.extend(modifications_from_dependencies(
|
||||
pkg.spec, context, custom_mods_only=False))
|
||||
|
||||
# architecture specific setup
|
||||
platform = spack.platforms.by_name(pkg.spec.architecture.platform)
|
||||
target = platform.target(pkg.spec.architecture.target)
|
||||
platform.setup_platform_environment(pkg, env)
|
||||
platform.setup_platform_environment(pkg, env_mods)
|
||||
|
||||
if context == 'build':
|
||||
pkg.setup_build_environment(env)
|
||||
pkg.setup_build_environment(env_mods)
|
||||
|
||||
if (not dirty) and (not env.is_unset('CPATH')):
|
||||
if (not dirty) and (not env_mods.is_unset('CPATH')):
|
||||
tty.debug("A dependency has updated CPATH, this may lead pkg-"
|
||||
"config to assume that the package is part of the system"
|
||||
" includes and omit it when invoked with '--cflags'.")
|
||||
elif context == 'test':
|
||||
env.extend(
|
||||
env_mods.extend(
|
||||
inspect_path(
|
||||
pkg.spec.prefix,
|
||||
spack.user_environment.prefix_inspections(pkg.spec.platform),
|
||||
exclude=is_system_path
|
||||
)
|
||||
)
|
||||
pkg.setup_run_environment(env)
|
||||
env.prepend_path('PATH', '.')
|
||||
pkg.setup_run_environment(env_mods)
|
||||
env_mods.prepend_path('PATH', '.')
|
||||
|
||||
# First apply the clean environment changes
|
||||
env_base.apply_modifications()
|
||||
|
||||
# Load modules on an already clean environment, just before applying Spack's
|
||||
# own environment modifications. This ensures Spack controls CC/CXX/... variables.
|
||||
@ -823,12 +826,16 @@ def setup_package(pkg, dirty, context='build'):
|
||||
|
||||
implicit_rpaths = pkg.compiler.implicit_rpaths()
|
||||
if implicit_rpaths:
|
||||
env.set('SPACK_COMPILER_IMPLICIT_RPATHS',
|
||||
':'.join(implicit_rpaths))
|
||||
env_mods.set('SPACK_COMPILER_IMPLICIT_RPATHS',
|
||||
':'.join(implicit_rpaths))
|
||||
|
||||
# Make sure nothing's strange about the Spack environment.
|
||||
validate(env, tty.warn)
|
||||
env.apply_modifications()
|
||||
validate(env_mods, tty.warn)
|
||||
env_mods.apply_modifications()
|
||||
|
||||
# Return all env modifications we controlled (excluding module related ones)
|
||||
env_base.extend(env_mods)
|
||||
return env_base
|
||||
|
||||
|
||||
def _make_runnable(pkg, env):
|
||||
@ -1016,8 +1023,8 @@ def _setup_pkg_and_run(serialized_pkg, function, kwargs, child_pipe,
|
||||
|
||||
if not kwargs.get('fake', False):
|
||||
kwargs['unmodified_env'] = os.environ.copy()
|
||||
setup_package(pkg, dirty=kwargs.get('dirty', False),
|
||||
context=context)
|
||||
kwargs['env_modifications'] = setup_package(
|
||||
pkg, dirty=kwargs.get('dirty', False), context=context)
|
||||
return_value = function(pkg, kwargs)
|
||||
child_pipe.send(return_value)
|
||||
|
||||
|
||||
@ -55,7 +55,7 @@
|
||||
import spack.repo
|
||||
import spack.store
|
||||
import spack.util.executable
|
||||
from spack.util.environment import dump_environment
|
||||
from spack.util.environment import EnvironmentModifications, dump_environment
|
||||
from spack.util.executable import which
|
||||
from spack.util.timer import Timer
|
||||
|
||||
@ -556,8 +556,8 @@ def log(pkg):
|
||||
log_file = os.path.join(os.path.dirname(packages_dir), log_file)
|
||||
fs.install(phase_log, log_file)
|
||||
|
||||
# Archive the environment used for the build
|
||||
fs.install(pkg.env_path, pkg.install_env_path)
|
||||
# Archive the environment modifications for the build.
|
||||
fs.install(pkg.env_mods_path, pkg.install_env_path)
|
||||
|
||||
if os.path.exists(pkg.configure_args_path):
|
||||
# Archive the args used for the build
|
||||
@ -1730,6 +1730,10 @@ def __init__(self, pkg, install_args):
|
||||
# env before starting installation
|
||||
self.unmodified_env = install_args.get('unmodified_env', {})
|
||||
|
||||
# env modifications by Spack
|
||||
self.env_mods = install_args.get(
|
||||
'env_modifications', EnvironmentModifications())
|
||||
|
||||
# timer for build phases
|
||||
self.timer = Timer()
|
||||
|
||||
@ -1819,6 +1823,15 @@ def _real_install(self):
|
||||
# Save the build environment in a file before building.
|
||||
dump_environment(pkg.env_path)
|
||||
|
||||
# Save just the changes to the environment. This file can be
|
||||
# safely installed, since it does not contain secret variables.
|
||||
with open(pkg.env_mods_path, 'w') as env_mods_file:
|
||||
mods = self.env_mods.shell_modifications(
|
||||
explicit=True,
|
||||
env=self.unmodified_env
|
||||
)
|
||||
env_mods_file.write(mods)
|
||||
|
||||
for attr in ('configure_args', 'cmake_args'):
|
||||
try:
|
||||
configure_args = getattr(pkg, attr)()
|
||||
|
||||
@ -73,6 +73,9 @@
|
||||
# Filename for the Spack build/install environment file.
|
||||
_spack_build_envfile = 'spack-build-env.txt'
|
||||
|
||||
# Filename for the Spack build/install environment modifications file.
|
||||
_spack_build_envmodsfile = 'spack-build-env-mods.txt'
|
||||
|
||||
# Filename of json with total build and phase times (seconds)
|
||||
_spack_times_log = 'install_times.json'
|
||||
|
||||
@ -1041,6 +1044,14 @@ def env_path(self):
|
||||
else:
|
||||
return os.path.join(self.stage.path, _spack_build_envfile)
|
||||
|
||||
@property
|
||||
def env_mods_path(self):
|
||||
"""
|
||||
Return the build environment modifications file path associated with
|
||||
staging.
|
||||
"""
|
||||
return os.path.join(self.stage.path, _spack_build_envmodsfile)
|
||||
|
||||
@property
|
||||
def metadata_dir(self):
|
||||
"""Return the install metadata directory."""
|
||||
|
||||
@ -175,6 +175,15 @@ def test_install_with_source(
|
||||
os.path.join(src, 'configure'))
|
||||
|
||||
|
||||
def test_install_env_variables(
|
||||
mock_packages, mock_archive, mock_fetch, config, install_mockery
|
||||
):
|
||||
spec = Spec('libdwarf')
|
||||
spec.concretize()
|
||||
install('libdwarf')
|
||||
assert os.path.isfile(spec.package.install_env_path)
|
||||
|
||||
|
||||
@pytest.mark.disable_clean_stage_check
|
||||
def test_show_log_on_error(mock_packages, mock_archive, mock_fetch,
|
||||
config, install_mockery, capfd):
|
||||
|
||||
@ -543,6 +543,7 @@ def _install(src, dest):
|
||||
with fs.working_dir(log_dir):
|
||||
fs.touch(log_path)
|
||||
fs.touch(spec.package.env_path)
|
||||
fs.touch(spec.package.env_mods_path)
|
||||
fs.touch(spec.package.configure_args_path)
|
||||
|
||||
install_path = os.path.dirname(spec.package.install_log_path)
|
||||
|
||||
@ -128,7 +128,8 @@ def dump_environment(path, environment=None):
|
||||
use_env = environment or os.environ
|
||||
hidden_vars = set(['PS1', 'PWD', 'OLDPWD', 'TERM_SESSION_ID'])
|
||||
|
||||
with open(path, 'w') as env_file:
|
||||
fd = os.open(path, os.O_WRONLY | os.O_CREAT, 0o600)
|
||||
with os.fdopen(fd, 'w') as env_file:
|
||||
for var, val in sorted(use_env.items()):
|
||||
env_file.write(''.join(['#' if var in hidden_vars else '',
|
||||
env_var_to_source_line(var, val),
|
||||
@ -596,21 +597,26 @@ def apply_modifications(self, env=None):
|
||||
for x in actions:
|
||||
x.execute(env)
|
||||
|
||||
def shell_modifications(self, shell='sh'):
|
||||
def shell_modifications(self, shell='sh', explicit=False, env=None):
|
||||
"""Return shell code to apply the modifications and clears the list."""
|
||||
modifications = self.group_by_name()
|
||||
new_env = os.environ.copy()
|
||||
|
||||
if env is None:
|
||||
env = os.environ
|
||||
|
||||
new_env = env.copy()
|
||||
|
||||
for name, actions in sorted(modifications.items()):
|
||||
for x in actions:
|
||||
x.execute(new_env)
|
||||
|
||||
cmds = ''
|
||||
|
||||
for name in set(new_env) | set(os.environ):
|
||||
for name in sorted(set(modifications)):
|
||||
new = new_env.get(name, None)
|
||||
old = os.environ.get(name, None)
|
||||
if new != old:
|
||||
old = env.get(name, None)
|
||||
if explicit or new != old:
|
||||
if new is None:
|
||||
cmds += _shell_unset_strings[shell].format(name)
|
||||
else:
|
||||
@ -989,7 +995,7 @@ def _source_single_file(file_and_args, environment):
|
||||
# go with sys.executable. Below we just need a working
|
||||
# Python interpreter, not necessarily sys.executable.
|
||||
python_cmd = executable.which('python3', 'python', 'python2')
|
||||
python_cmd = python_cmd.name if python_cmd else sys.executable
|
||||
python_cmd = python_cmd.path if python_cmd else sys.executable
|
||||
|
||||
dump_cmd = 'import os, json; print(json.dumps(dict(os.environ)))'
|
||||
dump_environment = python_cmd + ' -E -c "{0}"'.format(dump_cmd)
|
||||
|
||||
Loading…
Reference in New Issue
Block a user