refactor: stream zip rebuild instead of buffering

scripts/deterministic_zip.py

@@ -1,34 +1,23 @@
-"""Deterministic ZIP builder for MAME BIOS archives.
+"""Deterministic ZIP rebuilding for arcade BIOS and sample archives.
 
-Creates byte-identical ZIP files from individual ROM atoms, enabling:
+A ZIP's hash depends on entry order, timestamps, compression level, and
-- Reproducible builds: same ROMs -> same ZIP hash, always
+permission bits as much as on content. Packs ship archives rebuilt with
-- Version-agnostic assembly: build neogeo.zip for any MAME version
+all of that fixed, so the same ROMs always produce the same pack hash
-- Deduplication: store ROM atoms once, assemble any ZIP on demand
+regardless of how the source set was assembled.
-
-A ZIP's hash depends on: file content, filenames, order, timestamps,
-compression, and permissions. This module fixes all metadata to produce
-deterministic output.
 
 Usage:
-    from deterministic_zip import build_deterministic_zip, extract_atoms
+    from deterministic_zip import rebuild_zip_deterministic
 
-    # Extract atoms from an existing ZIP
+    sha1 = rebuild_zip_deterministic("neogeo.zip", "out/neogeo.zip")
-    atoms = extract_atoms("neogeo.zip")
-
-    # Build a ZIP from a recipe
-    recipe = [
-        {"name": "sp-s2.sp1", "crc32": "9036d879"},
-        {"name": "000-lo.lo", "crc32": "5a86cff2"},
-    ]
-    build_deterministic_zip("neogeo.zip", recipe, atom_store)
 """
 
 from __future__ import annotations
 
 import hashlib
+import os
+import shutil
+import tempfile
 import zipfile
-import zlib
-from io import BytesIO
 from pathlib import Path
 
 # Fixed metadata for deterministic ZIPs
@@ -36,168 +25,67 @@ _FIXED_DATE_TIME = (1980, 1, 1, 0, 0, 0)  # minimum ZIP timestamp
 _FIXED_CREATE_SYSTEM = 0  # FAT/DOS (most compatible)
 _FIXED_EXTERNAL_ATTR = 0o100644 << 16  # -rw-r--r--
 _COMPRESS_LEVEL = 9  # deflate level 9 for determinism
+_COPY_CHUNK = 1024 * 1024
 
 
-def build_deterministic_zip(
+def _sha1_file(path: str | Path) -> str:
-    output_path: str | Path,
-    recipe: list[dict],
-    atom_store: dict[str, bytes],
-    compression: int = zipfile.ZIP_DEFLATED,
-) -> str:
-    """Build a deterministic ZIP from a recipe and atom store.
-
-    Args:
-        output_path: Path for the output ZIP file.
-        recipe: List of dicts with 'name' and 'crc32' (lowercase hex, no 0x).
-            Files are sorted by name for determinism.
-        atom_store: Dict mapping CRC32 (lowercase hex) to ROM binary data.
-        compression: ZIP_DEFLATED (default) or ZIP_STORED.
-
-    Returns:
-        SHA1 hex digest of the generated ZIP.
-
-    Raises:
-        KeyError: If a recipe CRC32 is not found in the atom store.
-        ValueError: If a ROM's actual CRC32 doesn't match the recipe.
-    """
-    # Sort by filename for deterministic order
-    sorted_recipe = sorted(recipe, key=lambda r: r["name"])
-
-    with zipfile.ZipFile(
-        str(output_path), "w", compression, compresslevel=_COMPRESS_LEVEL
-    ) as zf:
-        for entry in sorted_recipe:
-            name = entry["name"]
-            expected_crc = entry.get("crc32", "").lower()
-
-            if expected_crc not in atom_store:
-                raise KeyError(
-                    f"ROM atom not found: {name} (crc32={expected_crc}). "
-                    f"Available: {len(atom_store)} atoms"
-                )
-
-            data = atom_store[expected_crc]
-
-            # Verify CRC32 of the atom data
-            actual_crc = format(zlib.crc32(data) & 0xFFFFFFFF, "08x")
-            if expected_crc and actual_crc != expected_crc:
-                raise ValueError(
-                    f"CRC32 mismatch for {name}: expected {expected_crc}, got {actual_crc}"
-                )
-
-            # Create ZipInfo with fixed metadata
-            info = zipfile.ZipInfo(filename=name, date_time=_FIXED_DATE_TIME)
-            info.compress_type = compression
-            info.create_system = _FIXED_CREATE_SYSTEM
-            info.external_attr = _FIXED_EXTERNAL_ATTR
-
-            zf.writestr(info, data)
-
-    # Compute and return the ZIP's SHA1
     sha1 = hashlib.sha1()
-    with open(output_path, "rb") as f:
+    with open(path, "rb") as f:
         for chunk in iter(lambda: f.read(65536), b""):
             sha1.update(chunk)
     return sha1.hexdigest()
 
 
-def extract_atoms(zip_path: str | Path) -> dict[str, bytes]:
-    """Extract all ROM atoms from a ZIP, indexed by CRC32.
-
-    Returns: Dict mapping CRC32 (lowercase hex) to raw ROM data.
-    """
-    atoms: dict[str, bytes] = {}
-    with zipfile.ZipFile(str(zip_path), "r") as zf:
-        for info in zf.infolist():
-            if info.is_dir():
-                continue
-            data = zf.read(info.filename)
-            crc = format(zlib.crc32(data) & 0xFFFFFFFF, "08x")
-            atoms[crc] = data
-    return atoms
-
-
-def extract_atoms_with_names(zip_path: str | Path) -> list[dict]:
-    """Extract atoms with full metadata from a ZIP.
-
-    Returns: List of dicts with 'name', 'crc32', 'size', 'data'.
-    """
-    result = []
-    with zipfile.ZipFile(str(zip_path), "r") as zf:
-        for info in sorted(zf.infolist(), key=lambda i: i.filename):
-            if info.is_dir():
-                continue
-            data = zf.read(info.filename)
-            crc = format(zlib.crc32(data) & 0xFFFFFFFF, "08x")
-            result.append(
-                {
-                    "name": info.filename,
-                    "crc32": crc,
-                    "size": len(data),
-                    "data": data,
-                }
-            )
-    return result
-
-
-def verify_zip_determinism(zip_path: str | Path) -> tuple[bool, str, str]:
-    """Verify a ZIP can be rebuilt deterministically.
-
-    Extracts atoms, rebuilds the ZIP, compares hashes.
-
-    Returns: (is_deterministic, original_sha1, rebuilt_sha1)
-    """
-    # Hash the original
-    orig_sha1 = hashlib.sha1(Path(zip_path).read_bytes()).hexdigest()
-
-    # Extract atoms
-    atoms_list = extract_atoms_with_names(zip_path)
-    atom_store = {a["crc32"]: a["data"] for a in atoms_list}
-    recipe = [{"name": a["name"], "crc32": a["crc32"]} for a in atoms_list]
-
-    # Rebuild to memory
-    buf = BytesIO()
-    sorted_recipe = sorted(recipe, key=lambda r: r["name"])
-    with zipfile.ZipFile(
-        buf, "w", zipfile.ZIP_DEFLATED, compresslevel=_COMPRESS_LEVEL
-    ) as zf:
-        for entry in sorted_recipe:
-            info = zipfile.ZipInfo(filename=entry["name"], date_time=_FIXED_DATE_TIME)
-            info.compress_type = zipfile.ZIP_DEFLATED
-            info.create_system = _FIXED_CREATE_SYSTEM
-            info.external_attr = _FIXED_EXTERNAL_ATTR
-            zf.writestr(info, atom_store[entry["crc32"]])
-
-    rebuilt_sha1 = hashlib.sha1(buf.getvalue()).hexdigest()
-    return orig_sha1 == rebuilt_sha1, orig_sha1, rebuilt_sha1
-
-
 def rebuild_zip_deterministic(
     source_zip: str | Path,
     output_zip: str | Path,
 ) -> str:
-    """Rebuild an existing ZIP deterministically.
+    """Rebuild a ZIP with fixed metadata and entries sorted by name.
+
+    Copies one chunk at a time rather than loading the archive: arcade
+    sample sets reach 279 MB, and holding a whole set plus its rebuilt
+    copy in memory costs hundreds of megabytes per pack for no benefit.
+
+    Reading through ZipFile.open verifies each entry against the CRC
+    recorded in the source archive, so a corrupt entry raises
+    BadZipFile instead of being copied through.
 
-    Extracts all files, reassembles with fixed metadata.
     Returns the SHA1 of the new ZIP.
     """
-    atoms_list = extract_atoms_with_names(source_zip)
+    with zipfile.ZipFile(str(source_zip)) as src:
-    atom_store = {a["crc32"]: a["data"] for a in atoms_list}
+        entries = sorted(
-    recipe = [{"name": a["name"], "crc32": a["crc32"]} for a in atoms_list]
+            (i for i in src.infolist() if not i.is_dir()),
-    return build_deterministic_zip(output_zip, recipe, atom_store)
+            key=lambda i: i.filename,
+        )
+        with zipfile.ZipFile(
+            str(output_zip),
+            "w",
+            zipfile.ZIP_DEFLATED,
+            compresslevel=_COMPRESS_LEVEL,
+        ) as dst:
+            for info in entries:
+                out_info = zipfile.ZipInfo(
+                    filename=info.filename, date_time=_FIXED_DATE_TIME
+                )
+                out_info.compress_type = zipfile.ZIP_DEFLATED
+                out_info.create_system = _FIXED_CREATE_SYSTEM
+                out_info.external_attr = _FIXED_EXTERNAL_ATTR
+                with src.open(info) as fsrc, dst.open(out_info, "w") as fdst:
+                    shutil.copyfileobj(fsrc, fdst, _COPY_CHUNK)
+
+    return _sha1_file(output_zip)
 
 
-def build_atom_store_from_zips(zip_dir: str | Path) -> dict[str, bytes]:
+def verify_zip_determinism(zip_path: str | Path) -> tuple[bool, str, str]:
-    """Build a global atom store from all ZIPs in a directory.
+    """Check whether a ZIP already matches its deterministic rebuild.
 
-    Scans all .zip files, extracts every ROM, indexes by CRC32.
+    Returns (is_deterministic, original_sha1, rebuilt_sha1).
-    Identical ROMs (same CRC32) from different ZIPs are stored once.
     """
-    store: dict[str, bytes] = {}
+    original = _sha1_file(zip_path)
-    for zip_path in sorted(Path(zip_dir).rglob("*.zip")):
+    fd, tmp_path = tempfile.mkstemp(suffix=".zip")
-        try:
+    os.close(fd)
-            atoms = extract_atoms(zip_path)
+    try:
-            store.update(atoms)
+        rebuilt = rebuild_zip_deterministic(zip_path, tmp_path)
-        except zipfile.BadZipFile:
+    finally:
-            continue
+        Path(tmp_path).unlink(missing_ok=True)
-    return store
+    return original == rebuilt, original, rebuilt

tests/test_deterministic_zip.py

@@ -0,0 +1,156 @@
+"""Tests for deterministic ZIP rebuilding."""
+
+from __future__ import annotations
+
+import tempfile
+import unittest
+import zipfile
+from pathlib import Path
+
+from scripts.deterministic_zip import (
+    _FIXED_DATE_TIME,
+    rebuild_zip_deterministic,
+    verify_zip_determinism,
+)
+
+CONTENT = {
+    "03.wav": b"\x00\x01\x02" * 500,
+    "01.wav": b"sample one",
+    "sub/02.wav": b"sample two",
+}
+
+
+def _make_source(path: Path, date_time=(2021, 6, 5, 4, 3, 2), compression=None) -> None:
+    """Write a ZIP with deliberately non-deterministic metadata."""
+    comp = zipfile.ZIP_STORED if compression is None else compression
+    with zipfile.ZipFile(path, "w", comp) as zf:
+        for name in ("sub/02.wav", "03.wav", "01.wav"):  # unsorted on purpose
+            info = zipfile.ZipInfo(filename=name, date_time=date_time)
+            info.compress_type = comp
+            info.external_attr = 0o100777 << 16
+            info.create_system = 3  # Unix
+            zf.writestr(info, CONTENT[name])
+
+
+class TestRebuild(unittest.TestCase):
+    def setUp(self):
+        self._tmp = tempfile.TemporaryDirectory()
+        self.tmp = Path(self._tmp.name)
+        self.src = self.tmp / "src.zip"
+        _make_source(self.src)
+
+    def tearDown(self):
+        self._tmp.cleanup()
+
+    def _rebuild(self, out_name="out.zip", src=None):
+        out = self.tmp / out_name
+        sha1 = rebuild_zip_deterministic(src or self.src, out)
+        return out, sha1
+
+    def test_content_is_preserved(self):
+        out, _ = self._rebuild()
+        with zipfile.ZipFile(out) as zf:
+            for name, data in CONTENT.items():
+                self.assertEqual(zf.read(name), data)
+
+    def test_entries_sorted_by_name(self):
+        out, _ = self._rebuild()
+        with zipfile.ZipFile(out) as zf:
+            self.assertEqual(zf.namelist(), ["01.wav", "03.wav", "sub/02.wav"])
+
+    def test_metadata_is_normalized(self):
+        out, _ = self._rebuild()
+        with zipfile.ZipFile(out) as zf:
+            for info in zf.infolist():
+                self.assertEqual(info.date_time, _FIXED_DATE_TIME)
+                self.assertEqual(info.create_system, 0)
+                self.assertEqual(info.external_attr, 0o100644 << 16)
+                self.assertEqual(info.compress_type, zipfile.ZIP_DEFLATED)
+
+    def test_same_input_gives_same_hash(self):
+        _, first = self._rebuild("a.zip")
+        _, second = self._rebuild("b.zip")
+        self.assertEqual(first, second)
+
+    def test_source_metadata_does_not_affect_hash(self):
+        """Different timestamps and compression, identical rebuild."""
+        other = self.tmp / "other.zip"
+        _make_source(other, date_time=(1999, 1, 1, 1, 1, 2), compression=zipfile.ZIP_DEFLATED)
+        _, from_first = self._rebuild("a.zip")
+        _, from_other = self._rebuild("b.zip", src=other)
+        self.assertEqual(from_first, from_other)
+
+    def test_returns_sha1_of_output(self):
+        import hashlib
+
+        out, sha1 = self._rebuild()
+        self.assertEqual(sha1, hashlib.sha1(out.read_bytes()).hexdigest())
+
+    def test_directory_entries_are_dropped(self):
+        src = self.tmp / "withdir.zip"
+        with zipfile.ZipFile(src, "w") as zf:
+            zf.writestr(zipfile.ZipInfo("adir/"), b"")
+            zf.writestr("adir/file.bin", b"x")
+        out, _ = self._rebuild("nodir.zip", src=src)
+        with zipfile.ZipFile(out) as zf:
+            self.assertEqual(zf.namelist(), ["adir/file.bin"])
+
+    def test_corrupt_entry_raises(self):
+        """A tampered payload must fail the source CRC, not copy through."""
+        raw = bytearray(self.src.read_bytes())
+        marker = CONTENT["01.wav"]
+        idx = raw.find(marker)
+        self.assertGreater(idx, 0)
+        raw[idx : idx + len(marker)] = b"tampered!!"
+        corrupt = self.tmp / "corrupt.zip"
+        corrupt.write_bytes(bytes(raw))
+        with self.assertRaises(zipfile.BadZipFile):
+            rebuild_zip_deterministic(corrupt, self.tmp / "never.zip")
+
+    def test_empty_zip(self):
+        src = self.tmp / "empty.zip"
+        with zipfile.ZipFile(src, "w"):
+            pass
+        out, sha1 = self._rebuild("emptyout.zip", src=src)
+        with zipfile.ZipFile(out) as zf:
+            self.assertEqual(zf.namelist(), [])
+        self.assertTrue(sha1)
+
+    def test_larger_than_copy_chunk(self):
+        """Streaming path must handle payloads bigger than one chunk."""
+        src = self.tmp / "big.zip"
+        payload = bytes(range(256)) * 8192  # 2 MiB, two chunks
+        with zipfile.ZipFile(src, "w") as zf:
+            zf.writestr("big.bin", payload)
+        out, _ = self._rebuild("bigout.zip", src=src)
+        with zipfile.ZipFile(out) as zf:
+            self.assertEqual(zf.read("big.bin"), payload)
+
+
+class TestVerifyDeterminism(unittest.TestCase):
+    def setUp(self):
+        self._tmp = tempfile.TemporaryDirectory()
+        self.tmp = Path(self._tmp.name)
+
+    def tearDown(self):
+        self._tmp.cleanup()
+
+    def test_non_deterministic_source_reported(self):
+        src = self.tmp / "src.zip"
+        _make_source(src)
+        ok, original, rebuilt = verify_zip_determinism(src)
+        self.assertFalse(ok)
+        self.assertNotEqual(original, rebuilt)
+
+    def test_already_deterministic_source_reported(self):
+        src = self.tmp / "src.zip"
+        _make_source(src)
+        normalized = self.tmp / "norm.zip"
+        rebuild_zip_deterministic(src, normalized)
+        ok, original, rebuilt = verify_zip_determinism(normalized)
+        self.assertTrue(ok)
+        self.assertEqual(original, rebuilt)
+
+
+if __name__ == "__main__":
+    unittest.main()