relocate: do not change library id to use rpaths on package install (#27139)

After #26608 I got a report about missing rpaths when building a
downstream package independently using a spack-installed toolchain
(@tmdelellis). This occurred because the spack-installed libraries were
being linked into the downstream app, but the rpaths were not being
manually added. Prior to #26608 autotools-installed libs would retain
their hard-coded path and would thus propagate their link information
into the downstream library on mac.

We could solve this problem *if* the mac linker (ld) respected
`LD_RUN_PATH` like it does on GNU systems, i.e. adding `rpath` entries
to each item in the environment variable. However on mac we would have
to manually add rpaths either using spack's compiler wrapper scripts or
manually (e.g. using `CMAKE_BUILD_RPATH` and pointing to the libraries of
all the autotools-installed spack libraries).

The easier and safer thing to do for now is to simply stop changing the
dylib IDs.

3 files changed, 7 insertions, 23 deletions

diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index c22898c5a3..8e1e81c124 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -1974,11 +1974,9 @@ class PackageBase(six.with_metaclass(PackageMeta, PackageViewMixin, object)):
         """On Darwin, make installed libraries more easily relocatable.
 
         Some build systems (handrolled, autotools, makefiles) can set their own
-        rpaths that are duplicated by spack's compiler wrapper. Additionally,
-        many simpler build systems do not link using ``-install_name
-        @rpath/foo.dylib``, which propagates the library's hardcoded
-        absolute path into downstream dependencies. This fixup interrogates,
-        and postprocesses if necessary, all libraries installed by the code.
+        rpaths that are duplicated by spack's compiler wrapper. This fixup
+        interrogates, and postprocesses if necessary, all libraries installed
+        by the code.
 
         It should be added as a @run_after to packaging systems (or individual
         packages) that do not install relocatable libraries by default.
diff --git a/lib/spack/spack/relocate.py b/lib/spack/spack/relocate.py
index 212223e7e0..78c5d72b23 100644
--- a/lib/spack/spack/relocate.py
+++ b/lib/spack/spack/relocate.py
@@ -379,6 +379,8 @@ def macholib_get_paths(cur_path):
             # Reproduce original behavior of only returning the last mach-O
             # header section
             tty.warn("Encountered fat binary: {0}".format(cur_path))
+        if headers[-1].filetype == 'dylib_stub':
+            tty.warn("File is a stub, not a full library: {0}".format(cur_path))
         commands = headers[-1].commands
 
     LC_ID_DYLIB = macholib.mach_o.LC_ID_DYLIB
@@ -1071,18 +1073,6 @@ def fixup_macos_rpath(root, filename):
             ))
             del_rpaths.add(rpath)
 
-    # Check for relocatable ID
-    if id_dylib is None:
-        tty.debug("No dylib ID is set for {0}".format(abspath))
-    elif not id_dylib.startswith('@'):
-        tty.debug("Non-relocatable dylib ID for {0}: {1}"
-                  .format(abspath, id_dylib))
-        if root in rpaths or root in add_rpaths:
-            args += ['-id', '@rpath/' + filename]
-        else:
-            tty.debug("Allowing hardcoded dylib ID because its rpath "
-                      "is *not* in the library already")
-
     # Delete bad rpaths
     for rpath in del_rpaths:
         args += ['-delete_rpath', rpath]
@@ -1101,16 +1091,13 @@ def fixup_macos_rpath(root, filename):
 
 
 def fixup_macos_rpaths(spec):
-    """Remove duplicate rpaths and make shared library IDs relocatable.
+    """Remove duplicate and nonexistent rpaths.
 
     Some autotools packages write their own ``-rpath`` entries in addition to
     those implicitly added by the Spack compiler wrappers. On Linux these
     duplicate rpaths are eliminated, but on macOS they result in multiple
     entries which makes it harder to adjust with ``install_name_tool
     -delete_rpath``.
-
-    Furthermore, many autotools programs (on macOS) set a library's install
-    paths to use absolute paths rather than relative paths.
     """
     if spec.external or spec.virtual:
         tty.warn('external or virtual package cannot be fixed up: {0!s}'
diff --git a/lib/spack/spack/test/relocate.py b/lib/spack/spack/test/relocate.py
index 386ca88d2d..1e4b771fa6 100644
--- a/lib/spack/spack/test/relocate.py
+++ b/lib/spack/spack/test/relocate.py
@@ -469,9 +469,8 @@ def test_fixup_macos_rpaths(make_dylib, make_object_file):
     assert fixup_rpath(root, filename)
     assert not fixup_rpath(root, filename)
 
-    # Bad but relocatable library id
+    # Hardcoded but relocatable library id (but we do NOT relocate)
     (root, filename) = make_dylib("abs_with_rpath", no_rpath)
-    assert fixup_rpath(root, filename)
     assert not fixup_rpath(root, filename)
 
     # Library id uses rpath but there are extra duplicate rpaths