Ensure that dynamic lib merge targets are unique.

changes/1481.misc.rst

@@ -0,0 +1 @@
+macOS dynamic libraries are now inspected to ensure that they are single platform before merging, and they are only merged once.

src/briefcase/platforms/macOS/__init__.py

@@ -95,7 +95,22 @@ def _install_app_requirements(
                     },
                 )
         else:
-            self.logger.info("All packages are pure python or universal.")
+            self.logger.info("All packages are pure Python, or universal.")
+
+        # If given the option of a single architecture binary or a universal2 binary,
+        # pip will install the single platform binary. However, a common situation on
+        # macOS is for there to be an x86_64 binary and a universal2 binary. This means
+        # you only get a universal2 binary in the "other" install pass. This then causes
+        # problems with merging, because the "other" binary contains a copy of the
+        # architecture that the "host" platform provides.
+        #
+        # To avoid this - ensure that the libraries in the app packages for the "other"
+        # arch are all thin.
+        #
+        # This doesn't matter if it happens the other way around - if the "host" arch
+        # installs a universal binary, then the "other" arch won't be asked to install
+        # a binary at all.
+        self.thin_app_packages(other_app_packages_path, arch=other_arch)
 
         # Merge the binaries
         self.merge_app_packages(

src/briefcase/platforms/macOS/utils.py

@@ -67,6 +67,61 @@ def find_binary_packages(
 
         return binary_packages
 
+    def ensure_thin_dylib(self, path: Path, arch: str):
+        """Ensure that a library is thin, targeting a given architecture.
+
+        If the library is already thin, it is left as-is.
+
+        :param path: The library file to process.
+        :param arch: The architecture that should be preserved.
+        """
+        try:
+            output = self.tools.subprocess.check_output(
+                ["lipo", "-info", path],
+            )
+        except subprocess.CalledProcessError as e:
+            raise BriefcaseCommandError(
+                f"Unable to inspect architectures in {path}"
+            ) from e
+        else:
+            if output.startswith("Non-fat file: "):
+                if self.logger.verbosity >= 1:
+                    self.logger.info(f"{path} is already thin.")
+            elif output.startswith("Architectures in the fat file: "):
+                architectures = set(output.strip().split(":")[-1].strip().split(" "))
+                if arch in architectures:
+                    if self.logger.verbosity >= 1:
+                        self.logger.info(f"Thinning {path}")
+                    try:
+                        thin_lib_path = path.parent / f"{path.name}.{arch}"
+                        self.tools.subprocess.run(
+                            [
+                                "lipo",
+                                "-thin",
+                                arch,
+                                "-output",
+                                thin_lib_path,
+                                path,
+                            ],
+                            check=True,
+                        )
+                    except subprocess.CalledProcessError as e:
+                        raise BriefcaseCommandError(
+                            f"Unable to create thin library from {path}"
+                        ) from e
+                    else:
+                        # Having extracted the single architecture into a temporary
+                        # file, replace the original with the thin version.
+                        self.tools.shutil.move(thin_lib_path, path)
+                else:
+                    raise BriefcaseCommandError(
+                        f"{path} does not contain a {arch} slice."
+                    )
+            else:
+                raise BriefcaseCommandError(
+                    f"Unable to determine architectures in {path}"
+                )
+
     def lipo_dylib(self, relative_path: Path, target_path: Path, sources: list[Path]):
         """Create a fat library by invoking lipo on multiple source libraries.
 
@@ -105,6 +160,40 @@ def lipo_dylib(self, relative_path: Path, target_path: Path, sources: list[Path]
                 f"Unable to create fat library for {relative_path}"
             ) from e
 
+    def thin_app_packages(
+        self,
+        app_packages: Path,
+        arch: str,
+    ):
+        """Ensure that all the dylibs in a given app_packages folder are thin."""
+        dylibs = []
+        for source_path in app_packages.glob("**/*"):
+            if source_path.suffix in {".so", ".dylib"}:
+                dylibs.append(source_path)
+
+        # Call lipo on each dylib that was found to ensure it is thin.
+        if dylibs:
+            # Do this in a threadpool to make it run faster.
+            progress_bar = self.input.progress_bar()
+            self.logger.info(f"Thinning libraries in {app_packages.name}...")
+            task_id = progress_bar.add_task("Create fat libraries", total=len(dylibs))
+            with progress_bar:
+                with concurrent.futures.ThreadPoolExecutor() as executor:
+                    futures = []
+                    for path in dylibs:
+                        future = executor.submit(
+                            self.ensure_thin_dylib,
+                            path=path,
+                            arch=arch,
+                        )
+                        futures.append(future)
+                    for future in concurrent.futures.as_completed(futures):
+                        progress_bar.update(task_id, advance=1)
+                        if future.exception():
+                            raise future.exception()
+        else:
+            self.logger.info("No libraries require thinning.")
+
     def merge_app_packages(
         self,
         target_app_packages: Path,
@@ -127,7 +216,7 @@ def merge_app_packages(
         # different, but they'll be purged later anyway. Don't warn for anything in a
         # .dist-info folder either; these are going to be different because of platform
         # difference, but core package metadata should be consistent.
-        dylibs = []
+        dylibs = set()
         digests = {}
         for source_app_packages in sources:
             with self.input.wait_bar(f"Merging {source_app_packages.name}..."):
@@ -139,7 +228,7 @@ def merge_app_packages(
                     else:
                         if source_path.suffix in {".so", ".dylib"}:
                             # Dynamic libraries need to be merged; do this in a second pass.
-                            dylibs.append(relative_path)
+                            dylibs.add(relative_path)
                         elif target_path.exists():
                             # The file already exists. Check for differences; if there are any
                             # differences outside `dist-info` or `__pycache__` folders, warn

tests/platforms/linux/system/test_package__pkg.py

@@ -31,8 +31,15 @@ def package_command(first_app, tmp_path):
     # Mock the app context
     command.tools.app_tools[first_app].app_context = mock.MagicMock()
 
-    # Mock shutil move and rmtree
-    command.tools.shutil.move = mock.MagicMock()
+    # Mock shutil
+    command.tools.shutil = mock.MagicMock()
+
+    # Make the mock copy still copy
+    command.tools.shutil.copy = mock.MagicMock(side_effect=shutil.copy)
+
+    # Make the mock make_archive still package tarballs
+    command.tools.shutil.make_archive = mock.MagicMock(side_effect=shutil.make_archive)
+
     # Make the mock rmtree still remove content
     command.tools.shutil.rmtree = mock.MagicMock(side_effect=shutil.rmtree)
 

tests/platforms/linux/system/test_package__rpm.py

@@ -31,8 +31,12 @@ def package_command(first_app, tmp_path):
     # Mock the app context
     command.tools.app_tools[first_app].app_context = mock.MagicMock()
 
-    # Mock shutil move and rmtree
-    command.tools.shutil.move = mock.MagicMock()
+    # Mock shutil
+    command.tools.shutil = mock.MagicMock()
+
+    # Make the mock make_archive still package tarballs
+    command.tools.shutil.make_archive = mock.MagicMock(side_effect=shutil.make_archive)
+
     # Make the mock rmtree still remove content
     command.tools.shutil.rmtree = mock.MagicMock(side_effect=shutil.rmtree)
 

tests/platforms/macOS/app/test_create.py

@@ -55,6 +55,10 @@ def test_install_app_packages(
             ("third", "3.4.5"),
         ]
     )
+
+    # Mock the thin command so we can confirm it was invoked.
+    create_command.thin_app_packages = mock.Mock()
+
     # Mock the merge command so we can confirm it was invoked.
     create_command.merge_app_packages = mock.Mock()
 
@@ -123,6 +127,12 @@ def test_install_app_packages(
     # versions is validated as a result of the underlying install/merge methods.
     assert (bundle_path / f"app_packages.{other_arch}").is_dir()
 
+    # An attempt was made thin the "other" arch packages.
+    create_command.thin_app_packages.assert_called_once_with(
+        bundle_path / f"app_packages.{other_arch}",
+        arch=other_arch,
+    )
+
     # An attempt was made to merge packages.
     create_command.merge_app_packages.assert_called_once_with(
         target_app_packages=bundle_path
@@ -163,6 +173,9 @@ def test_install_app_packages_no_binary(
     # Mock the result of finding no binary packages.
     create_command.find_binary_packages = mock.Mock(return_value=[])
 
+    # Mock the thin command so we can confirm it was invoked.
+    create_command.thin_app_packages = mock.Mock()
+
     # Mock the merge command so we can confirm it was invoked.
     create_command.merge_app_packages = mock.Mock()
 
@@ -205,7 +218,11 @@ def test_install_app_packages_no_binary(
     # result of the underlying install/merge methods.
     assert (bundle_path / f"app_packages.{other_arch}").is_dir()
 
-    # We still need to merge the app packages; this is effectively just a copy.
+    # We still need to thin and merge the app packages; this is effectively just a copy.
+    create_command.thin_app_packages.assert_called_once_with(
+        bundle_path / f"app_packages.{other_arch}",
+        arch=other_arch,
+    )
     create_command.merge_app_packages.assert_called_once_with(
         target_app_packages=bundle_path
         / "First App.app"
@@ -238,6 +255,9 @@ def test_install_app_packages_failure(create_command, first_app_templated, tmp_p
         ]
     )
 
+    # Mock the thin command so we can confirm it was invoked.
+    create_command.thin_app_packages = mock.Mock()
+
     # Mock the merge command so we can confirm it was invoked.
     create_command.merge_app_packages = mock.Mock()
 
@@ -323,7 +343,8 @@ def test_install_app_packages_failure(create_command, first_app_templated, tmp_p
     # result of the underlying install/merge methods.
     assert (bundle_path / "app_packages.x86_64").is_dir()
 
-    # We didn't attempt to merge, because we didn't complete installing.
+    # We didn't attempt to thin or  merge, because we didn't complete installing.
+    create_command.thin_app_packages.assert_not_called()
     create_command.merge_app_packages.assert_not_called()