EnvironmentModifications: allow disabling stack tracing (#26706)

Currently Spack keeps track of the origin in the code of any
modification to the environment variables. This is very slow 
and enabled unconditionally even in code paths where the 
origin of the modification is never queried.

The only place where we inspect the origins of environment 
modifications is before we start a build, If there's an override 
of the type `e.set(...)` after incremental changes like 
`e.append_path(..)`, which is a "suspicious" change.

This is very rare though.

If an override like this ever happens, it might mean a package is
broken. If that leads to build errors, we can just ask the user to run
`spack -d install ...` and check the warnings issued by Spack to find
the origins of the problem.
This commit is contained in:
Harmen Stoppels 2021-10-15 10:00:44 +02:00 committed by GitHub
parent 842e56efb8
commit e0fbf09239
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 29 additions and 14 deletions

View File

@ -43,6 +43,7 @@
import spack.spec import spack.spec
import spack.store import spack.store
import spack.util.debug import spack.util.debug
import spack.util.environment
import spack.util.executable as exe import spack.util.executable as exe
import spack.util.path import spack.util.path
from spack.error import SpackError from spack.error import SpackError
@ -478,6 +479,7 @@ def setup_main_options(args):
spack.error.debug = True spack.error.debug = True
spack.util.debug.register_interrupt_handler() spack.util.debug.register_interrupt_handler()
spack.config.set('config:debug', True, scope='command_line') spack.config.set('config:debug', True, scope='command_line')
spack.util.environment.tracing_enabled = True
if args.timestamp: if args.timestamp:
tty.set_timestamp(True) tty.set_timestamp(True)

View File

@ -21,6 +21,7 @@
import llnl.util.tty as tty import llnl.util.tty as tty
from llnl.util.lang import dedupe from llnl.util.lang import dedupe
import spack.config
import spack.platforms import spack.platforms
import spack.spec import spack.spec
import spack.util.executable as executable import spack.util.executable as executable
@ -45,6 +46,9 @@
} }
tracing_enabled = False
def is_system_path(path): def is_system_path(path):
"""Predicate that given a path returns True if it is a system path, """Predicate that given a path returns True if it is a system path,
False otherwise. False otherwise.
@ -365,14 +369,17 @@ class EnvironmentModifications(object):
* 'context' : line of code that issued the request that failed * 'context' : line of code that issued the request that failed
""" """
def __init__(self, other=None): def __init__(self, other=None, traced=None):
"""Initializes a new instance, copying commands from 'other' """Initializes a new instance, copying commands from 'other'
if it is not None. if it is not None.
Args: Args:
other (EnvironmentModifications): list of environment modifications other (EnvironmentModifications): list of environment modifications
to be extended (optional) to be extended (optional)
traced (bool): enable or disable stack trace inspection to log the origin
of the environment modifications.
""" """
self.traced = tracing_enabled if traced is None else bool(traced)
self.env_modifications = [] self.env_modifications = []
if other is not None: if other is not None:
self.extend(other) self.extend(other)
@ -393,7 +400,12 @@ def _check_other(other):
raise TypeError( raise TypeError(
'other must be an instance of EnvironmentModifications') 'other must be an instance of EnvironmentModifications')
def _get_outside_caller_attributes(self): def _maybe_trace(self, kwargs):
"""Provide the modification with stack trace info so that we can track its
origin to find issues in packages. This is very slow and expensive."""
if not self.traced:
return
stack = inspect.stack() stack = inspect.stack()
try: try:
_, filename, lineno, _, context, index = stack[2] _, filename, lineno, _, context, index = stack[2]
@ -402,8 +414,7 @@ def _get_outside_caller_attributes(self):
filename = 'unknown file' filename = 'unknown file'
lineno = 'unknown line' lineno = 'unknown line'
context = 'unknown context' context = 'unknown context'
args = {'filename': filename, 'lineno': lineno, 'context': context} kwargs.update({'filename': filename, 'lineno': lineno, 'context': context})
return args
def set(self, name, value, **kwargs): def set(self, name, value, **kwargs):
"""Stores a request to set an environment variable. """Stores a request to set an environment variable.
@ -412,7 +423,7 @@ def set(self, name, value, **kwargs):
name: name of the environment variable to be set name: name of the environment variable to be set
value: value of the environment variable value: value of the environment variable
""" """
kwargs.update(self._get_outside_caller_attributes()) self._maybe_trace(kwargs)
item = SetEnv(name, value, **kwargs) item = SetEnv(name, value, **kwargs)
self.env_modifications.append(item) self.env_modifications.append(item)
@ -425,7 +436,7 @@ def append_flags(self, name, value, sep=' ', **kwargs):
value: value to append to the environment variable value: value to append to the environment variable
Appends with spaces separating different additions to the variable Appends with spaces separating different additions to the variable
""" """
kwargs.update(self._get_outside_caller_attributes()) self._maybe_trace(kwargs)
kwargs.update({'separator': sep}) kwargs.update({'separator': sep})
item = AppendFlagsEnv(name, value, **kwargs) item = AppendFlagsEnv(name, value, **kwargs)
self.env_modifications.append(item) self.env_modifications.append(item)
@ -436,7 +447,7 @@ def unset(self, name, **kwargs):
Args: Args:
name: name of the environment variable to be unset name: name of the environment variable to be unset
""" """
kwargs.update(self._get_outside_caller_attributes()) self._maybe_trace(kwargs)
item = UnsetEnv(name, **kwargs) item = UnsetEnv(name, **kwargs)
self.env_modifications.append(item) self.env_modifications.append(item)
@ -450,7 +461,7 @@ def remove_flags(self, name, value, sep=' ', **kwargs):
value: value to remove to the environment variable value: value to remove to the environment variable
sep: separator to assume for environment variable sep: separator to assume for environment variable
""" """
kwargs.update(self._get_outside_caller_attributes()) self._maybe_trace(kwargs)
kwargs.update({'separator': sep}) kwargs.update({'separator': sep})
item = RemoveFlagsEnv(name, value, **kwargs) item = RemoveFlagsEnv(name, value, **kwargs)
self.env_modifications.append(item) self.env_modifications.append(item)
@ -462,7 +473,7 @@ def set_path(self, name, elements, **kwargs):
name: name o the environment variable to be set. name: name o the environment variable to be set.
elements: elements of the path to set. elements: elements of the path to set.
""" """
kwargs.update(self._get_outside_caller_attributes()) self._maybe_trace(kwargs)
item = SetPath(name, elements, **kwargs) item = SetPath(name, elements, **kwargs)
self.env_modifications.append(item) self.env_modifications.append(item)
@ -473,7 +484,7 @@ def append_path(self, name, path, **kwargs):
name: name of the path list in the environment name: name of the path list in the environment
path: path to be appended path: path to be appended
""" """
kwargs.update(self._get_outside_caller_attributes()) self._maybe_trace(kwargs)
item = AppendPath(name, path, **kwargs) item = AppendPath(name, path, **kwargs)
self.env_modifications.append(item) self.env_modifications.append(item)
@ -484,7 +495,7 @@ def prepend_path(self, name, path, **kwargs):
name: name of the path list in the environment name: name of the path list in the environment
path: path to be pre-pended path: path to be pre-pended
""" """
kwargs.update(self._get_outside_caller_attributes()) self._maybe_trace(kwargs)
item = PrependPath(name, path, **kwargs) item = PrependPath(name, path, **kwargs)
self.env_modifications.append(item) self.env_modifications.append(item)
@ -495,7 +506,7 @@ def remove_path(self, name, path, **kwargs):
name: name of the path list in the environment name: name of the path list in the environment
path: path to be removed path: path to be removed
""" """
kwargs.update(self._get_outside_caller_attributes()) self._maybe_trace(kwargs)
item = RemovePath(name, path, **kwargs) item = RemovePath(name, path, **kwargs)
self.env_modifications.append(item) self.env_modifications.append(item)
@ -506,7 +517,7 @@ def deprioritize_system_paths(self, name, **kwargs):
Args: Args:
name: name of the path list in the environment. name: name of the path list in the environment.
""" """
kwargs.update(self._get_outside_caller_attributes()) self._maybe_trace(kwargs)
item = DeprioritizeSystemPaths(name, **kwargs) item = DeprioritizeSystemPaths(name, **kwargs)
self.env_modifications.append(item) self.env_modifications.append(item)
@ -517,7 +528,7 @@ def prune_duplicate_paths(self, name, **kwargs):
Args: Args:
name: name of the path list in the environment. name: name of the path list in the environment.
""" """
kwargs.update(self._get_outside_caller_attributes()) self._maybe_trace(kwargs)
item = PruneDuplicatePaths(name, **kwargs) item = PruneDuplicatePaths(name, **kwargs)
self.env_modifications.append(item) self.env_modifications.append(item)
@ -843,6 +854,8 @@ def validate(env, errstream):
Args: Args:
env: list of environment modifications env: list of environment modifications
""" """
if not env.traced:
return
modifications = env.group_by_name() modifications = env.group_by_name()
for variable, list_of_changes in sorted(modifications.items()): for variable, list_of_changes in sorted(modifications.items()):
set_or_unset_not_first(variable, list_of_changes, errstream) set_or_unset_not_first(variable, list_of_changes, errstream)