diff --git a/scripts/generate_readme.py b/scripts/generate_readme.py index cb5daaaa..dcfee052 100644 --- a/scripts/generate_readme.py +++ b/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]]: diff --git a/scripts/release_notes.py b/scripts/release_notes.py index 995d18f6..9a40d660 100644 --- a/scripts/release_notes.py +++ b/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: diff --git a/scripts/release_record.py b/scripts/release_record.py index f8510686..1926398f 100644 --- a/scripts/release_record.py +++ b/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()), diff --git a/scripts/split_pack.py b/scripts/split_pack.py index af6ec7e9..9e830ff7 100644 --- a/scripts/split_pack.py +++ b/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.+_BIOS_Pack)\.part(?P\d+)of(?P\d+)\.zip$") +_MARKER_NAME = re.compile(r"^(?P.+_BIOS_Pack)\.part(?P\d+)of(?P\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: diff --git a/tests/test_doc_surfaces.py b/tests/test_doc_surfaces.py index f873f593..21cfd770 100644 --- a/tests/test_doc_surfaces.py +++ b/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) diff --git a/tests/test_download.py b/tests/test_download.py index 5c5c9927..a3b28627 100644 --- a/tests/test_download.py +++ b/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: diff --git a/tests/test_pack_counts.py b/tests/test_pack_counts.py index 43629cef..cc0d4541 100644 --- a/tests/test_pack_counts.py +++ b/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) diff --git a/tests/test_split_pack.py b/tests/test_split_pack.py index 3d0f0aa0..497380d2 100644 --- a/tests/test_split_pack.py +++ b/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