osxrelocator: Fix dyld being unable to load all our librariesHEADmain

When researching the construction of the monolithic GStreamer
library/framework (see !1466), I found that Qt applications
were totally unable to load GStreamer once deployed through
macdeployqt. In my case, I was consuming the libraries in raw form,
through a tarball I packaged myself, but @thewildtree also ran into
the same issue when testing an app that consumes the official release.

Upon looking at the libraries, I quickly realised that all libraries
had what looked like wrongly nested load commands, of the form
`@rpath/lib/libyadda.dylib`. Although the RPATH entries looked
reasonable at first glance, this is quickly not the case once the
libraries are deployed, because the @rpath of such an app will point to
the root of the Frameworks folder, and macdeployqt deploys the libraries
in raw form there.

However, that's not all the story. @thewildtree's case revealed a much
subtler and deadlier problem: the load commands themselves do not
respect Apple's convention, leading dyld(1) to kill the application on
sight. This is because, although OSXUniversalGenerator tries making the
fat libraries relocatable (correctly) by changing their ID, there's no
equivalent change made to any consumer. All load commands must equal the
ID of the dylib being loaded [1].

This is easily fixed at a given recipe's post-install time by adjusting
the library ID there, and fixing the rpaths so that they always point to
the root of the library path.

[1]: https://developer.apple.com/forums/thread/736728

Part-of: <https://gitlab.freedesktop.org/gstreamer/cerbero/-/merge_requests/1478>

3 files changed, 54 insertions, 26 deletions

diff --git a/cerbero/build/recipe.py b/cerbero/build/recipe.py
index a0b98938..30e440fe 100644
--- a/cerbero/build/recipe.py
+++ b/cerbero/build/recipe.py
@@ -577,7 +577,7 @@ SOFTWARE LICENSE COMPLIANCE.\n\n'''
             return fp.split('/')[0] in ['lib', 'bin', 'libexec'] and \
                     os.path.splitext(fp)[1] not in ['.a', '.pc', '.la']
 
-        relocator = OSXRelocator(self.config.prefix, self.config.prefix, True,
+        relocator = OSXRelocator(self.config.prefix, self.config.libdir, True,
                 logfile=self.logfile)
         # Only relocate files are that are potentially relocatable and
         # remove duplicates by symbolic links so we relocate libs only
diff --git a/cerbero/tools/osxrelocator.py b/cerbero/tools/osxrelocator.py
index 40483d70..737d72d5 100755
--- a/cerbero/tools/osxrelocator.py
+++ b/cerbero/tools/osxrelocator.py
@@ -53,6 +53,11 @@ class OSXRelocator(object):
         self.change_libs_path(object_file, original_file)
 
     def change_id(self, object_file, id=None):
+        """
+        Changes the `LC_ID_DYLIB` of the given object file.
+        @object_file: Path to the object file
+        @id: New ID; if None, it'll be `@rpath/<basename>`
+        """
         id = id or object_file.replace(self.lib_prefix, '@rpath')
         if not self._is_mach_o_file(object_file):
             return
@@ -60,42 +65,68 @@ class OSXRelocator(object):
         shell.new_call(cmd, fail=False, logfile=self.logfile)
 
     def change_libs_path(self, object_file, original_file=None):
-        # @object_file: the actual file location
-        # @original_file: where the file will end up in the output directory
-        # structure and the basis of how to calculate rpath entries.  This may
-        # be different from where the file is currently located e.g. when
-        # creating a fat binary from copy of the original file in a temporary
-        # location.
+        """
+        Sanitizes the `LC_LOAD_DYLIB` and `LC_RPATH` load commands,
+        setting the former to be of the form `@rpath/libyadda.dylib`,
+        and the latter to point to the /lib folder within the GStreamer prefix.
+        @object_file: the actual file location
+        @original_file: where the file will end up in the output directory
+        structure and the basis of how to calculate rpath entries.  This may
+        be different from where the file is currently located e.g. when
+        creating a fat binary from copy of the original file in a temporary
+        location.
+        """
+        if not self._is_mach_o_file(object_file):
+            return
         if original_file is None:
             original_file = object_file
+        # First things first: ensure the load command of future consumers
+        # points to the real ID of this library
+        # This used to be done only at Universal lipo time, but by then
+        # it's too late -- unless one wants to run through all load commands
+        self.change_id(object_file, id='@rpath/{}'.format(os.path.basename(original_file)))
+        # With that out of the way, we need to sort out how many parents
+        # need to be navigated to reach the root of the GStreamer prefix
         depth = len(original_file.split('/')) - len(self.lib_prefix.split('/')) - 1
         p_depth = '/..' * depth
-        rpaths = ['.']
-        rpaths += ['@loader_path' + p_depth, '@executable_path' + p_depth]
-        rpaths += ['@loader_path' + '/../lib', '@executable_path' + '/../lib']
-        if not self._is_mach_o_file(object_file):
-            return
+        rpaths = [
+            # From a deeply nested library
+            f'@loader_path{p_depth}',
+            # From a deeply nested framework or binary
+            f'@executable_path{p_depth}',
+            # From a library within the prefix
+            '@loader_path/../lib',
+            # From a binary within the prefix
+            '@executable_path/../lib',
+        ]
         if depth > 1:
-            rpaths += ['@loader_path/..', '@executable_path/..']
-        existing_rpaths = self.list_rpaths(object_file)
+            rpaths += [
+                # Allow loading from the parent (e.g. GIO plugin)
+                '@loader_path/..',
+                '@executable_path/..',
+            ]
+        # Make them unique
+        rpaths = list(set(rpaths))
         # Remove absolute RPATHs, we don't want or need these
-        for p in existing_rpaths:
-            if not p.startswith('/'):
-                continue
+        existing_rpaths = list(set(self.list_rpaths(object_file)))
+        for p in filter(lambda p: p.startswith('/'), self.list_rpaths(object_file)):
             cmd = [INT_CMD, '-delete_rpath', p, object_file]
             shell.new_call(cmd, fail=False)
         # Add relative RPATHs
-        for p in rpaths:
-            if p in existing_rpaths:
-                continue
+        for p in filter(lambda p: p not in existing_rpaths, rpaths):
             cmd = [INT_CMD, '-add_rpath', p, object_file]
             shell.new_call(cmd, fail=False)
-        # Change dependent library names from absolute to @rpath/
+        # Change dependencies' paths from absolute to @rpath/
         for lib in self.list_shared_libraries(object_file):
             if self.lib_prefix in lib:
                 new_lib = lib.replace(self.lib_prefix, '@rpath')
-                cmd = [INT_CMD, '-change', lib, new_lib, object_file]
-                shell.new_call(cmd, fail=False, logfile=self.logfile)
+            elif '@rpath/lib/' in lib:
+                # These are leftovers from meson thinking RPATH == prefix
+                new_lib = lib.replace('@rpath/lib/', '@rpath/')
+            else:
+                continue
+            cmd = [INT_CMD, '-change', lib, new_lib, object_file]
+            shell.new_call(cmd, fail=False, logfile=self.logfile)
 
     def change_lib_path(self, object_file, old_path, new_path):
         for lib in self.list_shared_libraries(object_file):
diff --git a/cerbero/tools/osxuniversalgenerator.py b/cerbero/tools/osxuniversalgenerator.py
index 2ad6f627..b12bc22c 100755
--- a/cerbero/tools/osxuniversalgenerator.py
+++ b/cerbero/tools/osxuniversalgenerator.py
@@ -127,10 +127,7 @@ class OSXUniversalGenerator(object):
             shutil.copy(f, tmp.name)
             prefix_to_replace = [d for d in dirs if d in f][0]
             relocator = OSXRelocator(self.output_root, prefix_to_replace, False, logfile=self.logfile)
-            # since we are using a temporary file, we must force the library id
-            # name to real one and not based on the filename
             relocator.relocate_file(tmp.name, f)
-            relocator.change_id(tmp.name, id='@rpath/{}'.format(os.path.basename(f)))
         cmd = [self.LIPO_CMD, '-create'] + [f.name for f in tmp_inputs] + ['-output', output]
         shell.new_call(cmd)
         for tmp in tmp_inputs: