Make macOS installed libraries more relocatable #26608

sethrj · 2021-10-08T15:44:47Z

Fixes #26544 and #26552 by adding a post-install step on macOS for autotools-based and package-based packages. The fixup will:

Replace hard-coded $prefix/foo.dylib paths with @rpath/foo.dylib if and only if $prefix is in the library's list of rpaths
Replace $spack_root/$other/bar.dylib with @rpath/bar.dylib and add $spack_root/$other to the rpath list
Remove duplicate rpath entries due to duplication between libtool and spack's compiler wrapper

I used the following script to successfully fix up a number of affected (already installed) libraries:

#!/usr/bin/env spack-python

import spack.store
from spack.relocate import fixup_macos_rpaths

def needs_fixup(spec):
    if spec.external or spec.virtual:
        return False
    return True

def fixup(specs):
    for spec in specs:
        fixup_macos_rpaths(spec)
        break

def main():
    specs = [s for s in spack.store.db.query() if needs_fixup(s)]
    fixup(specs)

if __name__ == '__main__':
    main()

Of course, libraries that link against the previously broken ones are unaffected since they may still contain hard-coded library IDs. But newly installed binaries from here out will not.

Multiple times on my mac, trying to install in parallel led to failures from multiple tasks trying to simultaneously create `$PREFIX/lib`.

gartung

As long as is passes spack unit-test -k bindist I am OK with the changes.

sethrj · 2021-10-09T16:15:06Z

@gartung @alalazo Please take a look at this when you have a minute. With this change, making build caches on macOS should be much easier and it should be easier for external tools to relocate spack libraries: in both cases, the only change to be made to shared libraries will be to update their rpaths.

I've tested this out on my macOS and it correctly patches my entire toolchain so far (including perl, pcre, gcc, zstd, zlib, glib, etc.), but I'll update when I can confirm that our downstream tools (and CMake's install(RUNTIME_DEPENDENCY_SET tool) is able to relocate the files.

sethrj · 2021-10-09T16:30:14Z

lib/spack/spack/package.py

+        packages) that do not install relocatable libraries by default.
+        """
+        if 'platform=darwin' not in self.spec:
+            return


Do you think it would be a good idea to add a config option to control the automatic behavior? That way we could turn it off by default if we're worried about side effects or longer build+install times.

sethrj · 2021-10-09T16:31:03Z

lib/spack/spack/relocate.py

@@ -82,7 +84,7 @@ def _patchelf():
    Return None on Darwin or if patchelf cannot be found.
    """
    # Check if patchelf is already in the PATH
-    patchelf = spack.util.executable.which('patchelf')
+    patchelf = executable.which('patchelf')


@alalazo Was this working before?

sethrj · 2021-10-09T18:46:11Z

Found one issue due to this change: the relocatable code change the absolute path of libgfortran to an rpath, but this exposes a bug related to my other lovely walk through RPATH hell, #26582 !!

Without the library fixup, code compiled/linked with gfortran links to the hard-coded absolute path to libgfortran.dylib. Thanks to Spack's extra modifications to the GCC specs file, it also gets an additional -rpath $GCC/lib:$GCC/lib64. This rpath command is not valid on macOS! The rpaths on mac are not colon-separated: they must be provided as separate commands.

Linking against libgfortran with the fixup exposes this, since for some reason the RPATH entries for libgfortran are not propagated into the downstream fortran executable. Instead, the fortran code gets linked against @rpath/libgfortran.dylib (which is what we want) but it has an invalid colon-separate rpath.

So I will update #26590 to use the correct rpath command on macOS, which should be merged before this PR.

sethrj · 2021-10-10T13:45:16Z

I'm going to examine this a bit more. It has consequences when using spack GCC Fortran as part of a mixed toolchain on macOS.

This restores the hardcoded library path for GCC.

sethrj · 2021-10-11T14:41:00Z

OK, this should be much more reliable. I'm testing this (as well as #26590) on my mac now.

@tmdelellis

After spack#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 spack#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.

@tmdelellis

…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.

sethrj added 3 commits October 8, 2021 11:33

relocate: call install_name_tool less

74feac1

zstd: fix race condition

57f299b

Multiple times on my mac, trying to install in parallel led to failures from multiple tasks trying to simultaneously create `$PREFIX/lib`.

PackageMeta: simplify callback flush

cc9d3cc

sethrj added macOS rpath labels Oct 8, 2021

sethrj requested review from alalazo and gartung October 8, 2021 15:44

spackbot-app bot added build-systems update-package labels Oct 8, 2021

spackbot-app bot requested review from haampie and michaelkuhn October 8, 2021 15:44

gartung reviewed Oct 8, 2021

View reviewed changes

spackbot-app bot added the tests General test capability(ies) label Oct 8, 2021

sethrj changed the title ~~Make macOS installed libraries more relocatable.~~ Make macOS installed libraries more relocatable Oct 8, 2021

sethrj added 4 commits October 9, 2021 10:47

Relocate: use spack.platforms instead of platform

22979e1

Relocate: code improvements

eda359c

fix zstd

e1fb743

Automatically fix rpaths for packages on macOS

9094be5

sethrj force-pushed the relocatable-libs branch from c886c85 to 9094be5 Compare October 9, 2021 14:52

sethrj requested a review from gartung October 9, 2021 15:13

sethrj commented Oct 9, 2021

View reviewed changes

gartung previously approved these changes Oct 10, 2021

View reviewed changes

Only change library IDs when the path is already in the rpath

bebb5d2

This restores the hardcoded library path for GCC.

sethrj dismissed gartung’s stale review via bebb5d2 October 11, 2021 13:22

sethrj mentioned this pull request Oct 11, 2021

gcc+binutils: rpath for zlib doesn't get propagated to lgfortran #21870

Open

3 tasks

sethrj requested a review from gartung October 11, 2021 16:34

sethrj added 2 commits October 12, 2021 05:52

Delete nonexistent rpaths and add more testing

3cebcca

Relocate: Allow @executable_path and @loader_path

d006e82

gartung approved these changes Oct 18, 2021

View reviewed changes

sethrj merged commit c48b733 into spack:develop Oct 18, 2021

sethrj deleted the relocatable-libs branch October 18, 2021 17:37

sethrj mentioned this pull request Nov 2, 2021

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

Merged

SpaceIm mentioned this pull request Jan 24, 2022

[conan-center] Add a hook to check whether installed shared libs are relocatable on Linux & macOS conan-io/hooks#376

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make macOS installed libraries more relocatable #26608

Make macOS installed libraries more relocatable #26608

sethrj commented Oct 8, 2021 •

edited

gartung left a comment

sethrj commented Oct 9, 2021 •

edited

sethrj Oct 9, 2021

sethrj Oct 9, 2021

sethrj commented Oct 9, 2021 •

edited

sethrj commented Oct 10, 2021

sethrj commented Oct 11, 2021

Make macOS installed libraries more relocatable #26608

Make macOS installed libraries more relocatable #26608

Conversation

sethrj commented Oct 8, 2021 • edited

gartung left a comment

Choose a reason for hiding this comment

sethrj commented Oct 9, 2021 • edited

sethrj Oct 9, 2021

Choose a reason for hiding this comment

sethrj Oct 9, 2021

Choose a reason for hiding this comment

sethrj commented Oct 9, 2021 • edited

sethrj commented Oct 10, 2021

sethrj commented Oct 11, 2021

sethrj commented Oct 8, 2021 •

edited

sethrj commented Oct 9, 2021 •

edited

sethrj commented Oct 9, 2021 •

edited