Inject the dtag option that needs to be enabled using SPACK_LDFLAGS

Instead of using an ad-hoc variable, the option `--disable-new-dtags`
is injected using the variable SPACK_LDFLAGS. This required a
refactoring of the part of the wrapper that was dealing with the
injection to distinguish between ccld and ld.

lib/spack/env/cc

@@ -51,7 +51,6 @@ parameters=(
     SPACK_CXX_RPATH_ARG
     SPACK_F77_RPATH_ARG
     SPACK_FC_RPATH_ARG
-    SPACK_DTAGS_TO_ENABLE
     SPACK_DTAGS_TO_DISABLE
     SPACK_LINKER_ARG
     SPACK_SHORT_SPEC
@@ -193,10 +192,6 @@ fi
 # This is needed to ensure we set RPATH instead of RUNPATH
 # (or the opposite, depending on the configuration in config.yaml)
 #
-# Documentation on this mechanism is lacking at best. A few sources
-# of information are (note that some of them take explicitly the
-# opposite stance that Spack does):
-#
 # http://blog.qt.io/blog/2011/10/28/rpath-and-runpath/
 # https://wiki.debian.org/RpathIssue
 #
@@ -205,7 +200,6 @@ fi
 #
 # https://sourceware.org/ml/binutils/2013-01/msg00307.html
 #
-dtags_to_enable="${SPACK_DTAGS_TO_ENABLE}"
 dtags_to_disable="${SPACK_DTAGS_TO_DISABLE}"
 linker_arg="${SPACK_LINKER_ARG}"
 
@@ -427,7 +421,10 @@ esac
 
 # Linker flags
 case "$mode" in
-    ld|ccld)
+    ccld)
+        for flag in "${SPACK_LDFLAGS[@]}"; do flags+=("$linker_arg$flag"); done
+        ;;
+    ld)
         flags=("${flags[@]}" "${SPACK_LDFLAGS[@]}") ;;
 esac
 
@@ -516,12 +513,10 @@ for dir in "${system_libdirs[@]}";   do args+=("-L$dir"); done
 # RPATHs arguments
 case "$mode" in
     ccld)
-        if [ ! -z "$dtags_to_enable" ] ; then args+=("$linker_arg$dtags_to_enable") ; fi
         for dir in "${rpaths[@]}";        do args+=("$rpath$dir"); done
         for dir in "${system_rpaths[@]}"; do args+=("$rpath$dir"); done
         ;;
     ld)
-        if [ ! -z "$dtags_to_enable" ] ; then args+=("$dtags_to_enable") ; fi
         for dir in "${rpaths[@]}";        do args+=("-rpath" "$dir"); done
         for dir in "${system_rpaths[@]}"; do args+=("-rpath" "$dir"); done
         ;;

lib/spack/spack/build_environment.py

@@ -211,10 +211,8 @@ def set_compiler_environment_variables(pkg, env):
     # 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)
-        env.set('SPACK_DTAGS_TO_ENABLE', compiler.disable_new_dtags)
     else:
         env.set('SPACK_DTAGS_TO_DISABLE', compiler.disable_new_dtags)
-        env.set('SPACK_DTAGS_TO_ENABLE', compiler.enable_new_dtags)
 
     # Trap spack-tracked compiler flags as appropriate.
     # env_flags are easy to accidentally override.
@@ -223,7 +221,7 @@ def set_compiler_environment_variables(pkg, env):
     build_system_flags = {}
     for flag in spack.spec.FlagMap.valid_compiler_flags():
         # Always convert flag_handler to function type.
-        # This avoids discrepencies in calling conventions between functions
+        # This avoids discrepancies in calling conventions between functions
         # and methods, or between bound and unbound methods in python 2.
         # We cannot effectively convert everything to a bound method, which
         # would be the simpler solution.
@@ -239,6 +237,12 @@ def set_compiler_environment_variables(pkg, env):
         env_flags[flag] = envf or []
         build_system_flags[flag] = bsf or []
 
+    # 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)
+    else:
+        inject_flags['ldflags'].append(compiler.enable_new_dtags)
+
     # Place compiler flags as specified by flag_handler
     for flag in spack.spec.FlagMap.valid_compiler_flags():
         # Concreteness guarantees key safety here

lib/spack/spack/test/build_environment.py

@@ -59,7 +59,6 @@ def build_environment():
     os.environ['SPACK_F77_RPATH_ARG'] = "-Wl,-rpath,"
     os.environ['SPACK_FC_RPATH_ARG']  = "-Wl,-rpath,"
     os.environ['SPACK_LINKER_ARG'] = '-Wl,'
-    os.environ['SPACK_DTAGS_TO_ENABLE'] = '--disable-new-dtags'
     os.environ['SPACK_DTAGS_TO_DISABLE'] = '--enable-new-dtags'
 
     os.environ['SPACK_SYSTEM_DIRS'] = '/usr/include /usr/lib'
@@ -83,12 +82,10 @@ def test_static_to_shared_library(build_environment):
 
     expected = {
         'linux': ('/bin/mycc -Wl,-rpath,/spack-test-prefix/lib'
-                  ' -Wl,--disable-new-dtags'
                   ' -Wl,-rpath,/spack-test-prefix/lib64 -shared'
                   ' -Wl,-soname,{2} -Wl,--whole-archive {0}'
                   ' -Wl,--no-whole-archive -o {1}'),
         'darwin': ('/bin/mycc -Wl,-rpath,/spack-test-prefix/lib'
-                   ' -Wl,--disable-new-dtags'
                    ' -Wl,-rpath,/spack-test-prefix/lib64 -dynamiclib'
                    ' -install_name {1} -Wl,-force_load,{0} -o {1}')
     }

lib/spack/spack/test/cc.py

@@ -130,7 +130,7 @@ def wrapper_environment():
             SPACK_F77_RPATH_ARG='-Wl,-rpath,',
             SPACK_FC_RPATH_ARG='-Wl,-rpath,',
             SPACK_LINKER_ARG='-Wl,',
-            SPACK_DTAGS_TO_ENABLE='--disable-new-dtags',
+            SPACK_LDFLAGS=None,  # Disable previous SPACK_LDFLAGS
             SPACK_DTAGS_TO_DISABLE='--enable-new-dtags',
             SPACK_DEPENDENCIES=None):
         yield
@@ -239,7 +239,6 @@ def test_ld_flags(wrapper_flags):
         spack_ldflags +
         test_include_paths +
         test_library_paths +
-        ['--disable-new-dtags'] +
         test_rpaths +
         pkg_rpaths +
         test_args_without_paths +
@@ -262,10 +261,9 @@ def test_cc_flags(wrapper_flags):
         [real_cc] +
         spack_cppflags +
         spack_cflags +
-        spack_ldflags +
+        ['-Wl,' + x for x in spack_ldflags] +
         test_include_paths +
         test_library_paths +
-        ['-Wl,--disable-new-dtags'] +
         test_wl_rpaths +
         pkg_wl_rpaths +
         test_args_without_paths +
@@ -278,10 +276,9 @@ def test_cxx_flags(wrapper_flags):
         [real_cc] +
         spack_cppflags +
         spack_cxxflags +
-        spack_ldflags +
+        ['-Wl,' + x for x in spack_ldflags] +
         test_include_paths +
         test_library_paths +
-        ['-Wl,--disable-new-dtags'] +
         test_wl_rpaths +
         pkg_wl_rpaths +
         test_args_without_paths +
@@ -294,10 +291,9 @@ def test_fc_flags(wrapper_flags):
         [real_cc] +
         spack_fflags +
         spack_cppflags +
-        spack_ldflags +
+        ['-Wl,' + x for x in spack_ldflags] +
         test_include_paths +
         test_library_paths +
-        ['-Wl,--disable-new-dtags'] +
         test_wl_rpaths +
         pkg_wl_rpaths +
         test_args_without_paths +
@@ -311,7 +307,6 @@ def test_dep_rpath():
         [real_cc] +
         test_include_paths +
         test_library_paths +
-        ['-Wl,--disable-new-dtags'] +
         test_wl_rpaths +
         pkg_wl_rpaths +
         test_args_without_paths)
@@ -328,7 +323,6 @@ def test_dep_include(dep4):
             test_include_paths +
             ['-I' + dep4 + '/include'] +
             test_library_paths +
-            ['-Wl,--disable-new-dtags'] +
             test_wl_rpaths +
             pkg_wl_rpaths +
             test_args_without_paths)
@@ -345,7 +339,6 @@ def test_dep_lib(dep2):
             test_include_paths +
             test_library_paths +
             ['-L' + dep2 + '/lib64'] +
-            ['-Wl,--disable-new-dtags'] +
             test_wl_rpaths +
             pkg_wl_rpaths +
             ['-Wl,-rpath,' + dep2 + '/lib64'] +
@@ -362,7 +355,6 @@ def test_dep_lib_no_rpath(dep2):
             test_include_paths +
             test_library_paths +
             ['-L' + dep2 + '/lib64'] +
-            ['-Wl,--disable-new-dtags'] +
             test_wl_rpaths +
             pkg_wl_rpaths +
             test_args_without_paths)
@@ -377,7 +369,6 @@ def test_dep_lib_no_lib(dep2):
             [real_cc] +
             test_include_paths +
             test_library_paths +
-            ['-Wl,--disable-new-dtags'] +
             test_wl_rpaths +
             pkg_wl_rpaths +
             ['-Wl,-rpath,' + dep2 + '/lib64'] +
@@ -401,7 +392,6 @@ def test_ccld_deps(dep1, dep2, dep3, dep4):
             ['-L' + dep1 + '/lib',
              '-L' + dep2 + '/lib64',
              '-L' + dep3 + '/lib64'] +
-            ['-Wl,--disable-new-dtags'] +
             test_wl_rpaths +
             pkg_wl_rpaths +
             ['-Wl,-rpath,' + dep1 + '/lib',
@@ -455,7 +445,6 @@ def test_ccld_with_system_dirs(dep1, dep2, dep3, dep4):
              '-L' + dep3 + '/lib64'] +
             ['-L/usr/local/lib',
              '-L/lib64/'] +
-            ['-Wl,--disable-new-dtags'] +
             test_wl_rpaths +
             pkg_wl_rpaths +
             ['-Wl,-rpath,' + dep1 + '/lib',
@@ -479,7 +468,6 @@ def test_ld_deps(dep1, dep2, dep3, dep4):
             ['-L' + dep1 + '/lib',
              '-L' + dep2 + '/lib64',
              '-L' + dep3 + '/lib64'] +
-            ['--disable-new-dtags'] +
             test_rpaths +
             pkg_rpaths +
             ['-rpath', dep1 + '/lib',
@@ -501,7 +489,6 @@ def test_ld_deps_no_rpath(dep1, dep2, dep3, dep4):
             ['-L' + dep1 + '/lib',
              '-L' + dep2 + '/lib64',
              '-L' + dep3 + '/lib64'] +
-            ['--disable-new-dtags'] +
             test_rpaths +
             pkg_rpaths +
             test_args_without_paths)
@@ -517,7 +504,6 @@ def test_ld_deps_no_link(dep1, dep2, dep3, dep4):
             ['ld'] +
             test_include_paths +
             test_library_paths +
-            ['--disable-new-dtags'] +
             test_rpaths +
             pkg_rpaths +
             ['-rpath', dep1 + '/lib',
@@ -542,7 +528,6 @@ def test_ld_deps_partial(dep1):
             test_include_paths +
             test_library_paths +
             ['-L' + dep1 + '/lib'] +
-            ['--disable-new-dtags'] +
             test_rpaths +
             pkg_rpaths +
             ['-rpath', dep1 + '/lib'] +
@@ -558,7 +543,6 @@ def test_ld_deps_partial(dep1):
             test_include_paths +
             test_library_paths +
             ['-L' + dep1 + '/lib'] +
-            ['--disable-new-dtags'] +
             test_rpaths +
             ['-r'] +
             test_args_without_paths)
@@ -572,7 +556,6 @@ def test_ccache_prepend_for_cc():
             [real_cc] +
             test_include_paths +
             test_library_paths +
-            ['-Wl,--disable-new-dtags'] +
             test_wl_rpaths +
             pkg_wl_rpaths +
             test_args_without_paths)
@@ -585,15 +568,17 @@ def test_no_ccache_prepend_for_fc():
         [real_cc] +
         test_include_paths +
         test_library_paths +
-        ['-Wl,--disable-new-dtags'] +
         test_wl_rpaths +
         pkg_wl_rpaths +
         test_args_without_paths)
 
 
 @pytest.mark.regression('9160')
 def test_disable_new_dtags(wrapper_flags):
-    with set_env(SPACK_TEST_COMMAND='dump-args'):
+    with set_env(
+            SPACK_TEST_COMMAND='dump-args',
+            SPACK_LDFLAGS='--disable-new-dtags'
+    ):
         result = ld(*test_args, output=str).strip().split('\n')
         assert '--disable-new-dtags' in result
         result = cc(*test_args, output=str).strip().split('\n')

lib/spack/spack/test/data/config.yaml

@@ -13,3 +13,4 @@ config:
   verify_ssl: true
   checksum: true
   dirty: True
+  shared_linking: 'rpath'

lib/spack/spack/test/flag_handlers.py

@@ -164,7 +164,10 @@ def test_ld_flags_cmake(self, temp_env):
         pkg.flag_handler = pkg.build_system_flags
         spack.build_environment.setup_package(pkg, False)
 
-        assert 'SPACK_LDFLAGS' not in os.environ
+        # SPACK_LDFLAGS is there because we inject
+        # flags for RPATH or RUNPATH
+        assert 'SPACK_LDFLAGS' in os.environ
+        assert os.environ['SPACK_LDFLAGS'] == '--disable-new-dtags'
         assert 'LDFLAGS' not in os.environ
 
         expected = set(['-DCMAKE_EXE_LINKER_FLAGS=-mthreads',