Skip to content

Commit

Permalink
Rework build environment and cc to use smaller RPATHs.
Browse files Browse the repository at this point in the history 
- 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.
  • Loading branch information
@tgamblin
tgamblin committed Oct 4, 2016
1 parent 50fcade commit fa11b7c
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 13 deletions.
32 changes: 24 additions & 8 deletions lib/spack/env/cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 18 additions & 5 deletions lib/spack/spack/build_environment.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@
#
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'
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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
Expand Down
66 changes: 66 additions & 0 deletions lib/spack/spack/test/cc.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,8 @@ def test_dep_rpath(self):
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 ' +
Expand All @@ -233,6 +235,8 @@ def test_dep_include(self):
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 ' +
Expand All @@ -241,10 +245,34 @@ def test_dep_lib(self):
'-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
Expand Down Expand Up @@ -273,6 +301,8 @@ def test_ld_deps(self):
"""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 ' +
Expand All @@ -290,10 +320,46 @@ def test_ld_deps(self):

' '.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
Expand Down

0 comments on commit fa11b7c

Please sign in to comment.