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.

2 files changed, 29 insertions, 14 deletions

diff --git a/lib/spack/spack/main.py b/lib/spack/spack/main.py
index 5224f42345..484ba4553d 100644
--- a/lib/spack/spack/main.py
+++ b/lib/spack/spack/main.py
@@ -43,6 +43,7 @@ import spack.repo
 import spack.spec
 import spack.store
 import spack.util.debug
+import spack.util.environment
 import spack.util.executable as exe
 import spack.util.path
 from spack.error import SpackError
@@ -478,6 +479,7 @@ def setup_main_options(args):
         spack.error.debug = True
         spack.util.debug.register_interrupt_handler()
         spack.config.set('config:debug', True, scope='command_line')
+        spack.util.environment.tracing_enabled = True
 
     if args.timestamp:
         tty.set_timestamp(True)
diff --git a/lib/spack/spack/util/environment.py b/lib/spack/spack/util/environment.py
index 398af5f018..a0a6c4a3aa 100644
--- a/lib/spack/spack/util/environment.py
+++ b/lib/spack/spack/util/environment.py
@@ -21,6 +21,7 @@ from six.moves import shlex_quote as cmd_quote
 import llnl.util.tty as tty
 from llnl.util.lang import dedupe
 
+import spack.config
 import spack.platforms
 import spack.spec
 import spack.util.executable as executable
@@ -45,6 +46,9 @@ _shell_unset_strings = {
 }
 
 
+tracing_enabled = False
+
+
 def is_system_path(path):
     """Predicate that given a path returns True if it is a system path,
     False otherwise.
@@ -365,14 +369,17 @@ class EnvironmentModifications(object):
         * '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'
         if it is not None.
 
         Args:
             other (EnvironmentModifications): list of environment modifications
                 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 = []
         if other is not None:
             self.extend(other)
@@ -393,7 +400,12 @@ class EnvironmentModifications(object):
             raise TypeError(
                 '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()
         try:
             _, filename, lineno, _, context, index = stack[2]
@@ -402,8 +414,7 @@ class EnvironmentModifications(object):
             filename = 'unknown file'
             lineno = 'unknown line'
             context = 'unknown context'
-        args = {'filename': filename, 'lineno': lineno, 'context': context}
-        return args
+        kwargs.update({'filename': filename, 'lineno': lineno, 'context': context})
 
     def set(self, name, value, **kwargs):
         """Stores a request to set an environment variable.
@@ -412,7 +423,7 @@ class EnvironmentModifications(object):
             name: name of the environment variable to be set
             value: value of the environment variable
         """
-        kwargs.update(self._get_outside_caller_attributes())
+        self._maybe_trace(kwargs)
         item = SetEnv(name, value, **kwargs)
         self.env_modifications.append(item)
 
@@ -425,7 +436,7 @@ class EnvironmentModifications(object):
             value: value to append to the environment 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})
         item = AppendFlagsEnv(name, value, **kwargs)
         self.env_modifications.append(item)
@@ -436,7 +447,7 @@ class EnvironmentModifications(object):
         Args:
             name: name of the environment variable to be unset
         """
-        kwargs.update(self._get_outside_caller_attributes())
+        self._maybe_trace(kwargs)
         item = UnsetEnv(name, **kwargs)
         self.env_modifications.append(item)
 
@@ -450,7 +461,7 @@ class EnvironmentModifications(object):
             value: value to remove to the environment variable
             sep: separator to assume for environment variable
         """
-        kwargs.update(self._get_outside_caller_attributes())
+        self._maybe_trace(kwargs)
         kwargs.update({'separator': sep})
         item = RemoveFlagsEnv(name, value, **kwargs)
         self.env_modifications.append(item)
@@ -462,7 +473,7 @@ class EnvironmentModifications(object):
             name: name o the environment variable to be set.
             elements: elements of the path to set.
         """
-        kwargs.update(self._get_outside_caller_attributes())
+        self._maybe_trace(kwargs)
         item = SetPath(name, elements, **kwargs)
         self.env_modifications.append(item)
 
@@ -473,7 +484,7 @@ class EnvironmentModifications(object):
             name: name of the path list in the environment
             path: path to be appended
         """
-        kwargs.update(self._get_outside_caller_attributes())
+        self._maybe_trace(kwargs)
         item = AppendPath(name, path, **kwargs)
         self.env_modifications.append(item)
 
@@ -484,7 +495,7 @@ class EnvironmentModifications(object):
             name: name of the path list in the environment
             path: path to be pre-pended
         """
-        kwargs.update(self._get_outside_caller_attributes())
+        self._maybe_trace(kwargs)
         item = PrependPath(name, path, **kwargs)
         self.env_modifications.append(item)
 
@@ -495,7 +506,7 @@ class EnvironmentModifications(object):
             name: name of the path list in the environment
             path: path to be removed
         """
-        kwargs.update(self._get_outside_caller_attributes())
+        self._maybe_trace(kwargs)
         item = RemovePath(name, path, **kwargs)
         self.env_modifications.append(item)
 
@@ -506,7 +517,7 @@ class EnvironmentModifications(object):
         Args:
             name: name of the path list in the environment.
         """
-        kwargs.update(self._get_outside_caller_attributes())
+        self._maybe_trace(kwargs)
         item = DeprioritizeSystemPaths(name, **kwargs)
         self.env_modifications.append(item)
 
@@ -517,7 +528,7 @@ class EnvironmentModifications(object):
         Args:
             name: name of the path list in the environment.
         """
-        kwargs.update(self._get_outside_caller_attributes())
+        self._maybe_trace(kwargs)
         item = PruneDuplicatePaths(name, **kwargs)
         self.env_modifications.append(item)
 
@@ -843,6 +854,8 @@ def validate(env, errstream):
     Args:
         env: list of environment modifications
     """
+    if not env.traced:
+        return
     modifications = env.group_by_name()
     for variable, list_of_changes in sorted(modifications.items()):
         set_or_unset_not_first(variable, list_of_changes, errstream)