Reimplement @rpath support for macOS

Author: jimporter

CHANGES.md

@@ -9,6 +9,8 @@
   result of `find_files` hasn't actually changed
 - Add support for system headers when compiling with MSVC
 - New `--verbose` option for configuration
+- The `install_name` for libraries on macOS now (again) uses `@rpath`-based
+  paths prior to installation
 
 ### Bug fixes
 - Automatically update dependencies via `mopack` if the toolchain file has

bfg9000/builtins/pkg_config.py

@@ -14,7 +14,7 @@
 from ..safe_str import literal, shell_literal
 from ..shell import posix as pshell
 from ..shell.syntax import Syntax, Writer
-from ..tools.install_name_tool import darwin_install_name
+from ..tools.install_name_tool import install_name as darwin_install_name
 from ..tools.pkg_config import GeneratedPkgConfigPackage, PkgConfigPackage
 from ..versioning import simplify_specifiers, Specifier, SpecifierSet
 
@@ -423,7 +423,7 @@ def _write(self, out, data, installed):
         if builder.object_format == 'mach-o':
             all_libs = flatten(i.all for i in data['libs'])
             install_names = filter(None, (darwin_install_name(
-                installify_fn(i), env, strict=False
+                env, installify_fn(i), strict=False
             ) for i in all_libs))
             self._write_variable(out, 'install_names', install_names,
                                  Syntax.shell)

bfg9000/tools/cc/linker.py

@@ -119,33 +119,38 @@ def _always_flags(self):
         return []
 
     def _local_rpath(self, library, output):
-        if ( not isinstance(library, Library) or
-             self.builder.object_format != 'elf' ):
+        if not isinstance(library, Library):
             return None, []
-
-        rpath = patchelf.local_rpath(self.env, library, output)
-        if rpath is None:
+        elif self.builder.object_format == 'elf':
+            rpath = patchelf.local_rpath(self.env, library, output)
+            if rpath is None:
+                return None, []
+
+            # Prior to binutils 2.28, GNU's BFD-based ld doesn't correctly
+            # respect $ORIGIN in a shared library's DT_RPATH/DT_RUNPATH field.
+            # This results in ld being unable to find other shared libraries
+            # needed by the directly-linked library. For more information, see:
+            # <https://sourceware.org/bugzilla/show_bug.cgi?id=20535>.
+            try:
+                ld = self.builder.linker('raw')
+                fix_rpath = (ld.brand == 'bfd' and ld.version in
+                             SpecifierSet('<2.28'))
+            except KeyError:
+                fix_rpath = False
+
+            rpath_link = []
+            if output and fix_rpath:
+                rpath_link = [
+                    i.path.parent() for i in
+                    recursive_walk(library.runtime_file, 'runtime_deps')
+                ]
+
+            return rpath, rpath_link
+        elif self.builder.object_format == 'mach-o':
+            return install_name_tool.local_rpath(self.env, library, output), []
+        else:
             return None, []
 
-        # Prior to binutils 2.28, GNU's BFD-based ld doesn't correctly respect
-        # $ORIGIN in a shared library's DT_RPATH/DT_RUNPATH field. This results
-        # in ld being unable to find other shared libraries needed by the
-        # directly-linked library. For more information, see:
-        # <https://sourceware.org/bugzilla/show_bug.cgi?id=20535>.
-        try:
-            ld = self.builder.linker('raw')
-            fix_rpath = (ld.brand == 'bfd' and ld.version in
-                         SpecifierSet('<2.28'))
-        except KeyError:
-            fix_rpath = False
-
-        rpath_link = []
-        if output and fix_rpath:
-            rpath_link = [i.path.parent() for i in
-                          recursive_walk(library.runtime_file, 'runtime_deps')]
-
-        return rpath, rpath_link
-
     def always_libs(self, primary):
         # XXX: Don't just asssume that these are the right libraries to use.
         # For instance, clang users might want to use libc++ instead.
@@ -280,8 +285,13 @@ def flags(self, options, global_options=None, output=None, mode='normal'):
 
         flags.extend('-L' + i for i in uniques(lib_dirs))
         if rpaths:
-            flags.append('-Wl,-rpath,' + safe_str.join(rpaths, ':'))
+            rpaths = uniques(rpaths)
+            if self.builder.linker('raw').brand == 'apple':
+                flags.extend('-Wl,-rpath,' + i for i in rpaths)
+            else:
+                flags.append('-Wl,-rpath,' + safe_str.join(rpaths, ':'))
         if rpath_links:
+            rpath_links = uniques(rpath_links)
             flags.append('-Wl,-rpath-link,' + safe_str.join(rpath_links, ':'))
         return flags
 
@@ -380,9 +390,8 @@ def _always_flags(self):
 
     def _soname(self, library):
         if self.env.target_platform.genus == 'darwin':
-            return ['-install_name', install_name_tool.darwin_install_name(
-                library, self.env
-            )]
+            install_name = install_name_tool.install_name(self.env, library)
+            return ['-install_name', install_name]
         else:
             return ['-Wl,-soname,' + library.runtime_file.path.basename()]
 

bfg9000/tools/install_name_tool.py

@@ -1,7 +1,10 @@
+import os
+
 from . import tool
 from .. import file_types, options as opts
 from .common import SimpleCommand
-from ..iterutils import flatten, listify
+from ..iterutils import flatten
+from ..path import BasePath, Path, Root, InstallRoot
 
 
 @tool('install_name_tool')
@@ -14,37 +17,100 @@ def __init__(self, env):
             default='install_name_tool'
         )
 
-    def _call(self, cmd, file, *, id=None, changes=[]):
+    def _call(self, cmd, file, *, id=None, changes=[], rpaths=[]):
         args = []
         if id:
             args += ['-id', id]
-        args += flatten(['-change'] + listify(i) for i in changes)
+        args += flatten(['-change', old, new] for old, new in changes)
+        for old, new in rpaths:
+            if old and new:
+                args += ['-rpath', old, new]
+            elif new:
+                args += ['-add_rpath', new]
+            elif old:
+                args += ['-delete_rpath', old]
 
         if args:
             return cmd + args + [file]
 
 
-def darwin_install_name(library, env, strict=True):
-    while isinstance(library, file_types.LinkLibrary):
-        library = library.library
+def install_name(env, library, *, strict=True):
+    if library.runtime_file:
+        path = library.runtime_file.path
+        if path.root in InstallRoot:
+            return path.cross(env)
+        else:
+            return os.path.join('@rpath', path.suffix)
+    elif not strict:
+        return None
+
+    raise TypeError('unable to create install_name')  # pragma: no cover
+
+
+def local_rpath(env, library, output):
+    if not library.runtime_file:
+        return None
+
+    rpath = library.runtime_file.path.parent().cross(env)
+    if rpath.root != Root.absolute and rpath.root not in InstallRoot:
+        if not output:
+            raise ValueError('unable to construct runtime search path')
+        # This should almost always be true, except for when linking to a
+        # shared library stored in the srcdir.
+        if rpath.root == output.path.root:
+            rpath = Path('.', rpath.root).relpath(output.path.parent(),
+                                                  prefix='@loader_path')
+    return rpath
+
 
-    if isinstance(library, file_types.VersionedSharedLibrary):
-        return library.soname.path.string(env.base_dirs)
-    elif isinstance(library, file_types.SharedLibrary):
-        return library.path.string(env.base_dirs)
-    elif strict:  # pragma: no cover
-        raise TypeError('unable to create darwin install_name')
-    else:
+def installed_rpath(env, library, install_db):
+    if not library.runtime_file:
         return None
+    try:
+        return install_db.target[library.runtime_file].path.parent()
+    except KeyError:
+        if library.runtime_file.path.root == Root.absolute:
+            return library.runtime_file.path.parent()
+        raise
 
 
 def post_install(env, options, output, install_db, *, is_library=False):
+    new_id = None
+    if is_library:
+        installed_id = install_name(env, install_db.host[output])
+        if installed_id != install_name(env, output):
+            new_id = installed_id
+
     change_opts = options.filter(opts.install_name_change)
     changes = ([(i.old, i.new) for i in change_opts] +
-               [(darwin_install_name(i, env), install_db.target[i].path)
+               [(install_name(env, i), install_name(env, install_db.target[i]))
                 for i in output.runtime_deps])
 
+    # Compute all the changed rpaths, preserving the original order. To do
+    # this, add all the entries into a dict with the key being the old rpath;
+    # for new entries, set the key to a sentinel value for uniqueness, which
+    # we'll remove at the end.
+    new_rpaths = {}
+    sentinel = 1
+    for i in options:
+        if isinstance(i, opts.lib):
+            if not isinstance(i.library, file_types.Library):
+                continue
+            local = local_rpath(env, i.library, output)
+            if local is not None:
+                installed = installed_rpath(env, i.library, install_db)
+                if not isinstance(local, BasePath) or local != installed:
+                    new_rpaths[local] = installed
+        elif isinstance(i, opts.rpath_dir):
+            if i.when == opts.RpathWhen.installed:
+                new_rpaths[sentinel] = i.path
+                sentinel += 1
+            elif i.when == opts.RpathWhen.uninstalled:
+                new_rpaths[i.path] = None
+    new_rpaths = ((k if not isinstance(k, int) else None, v)
+                  for k, v in new_rpaths.items())
+
     path = install_db.host[output].path
     return env.tool('install_name_tool')(
-        path, id=path.cross(env) if is_library else None, changes=changes
+        path, id=new_id, changes=changes, rpaths=new_rpaths
     )

bfg9000/tools/pkg_config.py

@@ -235,7 +235,8 @@ def link_options(self, linker, *, raw=False):
                 # should check if there are any install_names set that we need
                 # to update when installing. For more information, see the
                 # pkg-config builtin.
-                extra_opts = self._get_install_name_changes()
+                extra_opts = (self._get_rpaths() +
+                              self._get_install_name_changes())
 
         return flags + libdirs + libs + extra_opts
 

doc/about/changes.md

@@ -11,6 +11,8 @@ in progress
   result of `find_files` hasn't actually changed
 - Add support for system headers when compiling with MSVC
 - New `--verbose` option for configuration
+- The `install_name` for libraries on macOS now (again) uses `@rpath`-based
+  paths prior to installation
 
 ### Bug fixes
 - Automatically update dependencies via `mopack` if the toolchain file has

test/unit/tools/cc/test_cc.py

@@ -273,8 +273,8 @@ def check_package(self, pkg):
         self.assertEqual(pkg.link_options(self.linker), option_list(
             '-pthread', opts.lib_dir(Directory(Path('/path'))),
             opts.lib_literal('-lfoo'),
-            (opts.rpath_dir(Path('/path')) if self.platform_name == 'linux'
-             else None)
+            (opts.rpath_dir(Path('/path'))
+             if self.platform_name in ['linux', 'macos'] else None)
         ))
 
     def test_resolve_pkg_config(self):

test/unit/tools/cc/test_linker.py

@@ -97,7 +97,7 @@ def test_flags_empty(self):
         self.assertEqual(self.linker.flags(opts.option_list()), [])
 
     def test_flags_lib_dir(self):
-        libdir = self.Path('/path/to/lib')
+        libdir = self.Path('/path/to/lib/')
         lib = self.Path('/path/to/lib/libfoo.a')
         srclibdir = self.Path('.', Root.srcdir)
         srclib = self.Path('libfoo.a', Root.srcdir)
@@ -107,15 +107,14 @@ def test_flags_lib_dir(self):
         if self.shared:
             output = SharedLibrary(self.Path('out'), 'native')
             if self.env.target_platform.genus == 'darwin':
-                soname = ['-install_name',
-                          self.Path('out').string(self.env.base_dirs)]
+                soname = ['-install_name', os.path.join('@rpath', 'out')]
             else:
                 soname = ['-Wl,-soname,out']
         else:
             output = Executable(self.Path('exe'), 'native')
             soname = []
 
-        if self.env.target_platform.genus == 'linux':
+        if self.env.target_platform.genus in ['linux', 'darwin']:
             rpath = ['-Wl,-rpath,' + libdir]
             srcdir_rpath = ['-Wl,-rpath,' + srclibdir]
         else:
@@ -493,8 +492,10 @@ def test_post_installed_macos(self):
             self.assertEqual(fn(install_outputs), (
                 [install_name_tool] +
                 (['-id', installed.cross(self.env)] if self.shared else []) +
-                ['-change', deplib.path.string(self.env.base_dirs),
-                 installify(deplib, cross=self.env).path, installed]
+                ['-change', os.path.join('@rpath', deplib.path.suffix),
+                 installify(deplib, cross=self.env).path,
+                 '-rpath', '@loader_path', self.Path('.', InstallRoot.libdir),
+                 installed]
             ))
 
 

test/unit/tools/msvc/test_msvc.py

@@ -172,8 +172,8 @@ def check_package(self, pkg):
         self.assertEqual(pkg.link_options(self.linker), option_list(
             '/DEBUG', opts.lib_dir(Directory(Path('/path'))),
             opts.lib_literal('foo.lib'),
-            (opts.rpath_dir(Path('/path')) if self.platform_name == 'linux'
-             else None)
+            (opts.rpath_dir(Path('/path'))
+             if self.platform_name in ['linux', 'macos'] else None)
         ))
 
     def test_lang(self):