module files: system paths are excluded from path inspection (#5460)

closes #5201

Currently, if a user sets an external package to have a prefix that is
one of the system paths (like '/usr') the module files that are
generated will prepend '/usr/bin' to 'PATH', etc. This is particularly
nasty at the time when a module file is unloaded, and e.g. paths like
'/usr/bin' will be discarded from PATH.

This PR solves the issue skipping system paths when a prefix inspection
is made to generate module files.

4 files changed, 72 insertions, 11 deletions

diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
index 0078245d70..72bbc98625 100644
--- a/lib/spack/spack/environment.py
+++ b/lib/spack/spack/environment.py
@@ -502,25 +502,56 @@ def filter_environment_blacklist(env, variables):
             yield item
 
 
-def inspect_path(root, inspections):
-    """Inspects a path to search for the subdirectories specified in the
-    inspection dictionary. Return a list of commands that will modify the
-    environment accordingly.
+def inspect_path(root, inspections, exclude=None):
+    """Inspects ``root`` to search for the subdirectories in ``inspections``.
+    Adds every path found to a list of prepend-path commands and returns it.
 
     Args:
-        root: path where to search for subdirectories
-        inspections: dictionary that maps subdirectories to a list of
-            variables that we want to pre-pend with a path, if found
+        root (str): absolute path where to search for subdirectories
+
+        inspections (dict): maps relative paths to a list of environment
+            variables that will be modified if the path exists. The
+            modifications are not performed immediately, but stored in a
+            command object that is returned to client
+
+        exclude (callable): optional callable. If present it must accept an
+            absolute path and return True if it should be excluded from the
+            inspection
+
+    Examples:
+
+    The following lines execute an inspection in ``/usr`` to search for
+    ``/usr/include`` and ``/usr/lib64``. If found we want to prepend
+    ``/usr/include`` to ``CPATH`` and ``/usr/lib64`` to ``MY_LIB64_PATH``.
+
+        .. code-block:: python
+
+            # Set up the dictionary containing the inspection
+            inspections = {
+                'include': ['CPATH'],
+                'lib64': ['MY_LIB64_PATH']
+            }
+
+            # Get back the list of command needed to modify the environment
+            env = inspect_path('/usr', inspections)
+
+            # Eventually execute the commands
+            env.apply_modifications()
 
     Returns:
         instance of EnvironmentModifications containing the requested
         modifications
     """
+    if exclude is None:
+        exclude = lambda x: False
+
     env = EnvironmentModifications()
     # Inspect the prefix to check for the existence of common directories
     for relative_path, variables in inspections.items():
         expected = os.path.join(root, relative_path)
-        if os.path.isdir(expected):
+
+        if os.path.isdir(expected) and not exclude(expected):
             for variable in variables:
                 env.prepend_path(variable, expected)
+
     return env
diff --git a/lib/spack/spack/modules/common.py b/lib/spack/spack/modules/common.py
index faf15e4150..f5cbacc078 100644
--- a/lib/spack/spack/modules/common.py
+++ b/lib/spack/spack/modules/common.py
@@ -61,6 +61,7 @@ import spack.build_environment as build_environment
 import spack.environment
 import spack.tengine as tengine
 import spack.util.path
+import spack.util.environment
 import spack.error
 
 #: Root folders where the various module files should be written
@@ -473,7 +474,9 @@ class BaseContext(tengine.Context):
         """List of environment modifications to be processed."""
         # Modifications guessed inspecting the spec prefix
         env = spack.environment.inspect_path(
-            self.spec.prefix, prefix_inspections
+            self.spec.prefix,
+            prefix_inspections,
+            exclude=spack.util.environment.is_system_path
         )
 
         # Modifications that are coded at package level
diff --git a/lib/spack/spack/test/environment.py b/lib/spack/spack/test/environment.py
index e31d672d31..1c787c8e69 100644
--- a/lib/spack/spack/test/environment.py
+++ b/lib/spack/spack/test/environment.py
@@ -30,7 +30,7 @@ from spack import spack_root
 from spack.environment import EnvironmentModifications
 from spack.environment import RemovePath, PrependPath, AppendPath
 from spack.environment import SetEnv, UnsetEnv
-from spack.util.environment import filter_system_paths
+from spack.util.environment import filter_system_paths, is_system_path
 
 
 def test_inspect_path(tmpdir):
@@ -60,6 +60,20 @@ def test_inspect_path(tmpdir):
     assert 'CPATH' in names
 
 
+def test_exclude_paths_from_inspection():
+    inspections = {
+        'lib': ['LIBRARY_PATH', 'LD_LIBRARY_PATH'],
+        'lib64': ['LIBRARY_PATH', 'LD_LIBRARY_PATH'],
+        'include': ['CPATH']
+    }
+
+    env = environment.inspect_path(
+        '/usr', inspections, exclude=is_system_path
+    )
+
+    assert len(env) == 0
+
+
 @pytest.fixture()
 def prepare_environment_for_tests():
     """Sets a few dummy variables in the current environment, that will be
diff --git a/lib/spack/spack/util/environment.py b/lib/spack/spack/util/environment.py
index 38576d6e9d..e866a34bd1 100644
--- a/lib/spack/spack/util/environment.py
+++ b/lib/spack/spack/util/environment.py
@@ -30,8 +30,21 @@ system_dirs = [os.path.join(p, s) for s in suffixes for p in system_paths] + \
     system_paths
 
 
+def is_system_path(path):
+    """Predicate that given a path returns True if it is a system path,
+    False otherwise.
+
+    Args:
+        path (str): path to a directory
+
+    Returns:
+        True or False
+    """
+    return os.path.normpath(path) in system_dirs
+
+
 def filter_system_paths(paths):
-    return [p for p in paths if os.path.normpath(p) not in system_dirs]
+    return [p for p in paths if not is_system_path(p)]
 
 
 def get_path(name):