Correct signing order for macOS bundles

automation/src/automation/bootstraps/pyside6.py

@@ -49,4 +49,15 @@ def main():
     app = QtWidgets.QApplication(sys.argv)
     main_window = {{{{ cookiecutter.class_name }}}}()
     sys.exit(app.exec())
+"""
+
+    def pyproject_table_briefcase_app_extra_content(self):
+        # Include PySide6-Addons, even though it isn't used, as a way to exercise
+        # signing of apps that include nested frameworks and apps. See #1891 for
+        # details.
+        return """
+requires = [
+    "PySide6-Essentials~=6.5",
+    "PySide6-Addons~=6.5",
+]
 """

changes/1891.bugfix.rst

@@ -0,0 +1 @@
+The order in which nested frameworks and apps are signed on macOS was corrected.

src/briefcase/integrations/file.py

@@ -34,6 +34,44 @@ def verify_install(cls, tools: ToolCache, **kwargs) -> File:
         tools.file = File(tools=tools)
         return tools.file
 
+    @classmethod
+    def sorted_depth_first(cls, paths):
+        """Sort a list of paths, so that they appear in lexical order, with
+        subdirectories of a folder sorting before files in the same directory.
+
+        :param paths: The list of paths to sort.
+        :param reverse: Should the list be returned in reverse sorting order
+        :returns: The sorted list of paths
+        """
+
+        # The sort key for a path is is a list of triples. The path is split into
+        # constituent parts; each part is converted into a triple of:
+        #
+        #    (is not a dir, is a leaf, path to this part)
+        #
+        # For example, the path "/foo/bar/whiz.txt" would have the key:
+        #
+        #  [ (0, 0, "/foo"), (0, 0, "/foo/bar"), (1, 1, "/foo/bar/whiz.txt")
+        #
+        # To see how this works, consider a comparison when sorting the contents of the
+        # folder /foo. All subfolders of /foo/bar will return (0, 0, "/foo/bar") as the
+        # second entry in the key, so they'll sort as equal, with ties being resolved by
+        # later elements in the key. The folder /foo/bar itself will have a key of (0,
+        # 1, "/foo/bar"), so it will sort *after* any subfolder content. The file
+        # /foo/something.txt will have the key (1, 1, "foo/something.txt"); this means
+        # that files in foo will come *after* any subfolders.
+        def sort_key(p):
+            return [
+                (
+                    not Path(*p.parts[:t]).is_dir(),
+                    t == len(p.parts),
+                    Path(*p.parts[:t]),
+                )
+                for t in range(len(p.parts) + 1)
+            ]
+
+        return sorted(paths, key=sort_key)
+
     def is_archive(self, filename: str | os.PathLike) -> bool:
         """Can a file be unpacked via `shutil.unpack_archive()`?
 

src/briefcase/platforms/macOS/__init__.py

@@ -602,26 +602,22 @@ def sign_app(self, app: AppConfig, identity: SigningIdentity):
         sign_targets.append(bundle_path)
 
         # Run signing through a ThreadPoolExecutor so that they run in parallel.
-        # However, we need to ensure that objects are signed from the inside out
-        # (i.e., a folder must be signed *after* all it's contents has been
-        # signed). To do this, we sort the list of signing targets in reverse
-        # lexicographic order, and then group all the signing targets by parent.
-        # This sorts all the signable files into folders; and sign all files in
-        # a folder before sorting the next group. This ensures that longer paths
-        # are signed first, and all files in a folder are signed before the
-        # folder is signed.
+        # However, we need to ensure that objects are signed from the inside out (i.e.,
+        # a folder must be signed *after* all it's contents has been signed, and files
+        # in a folder must be signed *after* all subfolders in that same folder). To do
+        # this, we sort the list of signing targets in depth-first directory order.
         #
-        # NOTE: We are relying on the fact that the final iteration order
-        # produced by groupby() reflects the order in which groups are found in
-        # the input data. The documentation for groupby() says that a new break
-        # is created every time a new group is found in the input data; sorting
-        # the input in reverse order ensures that only one group is found per folder,
-        # and that the deepest folder is found first.
+        # NOTE: We are relying on the fact that the final iteration order produced by
+        # groupby() reflects the order in which groups are found in the input data. The
+        # documentation for groupby() says that a new break is created every time a new
+        # group is found in the input data; sorting the input in reverse order ensures
+        # that only one group is found per folder, and that the deepest folder is found
+        # first.
         progress_bar = self.input.progress_bar()
         task_id = progress_bar.add_task("Signing App", total=len(sign_targets))
         with progress_bar:
             for _, names in itertools.groupby(
-                sorted(sign_targets, reverse=True),
+                self.tools.file.sorted_depth_first(sign_targets),
                 lambda name: name.parent,
             ):
                 with concurrent.futures.ThreadPoolExecutor() as executor:

tests/integrations/file/test_File__sorted_depth_first.py

@@ -0,0 +1,60 @@
+import pytest
+
+from briefcase.integrations.file import File
+
+from ...utils import create_file
+
+
+@pytest.mark.parametrize(
+    "files, sorted",
+    [
+        # Files in a directory are sorted lexically
+        (
+            ["foo/bar/a.txt", "foo/bar/c.txt", "foo/bar/b.txt"],
+            ["foo/bar/a.txt", "foo/bar/b.txt", "foo/bar/c.txt"],
+        ),
+        # Subfolders are sorted before files in that directory; but sorted lexically in themselves
+        (
+            [
+                "foo/bar/b",
+                "foo/bar/b/aaa.txt",
+                "foo/bar/b/zzz.txt",
+                "foo/bar/b/deeper",
+                "foo/bar/b/deeper/deeper_db2.txt",
+                "foo/bar/b/deeper/deeper_db1.txt",
+                "foo/bar/a.txt",
+                "foo/bar/c.txt",
+                "foo/bar/e.txt",
+                "foo/bar/d",
+                "foo/bar/d/deep_d2.txt",
+                "foo/bar/d/deep_d1.txt",
+            ],
+            [
+                "foo/bar/b/deeper/deeper_db1.txt",
+                "foo/bar/b/deeper/deeper_db2.txt",
+                "foo/bar/b/deeper",
+                "foo/bar/b/aaa.txt",
+                "foo/bar/b/zzz.txt",
+                "foo/bar/d/deep_d1.txt",
+                "foo/bar/d/deep_d2.txt",
+                "foo/bar/b",
+                "foo/bar/d",
+                "foo/bar/a.txt",
+                "foo/bar/c.txt",
+                "foo/bar/e.txt",
+            ],
+        ),
+    ],
+)
+def test_sorted_depth_first(files, sorted, tmp_path):
+    # Convert the strings into paths in the temp folder
+    paths = [tmp_path / file_path for file_path in files]
+
+    # Create all the paths that have a suffix
+    for file_path in paths:
+        if file_path.suffix:
+            create_file(tmp_path / file_path, content=str(file_path))
+
+    assert File.sorted_depth_first(paths) == [
+        tmp_path / file_path for file_path in sorted
+    ]

tests/platforms/macOS/app/test_signing.py

@@ -713,6 +713,6 @@ def _codesign(args, **kwargs):
         # coverage we need to. However, win32 doesn't handle executable permissions
         # the same as linux/unix, `unknown.binary` is identified as a signing target.
         # We ignore this discrepancy for testing purposes.
-        assert len(output.strip("\n").split("\n")) == (7 if verbose else 1)
+        assert len(output.strip("\n").split("\n")) == (11 if verbose else 1)
     else:
-        assert len(output.strip("\n").split("\n")) == (6 if verbose else 1)
+        assert len(output.strip("\n").split("\n")) == (10 if verbose else 1)