From 71276db4eacf8541827037c98d1dfba5af1a1925 Mon Sep 17 00:00:00 2001 From: Nico Hinderling Date: Tue, 29 Sep 2026 18:02:10 -0700 Subject: [PATCH] perf(size): Compute zip metadata size without building a zip The download-size estimate compressed the whole app bundle into a temp zip only to read back its header sizes. Those depend only on entry names and count, so compute them directly. Results are byte-identical on all bundles tested, and a large app drops from about 8s to 0.2s. zip_directory no longer needs its follow-symlinks mode, so remove it. --- .../artifacts/apple/zipped_xcarchive.py | 2 +- src/launchpad/size/utils/apple_bundle_size.py | 52 +++++-------------- src/launchpad/utils/zip_utils.py | 28 +++------- tests/unit/utils/test_zip_utils.py | 17 +----- 4 files changed, 21 insertions(+), 78 deletions(-) diff --git a/src/launchpad/artifacts/apple/zipped_xcarchive.py b/src/launchpad/artifacts/apple/zipped_xcarchive.py index c9ea2222..8d9a1a8d 100644 --- a/src/launchpad/artifacts/apple/zipped_xcarchive.py +++ b/src/launchpad/artifacts/apple/zipped_xcarchive.py @@ -234,7 +234,7 @@ def generate_ipa(self, output_path: Path): # Zip the Payload directory, preserving symlinks and permissions try: - zip_directory(payload_dir, output_path, preserve_symlinks=True) + zip_directory(payload_dir, output_path) except OSError as e: raise RuntimeError("Failed to generate IPA file") from e diff --git a/src/launchpad/size/utils/apple_bundle_size.py b/src/launchpad/size/utils/apple_bundle_size.py index e2dd2855..b9dcc028 100644 --- a/src/launchpad/size/utils/apple_bundle_size.py +++ b/src/launchpad/size/utils/apple_bundle_size.py @@ -1,8 +1,5 @@ import os import plistlib -import tempfile -import uuid -import zipfile from pathlib import Path from typing import List, NamedTuple @@ -13,7 +10,6 @@ from launchpad.size.models.common import AppComponent, ComponentType from launchpad.utils.file_utils import get_file_size, to_nearest_block_size from launchpad.utils.logging import get_logger -from launchpad.utils.zip_utils import zip_directory logger = get_logger(__name__) @@ -366,42 +362,18 @@ def _calculate_install_size(path: Path) -> int: return total_size -# We used to build this zip with Info-ZIP's `zip`, which quietly adds a UT (timestamps) and a ux -# (uid/gid) extra field to every entry: 28 bytes in the local header, 24 in the central directory. -# Python's zipfile writes neither, so add them back and the sizes we report stay where they were. -_INFOZIP_EXTRA_FIELD_BYTES_PER_ENTRY = 52 +# Per-entry bytes Info-ZIP's `zip -r` writes besides the name: local header (30), central header (46) +# and its UT/ux extra fields (52). The name appears in both headers; the end record adds 22. +_ZIP_ENTRY_OVERHEAD_BYTES = 30 + 46 + 52 +_ZIP_END_RECORD_BYTES = 22 def _zip_metadata_size_for_bundle(bundle_url: Path) -> int: - zip_file_path = Path(tempfile.gettempdir()) / f"{uuid.uuid4()}.zip" - - try: - logger.debug(f"Creating ZIP file: {zip_file_path} from {bundle_url.name}") - zip_directory(bundle_url, zip_file_path, preserve_symlinks=False) - - with zipfile.ZipFile(zip_file_path) as zf: - infos = zf.infolist() - total_compressed = sum(info.compress_size for info in infos) - - # Get actual ZIP file size - total_zip_size = os.path.getsize(zip_file_path) - - # Metadata is everything that isn't payload: headers, names, central directory, EOCD. - # Compressed bytes sit in both terms and cancel, so the compression level can't skew this. - metadata_size = total_zip_size - total_compressed + _INFOZIP_EXTRA_FIELD_BYTES_PER_ENTRY * len(infos) - - if metadata_size < 0: - logger.warning( - f"Negative metadata size calculated: {metadata_size}. ZIP size: {total_zip_size}, Compressed content: {total_compressed}" - ) - return 0 - - return metadata_size - - except Exception: - logger.exception("Error calculating ZIP metadata size") - return 0 - - finally: - if zip_file_path.exists(): - zip_file_path.unlink() + names = [] + for dirpath, dirnames, filenames in os.walk(bundle_url, followlinks=True): + real = Path(dirpath).resolve() + dirnames[:] = [d for d in dirnames if not real.is_relative_to(Path(dirpath, d).resolve())] + rel = Path(dirpath).relative_to(bundle_url.parent).as_posix() + names.append(f"{rel}/") + names.extend(f"{rel}/{f}" for f in filenames if os.path.exists(os.path.join(dirpath, f))) + return _ZIP_END_RECORD_BYTES + sum(_ZIP_ENTRY_OVERHEAD_BYTES + 2 * len(os.fsencode(n)) for n in names) diff --git a/src/launchpad/utils/zip_utils.py b/src/launchpad/utils/zip_utils.py index 5aa2f468..3de10c19 100644 --- a/src/launchpad/utils/zip_utils.py +++ b/src/launchpad/utils/zip_utils.py @@ -23,34 +23,20 @@ def _zip_info_for(arcname: str, st: os.stat_result) -> zipfile.ZipInfo: return info -def zip_directory(source_dir: Path, output_path: Path, *, preserve_symlinks: bool = True) -> None: - """Recursively zip ``source_dir`` into ``output_path``. - - Mirrors ``zip -r`` (or ``zip -r -y`` when ``preserve_symlinks`` is set): entries are - named relative to the parent of ``source_dir`` so the archive root is ``source_dir.name``, - directory entries are included, and unix permissions are preserved. - - Args: - source_dir: Directory to archive - output_path: Destination ``.zip`` path (overwritten if it exists) - preserve_symlinks: Store symlinks as symlink entries instead of following them - """ +def zip_directory(source_dir: Path, output_path: Path) -> None: + """Zip ``source_dir`` like ``zip -r -y``: symlinks kept as links, archive root is ``source_dir.name``.""" source_dir = Path(source_dir) base = source_dir.parent with zipfile.ZipFile(output_path, "w", zipfile.ZIP_DEFLATED) as zf: - _add_path(zf, source_dir, base, preserve_symlinks) + _add_path(zf, source_dir, base) -def _add_path(zf: zipfile.ZipFile, path: Path, base: Path, preserve_symlinks: bool) -> None: - try: - st = path.lstat() if preserve_symlinks else path.stat() - except FileNotFoundError: - # Dangling symlink while following links; Info-ZIP skips these too - return +def _add_path(zf: zipfile.ZipFile, path: Path, base: Path) -> None: + st = path.lstat() arcname = path.relative_to(base).as_posix() - if preserve_symlinks and stat.S_ISLNK(st.st_mode): + if stat.S_ISLNK(st.st_mode): info = _zip_info_for(arcname, st) zf.writestr(info, os.readlink(path)) return @@ -60,7 +46,7 @@ def _add_path(zf: zipfile.ZipFile, path: Path, base: Path, preserve_symlinks: bo info.external_attr |= 0x10 # MS-DOS directory flag, as Info-ZIP sets it zf.writestr(info, b"") for child in sorted(path.iterdir()): - _add_path(zf, child, base, preserve_symlinks) + _add_path(zf, child, base) return info = _zip_info_for(arcname, st) diff --git a/tests/unit/utils/test_zip_utils.py b/tests/unit/utils/test_zip_utils.py index 8036962f..c3843c55 100644 --- a/tests/unit/utils/test_zip_utils.py +++ b/tests/unit/utils/test_zip_utils.py @@ -23,7 +23,7 @@ def test_zip_directory_preserves_symlinks_and_permissions(tmp_path: Path) -> Non bundle = _make_bundle(tmp_path) out = tmp_path / "out.zip" - zip_directory(bundle, out, preserve_symlinks=True) + zip_directory(bundle, out) with zipfile.ZipFile(out) as zf: assert zf.testzip() is None @@ -39,18 +39,3 @@ def test_zip_directory_preserves_symlinks_and_permissions(tmp_path: Path) -> Non assert zf.read(link) == b"Versions/A/Foo" assert stat.S_IMODE(infos["Test.app/Test"].external_attr >> 16) == 0o755 - - -def test_zip_directory_follows_symlinks_when_requested(tmp_path: Path) -> None: - bundle = _make_bundle(tmp_path) - os.symlink("does-not-exist", bundle / "dangling") - out = tmp_path / "out.zip" - - zip_directory(bundle, out, preserve_symlinks=False) - - with zipfile.ZipFile(out) as zf: - infos = {i.filename: i for i in zf.infolist()} - link = infos["Test.app/Frameworks/Foo.framework/Foo"] - assert stat.S_ISREG(link.external_attr >> 16) - assert zf.read(link) == b"binary" * 100 - assert "Test.app/dangling" not in infos