Skip to content

Commit

Permalink
Strip flags from ld and ccld with a more general mechanism.
Browse files Browse the repository at this point in the history
  • Loading branch information
alalazo committed Sep 13, 2018
1 parent ba0d18b commit 0f053e0
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 21 deletions.
20 changes: 5 additions & 15 deletions lib/spack/env/cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ parameters=(
SPACK_CXX_RPATH_ARG
SPACK_F77_RPATH_ARG
SPACK_FC_RPATH_ARG
SPACK_DTAGS_TO_DISABLE
SPACK_LINKER_ARG
SPACK_SHORT_SPEC
SPACK_SYSTEM_DIRS
Expand All @@ -62,6 +61,8 @@ parameters=(
# The default compiler flags are passed from these variables:
# SPACK_CFLAGS, SPACK_CXXFLAGS, SPACK_FCFLAGS, SPACK_FFLAGS,
# SPACK_LDFLAGS, SPACK_LDLIBS
# The flags that needs to be stripped are in these variables:
# SPACK_STRIP_LDFLAGS
# Debug env var is optional; set to "TRUE" for debug logging:
# SPACK_DEBUG
# Test command is used to unit test the compiler script.
Expand All @@ -86,6 +87,7 @@ IFS=' ' read -ra SPACK_CPPFLAGS <<< "$SPACK_CPPFLAGS"
IFS=' ' read -ra SPACK_CFLAGS <<< "$SPACK_CFLAGS"
IFS=' ' read -ra SPACK_CXXFLAGS <<< "$SPACK_CXXFLAGS"
IFS=' ' read -ra SPACK_LDFLAGS <<< "$SPACK_LDFLAGS"
IFS=' ' read -ra SPACK_STRIP_LDFLAGS <<< "$SPACK_STRIP_LDFLAGS"
IFS=' ' read -ra SPACK_LDLIBS <<< "$SPACK_LDLIBS"

# test whether a path is a system directory
Expand Down Expand Up @@ -189,18 +191,6 @@ if [[ -z $mode ]]; then
done
fi

# This is needed to ensure we set RPATH instead of RUNPATH
# (or the opposite, depending on the configuration in config.yaml)
#
# http://blog.qt.io/blog/2011/10/28/rpath-and-runpath/
# https://wiki.debian.org/RpathIssue
#
# The only discussion I could find on enabling new dynamic tags by
# default on ld is the following:
#
# https://sourceware.org/ml/binutils/2013-01/msg00307.html
#
dtags_to_disable="${SPACK_DTAGS_TO_DISABLE}"
linker_arg="${SPACK_LINKER_ARG}"

# Set up rpath variable according to language.
Expand Down Expand Up @@ -329,7 +319,7 @@ while [ -n "$1" ]; do
die "-Wl,-rpath was not followed by -Wl,*"
fi
rp="${arg#-Wl,}"
elif [[ "$arg" = "$dtags_to_disable" ]] ; then
elif [[ " ${SPACK_STRIP_LDFLAGS[@]} " =~ " $arg " ]] ; then
: # We want to remove explicitly this flag
else
other_args+=("-Wl,$arg")
Expand Down Expand Up @@ -362,7 +352,7 @@ while [ -n "$1" ]; do
fi
;;
*)
if [[ "$1" = "$dtags_to_disable" ]] ; then
if [[ " ${SPACK_STRIP_LDFLAGS[@]} " =~ " $1 " ]] ; then
: # We want to remove explicitly this flag
else
other_args+=("$1")
Expand Down
8 changes: 2 additions & 6 deletions lib/spack/spack/build_environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,12 +208,6 @@ def set_compiler_environment_variables(pkg, env):
env.set('SPACK_FC_RPATH_ARG', compiler.fc_rpath_arg)
env.set('SPACK_LINKER_ARG', compiler.linker_arg)

# Check whether we want to force RPATH or RUNPATH
if spack.config.get('config:shared_linking') == 'rpath':
env.set('SPACK_DTAGS_TO_DISABLE', compiler.enable_new_dtags)
else:
env.set('SPACK_DTAGS_TO_DISABLE', compiler.disable_new_dtags)

# Trap spack-tracked compiler flags as appropriate.
# env_flags are easy to accidentally override.
inject_flags = {}
Expand All @@ -240,8 +234,10 @@ def set_compiler_environment_variables(pkg, env):
# Inject flags to use either RPATH or RUNPATH for ELF binaries
if spack.config.get('config:shared_linking') == 'rpath':
inject_flags['ldflags'].append(compiler.disable_new_dtags)
env.set('SPACK_STRIP_LDFLAGS', compiler.enable_new_dtags)
else:
inject_flags['ldflags'].append(compiler.enable_new_dtags)
env.set('SPACK_STRIP_LDFLAGS', compiler.disable_new_dtags)

# Place compiler flags as specified by flag_handler
for flag in spack.spec.FlagMap.valid_compiler_flags():
Expand Down
31 changes: 31 additions & 0 deletions lib/spack/spack/test/build_environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -236,3 +236,34 @@ def _set_wrong_cc(x):
paths = os.environ['PATH'].split(':')

assert paths.index(spack_path) < paths.index(module_path)


@pytest.mark.regression('9160')
def test_dtags_in_build_environment(config, mock_packages, monkeypatch):
# Test the default configuration, that sets RPATH
pkg = spack.spec.Spec('cmake').concretized().package
spack.build_environment.setup_package(pkg, False)

assert 'SPACK_LDFLAGS' in os.environ
assert 'SPACK_STRIP_LDFLAGS' in os.environ
assert '--disable-new-dtags' in os.environ['SPACK_LDFLAGS']
assert '--enable-new-dtags' in os.environ['SPACK_STRIP_LDFLAGS']

old_get = spack.config.get

def _mock_get(request):
if request == 'config:shared_linking':
return 'runpath'
return old_get(request)

# Now use the mock function, that sets RUNPATH
monkeypatch.setattr(spack.config, 'get', _mock_get)
del os.environ['SPACK_LDFLAGS']
del os.environ['SPACK_STRIP_LDFLAGS']

spack.build_environment.setup_package(pkg, False)

assert 'SPACK_LDFLAGS' in os.environ
assert 'SPACK_STRIP_LDFLAGS' in os.environ
assert '--enable-new-dtags' in os.environ['SPACK_LDFLAGS']
assert '--disable-new-dtags' in os.environ['SPACK_STRIP_LDFLAGS']

0 comments on commit 0f053e0

Please sign in to comment.