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.
This commit is contained in:
		 Massimiliano Culpo
					Massimiliano Culpo
				
			
				
					committed by
					
						 Todd Gamblin
						Todd Gamblin
					
				
			
			
				
	
			
			
			 Todd Gamblin
						Todd Gamblin
					
				
			
						parent
						
							64311e8510
						
					
				
				
					commit
					8864d145e9
				
			| @@ -502,25 +502,56 @@ def filter_environment_blacklist(env, variables): | |||||||
|             yield item |             yield item | ||||||
|  |  | ||||||
|  |  | ||||||
| def inspect_path(root, inspections): | def inspect_path(root, inspections, exclude=None): | ||||||
|     """Inspects a path to search for the subdirectories specified in the |     """Inspects ``root`` to search for the subdirectories in ``inspections``. | ||||||
|     inspection dictionary. Return a list of commands that will modify the |     Adds every path found to a list of prepend-path commands and returns it. | ||||||
|     environment accordingly. |  | ||||||
|  |  | ||||||
|     Args: |     Args: | ||||||
|         root: path where to search for subdirectories |         root (str): absolute 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 |         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: |     Returns: | ||||||
|         instance of EnvironmentModifications containing the requested |         instance of EnvironmentModifications containing the requested | ||||||
|         modifications |         modifications | ||||||
|     """ |     """ | ||||||
|  |     if exclude is None: | ||||||
|  |         exclude = lambda x: False | ||||||
|  |  | ||||||
|     env = EnvironmentModifications() |     env = EnvironmentModifications() | ||||||
|     # Inspect the prefix to check for the existence of common directories |     # Inspect the prefix to check for the existence of common directories | ||||||
|     for relative_path, variables in inspections.items(): |     for relative_path, variables in inspections.items(): | ||||||
|         expected = os.path.join(root, relative_path) |         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: |             for variable in variables: | ||||||
|                 env.prepend_path(variable, expected) |                 env.prepend_path(variable, expected) | ||||||
|  |  | ||||||
|     return env |     return env | ||||||
|   | |||||||
| @@ -61,6 +61,7 @@ | |||||||
| import spack.environment | import spack.environment | ||||||
| import spack.tengine as tengine | import spack.tengine as tengine | ||||||
| import spack.util.path | import spack.util.path | ||||||
|  | import spack.util.environment | ||||||
| import spack.error | import spack.error | ||||||
|  |  | ||||||
| #: Root folders where the various module files should be written | #: Root folders where the various module files should be written | ||||||
| @@ -473,7 +474,9 @@ def environment_modifications(self): | |||||||
|         """List of environment modifications to be processed.""" |         """List of environment modifications to be processed.""" | ||||||
|         # Modifications guessed inspecting the spec prefix |         # Modifications guessed inspecting the spec prefix | ||||||
|         env = spack.environment.inspect_path( |         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 |         # Modifications that are coded at package level | ||||||
|   | |||||||
| @@ -30,7 +30,7 @@ | |||||||
| from spack.environment import EnvironmentModifications | from spack.environment import EnvironmentModifications | ||||||
| from spack.environment import RemovePath, PrependPath, AppendPath | from spack.environment import RemovePath, PrependPath, AppendPath | ||||||
| from spack.environment import SetEnv, UnsetEnv | 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): | def test_inspect_path(tmpdir): | ||||||
| @@ -60,6 +60,20 @@ def test_inspect_path(tmpdir): | |||||||
|     assert 'CPATH' in names |     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() | @pytest.fixture() | ||||||
| def prepare_environment_for_tests(): | def prepare_environment_for_tests(): | ||||||
|     """Sets a few dummy variables in the current environment, that will be |     """Sets a few dummy variables in the current environment, that will be | ||||||
|   | |||||||
| @@ -30,8 +30,21 @@ | |||||||
|     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): | 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): | def get_path(name): | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user