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.

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