From 83d49d93a8761d05d5ebfda03c690f5c014aecae Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 05:03:42 +0200 Subject: [PATCH 01/14] fix: name every --all-variants pack in its check --verify-packs after an --all-variants build rebuilt only the full pack's name and opened it alone. The five narrowed packs of the same run passed without a line. Each one is now named with the hash check the build ran on its members. --- scripts/generate_pack.py | 77 +++++++++++++++++++------------- tests/test_verify_pack_choice.py | 22 +++++++++ wiki/tools.md | 4 +- 3 files changed, 72 insertions(+), 31 deletions(-) diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index 56975f37..fa5533ff 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -2392,14 +2392,7 @@ def _run_manifest_mode( with open(registry_path) as _rf: registry = yaml_load(_rf) or {} - if args.all_variants: - variants = [ - ("full", False), ("full", True), - ("platform", False), ("platform", True), - ("truth", False), ("truth", True), - ] - else: - variants = [(args.source, args.required_only)] + variants = ALL_VARIANTS if args.all_variants else ((args.source, args.required_only),) failed: list[str] = [] for source, required_only in variants: @@ -2516,8 +2509,14 @@ def _run_verify_packs(args: argparse.Namespace) -> None: verify_regions = getattr(args, "regions", None) verify_profiles = load_emulator_profiles(args.emulators_dir) verify_data_registry = load_data_dir_registry(args.platforms_dir) + # After an --all-variants build every variant is named, so none of the + # six packs the run wrote passes unmentioned. expected = _expected_pack_names( - platforms, args.platforms_dir, verify_regions, args.target + platforms, + args.platforms_dir, + verify_regions, + args.target, + ALL_VARIANTS if args.all_variants else (("full", False),), ) on_disk = ( set(os.listdir(args.output_dir)) if os.path.isdir(args.output_dir) else set() @@ -2544,6 +2543,16 @@ def _run_verify_packs(args: argparse.Namespace) -> None: # A group pack and a pack built for the platform alone can both be # present, and each one is a pack of this platform. for pack_name in found: + if _narrows_contents(pack_name): + # Only reached after an --all-variants build, whose stage 1 + # re-hashed every member against the collection above. The + # platform's list describes the full pack, so a narrowed one + # is not compared to it. + print( + f" {platform_name} [{pack_name}]: hashes checked during the build; " + "narrowed by design, so not compared to the platform's full list" + ) + continue zip_path = os.path.join(args.output_dir, pack_name) result = verify_pack_against_platform( zip_path, @@ -2582,6 +2591,14 @@ def _run_verify_packs(args: argparse.Namespace) -> None: sys.exit(1) +# The source x required combinations --all-variants builds, in build order. +ALL_VARIANTS: tuple[tuple[str, bool], ...] = ( + ("full", False), ("full", True), + ("platform", False), ("platform", True), + ("truth", False), ("truth", True), +) + + def _pack_label(source: str, required_only: bool) -> str: """The variant tag pipeline.parse_pack_counts reads after a pack name.""" return f"[source={source}, required]" if required_only else f"[source={source}]" @@ -2640,14 +2657,7 @@ def _run_platform_packs( system_filter: list[str] | None, ) -> None: """Generate ZIP packs for platform groups and verify.""" - if args.all_variants: - variants = [ - ("full", False), ("full", True), - ("platform", False), ("platform", True), - ("truth", False), ("truth", True), - ] - else: - variants = [(args.source, args.required_only)] + variants = ALL_VARIANTS if args.all_variants else ((args.source, args.required_only),) failed: list[str] = [] for source, required_only in variants: @@ -4485,22 +4495,31 @@ def _expected_pack_names( platforms_dir: str, regions: list[str] | None, target_name: str | None, + variants: tuple[tuple[str, bool], ...] = (("full", False),), ) -> dict[str, set[str]]: """Exact pack file names --verify-packs may check, per platform. Rebuilt from the name builder, never matched by substring: a substring picks the full pack when a regional or targeted one was asked for, or a narrowed pack when the full one was. A platform answers to its own name - and to the name of the group it shares a pack with. + and to the name of the group it shares a pack with. ``variants`` lists + the (source, required_only) combinations the run built. """ - def narrow_tags(platform_name: str) -> str: + def narrow_tags(platform_name: str) -> list[str]: named = _platform_target(platform_name, target_name, platforms_dir) - return "".join( - tag for tag, _label in _narrowings("full", regions, named, False, False) - ) + return [ + "".join( + tag + for tag, _label in _narrowings(source, regions, named, False, required) + ) + for source, required in variants + ] names: dict[str, set[str]] = { - p: {f"{_platform_pack_stem([p], p, platforms_dir)}{narrow_tags(p)}_BIOS_Pack.zip"} + p: { + f"{_platform_pack_stem([p], p, platforms_dir)}{tags}_BIOS_Pack.zip" + for tags in narrow_tags(p) + } for p in platforms } # Groups depend on which platforms a run built: --all alone or with @@ -4523,13 +4542,11 @@ def _expected_pack_names( ): if len(group) < 2: continue - name = ( - f"{_platform_pack_stem(group, representative, platforms_dir)}" - f"{narrow_tags(representative)}_BIOS_Pack.zip" - ) - for p in group: - if p in names: - names[p].add(name) + stem = _platform_pack_stem(group, representative, platforms_dir) + for tags in narrow_tags(representative): + for p in group: + if p in names: + names[p].add(f"{stem}{tags}_BIOS_Pack.zip") return names diff --git a/tests/test_verify_pack_choice.py b/tests/test_verify_pack_choice.py index 1898de8a..0de0a352 100644 --- a/tests/test_verify_pack_choice.py +++ b/tests/test_verify_pack_choice.py @@ -53,6 +53,7 @@ class VerifyPackChoice(unittest.TestCase): "output_dir": str(self.out), "target": None, "regions": [], + "all_variants": False, } values.update(overrides) return argparse.Namespace(**values) @@ -93,6 +94,27 @@ class VerifyPackChoice(unittest.TestCase): self._run(self._args()) self.assertEqual(ctx.exception.code, 1) + def test_all_variants_names_every_variant_it_built(self): + """After --all-variants the check opened the full pack only: the five + narrowed packs the same run wrote passed without a line.""" + import contextlib + import io + + names = [] + for source, required in self.gp.ALL_VARIANTS: + tags = "".join( + tag for tag, _ in self.gp._narrowings(source, None, None, False, required) + ) + names.append(f"{self.stem}{tags}_BIOS_Pack.zip") + self._touch(names[-1]) + out = io.StringIO() + with contextlib.redirect_stdout(out): + checked = self._run(self._args(all_variants=True)) + self.assertEqual(checked, [("retroarch", f"{self.stem}_BIOS_Pack.zip")]) + printed = out.getvalue() + for name in names[1:]: + self.assertIn(f"[{name}]: hashes checked during the build", printed) + def test_include_archived_reaches_the_archived_pack(self): stem = self.gp._platform_pack_stem(["retropie"], "retropie", "platforms") self._touch(f"{stem}_BIOS_Pack.zip") diff --git a/wiki/tools.md b/wiki/tools.md index 8164c5d4..ebe0395e 100644 --- a/wiki/tools.md +++ b/wiki/tools.md @@ -229,7 +229,9 @@ would have loaded. - `--db`, `--platforms-dir`, `--emulators-dir`: read another database or source tree instead of the repository's - `--source {platform,truth,full}`: select file source (platform YAML only, emulator profiles only, or both) -- `--all-variants`: generate all 6 combinations of source x required_only +- `--all-variants`: generate all 6 combinations of source x required_only. With + `--verify-packs`, the full pack is compared to the platform's list and each + narrowed variant is named with the hash check the build ran on its members - `--refresh-data`: force re-download all data directories before packing - `--no-refresh-data`: pack the data directories as cached, still fetching release assets (what `pipeline.py` does after its own refresh step) From 5fc2423ca54ad69207a128b68556823e05fe88a5 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 05:05:49 +0200 Subject: [PATCH 02/14] fix: apply --required-only to data directories The Required packs and manifests carried every data tree, the FreeIntv overlays included. A data_directories entry now accepts required: false, and the platform, emulator and manifest walks leave such a tree out under --required-only. FreeIntv reads its overlays only with the multi-screen option and draws a placeholder keypad without one (src/libretro.c:294-296, 350-369). --- emulators/freeintv_ts_overlay.yml | 1 + platforms/retroarch.yml | 1 + schemas/emulator.schema.json | 4 +++ schemas/platform.schema.json | 4 +++ scripts/generate_pack.py | 13 +++++-- scripts/scraper/libretro_scraper.py | 9 ++++- tests/test_audit_regressions.py | 54 +++++++++++++++++++++++++++++ wiki/profiling.md | 2 +- wiki/tools.md | 3 +- 9 files changed, 86 insertions(+), 5 deletions(-) diff --git a/emulators/freeintv_ts_overlay.yml b/emulators/freeintv_ts_overlay.yml index 9435413b..f95d5bca 100644 --- a/emulators/freeintv_ts_overlay.yml +++ b/emulators/freeintv_ts_overlay.yml @@ -31,6 +31,7 @@ bios_identical_to: freeintv data_directories: - ref: freeintv-overlays destination: freeintv_overlays + required: false files: - name: "exec.bin" diff --git a/platforms/retroarch.yml b/platforms/retroarch.yml index 50e32746..7bdc9399 100644 --- a/platforms/retroarch.yml +++ b/platforms/retroarch.yml @@ -1234,6 +1234,7 @@ systems: data_directories: - ref: freeintv-overlays destination: freeintv_overlays + required: false microsoft-msx: files: - name: CARTS.SHA diff --git a/schemas/emulator.schema.json b/schemas/emulator.schema.json index 4adcd508..c53b6bdf 100644 --- a/schemas/emulator.schema.json +++ b/schemas/emulator.schema.json @@ -279,6 +279,10 @@ "destination": { "type": "string" }, + "required": { + "type": "boolean", + "description": "false when the core runs without the tree; --required-only leaves it out" + }, "mode": { "type": "string", "enum": [ diff --git a/schemas/platform.schema.json b/schemas/platform.schema.json index 8a5dda57..f45be8a6 100644 --- a/schemas/platform.schema.json +++ b/schemas/platform.schema.json @@ -297,6 +297,10 @@ }, "destination": { "type": "string" + }, + "required": { + "type": "boolean", + "description": "false when the core runs without the tree; --required-only leaves it out" } } } diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index fa5533ff..ff3cc9ce 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -383,6 +383,7 @@ def _data_directory_members( seen_lower: set, seen_parents: set, refs_out: dict | None = None, + required_only: bool = False, ) -> Iterator[tuple[str, str]]: """Yield (source, destination) for the cached data directories a platform's systems declare. @@ -394,6 +395,8 @@ def _data_directory_members( trees an emulator reads, refreshed from upstream, so what ships is whatever the cache holds. The pack and the install manifest both read this, the first to write the files and the second to count them. + ``required_only`` leaves out a tree declared ``required: false``, one the + core runs without, as it leaves out such a file. """ # Data directories from _data_dirs.yml for sys_id, system in sorted(pack_systems.items()): @@ -401,6 +404,8 @@ def _data_directory_members( ref_key = dd.get("ref", "") if not ref_key or not data_registry or ref_key not in data_registry: continue + if required_only and dd.get("required") is False: + continue if ref_key in installed_data_refs(platform_name): continue entry = data_registry[ref_key] @@ -455,12 +460,14 @@ def _pack_data_directories( seen_destinations: set, seen_lower: set, seen_parents: set, + required_only: bool = False, ) -> int: """Add the data directories to the pack. Returns how many files went in.""" added = 0 for src, full in _data_directory_members( pack_systems, data_registry, platform_name, base_dest, case_insensitive, seen_destinations, seen_lower, seen_parents, + required_only=required_only, ): _add_pack_member(zf, src, _flat(full, base_dest, flatten)) added += 1 @@ -1142,7 +1149,7 @@ def generate_pack( total_files += _pack_data_directories( zf, pack_systems, data_registry, platform_name, base_dest, flatten, case_insensitive, seen_destinations, seen_lower, - seen_parents, + seen_parents, required_only=required_only, ) # README.txt for users: personalized step-by-step per platform @@ -1682,6 +1689,8 @@ def generate_emulator_pack( for dd in deliverable( profile.get("data_directories", []), standalone ): + if required_only and dd.get("required") is False: + continue ref_key = dd.get("ref", "") if not ref_key or not data_registry or ref_key not in data_registry: if ref_key: @@ -4264,7 +4273,7 @@ def generate_manifest( data_members = list(_data_directory_members( pack_systems, data_registry, platform_name, base_dest, case_insensitive, seen_destinations, seen_lower, seen_parents, - data_refs, + data_refs, required_only=required_only, )) data_sizes = pack_only_sizes + [os.path.getsize(src) for src, _dest in data_members] data_directories = _manifest_data_directories( diff --git a/scripts/scraper/libretro_scraper.py b/scripts/scraper/libretro_scraper.py index 1e2a8c59..74571c11 100644 --- a/scripts/scraper/libretro_scraper.py +++ b/scripts/scraper/libretro_scraper.py @@ -495,8 +495,15 @@ class Scraper(BaseScraper): # so final path is system/freeintv_overlays/FreeIntvTS_Overlays/.png # but core expects system/freeintv_overlays/.png # fix: point destination into the subfolder + # Optional: read only with the multi-screen option on, and a + # missing PNG falls back to a drawn placeholder keypad + # (FreeIntv/src/libretro.c:294-296, 350-369). "mattel-intellivision": [ - {"ref": "freeintv-overlays", "destination": "freeintv_overlays"}, + { + "ref": "freeintv-overlays", + "destination": "freeintv_overlays", + "required": False, + }, ], } for sys_id, data_dirs in SYSTEM_DATA_DIRS.items(): diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index 2c500274..38750698 100644 --- a/tests/test_audit_regressions.py +++ b/tests/test_audit_regressions.py @@ -5794,5 +5794,59 @@ class AFileTheLauncherNeverOpens(unittest.TestCase): self.assertIn('unread_on(f, config.get("_key", ""))', source) +class RequiredOnlyReachesDataDirectories(unittest.TestCase): + """--required-only filtered files and packed every data tree anyway: the + Required RetroArch pack still carried the 90 FreeIntv overlays, which the + core reads only with its multi-screen option and replaces by a drawn + keypad when absent (FreeIntv src/libretro.c:294-296, 350-369).""" + + def _members(self, required_only: bool) -> list[str]: + from scripts import generate_pack as gp + + with tempfile.TemporaryDirectory(dir=TMP_ROOT) as tmp: + for key in ("needed", "optional"): + Path(tmp, key).mkdir() + Path(tmp, key, f"{key}.bin").write_bytes(b"x") + registry = { + key: {"local_cache": str(Path(tmp, key))} + for key in ("needed", "optional") + } + systems = { + "sys": { + "data_directories": [ + {"ref": "needed", "destination": "a"}, + {"ref": "optional", "destination": "b", "required": False}, + ] + } + } + return [ + dest + for _src, dest in gp._data_directory_members( + systems, registry, "retroarch", "", False, set(), set(), set(), + required_only=required_only, + ) + ] + + def test_an_optional_tree_leaves_the_required_pack(self): + self.assertEqual(self._members(False), ["a/needed.bin", "b/optional.bin"]) + self.assertEqual(self._members(True), ["a/needed.bin"]) + + def test_every_tree_walk_reads_the_flag(self): + source = (ROOT / "scripts" / "generate_pack.py").read_text(encoding="utf-8") + self.assertEqual( + source.count('if required_only and dd.get("required") is False:'), 2 + ) + self.assertIn("seen_parents, required_only=required_only,", source) + self.assertIn("data_refs, required_only=required_only,", source) + + def test_the_overlays_are_declared_optional(self): + retroarch = yaml.safe_load((ROOT / "platforms" / "retroarch.yml").read_text()) + trees = retroarch["systems"]["mattel-intellivision"]["data_directories"] + self.assertEqual(trees, [ + {"ref": "freeintv-overlays", "destination": "freeintv_overlays", + "required": False}, + ]) + + if __name__ == "__main__": unittest.main() diff --git a/wiki/profiling.md b/wiki/profiling.md index d0d17163..34d87d1a 100644 --- a/wiki/profiling.md +++ b/wiki/profiling.md @@ -493,7 +493,7 @@ which CI validates every profile against. | `mode` | no | default mode: `standalone`, `libretro`, or `both` | | `verification` | no | how the core verifies BIOS: `existence`, `md5`, `sha1`, `crc32` | | `files` | yes, unless `type: alias` | list of file entries | -| `data_directories` | no | whole directory trees the core needs, referencing `_data_dirs.yml` keys. An entry may carry `mode:` when only one build reads the tree | +| `data_directories` | no | whole directory trees the core needs, referencing `_data_dirs.yml` keys. An entry may carry `mode:` when only one build reads the tree, and `required: false` when the core runs without it (`--required-only` then leaves it out) | | `notes` | no | free-form technical notes | | `note` | no | single-paragraph variant of `notes` | | `exclusion_note` | no | why the profile has no files despite .info declaring firmware | diff --git a/wiki/tools.md b/wiki/tools.md index ebe0395e..709cfa06 100644 --- a/wiki/tools.md +++ b/wiki/tools.md @@ -213,7 +213,8 @@ would have loaded. ### Granular options - `--system` with `--platform`: filter to specific systems within a platform pack -- `--required-only`: exclude optional files, keep only required +- `--required-only`: exclude optional files and the data directories declared + `required: false`, keep only required - `--split`: generate one ZIP per system instead of one big pack - `--split --group-by manufacturer`: group split packs by manufacturer (Sony, Nintendo, Sega...) - `--from-md5`: look up a hash in the database, or build a custom pack with `--platform`/`--emulator` From 80084390339ff527edb02c1b7a77eb7c04639125 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 05:06:36 +0200 Subject: [PATCH 03/14] fix: keep manifests and target lists apart Both are written as .json. --manifest-targets aimed at install/ replaced install/retroarch.json with a target table, and --manifest aimed at install/targets/ did the reverse, both with exit 0. Each mode now stops when the output directory holds the other kind and names the directory to use. --- scripts/generate_pack.py | 49 +++++++++++++++++++++++++++++++++ tests/test_audit_regressions.py | 36 ++++++++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index ff3cc9ce..c4b06d0f 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -2181,6 +2181,53 @@ def generate_md5_pack( return zip_path +def _json_artifact_kind(path: Path) -> str | None: + """"manifest" for an install manifest, "targets" for a per-target core + list, None for anything else. The two share their file names: install.py + fetches install/.json and install/targets/.json.""" + try: + data = json.loads(path.read_text(encoding="utf-8")) + except (OSError, UnicodeDecodeError, json.JSONDecodeError): + return None + if not isinstance(data, dict) or not data: + return None + if "manifest_version" in data: + return "manifest" + if all(v is None or isinstance(v, list) for v in data.values()): + return "targets" + return None + + +def _refuse_mixed_output(output_dir: str, writing: str) -> None: + """Stop before one kind of installer JSON overwrites the other. + + Both are named .json, so --manifest-targets aimed at install/ + replaced install/retroarch.json, the file every installer downloads, with + a target table, and the reverse replaced a target list by a manifest. + """ + directory = Path(output_dir) + if not directory.is_dir(): + return + other = "targets" if writing == "manifest" else "manifest" + held = sorted( + p.name for p in directory.glob("*.json") if _json_artifact_kind(p) == other + ) + if not held: + return + what = { + "manifest": "install manifests (install/ in the repository)", + "targets": "per-target core lists (install/targets/ in the repository)", + } + flag = "--manifest" if writing == "manifest" else "--manifest-targets" + print( + f"ERROR: {output_dir} holds {what[other]}: {', '.join(held[:3])}" + f"{', ...' if len(held) > 3 else ''}. {flag} writes {what[writing]} under the " + f"same file names; pass the --output-dir that holds those instead.", + file=sys.stderr, + ) + sys.exit(1) + + def generate_target_manifests(targets_dir: str, output_dir: str) -> None: """Convert target YAMLs to installer JSON manifests.""" os.makedirs(output_dir, exist_ok=True) @@ -3100,6 +3147,7 @@ def _main(held: contextlib.ExitStack) -> None: _run_verify_packs(args) return if args.manifest_targets: + _refuse_mixed_output(args.output_dir, "targets") with _pack_output_lock(args.output_dir): generate_target_manifests( os.path.join(args.platforms_dir, "targets"), args.output_dir @@ -3276,6 +3324,7 @@ def _main(held: contextlib.ExitStack) -> None: ) if args.manifest: + _refuse_mixed_output(args.output_dir, "manifest") with _pack_output_lock(args.output_dir): _run_manifest_mode( args, groups, db, zip_contents, emu_profiles, target_cores_cache diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index 38750698..c098693b 100644 --- a/tests/test_audit_regressions.py +++ b/tests/test_audit_regressions.py @@ -5848,5 +5848,41 @@ class RequiredOnlyReachesDataDirectories(unittest.TestCase): ]) +class InstallerJsonKindsNeverOverwriteEachOther(unittest.TestCase): + """Install manifests and target core lists are both .json. + --manifest-targets aimed at install/ replaced install/retroarch.json, the + file every installer downloads, with a target table, exit 0.""" + + def _run(self, *flags: str, held: str) -> tuple: + with tempfile.TemporaryDirectory(dir=TMP_ROOT) as tmp: + target = Path(tmp, "retroarch.json") + before = (ROOT / held).read_text(encoding="utf-8") + target.write_text(before, encoding="utf-8") + proc = subprocess.run( + [sys.executable, "scripts/generate_pack.py", *flags, + "--offline", "--output-dir", tmp], + cwd=ROOT, capture_output=True, text=True, timeout=600, + ) + return proc, before, target.read_text(encoding="utf-8") + + def test_target_lists_refuse_a_manifest_directory(self): + proc, before, after = self._run( + "--manifest-targets", held="install/retroarch.json" + ) + self.assertEqual(proc.returncode, 1, proc.stdout + proc.stderr) + self.assertIn("holds install manifests", proc.stderr) + self.assertIn("--output-dir", proc.stderr) + self.assertEqual(after, before) + + def test_manifests_refuse_a_target_list_directory(self): + proc, before, after = self._run( + "--manifest", "--platform", "retroarch", + held="install/targets/retroarch.json", + ) + self.assertEqual(proc.returncode, 1, proc.stdout + proc.stderr) + self.assertIn("holds per-target core lists", proc.stderr) + self.assertEqual(after, before) + + if __name__ == "__main__": unittest.main() From ae874534e2049b8d956dc6853dcd8b2587493ae2 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 05:07:06 +0200 Subject: [PATCH 04/14] fix: reread a contradicted hash from disk _stable_hashes retried through the hash cache, so both retries returned the first reading and the file was read once before the manifest run stopped. The retries now read the file again. --- scripts/generate_pack.py | 4 +++- tests/test_install_experience.py | 27 +++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index c4b06d0f..acf92ef9 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -3468,7 +3468,9 @@ def _stable_hashes(local_path: str, db: dict) -> dict: if expected is None or hashes["sha1"] == expected: _STABLE_HASHES[key] = hashes return hashes - hashes = compute_hashes(local_path, _MANIFEST_HASHES) + # Read from disk again: the cached digest is the one in doubt, and a + # retry through the cache answered with it without a second read. + hashes = compute_hashes(local_path, _MANIFEST_HASHES, cache=False) raise RuntimeError( f"{local_path}: sha1 {hashes['sha1']} contradicts the database scan " f"({expected}). Rehash the file or regenerate database.json" diff --git a/tests/test_install_experience.py b/tests/test_install_experience.py index 4a0aaae6..f356a2f3 100644 --- a/tests/test_install_experience.py +++ b/tests/test_install_experience.py @@ -237,6 +237,33 @@ class ManifestHashesMatchTheScan(unittest.TestCase): generate_pack._stable_hashes(str(path), bad) self.assertEqual(generate_pack._stable_hashes(str(path), {"files": {}})["sha1"], real) + def test_a_wrong_first_reading_is_read_again_from_disk(self): + """The retries went through the hash cache and got the first, wrong + value back: one read of the file, then a refusal a second read would + have avoided.""" + sys.path.insert(0, str(REPO_ROOT / "scripts")) + import generate_pack + import hashing + + real_compute = hashing._compute_hashes + reads: list[str] = [] + + def flaky(filepath, algos): + reads.append(str(filepath)) + result = real_compute(filepath, algos) + if len(reads) == 1: + result = {**result, "sha1": "0" * 40} + return result + + with tempfile.TemporaryDirectory() as tmp: + path = Path(tmp) / "nand.bin" + path.write_bytes(b"flaky") + real = hashlib.sha1(b"flaky").hexdigest() + db = {"files": {real: {"path": str(path)}}} + with mock.patch.object(hashing, "_compute_hashes", flaky): + self.assertEqual(generate_pack._stable_hashes(str(path), db)["sha1"], real) + self.assertEqual(len(reads), 2) + def test_the_cache_is_keyed_by_what_was_checked(self): """Keyed by id(db), a database freed and replaced at the same address got hashes verified against the first one back, unchecked.""" From f7d561e42f97a437eb3250051ec8299309cd6a9d Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 05:10:00 +0200 Subject: [PATCH 05/14] fix: store already-compressed pack members Every member was deflated, archives and PNGs included. On the 2026-10-20 RetroArch pack the .zip, .7z, .chd, .png, .ogg and .pk4 members are 2.1 GB that took 28 s to deflate again for 9.6 MB saved (0.45 %), and about 16 GB for 72 MB across all packs. Those formats are now stored. The choice reads the member name only, so a pack stays byte-reproducible. --- scripts/generate_pack.py | 20 ++++++++++++++++-- tests/test_deterministic_zip.py | 36 +++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index acf92ef9..8aa52869 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -105,6 +105,22 @@ DEFAULT_BIOS_DIR = "bios" +# Formats that are already compressed: deflating them again took 28 s of the +# RetroArch build for 2.1 GB of members and saved 9.6 MB (0.45 %), 16 GB and +# about 72 MB across all packs. Measured per extension on the 2026-10-20 +# packs; .pk3 (3 %), .jpg (6 %), .avi (44 %) and .jar (60 %) still shrink +# and stay deflated. Chosen by name alone, so a pack stays a function of its +# inputs. +_STORED_EXTENSIONS = frozenset({".7z", ".chd", ".ogg", ".pk4", ".png", ".zip"}) + + +def _member_compression(arcname: str) -> int: + """ZIP_STORED for a member whose format is already compressed.""" + if os.path.splitext(arcname)[1].lower() in _STORED_EXTENSIONS: + return zipfile.ZIP_STORED + return zipfile.ZIP_DEFLATED + + def _add_pack_member(zf: zipfile.ZipFile, source_path: str, arcname: str) -> None: """Add a file to the pack with metadata that does not depend on the build. @@ -117,7 +133,7 @@ def _add_pack_member(zf: zipfile.ZipFile, source_path: str, arcname: str) -> Non because some members are firmware images of several hundred megabytes. """ info = zipfile.ZipInfo(filename=arcname, date_time=_FIXED_DATE_TIME) - info.compress_type = zipfile.ZIP_DEFLATED + info.compress_type = _member_compression(arcname) # The executable bit is the one permission worth carrying: the RetroDECK # pack ships the two Voxatron engine binaries, and a user who extracts # them at 644 cannot run them. Git records the bit, so reading it from @@ -1218,7 +1234,7 @@ def _extract_zip_to_archive( member = zipfile.ZipInfo( filename=target_path, date_time=_FIXED_DATE_TIME ) - member.compress_type = zipfile.ZIP_DEFLATED + member.compress_type = _member_compression(target_path) member.external_attr = 0o100644 << 16 target_zf.writestr(member, data) diff --git a/tests/test_deterministic_zip.py b/tests/test_deterministic_zip.py index 24ef3c0b..d5f09481 100644 --- a/tests/test_deterministic_zip.py +++ b/tests/test_deterministic_zip.py @@ -413,5 +413,41 @@ class ManifestInjectionIsAtomic(unittest.TestCase): ) +class PackMemberCompression(unittest.TestCase): + """A pack deflated every member, archives and PNGs included: 16 GB of + already-compressed data deflated again on each full build for 0.18 % of + the packs' size.""" + + def _pack(self, tmp: Path, name: str) -> Path: + from scripts.generate_pack import _add_pack_member + + source = tmp / "member" + source.write_bytes(b"\x00" * 4096) + pack = tmp / name + with zipfile.ZipFile(pack, "w", zipfile.ZIP_DEFLATED) as zf: + for arcname in ("bios/a.zip", "bios/b.PNG", "bios/c.bin", "bios/d.pk3"): + _add_pack_member(zf, str(source), arcname) + return pack + + def test_compressed_formats_are_stored_and_the_rest_deflated(self): + with tempfile.TemporaryDirectory() as tmp: + pack = self._pack(Path(tmp), "pack.zip") + with zipfile.ZipFile(pack) as zf: + kinds = {i.filename: i.compress_type for i in zf.infolist()} + self.assertEqual(kinds, { + "bios/a.zip": zipfile.ZIP_STORED, + "bios/b.PNG": zipfile.ZIP_STORED, + "bios/c.bin": zipfile.ZIP_DEFLATED, + "bios/d.pk3": zipfile.ZIP_DEFLATED, + }) + + def test_the_pack_stays_byte_reproducible(self): + with tempfile.TemporaryDirectory() as tmp: + first = self._pack(Path(tmp), "one.zip").read_bytes() + time.sleep(1.1) + second = self._pack(Path(tmp), "two.zip").read_bytes() + self.assertEqual(hashlib.sha256(first).hexdigest(), hashlib.sha256(second).hexdigest()) + + if __name__ == "__main__": unittest.main() From 959934d43cf82599a3228e4d897814b5a46c6f3e Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 05:18:53 +0200 Subject: [PATCH 06/14] refactor: compute the gap report once per build The region grouping and phase 2 of each pack and manifest called find_undeclared_files with the same arguments: 30 reports for the 15 manifests of --all --manifest, 2.8 s, now 15 and 1.7 s. The 15 manifests are byte-identical before and after. --- scripts/generate_pack.py | 30 +++++++++++++++++++++++++++++- scripts/packextras.py | 33 +++++++++++++++++++++++++++------ tests/test_directory_entries.py | 31 +++++++++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 7 deletions(-) diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index 8aa52869..6c2a5b51 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -546,6 +546,7 @@ def _select_variants( regions: list[str] | None, one_per_slot: bool, required_only: bool = False, + undeclared: list[dict] | None = None, ) -> tuple[set, list, list]: """Which regional and slot alternatives this pack leaves out. @@ -574,6 +575,7 @@ def _select_variants( include_all=(source == "truth"), required_only=required_only, held_out=held, + undeclared=undeclared, ) if regions: region_index = region_mod.platform_region_index( @@ -796,9 +798,20 @@ def generate_pack( # dedicated extras pass with an empty seen set: it only needs group # membership, never destinations, and a superset of members is the safe # direction. Runs only when --region is given. + # The gap report both extras passes read, computed once: the region + # grouping and the core complement below ask it the same question. + undeclared = ( + undeclared_report( + config, emulators_dir, db, emu_profiles, target_cores, + include_all=(source == "truth"), + ) + if source != "platform" and precomputed_extras is None + else None + ) region_drops, region_fallbacks, slot_undecidable = _select_variants( config, pack_systems, emulators_dir, db, base_dest, emu_profiles, target_cores, source, regions, one_per_slot, required_only, + undeclared=undeclared, ) with _writing_pack(zip_path) as zf: @@ -1060,6 +1073,7 @@ def generate_pack( _collect_emulator_extras( config, emulators_dir, db, set(), base_dest, emu_profiles, target_cores=target_cores, include_all=(source == "truth"), + undeclared=undeclared, ), system_filter, ) @@ -1073,6 +1087,7 @@ def generate_pack( emu_profiles, target_cores=target_cores, include_all=(source == "truth"), + undeclared=undeclared, ) core_count = 0 @@ -3888,6 +3903,7 @@ def _manifest_selection( source: str, required_only: bool, regions: list[str] | None, + undeclared: list[dict] | None = None, ) -> tuple[set[str], SelectionIndex]: """Destinations a region list removes, and the groups the installer replays. @@ -3909,6 +3925,7 @@ def _manifest_selection( required_only=required_only, extras_out=extras, held_out=held, + undeclared=undeclared, ) region_index = region_mod.platform_region_index( config, emu_profiles, target_cores @@ -4100,9 +4117,18 @@ def generate_manifest( ), } + # Read by the region grouping and by phase 2 with the same arguments. + undeclared = ( + undeclared_report( + config, emulators_dir, db, emu_profiles, target_cores, + include_all=(source == "truth"), + ) + if source != "platform" + else None + ) region_drops, selection = _manifest_selection( config, pack_systems, emulators_dir, db, base_dest, emu_profiles, - target_cores, source, required_only, regions, + target_cores, source, required_only, regions, undeclared=undeclared, ) # Phase 1: baseline files @@ -4296,6 +4322,7 @@ def generate_manifest( unplaceable=unplaceable, baseline_claims=baseline_claims, absent=absent, + undeclared=undeclared, ) else: core_files = [] @@ -4802,6 +4829,7 @@ from packextras import ( # noqa: E402,F401 emulator_region_drops, _emulator_region_group, expand_agnostic_entries, + undeclared_report, ) diff --git a/scripts/packextras.py b/scripts/packextras.py index e32dab34..4d68af1c 100644 --- a/scripts/packextras.py +++ b/scripts/packextras.py @@ -793,6 +793,26 @@ class ExtraPlacement: return self.slug_based and self.flat and "/" in dest +def undeclared_report( + config: dict, + emulators_dir: str, + db: dict, + emu_profiles: dict | None, + target_cores: set[str] | None = None, + include_all: bool = False, +) -> list[dict]: + """The gap report _collect_emulator_extras reads, for a caller that + collects the extras more than once with these arguments: the region + grouping and the pack or manifest phase 2 each computed it in full.""" + from verify import find_undeclared_files + + return find_undeclared_files( + config, emulators_dir, db, emu_profiles, target_cores=target_cores, + include_all=include_all, + declared_names=platform_declarations(config, db, enrich=False), + ) + + def _collect_emulator_extras( config: dict, emulators_dir: str, @@ -840,8 +860,6 @@ def _collect_emulator_extras( Works for ANY platform (RetroArch, Batocera, Recalbox, etc.) """ - from verify import find_undeclared_files - profiles = ( emu_profiles if emu_profiles is not None @@ -858,9 +876,8 @@ def _collect_emulator_extras( yaml_names = platform_declarations(config, db, enrich=False) if undeclared is None: - undeclared = find_undeclared_files( - config, emulators_dir, db, emu_profiles, target_cores=target_cores, - include_all=include_all, declared_names=yaml_names, + undeclared = undeclared_report( + config, emulators_dir, db, emu_profiles, target_cores, include_all ) # A destination two cores claim goes to the one that names its content: # hypseus-singe asks for gamecontrollerdb.txt by name alone and took the @@ -1153,6 +1170,7 @@ def platform_region_groups( required_only: bool = False, extras_out: list[dict] | None = None, held_out: set[str] | None = None, + undeclared: list[dict] | None = None, ) -> tuple[dict[str, list[tuple[str, str]]], dict[tuple[str, str, str], str]]: """Group a platform's pack candidates the way region filtering reads them. @@ -1165,7 +1183,9 @@ def platform_region_groups( withdraw from its report exactly what the builder withdraws from the pack: grouping the declared files here and the core extras there would let the two answer differently on the same request. `extras_out` receives the - core extras that were grouped, for the install manifest. + core extras that were grouped, for the install manifest. ``undeclared`` + is the gap report when the caller holds it (undeclared_report, with the + same target_cores and include_all). """ groups: dict[str, list[tuple[str, str]]] = {} for sys_id, system in systems.items(): @@ -1199,6 +1219,7 @@ def platform_region_groups( include_all=include_all, claimants=extra_dests, absent=absent, + undeclared=undeclared, ) held = {id(extra) for extra in extras} spellings = system_spellings(systems) diff --git a/tests/test_directory_entries.py b/tests/test_directory_entries.py index 1b3ce115..ef817563 100644 --- a/tests/test_directory_entries.py +++ b/tests/test_directory_entries.py @@ -14,6 +14,7 @@ import json import sys import tempfile import unittest +from unittest import mock import zipfile from pathlib import Path @@ -247,6 +248,36 @@ class PackCarriesADirectory(DirectoryFixture): self.assertTrue(self.EXPECTED <= dests, sorted(dests)) json.dumps(manifest) + def test_the_gap_report_is_computed_once_per_build(self): + """The region grouping and phase 2 each ran find_undeclared_files with + the same arguments: 30 reports for 15 manifests.""" + import verify + + real = verify.find_undeclared_files + calls: list[int] = [] + + def counted(*a, **kw): + calls.append(1) + return real(*a, **kw) + + profiles = self._profiles() + out = self.root / "counted" + out.mkdir() + with mock.patch.object(verify, "find_undeclared_files", counted): + builder.generate_manifest( + "demo", str(self.platforms), self._db(), str(self.bios), + str(self.platforms / "_registry.yml"), + emulators_dir=str(self.emulators), emu_profiles=profiles, + offline=True, + ) + self.assertEqual(len(calls), 1) + builder.generate_pack( + "demo", str(self.platforms), self._db(), str(self.bios), str(out), + emulators_dir=str(self.emulators), emu_profiles=profiles, + offline=True, regions=["north-america"], + ) + self.assertEqual(len(calls), 2) + def test_an_emulator_pack_ships_the_tree(self): self._profiles() out = self.root / "emu" From bc68e517cb30c2d2507b6c54bc35bbb7ea8d0188 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 05:19:27 +0200 Subject: [PATCH 07/14] fix: suggest the closest system for --platform --platform retroarch --system sony-playstaton listed the first ten systems of the alphabet. The warning now names the closest systems and the command that lists the platform's own, as --system alone already did. --- scripts/generate_pack.py | 10 +++++++--- tests/test_audit_regressions.py | 22 ++++++++++++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index 6c2a5b51..cf209d3e 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -769,10 +769,14 @@ def generate_pack( if sid in system_filter or _norm_system_id(sid) in norm_filter } if not filtered: - available = sorted(pack_systems.keys())[:10] + hint = suggest_names( + system_filter, + sorted(pack_systems), + f"--platform {platform_name} --list-systems", + ) print( - f" WARNING: no systems matched filter {system_filter} " - f"(available: {', '.join(available)})" + f" WARNING: {platform_name} has no system matching " + f"{', '.join(system_filter)}. {hint}" ) return None pack_systems = filtered diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index c098693b..49c738e1 100644 --- a/tests/test_audit_regressions.py +++ b/tests/test_audit_regressions.py @@ -5884,5 +5884,27 @@ class InstallerJsonKindsNeverOverwriteEachOther(unittest.TestCase): self.assertEqual(after, before) +class PlatformSystemTypoGetsTheClosestNames(unittest.TestCase): + """--platform retroarch --system sony-playstaton listed the first ten + systems of the alphabet, 3do to coleco-colecovision.""" + + def test_the_hint_names_the_close_systems(self): + from scripts import generate_pack as gp + + out = io.StringIO() + with tempfile.TemporaryDirectory(dir=TMP_ROOT) as tmp, \ + contextlib.redirect_stdout(out): + result = gp.generate_pack( + "retroarch", str(ROOT / "platforms"), {"files": {}, "indexes": {}}, + str(ROOT / "bios"), tmp, emulators_dir=str(ROOT / "emulators"), + system_filter=["sony-playstaton"], offline=True, + ) + self.assertIsNone(result) + printed = out.getvalue() + self.assertIn("Did you mean: sony-playstation", printed) + self.assertIn("--platform retroarch --list-systems", printed) + self.assertNotIn("3do", printed) + + if __name__ == "__main__": unittest.main() From a160a5bc6bb217e4024c38f1ae9b63e620beef68 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 05:20:59 +0200 Subject: [PATCH 08/14] fix: name the hash formats --from-md5 accepts A value of another length was looked up as an MD5 and reported NOT FOUND, a SHA-256 was dropped, and an input with no hash ended on "No valid hashes found in input". Digests are now read by length (CRC32, MD5 and its 29-digit Batocera form, SHA-1, SHA-256), SHA-256 is looked up in the database, and any other value is refused with the formats accepted. --- scripts/generate_pack.py | 42 ++++++------ scripts/packresolve.py | 116 ++++++++++++++++++++++---------- tests/test_audit_regressions.py | 39 +++++++++++ wiki/tools.md | 12 ++-- 4 files changed, 152 insertions(+), 57 deletions(-) diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index cf209d3e..a752b3f1 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -2081,8 +2081,6 @@ def generate_md5_pack( ) -> str | None: """Build a pack from an explicit list of hashes with layout context.""" files_db = db.get("files", {}) - by_md5 = db.get("indexes", {}).get("by_md5", {}) - by_crc32 = db.get("indexes", {}).get("by_crc32", {}) if zip_contents is None: zip_contents = {} @@ -2143,15 +2141,8 @@ def generate_md5_pack( with _writing_pack(zip_path) as zf: for hash_type, hash_val in hashes: - sha1 = None - if hash_type == "sha1" and hash_val in files_db: - sha1 = hash_val - elif hash_type == "md5": - sha1 = by_md5.get(hash_val) - elif hash_type == "crc32": - sha1 = by_crc32.get(hash_val) - - if not sha1 or sha1 not in files_db: + sha1 = sha1_for_hash(hash_type, hash_val, db) + if not sha1: not_in_db.append(hash_val) continue @@ -3146,9 +3137,13 @@ def _main(held: contextlib.ExitStack) -> None: help="List available targets for the platform", ) parser.add_argument( - "--from-md5", help="Hash(es) to look up or pack (comma-separated)" + "--from-md5", + help="Hash(es) to look up or pack, comma-separated: CRC32, MD5, SHA-1 or SHA-256", + ) + parser.add_argument( + "--from-md5-file", + help="File with hashes, one per line, alone or beside a file name as md5sum writes it", ) - parser.add_argument("--from-md5-file", help="File with hashes (one per line)") parser.add_argument( "--manifest", action="store_true", @@ -3223,13 +3218,20 @@ def _main(held: contextlib.ExitStack) -> None: hold_data_dirs(held, load_data_dir_registry(args.platforms_dir)) has_from_md5 = bool(args.from_md5 or getattr(args, "from_md5_file", None)) if has_from_md5: - hashes = ( - parse_hash_input(args.from_md5) - if args.from_md5 - else parse_hash_file(args.from_md5_file) - ) + try: + hashes = ( + parse_hash_input(args.from_md5) + if args.from_md5 + else parse_hash_file(args.from_md5_file) + ) + except ValueError as exc: + print(f"ERROR: {exc}", file=sys.stderr) + sys.exit(1) if not hashes: - print("No valid hashes found in input", file=sys.stderr) + print( + f"ERROR: no hash in the input. Accepted: {ACCEPTED_HASHES}", + file=sys.stderr, + ) sys.exit(1) db = load_database(args.db) if not args.platform and not args.emulator: @@ -4855,7 +4857,9 @@ from packverify import ( # noqa: E402,F401 # _OFFLINE is deliberately not re-exported: importing a value copies it, so a # facade name would keep saying False after set_offline() changed the flag. from packresolve import ( # noqa: E402,F401 + ACCEPTED_HASHES, _detect_hash_type, + sha1_for_hash, preferred_entries as _preferred_entries, parse_hash_input, parse_hash_file, diff --git a/scripts/packresolve.py b/scripts/packresolve.py index 33fe9e62..1d54413b 100644 --- a/scripts/packresolve.py +++ b/scripts/packresolve.py @@ -7,7 +7,6 @@ and the offline switch that forbids reaching for the network.""" from __future__ import annotations import re -_HEX_RE = re.compile(r"\b([0-9a-fA-F]{8,40})\b") from largefiles import fetch_large_file import os from common import ( @@ -81,43 +80,101 @@ def _verified_asset( return None -def _detect_hash_type(h: str) -> str: - n = len(h) - if n == 40: - return "sha1" - if n == 32: - return "md5" - if n == 8: - return "crc32" - return "md5" +# Digest length in hex digits -> kind. 29 is the MD5 Batocera writes with +# its last three digits cut, matched by prefix as the resolver does. +_HASH_KINDS = {8: "crc32", 29: "md5", 32: "md5", 40: "sha1", 64: "sha256"} +_HEX_DIGITS = re.compile(r"[0-9a-f]+") +_WORD = re.compile(r"[0-9A-Za-z]+") +ACCEPTED_HASHES = ( + "CRC32 (8 hex digits), MD5 (32, or the 29 Batocera writes), " + "SHA-1 (40) or SHA-256 (64)" +) + + +def _detect_hash_type(h: str) -> str | None: + """The digest a hex string is, by its length, or None.""" + return _HASH_KINDS.get(len(h)) + + +def _hash_token(token: str) -> tuple[str, str] | None: + """(kind, value) when a token is a whole digest of a known length.""" + value = token.strip().lower().removeprefix("*").removeprefix("0x") + kind = _detect_hash_type(value) + if kind and _HEX_DIGITS.fullmatch(value): + return kind, value + return None + + +def _refuse(rejected: list[str]) -> None: + shown = ", ".join(rejected[:5]) + (", ..." if len(rejected) > 5 else "") + raise ValueError(f"not a hash: {shown}. Accepted: {ACCEPTED_HASHES}") + def parse_hash_input(raw: str) -> list[tuple[str, str]]: - """Parse comma-separated hash string into (type, hash) tuples.""" + """Parse a comma-separated hash string into (type, hash) tuples. + + A part that is no digest is refused rather than dropped: a 12-digit + value was looked up as an MD5 and reported NOT FOUND, and a SHA-256 + vanished without a word. + """ results: list[tuple[str, str]] = [] + rejected: list[str] = [] for part in raw.split(","): - part = part.strip().lower() - if not part: + if not part.strip(): continue - m = _HEX_RE.search(part) - if m: - h = m.group(1) - results.append((_detect_hash_type(h), h)) + parsed = _hash_token(part) + if parsed: + results.append(parsed) + else: + rejected.append(part.strip()) + if rejected: + _refuse(rejected) return results + def parse_hash_file(path: str) -> list[tuple[str, str]]: - """Parse hash file (one per line, comments with #, mixed formats).""" + """Parse a hash file: one hash per line, alone or beside a file name as + md5sum writes it, ``#`` for comments. A line holding no digest is + refused with its line number.""" results: list[tuple[str, str]] = [] + rejected: list[str] = [] with open(path) as f: - for line in f: + for number, line in enumerate(f, 1): line = line.strip() if not line or line.startswith("#"): continue - m = _HEX_RE.search(line.lower()) - if m: - h = m.group(1) - results.append((_detect_hash_type(h), h)) + parsed = next( + (h for h in map(_hash_token, _WORD.findall(line)) if h), None + ) + if parsed: + results.append(parsed) + else: + rejected.append(f"line {number} ({line[:40]})") + if rejected: + _refuse(rejected) return results + +def sha1_for_hash(hash_type: str, hash_val: str, db: dict) -> str | None: + """The database key a looked-up hash names, or None.""" + files_db = db.get("files", {}) + indexes = db.get("indexes", {}) + if hash_type == "sha1": + sha1 = hash_val + elif hash_type == "crc32": + sha1 = indexes.get("by_crc32", {}).get(hash_val) + elif hash_type == "md5": + by_md5 = indexes.get("by_md5", {}) + sha1 = by_md5.get(hash_val) + if sha1 is None and len(hash_val) < 32: + prefixed = {v for k, v in by_md5.items() if k.startswith(hash_val)} + sha1 = prefixed.pop() if len(prefixed) == 1 else None + else: + sha1 = next( + (k for k, e in files_db.items() if e.get("sha256") == hash_val), None + ) + return sha1 if sha1 in files_db else None + def lookup_hashes( hashes: list[tuple[str, str]], db: dict, @@ -128,19 +185,10 @@ def lookup_hashes( """Print diagnostic info for each hash and return how many were not found.""" missing = 0 files_db = db.get("files", {}) - by_md5 = db.get("indexes", {}).get("by_md5", {}) - by_crc32 = db.get("indexes", {}).get("by_crc32", {}) for hash_type, hash_val in hashes: - sha1 = None - if hash_type == "sha1" and hash_val in files_db: - sha1 = hash_val - elif hash_type == "md5": - sha1 = by_md5.get(hash_val) - elif hash_type == "crc32": - sha1 = by_crc32.get(hash_val) - - if not sha1 or sha1 not in files_db: + sha1 = sha1_for_hash(hash_type, hash_val, db) + if not sha1: print(f"\n{hash_type.upper()}: {hash_val}") print(" NOT FOUND in database") missing += 1 diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index 49c738e1..7b204060 100644 --- a/tests/test_audit_regressions.py +++ b/tests/test_audit_regressions.py @@ -5906,5 +5906,44 @@ class PlatformSystemTypoGetsTheClosestNames(unittest.TestCase): self.assertNotIn("3do", printed) +class HashInputNamesWhatItAccepts(unittest.TestCase): + """A 12-digit value was looked up as an MD5 and reported NOT FOUND, a + SHA-256 vanished from the input, and the run said only "No valid hashes + found in input".""" + + def test_digests_are_read_by_length(self): + from scripts.packresolve import parse_hash_input + + sha256 = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + self.assertEqual(parse_hash_input(f"0x1A2B3C4D,{sha256}"), [ + ("crc32", "1a2b3c4d"), ("sha256", sha256), + ]) + self.assertEqual(parse_hash_input("a" * 29), [("md5", "a" * 29)]) + + def test_anything_else_is_refused_with_the_formats(self): + from scripts.packresolve import parse_hash_file, parse_hash_input + + for raw in ("1234567890ab", "zz", "490f666e1afb15b7362b406ed1cea246,zz"): + with self.subTest(raw=raw), self.assertRaises(ValueError) as ctx: + parse_hash_input(raw) + self.assertIn("SHA-256 (64)", str(ctx.exception)) + with tempfile.TemporaryDirectory(dir=TMP_ROOT) as tmp: + listing = Path(tmp, "hashes.txt") + listing.write_text("# list\n490f666e1afb15b7362b406ed1cea246 a.bin\nb.bin\n") + with self.assertRaises(ValueError) as ctx: + parse_hash_file(str(listing)) + self.assertIn("line 3 (b.bin)", str(ctx.exception)) + + def test_a_sha256_finds_its_file(self): + from scripts.packresolve import sha1_for_hash + + db = json.loads((ROOT / "database.json").read_text(encoding="utf-8")) + sha1, entry = next( + (k, e) for k, e in db["files"].items() if e.get("sha256") + ) + self.assertEqual(sha1_for_hash("sha256", entry["sha256"], db), sha1) + self.assertEqual(sha1_for_hash("md5", entry["md5"][:29], db), sha1) + + if __name__ == "__main__": unittest.main() diff --git a/wiki/tools.md b/wiki/tools.md index 709cfa06..3db2d9df 100644 --- a/wiki/tools.md +++ b/wiki/tools.md @@ -217,10 +217,14 @@ would have loaded. `required: false`, keep only required - `--split`: generate one ZIP per system instead of one big pack - `--split --group-by manufacturer`: group split packs by manufacturer (Sony, Nintendo, Sega...) -- `--from-md5`: look up a hash in the database, or build a custom pack with `--platform`/`--emulator` -- `--from-md5-file`: same, reading hashes from a file: one MD5, SHA-1 or CRC32 - per line, alone or beside a file name as `md5sum` writes it, `#` for comments. - A lookup exits 1 when a hash is not in the database +- `--from-md5`: look up a hash in the database, or build a custom pack with + `--platform`/`--emulator`. Any digest is read by its length: CRC32 (8 hex + digits), MD5 (32, or the 29 Batocera writes), SHA-1 (40), SHA-256 (64). + Anything else is refused with the value that is not a hash +- `--from-md5-file`: same, reading hashes from a file: one per line, alone or + beside a file name as `md5sum` writes it, `#` for comments. A line with no + hash is refused with its number. A lookup exits 1 when a hash is not in the + database - `--target`: filter by hardware target (e.g. `switch`, `rpi4`, `x86_64`) - `--region`: ordered priority list, best first. Regions nest, a system with no regional split keeps everything, and a group with no candidate in any named From 4fa598645ebec1f49778d06532d2d64538f78d10 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 05:22:05 +0200 Subject: [PATCH 09/14] fix: print pack and lock errors on stderr The lock refusal, "no pack for", "no manifest for" and "no pack to verify" went to stdout, so a run redirected to a log exited 1 with nothing on the terminal. A test reads both scripts and refuses an ERROR line printed to stdout. --- scripts/generate_pack.py | 12 ++++++------ scripts/pipeline.py | 4 ++-- tests/test_artifact_lock.py | 6 +++--- tests/test_audit_regressions.py | 32 +++++++++++++++++++++++++++++++- 4 files changed, 42 insertions(+), 12 deletions(-) diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index a752b3f1..0b5ba8a7 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -2547,13 +2547,13 @@ def _run_manifest_mode( _write_manifest_if_changed(alias_path, alias_manifest) print(f" {alias_path}: alias of {representative}") except (FileNotFoundError, OSError, yaml.YAMLError) as e: - print(f" ERROR: {e}") + print(f" ERROR: {e}", file=sys.stderr) failed.append(representative) # A platform whose manifest could not be built has none, and install.py # serves whatever an older run left there: that is a failed run. if failed: - print(f"ERROR: no manifest for {', '.join(sorted(set(failed)))}") + print(f"ERROR: no manifest for {', '.join(sorted(set(failed)))}", file=sys.stderr) sys.exit(1) @@ -2564,7 +2564,7 @@ def _pack_output_lock(output_dir: str, exclusive: bool = True) -> Iterator[None] with artifact_lock(output_dir, exclusive=exclusive): yield except ArtifactLockBusy as exc: - print(f"ERROR: {exc}") + print(f"ERROR: {exc}", file=sys.stderr) sys.exit(1) @@ -2667,7 +2667,7 @@ def _run_verify_packs(args: argparse.Namespace) -> None: if not verified: # Every platform skipped is no check at all: a mistyped or purged # output directory read as a passed integrity check. - print(f"ERROR: no pack to verify in {args.output_dir}") + print(f"ERROR: no pack to verify in {args.output_dir}", file=sys.stderr) all_ok = False if not all_ok: sys.exit(1) @@ -2829,7 +2829,7 @@ def _run_platform_packs( pack_name=group_name, ) except (FileNotFoundError, OSError, yaml.YAMLError) as e: - print(f" ERROR: {e}") + print(f" ERROR: {e}", file=sys.stderr) failed.append(representative) else: # A builder that declined (no system left under the target) @@ -2867,7 +2867,7 @@ def _run_platform_packs( if failed: # Verification only sees the ZIPs that exist, so a pack that was never # written does not fail it. - print(f"ERROR: no pack for {', '.join(sorted(set(failed)))}") + print(f"ERROR: no pack for {', '.join(sorted(set(failed)))}", file=sys.stderr) sys.exit(1) if not all_ok: sys.exit(1) diff --git a/scripts/pipeline.py b/scripts/pipeline.py index 8fd311f1..46b271b2 100644 --- a/scripts/pipeline.py +++ b/scripts/pipeline.py @@ -467,7 +467,7 @@ def main() -> None: try: held.enter_context(hold_artifact_lock(args.output_dir)) except ArtifactLockBusy as exc: - print(f"ERROR: {exc}") + print(f"ERROR: {exc}", file=sys.stderr) sys.exit(1) all_ok = True @@ -655,7 +655,7 @@ def main() -> None: if stale: print(f"Purged {len(stale)} stale pack file(s) from {out_dir}/") except ArtifactLockBusy as exc: - print(f"ERROR: {exc}") + print(f"ERROR: {exc}", file=sys.stderr) sys.exit(1) pack_cmd = [ diff --git a/tests/test_artifact_lock.py b/tests/test_artifact_lock.py index 905db767..2a8a100a 100644 --- a/tests/test_artifact_lock.py +++ b/tests/test_artifact_lock.py @@ -108,7 +108,7 @@ class PackLockCliTest(unittest.TestCase): ] ) self.assertEqual(proc.returncode, 1, proc.stdout + proc.stderr) - self.assertIn("is in use by another run", proc.stdout) + self.assertIn("is in use by another run", proc.stderr) def test_every_writing_mode_refuses_a_locked_output(self): """Two of seven modes took the lock, and five wrote straight into it. @@ -153,7 +153,7 @@ class PackLockCliTest(unittest.TestCase): ] ) self.assertEqual(proc.returncode, 1, proc.stdout + proc.stderr) - self.assertIn("is in use by another run", proc.stdout) + self.assertIn("is in use by another run", proc.stderr) def test_pipeline_refuses_locked_output(self): with artifact_lock(self.dir): @@ -167,7 +167,7 @@ class PackLockCliTest(unittest.TestCase): ] ) self.assertEqual(proc.returncode, 1, proc.stdout + proc.stderr) - self.assertIn("is in use by another run", proc.stdout) + self.assertIn("is in use by another run", proc.stderr) class HeldLockTest(unittest.TestCase): diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index 7b204060..c709c523 100644 --- a/tests/test_audit_regressions.py +++ b/tests/test_audit_regressions.py @@ -5192,7 +5192,7 @@ class ANamedPackThatIsAbsentFails(unittest.TestCase): cwd=ROOT, capture_output=True, text=True, timeout=600, ) self.assertEqual(result.returncode, 1) - self.assertIn("no pack to verify", result.stdout) + self.assertIn("no pack to verify", result.stderr) @@ -5945,5 +5945,35 @@ class HashInputNamesWhatItAccepts(unittest.TestCase): self.assertEqual(sha1_for_hash("md5", entry["md5"][:29], db), sha1) +class ErrorsGoToStderr(unittest.TestCase): + """The lock refusal and "no pack for" went to stdout: a run redirected to + a log showed nothing on the terminal and exited 1.""" + + @staticmethod + def _leading_text(node: ast.expr) -> str: + if isinstance(node, ast.Constant) and isinstance(node.value, str): + return node.value + if isinstance(node, ast.JoinedStr) and node.values: + return ErrorsGoToStderr._leading_text(node.values[0]) + return "" + + def test_no_error_line_is_printed_to_stdout(self): + for script in ("generate_pack.py", "pipeline.py"): + tree = ast.parse((ROOT / "scripts" / script).read_text(encoding="utf-8")) + seen = 0 + for node in ast.walk(tree): + if not (isinstance(node, ast.Call) and getattr(node.func, "id", "") == "print" + and node.args): + continue + text = self._leading_text(node.args[0]) + # Detail lines under a pack's own verdict stay with it. + if not text.lstrip().startswith("ERROR") or text.startswith(" "): + continue + seen += 1 + with self.subTest(script=script, line=node.lineno): + self.assertIn("file", [k.arg for k in node.keywords]) + self.assertGreater(seen, 0, script) + + if __name__ == "__main__": unittest.main() From 0457bd8f405d0e9af1802b78bc661c903e4151b6 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 05:24:25 +0200 Subject: [PATCH 10/14] fix: resolve the pipeline output dir once Every step runs from the repository root, but the lock and the purge of stale packs read --output-dir from the caller's directory. Started from scripts/, the run locked and purged scripts/dist/ and wrote to dist/, leaving the stale packs there. The directory is now made absolute before the lock, dist/ at the root by default. --- scripts/pipeline.py | 17 ++++++++++++++++- tests/test_audit_regressions.py | 20 ++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/scripts/pipeline.py b/scripts/pipeline.py index 46b271b2..e46ed4f2 100644 --- a/scripts/pipeline.py +++ b/scripts/pipeline.py @@ -92,6 +92,19 @@ class StepResults(dict): # Steps that report without failing the run. NON_BLOCKING = frozenset({"check_buildbot"}) +def _output_dir(typed: str | None) -> str: + """The pack directory as an absolute path. + + Every step runs from the repository root, while the lock and the purge + of stale packs ran here, against the caller's directory: started from + scripts/, the run locked and purged scripts/dist/ and wrote its packs to + dist/. A directory the caller typed is read from where they typed it. + """ + if typed is None: + return str(Path(__file__).resolve().parent.parent / "dist") + return str(Path(typed).resolve()) + + def run(cmd: list[str], label: str) -> tuple[bool, str]: """Run a command, echoing its output as it comes. Returns (success, output). @@ -425,7 +438,8 @@ def main() -> None: "assets already in .cache/large/" ) parser.add_argument( - "--output-dir", default="dist", help="Pack output directory (default: dist/)" + "--output-dir", + help="Pack output directory (default: dist/ at the repository root)", ) parser.add_argument("--target", "-t", help="Hardware target (e.g., switch, rpi4)") parser.add_argument( @@ -455,6 +469,7 @@ def main() -> None: ) args = parser.parse_args() _refuse_unapplied_flags(parser, args) + args.output_dir = _output_dir(args.output_dir) # A second run on the same output directory is refused before any work: # the database rebuild alone takes minutes, and the reader holding the diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index c709c523..42e2d6b5 100644 --- a/tests/test_audit_regressions.py +++ b/tests/test_audit_regressions.py @@ -5975,5 +5975,25 @@ class ErrorsGoToStderr(unittest.TestCase): self.assertGreater(seen, 0, script) +class PipelineOutputDirIsTheOneItWrites(unittest.TestCase): + """Started from scripts/, the pipeline locked and purged scripts/dist/ + while every step, run from the repository root, wrote to dist/.""" + + def test_the_default_is_the_root_dist_wherever_the_run_starts(self): + previous = os.getcwd() + self.addCleanup(os.chdir, previous) + os.chdir(ROOT / "scripts") + self.assertEqual(pipeline._output_dir(None), str(ROOT / "dist")) + self.assertEqual(pipeline._output_dir("out"), str(ROOT / "scripts" / "out")) + + def test_main_resolves_it_before_the_lock(self): + source = (ROOT / "scripts" / "pipeline.py").read_text(encoding="utf-8") + body = source[source.index("def main("):] + self.assertLess( + body.index("args.output_dir = _output_dir(args.output_dir)"), + body.index("hold_artifact_lock(args.output_dir)"), + ) + + if __name__ == "__main__": unittest.main() From 738577b5a4b7fe565a3b9e56aa241e62719ecab8 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 05:25:29 +0200 Subject: [PATCH 11/14] fix: run verify and generate_pack from any dir Started outside the repository root, both ended on a FileNotFoundError traceback: the database and the data registry name files from the root. They now move to the root after reading the paths the caller typed, which stay relative to where they were typed. --- scripts/generate_pack.py | 6 ++++++ scripts/verify.py | 25 +++++++++++++++++++++++++ tests/test_audit_regressions.py | 31 +++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+) diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index 0b5ba8a7..a7892e3b 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -3160,6 +3160,12 @@ def _main(held: contextlib.ExitStack) -> None: help="Extract and verify pack integrity (path + hash)", ) args = parser.parse_args() + from verify import work_from_repo_root + + work_from_repo_root( + args, parser, + ["db", "platforms_dir", "bios_dir", "output_dir", "emulators_dir", "from_md5_file"], + ) packresolve.set_offline(bool(args.offline)) if args.platform: require_platform(args.platform, args.platforms_dir) diff --git a/scripts/verify.py b/scripts/verify.py index dfae360a..c7078a22 100644 --- a/scripts/verify.py +++ b/scripts/verify.py @@ -109,6 +109,30 @@ DEFAULT_BIOS_DIR = "bios" # that pass no directory. _REPO_PLATFORMS = os.path.join(os.path.dirname(__file__), "..", "platforms") DEFAULT_EMULATORS_DIR = "emulators" +REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + + +def work_from_repo_root( + args: argparse.Namespace, parser: argparse.ArgumentParser, path_flags: list[str] +) -> None: + """Run from the repository root, whichever directory the command started in. + + database.json records every file from the root and the data registry + names its caches the same way, so a run from scripts/ ended on a + FileNotFoundError traceback. A path the caller typed is read from where + they typed it, a default from the root. + """ + if os.path.realpath(os.getcwd()) == os.path.realpath(REPO_ROOT): + return + for flag in path_flags: + value = getattr(args, flag, None) + if value and value != parser.get_default(flag): + setattr(args, flag, os.path.abspath(value)) + os.chdir(REPO_ROOT) + if getattr(args, "platform", None): + # Read before the move, the registry was not there to map + # "RetroArch" to its slug. + args.platform = platform_arg(args.platform) # Status model: aligned with Batocera BiosStatus (batocera-systems:967-969) @@ -2367,6 +2391,7 @@ def _main(held: contextlib.ExitStack) -> None: "present with an unconfirmed hash is a warning, not a failure", ) args = parser.parse_args() + work_from_repo_root(args, parser, ["db", "platforms_dir", "emulators_dir"]) if args.platform: require_platform(args.platform, args.platforms_dir) diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index 42e2d6b5..52d40241 100644 --- a/tests/test_audit_regressions.py +++ b/tests/test_audit_regressions.py @@ -5995,5 +5995,36 @@ class PipelineOutputDirIsTheOneItWrites(unittest.TestCase): ) +class ToolsRunFromAnyDirectory(unittest.TestCase): + """From scripts/, verify.py and generate_pack.py ended on a + FileNotFoundError traceback about platforms/_registry.yml.""" + + def _run(self, cwd: Path, *argv: str) -> subprocess.CompletedProcess: + return subprocess.run( + [sys.executable, *argv], cwd=cwd, capture_output=True, text=True, + timeout=600, + ) + + def test_listing_works_from_scripts(self): + for script in ("verify.py", "generate_pack.py"): + with self.subTest(script=script): + proc = self._run( + ROOT / "scripts", str(ROOT / "scripts" / script), + "--platform", "RetroArch", "--list-systems", + ) + self.assertEqual(proc.returncode, 0, proc.stderr) + self.assertNotIn("Traceback", proc.stderr) + self.assertIn("sony-playstation", proc.stdout) + + def test_a_typed_path_is_read_from_where_it_was_typed(self): + with tempfile.TemporaryDirectory(dir=TMP_ROOT) as tmp: + proc = self._run( + Path(tmp), str(ROOT / "scripts" / "generate_pack.py"), + "--manifest-targets", "--output-dir", "lists", + ) + self.assertEqual(proc.returncode, 0, proc.stderr) + self.assertTrue(Path(tmp, "lists", "retroarch.json").is_file()) + + if __name__ == "__main__": unittest.main() From 309231afc272dc5a717a7eeed4979c2166b005a4 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 05:27:07 +0200 Subject: [PATCH 12/14] docs: say what --skip-packs still runs The help said "Only regenerate DB and verify" while steps 1b, 4d, 7 and 8 still ran and rewrote derivable.json, README.md and docs/. It now names the steps it skips and points to --skip-docs. --- scripts/pipeline.py | 6 ++++-- tests/test_audit_regressions.py | 19 +++++++++++++++++++ wiki/tools.md | 2 +- 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/scripts/pipeline.py b/scripts/pipeline.py index e46ed4f2..aa815ed4 100644 --- a/scripts/pipeline.py +++ b/scripts/pipeline.py @@ -33,7 +33,7 @@ come from .cache/large/ only. Usage: python scripts/pipeline.py # active platforms python scripts/pipeline.py --include-archived # all platforms - python scripts/pipeline.py --skip-packs # steps 1 to 3b, then 7 and 8 + python scripts/pipeline.py --skip-packs # steps 0 to 3b, 4d, 7 and 8 python scripts/pipeline.py --skip-docs # skip steps 7 and 8 python scripts/pipeline.py --offline # no network, see above """ @@ -427,7 +427,9 @@ def main() -> None: parser.add_argument( "--skip-packs", action="store_true", - help="Only regenerate DB and verify, skip pack generation", + help="Skip steps 4 to 6 except 4d: packs, manifests and their checks. " + "Steps 7 and 8 still rewrite README.md and docs/, add --skip-docs to " + "stop after verify", ) parser.add_argument( "--skip-docs", action="store_true", help="Skip README and site generation" diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index 52d40241..4d4ae2eb 100644 --- a/tests/test_audit_regressions.py +++ b/tests/test_audit_regressions.py @@ -6026,5 +6026,24 @@ class ToolsRunFromAnyDirectory(unittest.TestCase): self.assertTrue(Path(tmp, "lists", "retroarch.json").is_file()) +class SkipPacksHelpSaysWhatRuns(unittest.TestCase): + """--help said "Only regenerate DB and verify" while the run went on to + rewrite derivable.json, README.md and docs/.""" + + def test_the_help_names_the_steps_that_still_run(self): + proc = subprocess.run( + [sys.executable, "scripts/pipeline.py", "--help"], + cwd=ROOT, capture_output=True, text=True, timeout=60, check=True, + ) + text = " ".join(proc.stdout.split()) + self.assertNotIn("Only regenerate DB and verify", text) + self.assertIn("Steps 7 and 8 still rewrite README.md and docs/", text) + + def test_every_step_it_names_is_one_it_skips(self): + source = (ROOT / "scripts" / "pipeline.py").read_text(encoding="utf-8") + skipped = set(re.findall(r"--- (\w+)/8 [^\n]*SKIPPED \(--skip-packs\)", source)) + self.assertEqual(skipped, {"4", "4b", "4c", "6"}) + + if __name__ == "__main__": unittest.main() diff --git a/wiki/tools.md b/wiki/tools.md index 3db2d9df..06731d05 100644 --- a/wiki/tools.md +++ b/wiki/tools.md @@ -53,7 +53,7 @@ Pipeline flags: | Flag | Effect | |------|--------| | `--offline` | skip every step that needs the network (2, 2a, 2a2, 2b, 2b2) | -| `--skip-packs` | skip steps 4, 4b, 4c, 6 and the consistency check | +| `--skip-packs` | skip steps 4, 4b, 4c, 6 and the consistency check. README and site are still rewritten unless `--skip-docs` is given too | | `--skip-docs` | skip steps 7 and 8 | | `--include-archived` | include archived platforms in verify, packs, truth and export | | `--target TARGET` | filter verify, packs and truth by hardware target | From bdcc8f0fb6aec72114b70766aed59ef585853c4e Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 05:32:50 +0200 Subject: [PATCH 13/14] fix: lower the pipeline floor to Python 3.9 The 3.12 guard rested on generate_readme f-strings that have since been rewritten. Every script compiles under CPython 3.9.25, 3.10 and 3.11, and only generate_pack's parenthesized context managers fail under 3.8. Under 3.9 the offline pipeline (--skip-packs) completed with no change to the committed artifacts, the 15 manifests matched 3.12 byte for byte, and the RomM pack was identical. CI now compiles every script under a real 3.9. --- .github/workflows/validate.yml | 6 ++++-- README.md | 4 ++-- pyproject.toml | 5 ++--- scripts/generate_readme.py | 2 +- scripts/pipeline.py | 12 ++++++----- tests/test_audit_regressions.py | 36 ++++++++++++++++++++++++++++++++- wiki/installer.md | 2 +- wiki/testing-guide.md | 2 +- wiki/tools.md | 2 +- 9 files changed, 54 insertions(+), 17 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 94b4c64f..50a8aaf8 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -165,12 +165,14 @@ jobs: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 # install.py stays on Python 3.8: the suite runs it end to end under a - # real 3.8 interpreter, which import-only and grammar checks miss. The - # last version listed is the default python. + # real 3.8 interpreter, which import-only and grammar checks miss. 3.9 + # is the floor pipeline.py states for the build scripts, compiled under + # a real 3.9. The last version listed is the default python. - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7 with: python-version: | 3.8 + 3.9 3.12 # markdown: the site validation test renders a wiki line to prove the diff --git a/README.md b/README.md index 37696e62..a0163372 100644 --- a/README.md +++ b/README.md @@ -131,7 +131,7 @@ python scripts/verify.py --emulator flycast python scripts/verify.py --platform retroarch --verbose # emulator ground truth ``` -Requires Python 3.12 and `pyyaml`. The one-line installer alone runs on Python 3.8. +Requires Python 3.9 or newer and `pyyaml`. The one-line installer alone runs on Python 3.8. ## Documentation site @@ -180,4 +180,4 @@ The scripts and tooling are released under the [MIT License](LICENSE). The BIOS and firmware files are not covered by that license: they are third-party system software, preserved and provided for personal backup, archival, and interoperability with emulation software. [NOTICE](NOTICE) sets out their status and how to ask for a file to be removed. The reasoning, and where it is weakest, is in the [FAQ](https://abdess.github.io/retrobios/wiki/faq/#is-this-legal). -*Auto-generated on 2026-10-20T20:53:07Z* +*Auto-generated on 2026-10-21T03:32:18Z* diff --git a/pyproject.toml b/pyproject.toml index 6e588335..ac501527 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,9 +4,8 @@ version = "1.0.0" description = "BIOS collection manager for retrogaming platforms" # install.py is the only entry point a user runs before cloning, and both # bootstraps accept 3.8, so the floor stays where the bootstraps put it. That -# floor covers install.py alone: the build scripts under scripts/ use syntax -# CPython 3.12 introduced, which is the version CI pins and the version a -# contributor needs to run the pipeline or the test suite. +# floor covers install.py alone: the build scripts under scripts/ need 3.9 +# (pipeline.py checks it), and CI runs the test suite on 3.12. requires-python = ">=3.8" dependencies = [ "pyyaml", diff --git a/scripts/generate_readme.py b/scripts/generate_readme.py index 52256e85..d9be6cdb 100644 --- a/scripts/generate_readme.py +++ b/scripts/generate_readme.py @@ -582,7 +582,7 @@ def generate_readme(db: dict, platforms_dir: str) -> str: "python scripts/verify.py --platform retroarch --verbose # emulator ground truth", "```", "", - "Requires Python 3.12 and `pyyaml`. The one-line installer alone runs on Python 3.8.", + "Requires Python 3.9 or newer and `pyyaml`. The one-line installer alone runs on Python 3.8.", "", "## Documentation site", "", diff --git a/scripts/pipeline.py b/scripts/pipeline.py index aa815ed4..8245dd24 100644 --- a/scripts/pipeline.py +++ b/scripts/pipeline.py @@ -51,11 +51,13 @@ import sys import time from pathlib import Path -# The build scripts use syntax CPython 3.12 introduced (generate_readme's -# f-strings, generate_pack's parenthesized context managers). On an older -# interpreter the run got through the packs and failed hours in, at step 7. -if sys.version_info < (3, 12): - sys.exit(f"pipeline.py needs Python 3.12 or newer, this is {sys.version.split()[0]}") +# generate_pack's parenthesized context managers do not compile before +# CPython 3.9, which builds the same database, manifests and packs as 3.12. +# On an older interpreter the run got through the first steps and failed in +# the packs, so it stops before step 0. A test compiles every script under +# this floor. +if sys.version_info < (3, 9): + sys.exit(f"pipeline.py needs Python 3.9 or newer, this is {sys.version.split()[0]}") sys.path.insert(0, str(Path(__file__).parent)) from common import ( diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index 4d4ae2eb..0b7a948e 100644 --- a/tests/test_audit_regressions.py +++ b/tests/test_audit_regressions.py @@ -108,7 +108,41 @@ class ReadmeRegressions(unittest.TestCase): cwd=ROOT, capture_output=True, text=True, timeout=60, check=False, ) self.assertEqual(proc.returncode, 1) - self.assertIn("needs Python 3.12", proc.stderr) + self.assertIn("needs Python 3.9", proc.stderr) + + def test_the_floor_interpreter_compiles_every_script(self): + """The floor said 3.12 for a reason the code no longer carried: every + script compiled under 3.9, and only the guard refused it. Checked + with a real interpreter of the floor version, as the installer's 3.8 + floor is.""" + import shutil # noqa: PLC0415 + + floor = re.search( + r"sys\.version_info < \((\d+), (\d+)\)", + (ROOT / "scripts" / "pipeline.py").read_text(encoding="utf-8"), + ) + name = f"python{floor.group(1)}.{floor.group(2)}" + interpreter = shutil.which(name) + if interpreter is None: + if os.environ.get("CI"): + self.fail(f"CI must provide {name}") + self.skipTest(f"no {name} on PATH") + check = ( + "import pathlib, sys\n" + "bad = []\n" + "for p in sorted(pathlib.Path('scripts').rglob('*.py')):\n" + " try:\n" + " compile(p.read_text(encoding='utf-8'), str(p), 'exec')\n" + " except SyntaxError as e:\n" + " bad.append(f'{p}:{e.lineno}')\n" + "print(bad)\n" + "sys.exit(1 if bad else 0)\n" + ) + proc = subprocess.run( + [interpreter, "-I", "-c", check], cwd=ROOT, capture_output=True, + text=True, timeout=300, check=False, + ) + self.assertEqual(proc.returncode, 0, proc.stdout + proc.stderr) class ReleaseSigningRegressions(unittest.TestCase): diff --git a/wiki/installer.md b/wiki/installer.md index d646ce5c..574faff4 100644 --- a/wiki/installer.md +++ b/wiki/installer.md @@ -471,7 +471,7 @@ followed. RetroPie image, so it stays on Python 3.8 and the standard library alone. Tests refuse any 3.9 syntax or API, and one runs the installer end to end under a real 3.8 interpreter, which CI installs beside 3.12 so the check cannot skip. -The build scripts need 3.12 and are not bound by this. +The build scripts need 3.9 and are not bound by this. Every change to `install.py` recomputes the SHA-256 pinned in `install.sh` and `install.ps1` in the same commit. The tests compare the pins with the file. diff --git a/wiki/testing-guide.md b/wiki/testing-guide.md index bf4c3c1d..ca71dd6c 100644 --- a/wiki/testing-guide.md +++ b/wiki/testing-guide.md @@ -24,7 +24,7 @@ python -m unittest tests.test_install tests.test_install_cores -v The table below says which module covers what. -Running the build needs Python 3.12 and `pyyaml` alone, and so does most of the suite. The +Running the build needs Python 3.9 or newer and `pyyaml` alone. CI runs the suite on 3.12. The schema contract tests also need `jsonschema[format-nongpl]`, and one site test renders a wiki line with `markdown`. The CI installs both: without them those tests are reported as skipped, so install them before a pull request diff --git a/wiki/tools.md b/wiki/tools.md index 06731d05..0027be20 100644 --- a/wiki/tools.md +++ b/wiki/tools.md @@ -1,6 +1,6 @@ # Tools -All tools are Python scripts in `scripts/`. They need Python 3.12 and `pyyaml`, +All tools are Python scripts in `scripts/`. They need Python 3.9 or newer and `pyyaml`, and only `validate_schemas.py` also needs `jsonschema[format-nongpl]`. `install.py` alone runs on Python 3.8. From 5b7f8526a85772f9d81eda5af183d80ef5620ab8 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 05:43:14 +0200 Subject: [PATCH 14/14] refactor: read the gap report through its helper test_launcher_layer looked for find_undeclared_files in the extras collector, which now reaches it through undeclared_report. --- tests/test_launcher_layer.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_launcher_layer.py b/tests/test_launcher_layer.py index f17efeac..dd029a6d 100644 --- a/tests/test_launcher_layer.py +++ b/tests/test_launcher_layer.py @@ -264,7 +264,10 @@ class LauncherLayerTest(unittest.TestCase): self.assertIn("LauncherLayer(", inspect.getsource(verify.find_undeclared_files)) self.assertIn(".settle(", inspect.getsource(verify.find_undeclared_files)) self.assertIn( - "find_undeclared_files(", inspect.getsource(packextras._collect_emulator_extras) + "undeclared_report(", inspect.getsource(packextras._collect_emulator_extras) + ) + self.assertIn( + "find_undeclared_files(", inspect.getsource(packextras.undeclared_report) ) for module in (generate_pack, packextras, verify): source = inspect.getsource(module)