feat: mark each pack part with its siblings

scripts/generate_readme.py

@@ -236,7 +236,9 @@ def download_links(
     """Markdown links to the files of a platform's pack in the recorded release.
 
     A pack published whole gets one link labelled *label*. A pack in parts
-    gets one link per part, since every part is needed. *repo* is the
+    gets one link per part, since every part is needed, after the number of
+    files to download: a reader who took one link for the pack extracted
+    half of it. *repo* is the
     repository URL, absolute or relative, and *attrs* an attribute list
     put after each link, or *button* the function that draws each link as a
     button on the site. Without a recorded release the link is the latest
@@ -263,8 +265,8 @@ def download_links(
         for number, url in enumerate(urls, 1)
     ]
     if styled:
-        return " ".join(parts)
-    return f"{label}: " + ", ".join(parts)
+        return f"{len(urls)} files to download, all needed: " + " ".join(parts)
+    return f"{label}, {len(urls)} files, all needed: " + ", ".join(parts)
 
 
 def extract_targets(platforms_dir: str) -> list[tuple[str, str]]:

scripts/release_notes.py

@@ -63,7 +63,8 @@ def _links(tag: str, assets: list[str]) -> str:
     base = f"https://github.com/{REPO}/releases/download/{tag}"
     if len(assets) == 1:
         return f"[{assets[0]}]({base}/{assets[0]})"
-    return " + ".join(f"[part {n}]({base}/{asset})" for n, asset in enumerate(assets, 1))
+    parts = " + ".join(f"[part {n}]({base}/{asset})" for n, asset in enumerate(assets, 1))
+    return f"{len(assets)} files, all needed: {parts}"
 
 
 def render(record: dict, platforms_dir: str, whats_new: str) -> str:

scripts/release_record.py

@@ -103,8 +103,11 @@ def _read_record(dist: Path, tag: str, owners: dict[str, list[str]]) -> dict:
         members: dict[str, int] = {}
         for path in paths:
             with zipfile.ZipFile(path) as archive:
+                # A part's marker names its siblings and is not pack content:
+                # the manifest's pack_files is counted before any split.
                 for info in archive.infolist():
-                    members[info.filename] = info.file_size
+                    if not split_pack.is_marker(info.filename):
+                        members[info.filename] = info.file_size
         packs[name] = {
             "files": len(members),
             "extracted_size": sum(members.values()),

scripts/split_pack.py

@@ -6,6 +6,11 @@ as parts, each a complete archive of whole files: any tool opens one, and
 the parts extracted into the same folder are the pack. Members are copied
 as stored, never recompressed, so the parts depend on the pack alone.
 
+Each part also carries a short text file, its marker, at the archive root:
+the pack it belongs to, its number, and the name and size of every part.
+Extracted with the files, the markers let `install.py --check` name a part
+that is missing from the folder.
+
 Usage:
     python scripts/split_pack.py dist/
     python scripts/split_pack.py dist/RetroArch_v1.22.2_BIOS_Pack.zip
@@ -20,6 +25,7 @@ import re
 import struct
 import sys
 import zipfile
+import zlib
 from pathlib import Path
 
 from artifacts import ArtifactLockBusy, artifact_lock
@@ -29,6 +35,10 @@ ASSET_LIMIT = 2 * 1024**3
 
 PACK_SUFFIX = "_BIOS_Pack.zip"
 _PART_NAME = re.compile(r"^(?P<stem>.+_BIOS_Pack)\.part(?P<index>\d+)of(?P<count>\d+)\.zip$")
+_MARKER_NAME = re.compile(r"^(?P<stem>.+_BIOS_Pack)\.part(?P<index>\d+)of(?P<count>\d+)\.txt$")
+# Sizes are written at a fixed width, so a marker's length, and with it the
+# size of its own part, does not depend on the sizes it records.
+_SIZE_WIDTH = 13
 
 _LOCAL = struct.Struct("<4s2B4HL2L2H")
 _CENTRAL = struct.Struct("<4s4B4HL2L5H2L")
@@ -58,6 +68,44 @@ def part_name(pack_name: str, index: int, count: int) -> str:
     return f"{pack_name[: -len('.zip')]}.part{index}of{count}.zip"
 
 
+def marker_name(pack_name: str, index: int, count: int) -> str:
+    return f"{pack_name[: -len('.zip')]}.part{index}of{count}.txt"
+
+
+def is_marker(name: str) -> bool:
+    """Whether an archive member is the marker a split writes in each part."""
+    return _MARKER_NAME.match(name) is not None
+
+
+def marker_text(pack_name: str, index: int, sizes: list[int]) -> bytes:
+    """The marker of part *index* of a pack published in ``len(sizes)`` parts.
+
+    Plain text a person can read, in a form install.py parses back: the
+    lines that matter are `pack:`, `part:` and one `file:` per part.
+    """
+    count = len(sizes)
+    lines = [
+        "RetroBIOS split pack",
+        f"This folder holds part {index} of {count} of {pack_name}.",
+        "Every part is needed: extract them all into this same folder.",
+        "The installer's --check names a part missing from the folder.",
+        "",
+        f"pack: {pack_name}",
+        f"part: {index} of {count}",
+    ]
+    lines += [
+        f"file: {size:>{_SIZE_WIDTH}} {part_name(pack_name, number, count)}"
+        for number, size in enumerate(sizes, 1)
+    ]
+    return ("\n".join(lines) + "\n").encode("utf-8")
+
+
+def _marker_weight(pack_name: str, index: int, count: int) -> int:
+    name = len(marker_name(pack_name, index, count).encode("utf-8"))
+    body = len(marker_text(pack_name, index, [0] * count))
+    return _LOCAL.size + name + body + _CENTRAL.size + name
+
+
 def _name_bytes(info: zipfile.ZipInfo) -> bytes:
     encoding = "utf-8" if info.flag_bits & _UTF8_NAME else "cp437"
     return info.orig_filename.encode(encoding)
@@ -70,23 +118,24 @@ def _weight(info: zipfile.ZipInfo) -> int:
 
 
 def plan_parts(
-    members: list[zipfile.ZipInfo], limit: int
+    members: list[zipfile.ZipInfo], limit: int, reserve: int = 0
 ) -> list[list[zipfile.ZipInfo]]:
-    """Fill each part in archive order, up to the limit."""
+    """Fill each part in archive order, up to the limit less *reserve*."""
     parts: list[list[zipfile.ZipInfo]] = [[]]
-    size = _END.size
+    floor = _END.size + reserve
+    size = floor
     for info in members:
         weight = _weight(info)
-        if _END.size + weight >= limit:
+        if floor + weight >= limit:
             raise ValueError(
                 f"{info.filename} needs {weight:,} bytes, more than a part of "
                 f"{limit:,} can hold"
             )
         if info.compress_size > _MAX_FIELD or info.file_size > _MAX_FIELD:
             raise ValueError(f"{info.filename} needs ZIP64, which a part does not use")
-        if size + weight >= limit or len(parts[-1]) >= _MAX_ENTRIES:
+        if size + weight >= limit or len(parts[-1]) >= _MAX_ENTRIES - 1:
             parts.append([])
-            size = _END.size
+            size = floor
         parts[-1].append(info)
         size += weight
     return parts
@@ -109,12 +158,24 @@ def _data_offset(source: io.BufferedReader, info: zipfile.ZipInfo) -> int:
     return info.header_offset + _LOCAL.size + header[10] + header[11]
 
 
+def _marker_info(name: str, body: bytes) -> zipfile.ZipInfo:
+    info = zipfile.ZipInfo(name, date_time=(1980, 1, 1, 0, 0, 0))
+    info.compress_type = zipfile.ZIP_STORED
+    info.CRC = zlib.crc32(body)
+    info.compress_size = info.file_size = len(body)
+    info.create_system = 3
+    info.external_attr = (0o100000 | 0o644) << 16
+    return info
+
+
 def _write_part(
-    source: io.BufferedReader, members: list[zipfile.ZipInfo], dest: Path
+    source: io.BufferedReader, members: list[zipfile.ZipInfo], dest: Path,
+    marker: tuple[str, bytes],
 ) -> None:
     directory = []
+    marker_info = _marker_info(*marker)
     with open(dest, "wb") as out:
-        for info in members:
+        for info in [marker_info, *members]:
             name = _name_bytes(info)
             flags = info.flag_bits & ~_DATA_DESCRIPTOR
             dos_time, dos_date = _dos_stamp(info)
@@ -127,8 +188,12 @@ def _write_part(
                 )
             )
             out.write(name)
-            source.seek(_data_offset(source, info))
-            remaining = info.compress_size
+            if info is marker_info:
+                out.write(marker[1])
+                remaining = 0
+            else:
+                source.seek(_data_offset(source, info))
+                remaining = info.compress_size
             while remaining:
                 chunk = source.read(min(_CHUNK, remaining))
                 if not chunk:
@@ -157,7 +222,33 @@ def _write_part(
 
 
 def _identity(archive: zipfile.ZipFile) -> list[tuple[str, int, int]]:
-    return [(info.filename, info.CRC, info.file_size) for info in archive.infolist()]
+    return [
+        (info.filename, info.CRC, info.file_size)
+        for info in archive.infolist()
+        if not is_marker(info.filename)
+    ]
+
+
+def marker_weight(pack_name: str, count: int) -> int:
+    """Bytes the largest marker of a pack in *count* parts adds to its part."""
+    return max(_marker_weight(pack_name, index, count) for index in (1, count))
+
+
+def _plan_with_markers(
+    pack_name: str, members: list[zipfile.ZipInfo], limit: int
+) -> list[list[zipfile.ZipInfo]]:
+    """Plan the parts with room for the marker each one carries.
+
+    The marker lists every part, so its length grows with their count: the
+    plan is made again until the count it assumed is the count it gives.
+    """
+    count = 1
+    while True:
+        reserve = marker_weight(pack_name, count)
+        plan = plan_parts(members, limit, reserve)
+        if len(plan) <= count:
+            return plan
+        count = len(plan)
 
 
 def split_pack(zip_path: Path, limit: int = ASSET_LIMIT) -> list[Path]:
@@ -174,15 +265,31 @@ def split_pack(zip_path: Path, limit: int = ASSET_LIMIT) -> list[Path]:
     with zipfile.ZipFile(zip_path) as archive:
         members = archive.infolist()
         expected = _identity(archive)
-    plan = plan_parts(members, limit)
+    taken = [info.filename for info in members if is_marker(info.filename)]
+    if taken:
+        raise ValueError(f"{zip_path.name} already holds {taken[0]}, a part marker name")
+    plan = _plan_with_markers(zip_path.name, members, limit)
+    count = len(plan)
     parts = [
-        zip_path.with_name(part_name(zip_path.name, index, len(plan)))
-        for index in range(1, len(plan) + 1)
+        zip_path.with_name(part_name(zip_path.name, index, count))
+        for index in range(1, count + 1)
+    ]
+    sizes = [
+        _END.size + _marker_weight(zip_path.name, index, count)
+        + sum(_weight(info) for info in chosen)
+        for index, chosen in enumerate(plan, 1)
     ]
     try:
         with open(zip_path, "rb") as source:
-            for dest, chosen in zip(parts, plan):
-                _write_part(source, chosen, dest)
+            for index, (dest, chosen) in enumerate(zip(parts, plan), 1):
+                marker = (marker_name(zip_path.name, index, count),
+                          marker_text(zip_path.name, index, sizes))
+                _write_part(source, chosen, dest, marker)
+                if dest.stat().st_size != sizes[index - 1]:
+                    raise ValueError(
+                        f"{dest.name} is {dest.stat().st_size:,} bytes, its marker "
+                        f"says {sizes[index - 1]:,}"
+                    )
         found: list[tuple[str, int, int]] = []
         for dest in parts:
             with zipfile.ZipFile(dest) as archive:

tests/test_doc_surfaces.py

@@ -111,6 +111,15 @@ class DownloadLinksNameTheFile(unittest.TestCase):
             )
         self.assertIn("part 1 of 2", cell)
 
+    def test_a_split_pack_says_how_many_files_to_download(self):
+        """One link taken for the pack left half of it out of the folder."""
+        self.assertIn(
+            "Download, 2 files, all needed: ",
+            download_links("batocera", SPLIT_RECORD, "../..", "Download"),
+        )
+        buttons = download_links("batocera", SPLIT_RECORD, REPO, "Pack", " { .x }")
+        self.assertTrue(buttons.startswith("2 files to download, all needed: "), buttons)
+
     def test_buttons_carry_their_attributes_on_each_part(self):
         cell = download_links("batocera", SPLIT_RECORD, REPO, "Pack", " { .x }")
         self.assertEqual(cell.count("{ .x }"), 2)

tests/test_download.py

@@ -446,8 +446,11 @@ class DownloaderCase(unittest.TestCase):
             "pack staged in the system temp directory",
         )
         searched = list(self.dest.parent.iterdir()) + list(self.dest.rglob("*"))
+        # A part's marker is extracted on purpose: install.py --check reads it.
         leftovers = sorted(
-            p.name for p in searched if "_BIOS_Pack" in p.name or p.name.startswith(".")
+            p.name for p in searched
+            if ("_BIOS_Pack" in p.name or p.name.startswith("."))
+            and not split_pack.is_marker(p.name)
         )
         self.assertEqual(leftovers, [], "download staging left behind")
 
@@ -573,6 +576,12 @@ class ZipPartsCase(DownloaderCase):
     """A pack published as parts that are each an archive."""
 
     zip_parts = True
+    # Bytes deflate cannot shrink: each member outweighs the marker a part
+    # carries, so the pack splits in two.
+    payload = {
+        "bios/scph5501.bin": b"".join(hashlib.sha256(b"us%d" % n).digest() for n in range(64)),
+        "bios/dc/dc_boot.bin": b"".join(hashlib.sha256(b"dc%d" % n).digest() for n in range(64)),
+    }
 
 
 class ZipPartsTests:

tests/test_pack_counts.py

@@ -341,6 +341,11 @@ class TheTableDescribesTheRelease(unittest.TestCase):
         self.assertTrue(f"| RetroArch | {files:,} |" in readme)
 
 
+def _incompressible(seed: int) -> bytes:
+    """960 bytes deflate cannot shrink, so a member outweighs a part marker."""
+    return b"".join(hashlib.sha256(bytes([seed, n])).digest() for n in range(30))
+
+
 class ReleaseRecord(unittest.TestCase):
     OWNERS = {
         "Whole_1.0_BIOS_Pack.zip": ["whole"],
@@ -356,7 +361,7 @@ class ReleaseRecord(unittest.TestCase):
         self._pack("Whole_1.0_BIOS_Pack.zip", {"a.bin": b"a" * 10, "README.txt": b"r"})
         big = self._pack(
             "Demo_2.0_BIOS_Pack.zip",
-            {f"dir/file{n}.bin": bytes([n]) * 700 + bytes(range(256)) for n in range(6)},
+            {f"dir/file{n}.bin": _incompressible(n) for n in range(6)},
         )
         self.parts = split_pack.split_pack(big, limit=big.stat().st_size - 1)
 
@@ -375,7 +380,7 @@ class ReleaseRecord(unittest.TestCase):
         self.assertEqual(record["tag"], "v1")
         demo = record["packs"]["Demo_2.0_BIOS_Pack.zip"]
         self.assertEqual(demo["files"], 6)
-        self.assertEqual(demo["extracted_size"], 6 * (700 + 256))
+        self.assertEqual(demo["extracted_size"], 6 * 960)
         self.assertEqual(demo["assets"], [part.name for part in self.parts])
         self.assertEqual(
             demo["download_size"], sum(part.stat().st_size for part in self.parts)
@@ -399,7 +404,7 @@ class ReleaseRecord(unittest.TestCase):
         pack, leaves both: summed, the download size doubled."""
         self._pack(
             "Demo_2.0_BIOS_Pack.zip",
-            {f"dir/file{n}.bin": bytes([n]) * 700 + bytes(range(256)) for n in range(6)},
+            {f"dir/file{n}.bin": _incompressible(n) for n in range(6)},
         )
         with self.assertRaises(release_record.UnfinishedSplitError):
             release_record.build_record(self.dist, "v1", owners=self.OWNERS)

tests/test_split_pack.py

@@ -67,6 +67,8 @@ class SplitFixture(unittest.TestCase):
             with zipfile.ZipFile(part) as archive:
                 self.assertIsNone(archive.testzip())
                 for name in archive.namelist():
+                    if split_pack.is_marker(name):
+                        continue
                     self.assertNotIn(name, found, "a file sits in two parts")
                     found[name] = archive.read(name)
         return found
@@ -137,6 +139,61 @@ class PartsAreArchives(SplitFixture):
         self.assertTrue(modes["bin/engine"] & stat.S_IXUSR)
 
 
+class EachPartNamesItsSiblings(SplitFixture):
+    """A part extracted alone looked like a complete pack. Each part now
+    carries a marker naming the pack, its number and every part with its
+    size, so a missing one can be named from the extracted folder."""
+
+    def _markers(self, parts: list[Path]) -> dict[str, str]:
+        found = {}
+        for part in parts:
+            with zipfile.ZipFile(part) as archive:
+                names = [n for n in archive.namelist() if split_pack.is_marker(n)]
+                self.assertEqual(len(names), 1, part.name)
+                found[part.name] = archive.read(names[0]).decode("utf-8")
+        return found
+
+    def test_every_part_carries_one_marker(self):
+        parts = split_pack.split_pack(self.pack, limit=LIMIT)
+        count = len(parts)
+        for index, part in enumerate(parts, 1):
+            with zipfile.ZipFile(part) as archive:
+                self.assertIn(f"Demo_1.0_BIOS_Pack.part{index}of{count}.txt",
+                              archive.namelist())
+
+    def test_the_marker_names_the_pack_the_part_and_every_size(self):
+        parts = split_pack.split_pack(self.pack, limit=LIMIT)
+        count = len(parts)
+        for index, (name, text) in enumerate(self._markers(parts).items(), 1):
+            with self.subTest(part=name):
+                lines = text.splitlines()
+                self.assertIn("pack: Demo_1.0_BIOS_Pack.zip", lines)
+                self.assertIn(f"part: {index} of {count}", lines)
+                listed = [line.split()[1:] for line in lines if line.startswith("file:")]
+                self.assertEqual(
+                    listed,
+                    [[str(p.stat().st_size), p.name] for p in parts],
+                )
+
+    def test_the_parts_still_fit_with_their_markers(self):
+        for part in split_pack.split_pack(self.pack, limit=LIMIT):
+            self.assertLess(part.stat().st_size, LIMIT)
+
+    def test_a_pack_holding_a_marker_name_is_refused(self):
+        members = dict(self.members)
+        members["Demo_1.0_BIOS_Pack.part1of2.txt"] = b"x"
+        self.pack.unlink()
+        self._build(self.pack, members)
+        with self.assertRaises(ValueError):
+            split_pack.split_pack(self.pack, limit=LIMIT)
+        self.assertTrue(self.pack.is_file())
+
+    def test_a_marker_is_not_a_part(self):
+        self.assertTrue(split_pack.is_marker("Demo_BIOS_Pack.part2of3.txt"))
+        self.assertFalse(split_pack.is_part("Demo_BIOS_Pack.part2of3.txt"))
+        self.assertFalse(split_pack.is_marker("Demo_BIOS_Pack.part2of3.zip"))
+
+
 class SplitHoldsTheDirectory(SplitFixture):
     """split_pack read and deleted packs in a directory a build was writing:
     the build could truncate the pack under the split, or the split delete