From 43337c67197e422f868fa1f140eeb8704fd25429 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 09:51:55 +0200 Subject: [PATCH] fix: match release packs by exact name release_record rebuilt nothing from the name builder: a substring match took RetroArch_v1.22.2_AndroidArm64V8A_BIOS_Pack.zip for the RetroArch pack. The record now refuses any pack that is not a registered platform's full pack and stores the platforms each pack serves, which the readers use instead of the pack name. --- release.json | 34 ++++++++++++ schemas/release.schema.json | 8 ++- scripts/release_notes.py | 26 ++++++--- scripts/release_record.py | 103 +++++++++++++++++++++++++----------- tests/test_doc_surfaces.py | 4 +- tests/test_pack_counts.py | 46 +++++++++++++--- wiki/tools.md | 2 +- 7 files changed, 176 insertions(+), 47 deletions(-) diff --git a/release.json b/release.json index df9c894a..962167ef 100644 --- a/release.json +++ b/release.json @@ -9,6 +9,9 @@ "Batocera_43.1_BIOS_Pack.part1of3.zip", "Batocera_43.1_BIOS_Pack.part2of3.zip", "Batocera_43.1_BIOS_Pack.part3of3.zip" + ], + "platforms": [ + "batocera" ] }, "BizHawk_2.11.1_BIOS_Pack.zip": { @@ -17,6 +20,9 @@ "download_size": 1469752558, "assets": [ "BizHawk_2.11.1_BIOS_Pack.zip" + ], + "platforms": [ + "bizhawk" ] }, "EmuDeck_2.7EA_BIOS_Pack.zip": { @@ -25,6 +31,9 @@ "download_size": 1939696164, "assets": [ "EmuDeck_2.7EA_BIOS_Pack.zip" + ], + "platforms": [ + "emudeck" ] }, "MiSTer_FPGA_2026-09-29_BIOS_Pack.zip": { @@ -33,6 +42,9 @@ "download_size": 18787872, "assets": [ "MiSTer_FPGA_2026-09-29_BIOS_Pack.zip" + ], + "platforms": [ + "misterfpga" ] }, "ROCKNIX_20261001_BIOS_Pack.zip": { @@ -42,6 +54,9 @@ "assets": [ "ROCKNIX_20261001_BIOS_Pack.part1of2.zip", "ROCKNIX_20261001_BIOS_Pack.part2of2.zip" + ], + "platforms": [ + "rocknix" ] }, "Recalbox_10.1.1_BIOS_Pack.zip": { @@ -51,6 +66,9 @@ "assets": [ "Recalbox_10.1.1_BIOS_Pack.part1of2.zip", "Recalbox_10.1.1_BIOS_Pack.part2of2.zip" + ], + "platforms": [ + "recalbox" ] }, "RetroArch_Lakka_v1.22.2_BIOS_Pack.zip": { @@ -60,6 +78,10 @@ "assets": [ "RetroArch_Lakka_v1.22.2_BIOS_Pack.part1of2.zip", "RetroArch_Lakka_v1.22.2_BIOS_Pack.part2of2.zip" + ], + "platforms": [ + "lakka", + "retroarch" ] }, "RetroBat_8.2.1_BIOS_Pack.zip": { @@ -70,6 +92,9 @@ "RetroBat_8.2.1_BIOS_Pack.part1of3.zip", "RetroBat_8.2.1_BIOS_Pack.part2of3.zip", "RetroBat_8.2.1_BIOS_Pack.part3of3.zip" + ], + "platforms": [ + "retrobat" ] }, "RetroDECK_0.10.10b_BIOS_Pack.zip": { @@ -80,6 +105,9 @@ "RetroDECK_0.10.10b_BIOS_Pack.part1of3.zip", "RetroDECK_0.10.10b_BIOS_Pack.part2of3.zip", "RetroDECK_0.10.10b_BIOS_Pack.part3of3.zip" + ], + "platforms": [ + "retrodeck" ] }, "RetroPie_4.8.12_BIOS_Pack.zip": { @@ -90,6 +118,9 @@ "RetroPie_4.8.12_BIOS_Pack.part1of3.zip", "RetroPie_4.8.12_BIOS_Pack.part2of3.zip", "RetroPie_4.8.12_BIOS_Pack.part3of3.zip" + ], + "platforms": [ + "retropie" ] }, "RomM_5.3.1_BIOS_Pack.zip": { @@ -98,6 +129,9 @@ "download_size": 973353236, "assets": [ "RomM_5.3.1_BIOS_Pack.zip" + ], + "platforms": [ + "romm" ] } } diff --git a/schemas/release.schema.json b/schemas/release.schema.json index a84a6ae3..34e8bcde 100644 --- a/schemas/release.schema.json +++ b/schemas/release.schema.json @@ -15,8 +15,14 @@ "additionalProperties": { "type": "object", "additionalProperties": false, - "required": ["files", "extracted_size", "download_size", "assets"], + "required": ["files", "extracted_size", "download_size", "assets", "platforms"], "properties": { + "platforms": { + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": { "type": "string" } + }, "files": { "type": "integer", "minimum": 1 }, "extracted_size": { "type": "integer", "minimum": 0 }, "download_size": { "type": "integer", "minimum": 1 }, diff --git a/scripts/release_notes.py b/scripts/release_notes.py index 06a111c1..995d18f6 100644 --- a/scripts/release_notes.py +++ b/scripts/release_notes.py @@ -14,6 +14,7 @@ Usage: from __future__ import annotations import argparse +import re import sys from pathlib import Path @@ -37,18 +38,27 @@ def _display(pack: str) -> str: def _extract_folders(record: dict, platforms_dir: str) -> dict[str, str]: """Pack name to the folder the registry says it extracts into.""" + registry = load_platform_registry(platforms_dir) folders: dict[str, str] = {} - for key, entry in sorted(load_platform_registry(platforms_dir).items()): - if not entry.get("extract_to"): - continue - needle = release_record._match_key(key) - for name in sorted(record.get("packs", {})): - # The name a pack starts with: RetroArch_Lakka is RetroArch's. - if needle and release_record._match_key(name).startswith(needle): - folders.setdefault(name, entry["extract_to"]) + for name, pack in sorted(record.get("packs", {}).items()): + # A shared pack extracts where the platform it is named after does: + # RetroArch_Lakka is RetroArch's. + key = _match_key(name) + for platform in sorted( + pack.get("platforms", ()), key=lambda p: key.find(_match_key(p)) % (len(key) + 1) + ): + folder = registry.get(platform, {}).get("extract_to") + if folder: + folders[name] = folder + break return folders +def _match_key(value: str) -> str: + """Letters and digits only: the platform is `misterfpga`, the pack MiSTer_FPGA.""" + return re.sub(r"[^a-z0-9]", "", value.lower()) + + def _links(tag: str, assets: list[str]) -> str: base = f"https://github.com/{REPO}/releases/download/{tag}" if len(assets) == 1: diff --git a/scripts/release_record.py b/scripts/release_record.py index 3c110e5f..f8510686 100644 --- a/scripts/release_record.py +++ b/scripts/release_record.py @@ -18,39 +18,70 @@ from __future__ import annotations import argparse import json -import re import sys import zipfile from pathlib import Path import split_pack -from common import ArtifactLockBusy, artifact_lock, write_text_atomic +from common import ( + ArtifactLockBusy, + artifact_lock, + list_registered_platforms, + write_text_atomic, +) RECORD = "release.json" -def _match_key(value: str) -> str: - """Letters and digits only: the platform is `misterfpga`, the pack MiSTer_FPGA.""" - return re.sub(r"[^a-z0-9]", "", value.lower()) - - class UnfinishedSplitError(Exception): """A pack lies beside its own parts: a split is running or was killed.""" -def build_record(dist: Path, tag: str) -> dict: +class NotAReleasePackError(Exception): + """A pack in the directory is not the full pack of a registered platform.""" + + +def release_owners(platforms_dir: str = "platforms") -> dict[str, list[str]]: + """Full pack file name to the platforms it serves, as the builder names it. + + Rebuilt from the name builder, never matched by substring: matched by + substring, RetroArch_v1.22.2_AndroidArm64V8A_BIOS_Pack.zip sorts before + the full pack and was taken for the RetroArch pack. + """ + import generate_pack + + platforms = list_registered_platforms(platforms_dir, include_archived=True) + owners: dict[str, list[str]] = {} + for platform, names in generate_pack._expected_pack_names( + platforms, platforms_dir, None, None + ).items(): + for name in names: + owners.setdefault(name, []).append(platform) + return {name: sorted(serves) for name, serves in owners.items()} + + +def build_record( + dist: Path, + tag: str, + platforms_dir: str = "platforms", + owners: dict[str, list[str]] | None = None, +) -> dict: """Read every pack of a directory, parts counted once across them. Read under the shared artifact lock, so a split that is reading its parts back cannot be recorded half done. A pack still beside its parts after that is a split that never finished: summed, it doubled the download size - and named an asset that is never published. + and named an asset that is never published. Each pack carries the + platforms it serves, so a reader finds a platform's pack by that list + after a later version or grouping has renamed the pack main would build. """ + if owners is None: + owners = release_owners(platforms_dir) with artifact_lock(str(dist), exclusive=False): - return _read_record(Path(dist), tag) + return _read_record(Path(dist), tag, owners) -def _read_record(dist: Path, tag: str) -> dict: +def _read_record(dist: Path, tag: str, owners: dict[str, list[str]]) -> dict: assets: dict[str, list[Path]] = {} for path in sorted(dist.glob("*_BIOS_Pack*.zip")): assets.setdefault(split_pack.pack_of(path.name), []).append(path) @@ -60,6 +91,12 @@ def _read_record(dist: Path, tag: str) -> dict: raise UnfinishedSplitError( f"{whole[0].name} lies beside its parts: the split did not finish" ) + foreign = sorted(name for name in assets if name not in owners) + if foreign: + raise NotAReleasePackError( + f"not the full pack of a registered platform: {', '.join(foreign)}" + " (a narrowed or older build left in the directory)" + ) packs = {} for name, paths in sorted(assets.items()): @@ -73,6 +110,7 @@ def _read_record(dist: Path, tag: str) -> dict: "extracted_size": sum(members.values()), "download_size": sum(path.stat().st_size for path in paths), "assets": [path.name for path in paths], + "platforms": owners[name], } return {"tag": tag, "packs": packs} @@ -86,15 +124,18 @@ def load_record(path: str | Path = RECORD) -> dict: return {} +def pack_name_for(platform: str, record: dict) -> str | None: + """Name of the release pack that serves a platform, if it has one.""" + for name, pack in sorted(record.get("packs", {}).items()): + if platform in pack.get("platforms", ()): + return name + return None + + def pack_for(platform: str, record: dict) -> dict | None: """The pack of the release that serves a platform, if it has one.""" - needle = _match_key(platform) - if not needle: - return None - for name, pack in sorted(record.get("packs", {}).items()): - if needle in _match_key(name): - return pack - return None + name = pack_name_for(platform, record) + return record["packs"][name] if name else None def manifest_mismatches(record: dict, install_dir: str | Path) -> list[str]: @@ -103,17 +144,15 @@ def manifest_mismatches(record: dict, install_dir: str | Path) -> list[str]: for manifest_path in sorted(Path(install_dir).glob("*.json")): manifest = json.loads(manifest_path.read_text(encoding="utf-8")) expected = manifest.get("pack_files") - if expected is None: + name = pack_name_for(manifest_path.stem, record) + if expected is None or name is None: continue - for name, pack in sorted(record.get("packs", {}).items()): - if _match_key(manifest_path.stem) not in _match_key(name): - continue - if pack["files"] != expected: - found.append( - f"{name} holds {pack['files']} files, " - f"install/{manifest_path.name} expects {expected}" - ) - break + pack = record["packs"][name] + if pack["files"] != expected: + found.append( + f"{name} holds {pack['files']} files, " + f"install/{manifest_path.name} expects {expected}" + ) return found @@ -131,6 +170,10 @@ def main() -> int: "--install-dir", default="install", type=Path, help="manifests the archives are compared with (default: %(default)s)", ) + parser.add_argument( + "--platforms-dir", default="platforms", + help="registry the pack names are rebuilt from (default: %(default)s)", + ) parser.add_argument( "--output", default=RECORD, type=Path, help="record to write (default: %(default)s)", @@ -138,8 +181,8 @@ def main() -> int: args = parser.parse_args() try: - record = build_record(args.dist, args.tag) - except (UnfinishedSplitError, ArtifactLockBusy) as exc: + record = build_record(args.dist, args.tag, args.platforms_dir) + except (UnfinishedSplitError, NotAReleasePackError, ArtifactLockBusy) as exc: print(f"Error: {exc}", file=sys.stderr) return 1 if not record["packs"]: diff --git a/tests/test_doc_surfaces.py b/tests/test_doc_surfaces.py index 6d825714..ced78597 100644 --- a/tests/test_doc_surfaces.py +++ b/tests/test_doc_surfaces.py @@ -40,12 +40,14 @@ SPLIT_RECORD = { "Batocera_1_BIOS_Pack.part1of2.zip", "Batocera_1_BIOS_Pack.part2of2.zip", ], + "platforms": ["batocera"], }, "RomM_1_BIOS_Pack.zip": { "files": 1, "extracted_size": 1024**2, "download_size": 1024**2, "assets": ["RomM_1_BIOS_Pack.zip"], + "platforms": ["romm"], }, }, } @@ -244,7 +246,7 @@ class SizesComeFromTheRecord(unittest.TestCase): tag, platform, download, extracted = quoted.groups() if tag != RECORD.get("tag"): self.skipTest(f"release.json records {RECORD.get('tag')}, not {tag}") - pack = release_record.pack_for(platform, RECORD) + pack = release_record.pack_for(platform.lower(), RECORD) self.assertEqual(download, format_size(pack["download_size"])) self.assertEqual(extracted, format_size(pack["extracted_size"])) diff --git a/tests/test_pack_counts.py b/tests/test_pack_counts.py index be858eff..aa556bfb 100644 --- a/tests/test_pack_counts.py +++ b/tests/test_pack_counts.py @@ -283,12 +283,14 @@ RECORD = { "RetroArch_Lakka_v1.22.2_BIOS_Pack.zip.001", "RetroArch_Lakka_v1.22.2_BIOS_Pack.zip.002", ], + "platforms": ["lakka", "retroarch"], }, "MiSTer_FPGA_2026-08-29_BIOS_Pack.zip": { "files": 74, "extracted_size": 17000000, "download_size": 16769418, "assets": ["MiSTer_FPGA_2026-08-29_BIOS_Pack.zip"], + "platforms": ["misterfpga"], }, }, } @@ -340,6 +342,11 @@ class TheTableDescribesTheRelease(unittest.TestCase): class ReleaseRecord(unittest.TestCase): + OWNERS = { + "Whole_1.0_BIOS_Pack.zip": ["whole"], + "Demo_2.0_BIOS_Pack.zip": ["demo"], + } + def setUp(self): self._tmp = tempfile.TemporaryDirectory() self.dist = Path(self._tmp.name) / "dist" @@ -364,7 +371,7 @@ class ReleaseRecord(unittest.TestCase): return path def test_a_pack_in_parts_is_counted_once_across_them(self): - record = release_record.build_record(self.dist, "v1") + record = release_record.build_record(self.dist, "v1", owners=self.OWNERS) self.assertEqual(record["tag"], "v1") demo = record["packs"]["Demo_2.0_BIOS_Pack.zip"] self.assertEqual(demo["files"], 6) @@ -378,7 +385,7 @@ class ReleaseRecord(unittest.TestCase): def test_a_pack_that_disagrees_with_its_manifest_is_named(self): """The count main predicts and the count the archive holds are two computations of one thing. Apart, one of them is wrong.""" - record = release_record.build_record(self.dist, "v1") + record = release_record.build_record(self.dist, "v1", owners=self.OWNERS) (self.install / "demo.json").write_text(json.dumps({"pack_files": 6})) (self.install / "whole.json").write_text(json.dumps({"pack_files": 5})) self.assertEqual( @@ -395,12 +402,38 @@ class ReleaseRecord(unittest.TestCase): {f"dir/file{n}.bin": bytes([n]) * 700 + bytes(range(256)) for n in range(6)}, ) with self.assertRaises(release_record.UnfinishedSplitError): - release_record.build_record(self.dist, "v1") + release_record.build_record(self.dist, "v1", owners=self.OWNERS) + + def test_a_narrowed_pack_is_refused_not_recorded(self): + """A pack that sorts before the full one (Android targets, Asia) was + taken for the platform's pack by a substring match.""" + self._pack("Whole_1.0_Asia_BIOS_Pack.zip", {"a.bin": b"a"}) + with self.assertRaises(release_record.NotAReleasePackError) as raised: + release_record.build_record(self.dist, "v1", owners=self.OWNERS) + self.assertIn("Whole_1.0_Asia_BIOS_Pack.zip", str(raised.exception)) + + def test_a_platform_finds_the_pack_that_serves_it_by_name(self): + record = release_record.build_record(self.dist, "v1", owners=self.OWNERS) + record["packs"]["Whole_1.0_AAA_BIOS_Pack.zip"] = dict( + record["packs"]["Whole_1.0_BIOS_Pack.zip"], files=99, platforms=["other"] + ) + self.assertEqual( + release_record.pack_name_for("whole", record), "Whole_1.0_BIOS_Pack.zip" + ) + self.assertEqual(release_record.pack_for("whole", record)["files"], 2) + + def test_the_owners_are_the_full_pack_names_the_builder_writes(self): + owners = release_record.release_owners(str(REPO_ROOT / "platforms")) + retroarch = [name for name, serves in owners.items() if "retroarch" in serves] + self.assertEqual(len(retroarch), 1, retroarch) + for tags in ("_AndroidArm64V8A", "_Asia", "_Required"): + narrowed = retroarch[0].replace("_BIOS_Pack.zip", f"{tags}_BIOS_Pack.zip") + self.assertNotIn(narrowed, owners) def test_a_directory_being_written_is_not_read(self): with common.artifact_lock(str(self.dist)), \ self.assertRaises(common.ArtifactLockBusy): - release_record.build_record(self.dist, "v1") + release_record.build_record(self.dist, "v1", owners=self.OWNERS) class TheCollectionTotalNamesTheCollection(unittest.TestCase): @@ -483,10 +516,11 @@ class TheReleaseNotesReadTheRecord(unittest.TestCase): "RetroArch_Lakka_v1.22.2_BIOS_Pack.zip": { "files": 8504, "extracted_size": 6_400_000_000, "download_size": 4_000_000_000, "assets": ["RetroArch_Lakka_v1.22.2_BIOS_Pack.part1of2.zip", - "RetroArch_Lakka_v1.22.2_BIOS_Pack.part2of2.zip"]}, + "RetroArch_Lakka_v1.22.2_BIOS_Pack.part2of2.zip"], + "platforms": ["lakka", "retroarch"]}, "EmuDeck_2.7EA_BIOS_Pack.zip": { "files": 620, "extracted_size": 3_500_000_000, "download_size": 3_000_000_000, - "assets": ["EmuDeck_2.7EA_BIOS_Pack.zip"]}, + "assets": ["EmuDeck_2.7EA_BIOS_Pack.zip"], "platforms": ["emudeck"]}, }} def render(self, record=None): diff --git a/wiki/tools.md b/wiki/tools.md index 5a42d3ac..976d16dd 100644 --- a/wiki/tools.md +++ b/wiki/tools.md @@ -678,7 +678,7 @@ Further options, by script: | `provenance_report.py` | `--provenance-dir`, `--write-derivable PATH` (with `--derive`, and the pipeline writes `derivable.json`) | | `refresh_stale.py` | `--freshness-arg FLAG` passes a flag through to `check_freshness.py`, repeatable | | `bundled_only.py` | `--strict` (exit 1 while a collected file only an emulator install provides remains, pipeline step 4d), `--json`, `--install-dir`. Lists every copy of such a file, the database keeping one path per content | -| `release_record.py` | `--install-dir` (manifests the archives are held against), `--output` | +| `release_record.py` | `--install-dir` (manifests the archives are held against), `--platforms-dir` (registry the full pack names are rebuilt from: any other pack in the directory is refused), `--output` | | `release_notes.py` | `--whats-new FILE` (required, appended as written), `--record`, `--platforms-dir`, `--output` | | `restore_large_files.py` | `--cache` (downloaded assets), `--gitignore`, `--db` | | `romset_recipes.py` | `--dat-recipes PATH`, a snapshot or a directory of them (default `recipes/`) |