From 1f303c9ac89b3eee34ac8c07878f8ba1d00fac62 Mon Sep 17 00:00:00 2001 From: "Adam J. Stewart" Date: Sun, 30 Apr 2017 20:43:44 -0500 Subject: Don't add system paths to PATH (#3910) * Filter all system paths introduced by dependencies from PATH * Make sure path filtering works *even* for trailing slashes * Revert some of the changes to `filter_system_paths` * Yes, `bin64` is a real thing (sigh) * add tests: /usr, /usr/, /usr/local/../bin, etc. * Convert from rST to Google-style docstrings --- lib/spack/spack/build_environment.py | 104 ++++++++++++++++++++--------------- lib/spack/spack/test/environment.py | 38 +++---------- lib/spack/spack/util/environment.py | 14 +---- 3 files changed, 72 insertions(+), 84 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py index 9c3768b65b..2ac935291d 100644 --- a/lib/spack/spack/build_environment.py +++ b/lib/spack/spack/build_environment.py @@ -236,50 +236,47 @@ def set_compiler_environment_variables(pkg, env): def set_build_environment_variables(pkg, env, dirty=False): - """ - This ensures a clean install environment when we build packages. + """Ensure a clean install environment when we build packages. - Arguments: - dirty -- skip unsetting the user's environment settings. - """ - # Add spack build environment path with compiler wrappers first in - # the path. We add both spack.env_path, which includes default - # wrappers (cc, c++, f77, f90), AND a subdirectory containing - # compiler-specific symlinks. The latter ensures that builds that - # are sensitive to the *name* of the compiler see the right name - # when we're building with the wrappers. - # - # Conflicts on case-insensitive systems (like "CC" and "cc") are - # handled by putting one in the /case-insensitive - # directory. Add that to the path too. - env_paths = [] - compiler_specific = join_path(spack.build_env_path, pkg.compiler.name) - for item in [spack.build_env_path, compiler_specific]: - env_paths.append(item) - ci = join_path(item, 'case-insensitive') - if os.path.isdir(ci): - env_paths.append(ci) - - env_paths = filter_system_paths(env_paths) + This involves unsetting pesky environment variables that may + affect the build. It also involves setting environment variables + used by Spack's compiler wrappers. - for item in reversed(env_paths): - env.prepend_path('PATH', item) - env.set_path(SPACK_ENV_PATH, env_paths) + Args: + pkg: The package we are building + env: The build environment + dirty (bool): Skip unsetting the user's environment settings + """ + # Gather information about various types of dependencies + build_deps = pkg.spec.traverse(root=False, deptype=('build')) + link_deps = pkg.spec.traverse(root=False, deptype=('link')) + build_link_deps = pkg.spec.traverse(root=False, deptype=('build', 'link')) + rpath_deps = get_rpath_deps(pkg) + + build_prefixes = [dep.prefix for dep in build_deps] + link_prefixes = [dep.prefix for dep in link_deps] + build_link_prefixes = [dep.prefix for dep in build_link_deps] + rpath_prefixes = [dep.prefix for dep in rpath_deps] + + # Filter out system paths: ['/', '/usr', '/usr/local'] + # These paths can be introduced into the build when an external package + # is added as a dependency. The problem with these paths is that they often + # contain hundreds of other packages installed in the same directory. + # If these paths come first, they can overshadow Spack installations. + build_prefixes = filter_system_paths(build_prefixes) + link_prefixes = filter_system_paths(link_prefixes) + build_link_prefixes = filter_system_paths(build_link_prefixes) + rpath_prefixes = filter_system_paths(rpath_prefixes) # Prefixes of all of the package's dependencies go in SPACK_DEPENDENCIES - dep_prefixes = [d.prefix for d in - pkg.spec.traverse(root=False, deptype=('build', 'link'))] - dep_prefixes = filter_system_paths(dep_prefixes) - env.set_path(SPACK_DEPENDENCIES, dep_prefixes) + env.set_path(SPACK_DEPENDENCIES, build_link_prefixes) # These variables control compiler wrapper behavior - env.set_path(SPACK_RPATH_DEPS, filter_system_paths([ - d.prefix for d in get_rpath_deps(pkg)])) - env.set_path(SPACK_LINK_DEPS, filter_system_paths([ - d.prefix for d in pkg.spec.traverse(root=False, deptype=('link'))])) + env.set_path(SPACK_RPATH_DEPS, rpath_prefixes) + env.set_path(SPACK_LINK_DEPS, link_prefixes) # Add dependencies to CMAKE_PREFIX_PATH - env.set_path('CMAKE_PREFIX_PATH', dep_prefixes) + env.set_path('CMAKE_PREFIX_PATH', build_link_prefixes) # Install prefix env.set(SPACK_PREFIX, pkg.prefix) @@ -326,12 +323,33 @@ def set_build_environment_variables(pkg, env, dirty=False): env.set('SPACK_COMPILER_EXTRA_RPATHS', extra_rpaths) # Add bin directories from dependencies to the PATH for the build. - bin_dirs = reversed( - [d.prefix.bin for d in pkg.spec.dependencies(deptype='build') - if os.path.isdir(d.prefix.bin)]) - bin_dirs = filter_system_bin_paths(bin_dirs) - for item in bin_dirs: + for prefix in build_prefixes: + for dirname in ['bin', 'bin64']: + bin_dir = os.path.join(prefix, dirname) + if os.path.isdir(bin_dir): + env.prepend_path('PATH', bin_dir) + + # Add spack build environment path with compiler wrappers first in + # the path. We add both spack.env_path, which includes default + # wrappers (cc, c++, f77, f90), AND a subdirectory containing + # compiler-specific symlinks. The latter ensures that builds that + # are sensitive to the *name* of the compiler see the right name + # when we're building with the wrappers. + # + # Conflicts on case-insensitive systems (like "CC" and "cc") are + # handled by putting one in the /case-insensitive + # directory. Add that to the path too. + env_paths = [] + compiler_specific = join_path(spack.build_env_path, pkg.compiler.name) + for item in [spack.build_env_path, compiler_specific]: + env_paths.append(item) + ci = join_path(item, 'case-insensitive') + if os.path.isdir(ci): + env_paths.append(ci) + + for item in reversed(env_paths): env.prepend_path('PATH', item) + env.set_path(SPACK_ENV_PATH, env_paths) # Working directory for the spack command itself, for debug logs. if spack.debug: @@ -340,9 +358,9 @@ def set_build_environment_variables(pkg, env, dirty=False): env.set(SPACK_DEBUG_LOG_DIR, spack.spack_working_dir) # Add any pkgconfig directories to PKG_CONFIG_PATH - for pre in dep_prefixes: + for prefix in build_link_prefixes: for directory in ('lib', 'lib64', 'share'): - pcdir = join_path(pre, directory, 'pkgconfig') + pcdir = join_path(prefix, directory, 'pkgconfig') if os.path.isdir(pcdir): env.prepend_path('PKG_CONFIG_PATH', pcdir) diff --git a/lib/spack/spack/test/environment.py b/lib/spack/spack/test/environment.py index dbad2c5e1f..cdc2e68085 100644 --- a/lib/spack/spack/test/environment.py +++ b/lib/spack/spack/test/environment.py @@ -29,7 +29,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, filter_system_bin_paths +from spack.util.environment import filter_system_paths @pytest.fixture() @@ -64,25 +64,18 @@ def miscellaneous_paths(): '/usr/local/lib64', '/usr/local/opt/some-package/lib', '/usr/opt/lib', + '/usr/local/../bin', '/lib', '/', '/usr', + '/usr/', + '/usr/bin', + '/bin64', '/lib64', '/include', + '/include/', '/opt/some-package/include', - ] - - -@pytest.fixture -def bin_paths(): - """Returns a list of bin paths, including system ones.""" - return [ - '/usr/local/Cellar/gcc/5.3.0/bin', - '/usr/local/bin', - '/usr/local/opt/some-package/bin', - '/usr/opt/bin', - '/bin', - '/opt/some-package/bin', + '/opt/some-package/local/..', ] @@ -137,21 +130,8 @@ def test_filter_system_paths(miscellaneous_paths): '/usr/local/Cellar/gcc/5.3.0/lib', '/usr/local/opt/some-package/lib', '/usr/opt/lib', - '/opt/some-package/include' - ] - assert filtered == expected - - -def test_filter_system_bin_paths(bin_paths): - """Tests that the filtering of system bin paths works as expected.""" - filtered = filter_system_bin_paths(bin_paths) - expected = [ - '/usr/local/bin', - '/bin', - '/usr/local/Cellar/gcc/5.3.0/bin', - '/usr/local/opt/some-package/bin', - '/usr/opt/bin', - '/opt/some-package/bin' + '/opt/some-package/include', + '/opt/some-package/local/..', ] assert filtered == expected diff --git a/lib/spack/spack/util/environment.py b/lib/spack/spack/util/environment.py index 420cce8245..934a999327 100644 --- a/lib/spack/spack/util/environment.py +++ b/lib/spack/spack/util/environment.py @@ -25,23 +25,13 @@ import os system_paths = ['/', '/usr', '/usr/local'] -suffixes = ['lib', 'lib64', 'include'] +suffixes = ['bin', 'bin64', 'include', 'lib', 'lib64'] system_dirs = [os.path.join(p, s) for s in suffixes for p in system_paths] + \ system_paths -system_bins = [os.path.join(p, 'bin') for p in system_paths] def filter_system_paths(paths): - return [p for p in paths if p not in system_dirs] - - -def filter_system_bin_paths(paths): - # Turn the iterable into a list. Assume it's a list from here on. - _paths = list(paths) - bins = [p for p in _paths if p in system_bins] - nobins = [p for p in _paths if p not in system_bins] - # put bins infront as PATH is set by: prepend_path('PATH', item) - return bins + nobins + return [p for p in paths if os.path.normpath(p) not in system_dirs] def get_path(name): -- cgit v1.2.3-60-g2f50