From e9d4780bbc307f37e6385fc80c8fea1abaab5782 Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Tue, 4 Oct 2016 09:40:28 -0700 Subject: Rework build environment and cc to use smaller RPATHs. (#1894) - Fixed up dependency management so that: - build deps go in PATH and -I - link deps go in -L args - only *immediate* link deps are RPATH'd The latter reduces the number of libraries that need to be added to DT_NEEDED / LC_RPATH. This removes redundant RPATHs to transitive dependencies. --- lib/spack/env/cc | 32 ++++++++++++----- lib/spack/spack/build_environment.py | 23 ++++++++++--- lib/spack/spack/test/cc.py | 66 ++++++++++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 13 deletions(-) diff --git a/lib/spack/env/cc b/lib/spack/env/cc index c6bb50d261..4b8922178a 100755 --- a/lib/spack/env/cc +++ b/lib/spack/env/cc @@ -266,22 +266,38 @@ for dep in "${deps[@]}"; do # Prepend lib and RPATH directories if [[ -d $dep/lib ]]; then if [[ $mode == ccld ]]; then - $add_rpaths && args=("$rpath$dep/lib" "${args[@]}") - args=("-L$dep/lib" "${args[@]}") + if [[ $SPACK_RPATH_DEPS == *$dep* ]]; then + $add_rpaths && args=("$rpath$dep/lib" "${args[@]}") + fi + if [[ $SPACK_LINK_DEPS == *$dep* ]]; then + args=("-L$dep/lib" "${args[@]}") + fi elif [[ $mode == ld ]]; then - $add_rpaths && args=("-rpath" "$dep/lib" "${args[@]}") - args=("-L$dep/lib" "${args[@]}") + if [[ $SPACK_RPATH_DEPS == *$dep* ]]; then + $add_rpaths && args=("-rpath" "$dep/lib" "${args[@]}") + fi + if [[ $SPACK_LINK_DEPS == *$dep* ]]; then + args=("-L$dep/lib" "${args[@]}") + fi fi fi # Prepend lib64 and RPATH directories if [[ -d $dep/lib64 ]]; then if [[ $mode == ccld ]]; then - $add_rpaths && args=("$rpath$dep/lib64" "${args[@]}") - args=("-L$dep/lib64" "${args[@]}") + if [[ $SPACK_RPATH_DEPS == *$dep* ]]; then + $add_rpaths && args=("$rpath$dep/lib64" "${args[@]}") + fi + if [[ $SPACK_LINK_DEPS == *$dep* ]]; then + args=("-L$dep/lib64" "${args[@]}") + fi elif [[ $mode == ld ]]; then - $add_rpaths && args=("-rpath" "$dep/lib64" "${args[@]}") - args=("-L$dep/lib64" "${args[@]}") + if [[ $SPACK_RPATH_DEPS == *$dep* ]]; then + $add_rpaths && args=("-rpath" "$dep/lib64" "${args[@]}") + fi + if [[ $SPACK_LINK_DEPS == *$dep* ]]; then + args=("-L$dep/lib64" "${args[@]}") + fi fi fi done diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py index 9b5ed367d1..c3fa7418e8 100644 --- a/lib/spack/spack/build_environment.py +++ b/lib/spack/spack/build_environment.py @@ -77,6 +77,8 @@ SPACK_NO_PARALLEL_MAKE = 'SPACK_NO_PARALLEL_MAKE' # SPACK_ENV_PATH = 'SPACK_ENV_PATH' SPACK_DEPENDENCIES = 'SPACK_DEPENDENCIES' +SPACK_RPATH_DEPS = 'SPACK_RPATH_DEPS' +SPACK_LINK_DEPS = 'SPACK_LINK_DEPS' SPACK_PREFIX = 'SPACK_PREFIX' SPACK_INSTALL = 'SPACK_INSTALL' SPACK_DEBUG = 'SPACK_DEBUG' @@ -254,9 +256,15 @@ def set_build_environment_variables(pkg, env, dirty=False): env.set_path(SPACK_ENV_PATH, env_paths) # 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')] + dep_prefixes = [d.prefix for d in + pkg.spec.traverse(root=False, deptype=('build', 'link'))] env.set_path(SPACK_DEPENDENCIES, dep_prefixes) + + # These variables control compiler wrapper behavior + env.set_path(SPACK_RPATH_DEPS, [d.prefix for d in get_rpath_deps(pkg)]) + env.set_path(SPACK_LINK_DEPS, [ + d.prefix for d in pkg.spec.traverse(root=False, deptype=('link'))]) + # Add dependencies to CMAKE_PREFIX_PATH env.set_path('CMAKE_PREFIX_PATH', dep_prefixes) @@ -288,8 +296,8 @@ def set_build_environment_variables(pkg, env, dirty=False): env.remove_path('PATH', p) # Add bin directories from dependencies to the PATH for the build. - bin_dirs = reversed( - filter(os.path.isdir, ['%s/bin' % prefix for prefix in dep_prefixes])) + bin_dirs = reversed(filter(os.path.isdir, [ + '%s/bin' % d.prefix for d in pkg.spec.dependencies(deptype='build')])) for item in bin_dirs: env.prepend_path('PATH', item) @@ -382,10 +390,15 @@ def set_module_variables_for_package(pkg, module): m.dso_suffix = dso_suffix +def get_rpath_deps(pkg): + """We only need to RPATH immediate dependencies.""" + return pkg.spec.dependencies(deptype='link') + + def get_rpaths(pkg): """Get a list of all the rpaths for a package.""" rpaths = [pkg.prefix.lib, pkg.prefix.lib64] - deps = pkg.spec.dependencies(deptype='link') + deps = get_rpath_deps(pkg) rpaths.extend(d.prefix.lib for d in deps if os.path.isdir(d.prefix.lib)) rpaths.extend(d.prefix.lib64 for d in deps diff --git a/lib/spack/spack/test/cc.py b/lib/spack/spack/test/cc.py index f3e4bb31d2..73c711724c 100644 --- a/lib/spack/spack/test/cc.py +++ b/lib/spack/spack/test/cc.py @@ -223,6 +223,8 @@ class CompilerTest(unittest.TestCase): def test_dep_include(self): """Ensure a single dependency include directory is added.""" os.environ['SPACK_DEPENDENCIES'] = self.dep4 + os.environ['SPACK_RPATH_DEPS'] = os.environ['SPACK_DEPENDENCIES'] + os.environ['SPACK_LINK_DEPS'] = os.environ['SPACK_DEPENDENCIES'] self.check_cc('dump-args', test_command, self.realcc + ' ' + '-Wl,-rpath,' + self.prefix + '/lib ' + @@ -233,6 +235,8 @@ class CompilerTest(unittest.TestCase): def test_dep_lib(self): """Ensure a single dependency RPATH is added.""" os.environ['SPACK_DEPENDENCIES'] = self.dep2 + os.environ['SPACK_RPATH_DEPS'] = os.environ['SPACK_DEPENDENCIES'] + os.environ['SPACK_LINK_DEPS'] = os.environ['SPACK_DEPENDENCIES'] self.check_cc('dump-args', test_command, self.realcc + ' ' + '-Wl,-rpath,' + self.prefix + '/lib ' + @@ -241,10 +245,34 @@ class CompilerTest(unittest.TestCase): '-Wl,-rpath,' + self.dep2 + '/lib64 ' + ' '.join(test_command)) + def test_dep_lib_no_rpath(self): + """Ensure a single dependency link flag is added with no dep RPATH.""" + os.environ['SPACK_DEPENDENCIES'] = self.dep2 + os.environ['SPACK_LINK_DEPS'] = os.environ['SPACK_DEPENDENCIES'] + self.check_cc('dump-args', test_command, + self.realcc + ' ' + + '-Wl,-rpath,' + self.prefix + '/lib ' + + '-Wl,-rpath,' + self.prefix + '/lib64 ' + + '-L' + self.dep2 + '/lib64 ' + + ' '.join(test_command)) + + def test_dep_lib_no_lib(self): + """Ensure a single dependency RPATH is added with no -L.""" + os.environ['SPACK_DEPENDENCIES'] = self.dep2 + os.environ['SPACK_RPATH_DEPS'] = os.environ['SPACK_DEPENDENCIES'] + self.check_cc('dump-args', test_command, + self.realcc + ' ' + + '-Wl,-rpath,' + self.prefix + '/lib ' + + '-Wl,-rpath,' + self.prefix + '/lib64 ' + + '-Wl,-rpath,' + self.dep2 + '/lib64 ' + + ' '.join(test_command)) + def test_all_deps(self): """Ensure includes and RPATHs for all deps are added. """ os.environ['SPACK_DEPENDENCIES'] = ':'.join([ self.dep1, self.dep2, self.dep3, self.dep4]) + os.environ['SPACK_RPATH_DEPS'] = os.environ['SPACK_DEPENDENCIES'] + os.environ['SPACK_LINK_DEPS'] = os.environ['SPACK_DEPENDENCIES'] # This is probably more constrained than it needs to be; it # checks order within prepended args and doesn't strictly have @@ -273,6 +301,8 @@ class CompilerTest(unittest.TestCase): """Ensure no (extra) -I args or -Wl, are passed in ld mode.""" os.environ['SPACK_DEPENDENCIES'] = ':'.join([ self.dep1, self.dep2, self.dep3, self.dep4]) + os.environ['SPACK_RPATH_DEPS'] = os.environ['SPACK_DEPENDENCIES'] + os.environ['SPACK_LINK_DEPS'] = os.environ['SPACK_DEPENDENCIES'] self.check_ld('dump-args', test_command, 'ld ' + @@ -290,10 +320,46 @@ class CompilerTest(unittest.TestCase): ' '.join(test_command)) + def test_ld_deps_no_rpath(self): + """Ensure SPACK_RPATH_DEPS controls RPATHs for ld.""" + os.environ['SPACK_DEPENDENCIES'] = ':'.join([ + self.dep1, self.dep2, self.dep3, self.dep4]) + os.environ['SPACK_LINK_DEPS'] = os.environ['SPACK_DEPENDENCIES'] + + self.check_ld('dump-args', test_command, + 'ld ' + + '-rpath ' + self.prefix + '/lib ' + + '-rpath ' + self.prefix + '/lib64 ' + + + '-L' + self.dep3 + '/lib64 ' + + '-L' + self.dep2 + '/lib64 ' + + '-L' + self.dep1 + '/lib ' + + + ' '.join(test_command)) + + def test_ld_deps_no_link(self): + """Ensure SPACK_LINK_DEPS controls -L for ld.""" + os.environ['SPACK_DEPENDENCIES'] = ':'.join([ + self.dep1, self.dep2, self.dep3, self.dep4]) + os.environ['SPACK_RPATH_DEPS'] = os.environ['SPACK_DEPENDENCIES'] + + self.check_ld('dump-args', test_command, + 'ld ' + + '-rpath ' + self.prefix + '/lib ' + + '-rpath ' + self.prefix + '/lib64 ' + + + '-rpath ' + self.dep3 + '/lib64 ' + + '-rpath ' + self.dep2 + '/lib64 ' + + '-rpath ' + self.dep1 + '/lib ' + + + ' '.join(test_command)) + def test_ld_deps_reentrant(self): """Make sure ld -r is handled correctly on OS's where it doesn't support rpaths.""" os.environ['SPACK_DEPENDENCIES'] = ':'.join([self.dep1]) + os.environ['SPACK_RPATH_DEPS'] = os.environ['SPACK_DEPENDENCIES'] + os.environ['SPACK_LINK_DEPS'] = os.environ['SPACK_DEPENDENCIES'] os.environ['SPACK_SHORT_SPEC'] = "foo@1.2=linux-x86_64" reentrant_test_command = ['-r'] + test_command -- cgit v1.2.3-60-g2f50