From 717799b678d3445afc601c46e080352766f78b76 Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Fri, 12 Jul 2024 12:36:40 +0800 Subject: [PATCH 1/9] Add a utility for depth-first directory sorting. --- src/briefcase/integrations/file.py | 38 ++++++++++++ .../file/test_File__sorted_depth_first.py | 60 +++++++++++++++++++ 2 files changed, 98 insertions(+) create mode 100644 tests/integrations/file/test_File__sorted_depth_first.py diff --git a/src/briefcase/integrations/file.py b/src/briefcase/integrations/file.py index 3a6856da8..7263d1e8d 100644 --- a/src/briefcase/integrations/file.py +++ b/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()`? diff --git a/tests/integrations/file/test_File__sorted_depth_first.py b/tests/integrations/file/test_File__sorted_depth_first.py new file mode 100644 index 000000000..b908d989b --- /dev/null +++ b/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 [ + str(path.relative_to(tmp_path)) for path in File.sorted_depth_first(paths) + ] == sorted From 0647fa7ba72ba4f6a5672cc1b309b0a1abe5bbba Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Fri, 12 Jul 2024 12:38:14 +0800 Subject: [PATCH 2/9] Use depth-first sorting when signing macOS app bundles. --- src/briefcase/platforms/macOS/__init__.py | 26 ++++++++++------------- tests/platforms/macOS/app/test_signing.py | 4 ++-- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/src/briefcase/platforms/macOS/__init__.py b/src/briefcase/platforms/macOS/__init__.py index f0c7b8d5d..8d421f0d6 100644 --- a/src/briefcase/platforms/macOS/__init__.py +++ b/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: diff --git a/tests/platforms/macOS/app/test_signing.py b/tests/platforms/macOS/app/test_signing.py index c0e596b1c..5cf4e51b5 100644 --- a/tests/platforms/macOS/app/test_signing.py +++ b/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) From f03a8aebfeea79824f052c593dfb047c70bd4bbe Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Fri, 12 Jul 2024 12:38:25 +0800 Subject: [PATCH 3/9] Add changenote. --- changes/1891.bugfix.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/1891.bugfix.rst diff --git a/changes/1891.bugfix.rst b/changes/1891.bugfix.rst new file mode 100644 index 000000000..06fdc41de --- /dev/null +++ b/changes/1891.bugfix.rst @@ -0,0 +1 @@ +The order in which nested frameworks and apps are signed on macOS was corrected. From 3b2ca826341cc113280af2dbadac14b6291c1358 Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Fri, 12 Jul 2024 13:18:19 +0800 Subject: [PATCH 4/9] Modify test for windows compatibility. --- tests/integrations/file/test_File__sorted_depth_first.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integrations/file/test_File__sorted_depth_first.py b/tests/integrations/file/test_File__sorted_depth_first.py index b908d989b..3caeb6e11 100644 --- a/tests/integrations/file/test_File__sorted_depth_first.py +++ b/tests/integrations/file/test_File__sorted_depth_first.py @@ -55,6 +55,6 @@ def test_sorted_depth_first(files, sorted, tmp_path): if file_path.suffix: create_file(tmp_path / file_path, content=str(file_path)) - assert [ - str(path.relative_to(tmp_path)) for path in File.sorted_depth_first(paths) - ] == sorted + assert File.sorted_depth_first(paths) == [ + tmp_path / file_path for file_path in sorted + ] From cb61a725e7af4c0f21ece97f9754565d551b957e Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Fri, 12 Jul 2024 15:01:31 +0800 Subject: [PATCH 5/9] Add pytest-addons to the PySide6 automation bootstrap. --- automation/src/automation/bootstraps/pyside6.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/automation/src/automation/bootstraps/pyside6.py b/automation/src/automation/bootstraps/pyside6.py index 20e1ec110..0d723c306 100644 --- a/automation/src/automation/bootstraps/pyside6.py +++ b/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", +] """ From 3ec36b39f3601afbd2ac0163c029ce5fa5b1cf4d Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Wed, 17 Jul 2024 10:02:49 +0800 Subject: [PATCH 6/9] Corrections to docstrings, plus fix to grouping structure to allow for parallelism. --- src/briefcase/integrations/file.py | 47 +++++++++-- src/briefcase/platforms/macOS/__init__.py | 38 +++++---- .../file/test_File__sorted_depth_first.py | 15 ++++ .../test_File__sorted_depth_first_groups.py | 78 +++++++++++++++++++ 4 files changed, 154 insertions(+), 24 deletions(-) create mode 100644 tests/integrations/file/test_File__sorted_depth_first_groups.py diff --git a/src/briefcase/integrations/file.py b/src/briefcase/integrations/file.py index 7263d1e8d..1b5f1a99a 100644 --- a/src/briefcase/integrations/file.py +++ b/src/briefcase/integrations/file.py @@ -1,5 +1,6 @@ from __future__ import annotations +import itertools import os import shutil import sys @@ -7,6 +8,7 @@ from contextlib import suppress from email.message import Message from pathlib import Path +from typing import TYPE_CHECKING from urllib.parse import urlparse import requests.exceptions as requests_exceptions @@ -19,6 +21,9 @@ ) from briefcase.integrations.base import Tool, ToolCache +if TYPE_CHECKING: + from typing import Iterable, Sequence + class File(Tool): name = "file" @@ -35,12 +40,11 @@ def verify_install(cls, tools: ToolCache, **kwargs) -> File: return tools.file @classmethod - def sorted_depth_first(cls, paths): + def sorted_depth_first(cls, paths: Sequence[Path]) -> Iterable[Path]: """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 """ @@ -51,15 +55,15 @@ def sorted_depth_first(cls, paths): # # 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") + # [(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. + # later elements in the key. The folder /foo/bar itself will have a second key + # of (0, 1, "/foo/bar"), so it will sort *after* any subfolder content. The file + # /foo/something.txt will have the key entry (1, 1, "foo/something.txt"); this + # means that files in foo will come *after* any subfolders. def sort_key(p): return [ ( @@ -72,6 +76,35 @@ def sort_key(p): return sorted(paths, key=sort_key) + @classmethod + def sorted_depth_first_groups( + cls, + paths: Sequence[Path], + ) -> Iterable[Iterable[Path]]: + """Convert a list of paths into a collection of groups that are all at the same + "level", grouped depth-first. + + Subfolders in a folder will be in a separate group, returned *before* files in + the same folder. + + :param paths: The list of paths return grouped. + :returns: A generator returning a iterable groups of paths that are all at the + same sorting level. + """ + # The sort function guarantees depth first ordering, with folders before files + # at the same level. + # + # The grouping key is based on the parent of the path (so that all objects in + # the same folder group together), with an additional discriminator to ensure + # that subfolders are in a different group to files. + return ( + group_paths + for _, group_paths in itertools.groupby( + cls.sorted_depth_first(paths), + lambda path: (path.parent, path.is_dir()), + ) + ) + def is_archive(self, filename: str | os.PathLike) -> bool: """Can a file be unpacked via `shutil.unpack_archive()`? diff --git a/src/briefcase/platforms/macOS/__init__.py b/src/briefcase/platforms/macOS/__init__.py index 8d421f0d6..22ebb2195 100644 --- a/src/briefcase/platforms/macOS/__init__.py +++ b/src/briefcase/platforms/macOS/__init__.py @@ -1,7 +1,6 @@ from __future__ import annotations import concurrent.futures -import itertools import os import plistlib import re @@ -601,28 +600,33 @@ def sign_app(self, app: AppConfig, identity: SigningIdentity): # Sign the bundle path itself 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, 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. + # Run signing through a ThreadPoolExecutor so that they run in parallel. 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; files in a folder must be + # signed *after* all subfolders in that same folder); and an app that uses a + # library must be signed *after* all the libraries it uses. See + # https://developer.apple.com/documentation/xcode/creating-distribution-signed-code-for-the-mac#Determine-the-signing-order + # for details. # - # 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. + # To do this, we utilize grouping on a sorted list, and rely on the fact that a + # new group is created whenever the grouping key changes. The sorting process + # guarantees depth-first ordering; the grouping is applied over the sorted + # content, ensuring that that folders are signed before files. + # + # This approach isn't perfect. It will fail if there's an embedded app that uses + # a library in a different framework, and the app is lexically sorted before the + # library (e.g. if app_packages/foobar/Alpha.framework/Helpers/My App.app + # depends on app_packages/footbar/Beta.framework/libbeta). However, if you're + # embedding apps in frameworks in Python libraries that are installed by pip, + # you're already making poor life choices; this approach is enough to satisfy + # the one use of app-in-framework embedding that we're aware of (PySide). 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( - self.tools.file.sorted_depth_first(sign_targets), - lambda name: name.parent, - ): + for group in self.tools.file.sorted_depth_first_groups(sign_targets): with concurrent.futures.ThreadPoolExecutor() as executor: futures = [] - for path in names: + for path in group: future = executor.submit( self.sign_file, path, diff --git a/tests/integrations/file/test_File__sorted_depth_first.py b/tests/integrations/file/test_File__sorted_depth_first.py index 3caeb6e11..01153183b 100644 --- a/tests/integrations/file/test_File__sorted_depth_first.py +++ b/tests/integrations/file/test_File__sorted_depth_first.py @@ -44,6 +44,19 @@ "foo/bar/e.txt", ], ), + # If a folder contains both folders and files, the folders are returned first. + ( + [ + "foo/bar/a", + "foo/bar/b.txt", + "foo/bar/c", + ], + [ + "foo/bar/a", + "foo/bar/c", + "foo/bar/b.txt", + ], + ), ], ) def test_sorted_depth_first(files, sorted, tmp_path): @@ -54,6 +67,8 @@ def test_sorted_depth_first(files, sorted, tmp_path): for file_path in paths: if file_path.suffix: create_file(tmp_path / file_path, content=str(file_path)) + else: + file_path.mkdir(parents=True, exist_ok=True) assert File.sorted_depth_first(paths) == [ tmp_path / file_path for file_path in sorted diff --git a/tests/integrations/file/test_File__sorted_depth_first_groups.py b/tests/integrations/file/test_File__sorted_depth_first_groups.py new file mode 100644 index 000000000..c308b5ca5 --- /dev/null +++ b/tests/integrations/file/test_File__sorted_depth_first_groups.py @@ -0,0 +1,78 @@ +import pytest + +from briefcase.integrations.file import File + +from ...utils import create_file + + +@pytest.mark.parametrize( + "files, groups", + [ + # Files in a single directory are sorted lexically into the same group + ( + ["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, and in a different + # group; 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"], + ], + ), + # If a folder contains both folders and files, the folders are in a different + # group the files, and are returned first. + ( + [ + "foo/bar/a", + "foo/bar/b.txt", + "foo/bar/c", + ], + [ + [ + "foo/bar/a", + "foo/bar/c", + ], + [ + "foo/bar/b.txt", + ], + ], + ), + ], +) +def test_sorted_depth_first_groups(files, groups, tmp_path): + # Convert the strings into paths in the temp folder + paths = [tmp_path / file_path for file_path in files] + + # Create files for all paths that have a suffix; otherwise, ensure the folder + # exists. + for file_path in paths: + if file_path.suffix: + create_file(tmp_path / file_path, content=str(file_path)) + else: + file_path.mkdir(parents=True, exist_ok=True) + + assert list(list(group) for group in File.sorted_depth_first_groups(paths)) == [ + [tmp_path / file_path for file_path in group] for group in groups + ] From f20ff4bdcff2d0248764e96a9fd124d639306975 Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Thu, 18 Jul 2024 06:50:54 +0800 Subject: [PATCH 7/9] Only include PySide6-Addons when testing on macOS. --- automation/src/automation/bootstraps/pyside6.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/automation/src/automation/bootstraps/pyside6.py b/automation/src/automation/bootstraps/pyside6.py index 0d723c306..ee48f9040 100644 --- a/automation/src/automation/bootstraps/pyside6.py +++ b/automation/src/automation/bootstraps/pyside6.py @@ -51,13 +51,13 @@ def main(): 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 """ + def pyproject_table_macOS(self): + # Include PySide6-Addons on macOS, 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 """\ +universal_build = true requires = [ - "PySide6-Essentials~=6.5", "PySide6-Addons~=6.5", ] """ From ab7bb5f1154ca359f6e95a0a6131019ee8c0a7f3 Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Thu, 25 Jul 2024 07:47:50 +0800 Subject: [PATCH 8/9] Simplify depth-first sorting key. --- src/briefcase/integrations/file.py | 43 ++++++++----------- .../file/test_File__sorted_depth_first.py | 18 ++++---- .../test_File__sorted_depth_first_groups.py | 14 +++--- 3 files changed, 34 insertions(+), 41 deletions(-) diff --git a/src/briefcase/integrations/file.py b/src/briefcase/integrations/file.py index 1b5f1a99a..afdf71123 100644 --- a/src/briefcase/integrations/file.py +++ b/src/briefcase/integrations/file.py @@ -47,34 +47,23 @@ def sorted_depth_first(cls, paths: Sequence[Path]) -> Iterable[Path]: :param paths: The list of paths to sort. :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) + # The sort key for a path is a triple - the parent of the path; whether the path + # is a directory or not; and finally, the actual path. We then sort this in + # reverse order. # - # For example, the path "/foo/bar/whiz.txt" would have the key: + # Sorting by the parent of the path first (in reverse order) guarantees that + # long paths are sorted first; so a file in a folder will come *after* the + # folder it is in. # - # [(0, 0, "/foo"), (0, 0, "/foo/bar"), (1, 1, "/foo/bar/whiz.txt")] + # The second term (parent.is_dir()) guarantees that subfolders in a folder are + # found before files in the same folder (because in reverse order, booleans sort + # True before False). # - # 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 second key - # of (0, 1, "/foo/bar"), so it will sort *after* any subfolder content. The file - # /foo/something.txt will have the key entry (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) + # Lastly, we sort on the actual path itself. This provide a guaranteed lexical + # ordering inside any given folder. This isn't strictly required, but it's + # helpful for testing and reproducibility that the sort order is reliable and + # repeatable. + return sorted(paths, key=lambda p: (p.parent, p.is_dir(), p), reverse=True) @classmethod def sorted_depth_first_groups( @@ -97,6 +86,10 @@ def sorted_depth_first_groups( # The grouping key is based on the parent of the path (so that all objects in # the same folder group together), with an additional discriminator to ensure # that subfolders are in a different group to files. + # + # itertools.groupby guarantees a new group whenever the key changes; and the + # input sort order guarantees that directories come before files, so we get + # our desired sort grouping and ordering. return ( group_paths for _, group_paths in itertools.groupby( diff --git a/tests/integrations/file/test_File__sorted_depth_first.py b/tests/integrations/file/test_File__sorted_depth_first.py index 01153183b..4859541dc 100644 --- a/tests/integrations/file/test_File__sorted_depth_first.py +++ b/tests/integrations/file/test_File__sorted_depth_first.py @@ -11,7 +11,7 @@ # 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"], + ["foo/bar/c.txt", "foo/bar/b.txt", "foo/bar/a.txt"], ), # Subfolders are sorted before files in that directory; but sorted lexically in themselves ( @@ -30,18 +30,18 @@ "foo/bar/d/deep_d1.txt", ], [ - "foo/bar/b/deeper/deeper_db1.txt", + "foo/bar/d/deep_d2.txt", + "foo/bar/d/deep_d1.txt", "foo/bar/b/deeper/deeper_db2.txt", + "foo/bar/b/deeper/deeper_db1.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/b/aaa.txt", "foo/bar/d", - "foo/bar/a.txt", - "foo/bar/c.txt", + "foo/bar/b", "foo/bar/e.txt", + "foo/bar/c.txt", + "foo/bar/a.txt", ], ), # If a folder contains both folders and files, the folders are returned first. @@ -52,8 +52,8 @@ "foo/bar/c", ], [ - "foo/bar/a", "foo/bar/c", + "foo/bar/a", "foo/bar/b.txt", ], ), diff --git a/tests/integrations/file/test_File__sorted_depth_first_groups.py b/tests/integrations/file/test_File__sorted_depth_first_groups.py index c308b5ca5..b73e07020 100644 --- a/tests/integrations/file/test_File__sorted_depth_first_groups.py +++ b/tests/integrations/file/test_File__sorted_depth_first_groups.py @@ -12,7 +12,7 @@ ( ["foo/bar/a.txt", "foo/bar/c.txt", "foo/bar/b.txt"], [ - ["foo/bar/a.txt", "foo/bar/b.txt", "foo/bar/c.txt"], + ["foo/bar/c.txt", "foo/bar/b.txt", "foo/bar/a.txt"], ], ), # Subfolders are sorted before files in that directory, and in a different @@ -33,12 +33,12 @@ "foo/bar/d/deep_d1.txt", ], [ - ["foo/bar/b/deeper/deeper_db1.txt", "foo/bar/b/deeper/deeper_db2.txt"], + ["foo/bar/d/deep_d2.txt", "foo/bar/d/deep_d1.txt"], + ["foo/bar/b/deeper/deeper_db2.txt", "foo/bar/b/deeper/deeper_db1.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"], + ["foo/bar/b/zzz.txt", "foo/bar/b/aaa.txt"], + ["foo/bar/d", "foo/bar/b"], + ["foo/bar/e.txt", "foo/bar/c.txt", "foo/bar/a.txt"], ], ), # If a folder contains both folders and files, the folders are in a different @@ -51,8 +51,8 @@ ], [ [ - "foo/bar/a", "foo/bar/c", + "foo/bar/a", ], [ "foo/bar/b.txt", From e2fc32f80f31585d38b63878b0df054cb9b9f050 Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Thu, 25 Jul 2024 07:56:34 +0800 Subject: [PATCH 9/9] Correct test result dependent on sort order. --- tests/platforms/macOS/app/test_signing.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/platforms/macOS/app/test_signing.py b/tests/platforms/macOS/app/test_signing.py index 5cf4e51b5..f891c50c8 100644 --- a/tests/platforms/macOS/app/test_signing.py +++ b/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")) == (11 if verbose else 1) + assert len(output.strip("\n").split("\n")) == (9 if verbose else 1) else: - assert len(output.strip("\n").split("\n")) == (10 if verbose else 1) + assert len(output.strip("\n").split("\n")) == (8 if verbose else 1)