From 3545c1b333cc29a5140b50e36d2738e6bafde76f Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 09:49:49 +0200 Subject: [PATCH 1/9] fix: scope gap report entries by platform cross_reference --platform read bundled from the profile alone and never read unread_on. It now calls validation.installed_with_emulator and unread_on with the platform key, as verify does, and a file installed with its emulator is reported as bundled whether held or not. --- scripts/cross_reference.py | 41 +++++++++++++------- tests/test_cross_reference_sources.py | 55 +++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 13 deletions(-) diff --git a/scripts/cross_reference.py b/scripts/cross_reference.py index f82966b5..b9aaa16e 100644 --- a/scripts/cross_reference.py +++ b/scripts/cross_reference.py @@ -45,7 +45,13 @@ from common import ( select_emulator_profiles, ) from hashing import parse_hash_list -from validation import case_folded_slots, declaration_settles, outside_gap_scope +from validation import ( + case_folded_slots, + declaration_settles, + installed_with_emulator, + outside_gap_scope, + unread_on, +) yaml = require_yaml() @@ -606,9 +612,15 @@ def _cross_reference_profile( report: dict, index: dict, standalone_cores: set[str] | None = None, + platform: str = "", ) -> None: """Compare one emulator profile against what the platforms declare. + *platform* is the platform key under --platform, empty for the whole + tree. It decides, through the functions verify reads, which entries the + platform's launcher never opens and which ones its emulator package + already installs. + Split out of cross_reference, where it was a 200-line loop body that carried the whole function's branching: every file is resolved by name, by path, by hash and by archive membership before it can be called a @@ -667,7 +679,7 @@ def _cross_reference_profile( if len(layouts) > 1: # One file read at one path by both builds counts once. seen_key = (effective_path.casefold(), seen_key) - if not fname or seen_key in seen_files: + if not fname or seen_key in seen_files or unread_on(f, platform): continue # Collect unsourceable files separately (documented, not a gap) @@ -760,7 +772,7 @@ def _cross_reference_profile( "name": fname, "path": effective_path, "required": f.get("required", False), - "bundled": bool(f.get("bundled")), + "bundled": installed_with_emulator(f, platform), "note": f.get("note", ""), "source_ref": f.get("source_ref", ""), "in_platform": False, @@ -800,6 +812,7 @@ def cross_reference( all_declared: dict[str, list[tuple[int | None, str]]] | None = None, standalone_cores: set[str] | None = None, folded_slots: frozenset[str] = frozenset(), + platform: str = "", ) -> dict: """Compare emulator profiles against platform declarations. @@ -841,7 +854,8 @@ def cross_reference( } for emu_name, profile in profiles.items(): _cross_reference_profile( - emu_name, profile, declared, report, index, standalone_cores + emu_name, profile, declared, report, index, standalone_cores, + platform, ) return report @@ -872,14 +886,14 @@ def print_report( if gaps == 0 and not show_clean: continue - # Counted once, for the heading and the total alike: a file bundled - # with its emulator is not one to source. - emulator_bundled = sum( - 1 for g in data["gap_details"] - if g.get("bundled") and g.get("source") == "missing" - ) + # Counted once, for the heading and the total alike: a file installed + # with its emulator is neither one to source nor one the pack brings, + # held or not (validation.deliverable leaves it out of every pack). + bundled_gaps = [g for g in data["gap_details"] if g.get("bundled")] + emulator_bundled = len(bundled_gaps) counts = {key: data.get(f"gap_{key}", 0) for key in totals} - counts["missing"] -= emulator_bundled + for g in bundled_gaps: + counts[g.get("source", "missing")] -= 1 counts["bundled"] = emulator_bundled parts = [f"{count} {key}" for key, count in counts.items() if count] status = ", ".join(parts) if parts else "OK" @@ -896,7 +910,7 @@ def print_report( for g in data["gap_details"]: req = "*" if g["required"] else " " src = g.get("source", "missing").upper() - if g.get("bundled") and g.get("source") == "missing": + if g.get("bundled"): src = "BUNDLED" # One line per file: a note spanning lines fell to column 0 # between entries and read as an emulator heading. @@ -920,7 +934,7 @@ def print_report( f"large_file: {totals['large_file']})") print(f" {totals['missing']} missing (need to be sourced)") if bundled: - print(f" {bundled} bundled with their emulator, not collected") + print(f" {bundled} bundled with their emulator, not packed") def main() -> None: @@ -1023,6 +1037,7 @@ def main() -> None: all_declared=declared_names(args.platforms_dir, selected, db), standalone_cores=standalone_cores, folded_slots=folded_declared_slots(args.platforms_dir, selected, db), + platform=config.get("_key", args.platform) if args.platform else "", ) if args.json: diff --git a/tests/test_cross_reference_sources.py b/tests/test_cross_reference_sources.py index 40a6fa91..67de9cbf 100644 --- a/tests/test_cross_reference_sources.py +++ b/tests/test_cross_reference_sources.py @@ -110,5 +110,60 @@ class GapReportSources(unittest.TestCase): ) +class PlatformScopedEntries(unittest.TestCase): + """Under --platform the report reads the platform's scoping fields + through the functions verify and the builder call: blastem's cdbios.bin + is never opened by RetroBat (unread_on), touchHLE's fonts are not in + ROCKNIX's package (not_bundled_on), fMSX's fonts come with Recalbox's + (installed_on).""" + + PROFILES = {"emu": {"emulator": "Emu", "systems": ["sys"], "files": [ + {"name": "skipped.bin", "required": True, "unread_on": ["plat"]}, + {"name": "font.ttf", "required": True, "bundled": True, + "not_bundled_on": ["plat"]}, + {"name": "held.fnt", "installed_on": ["plat"]}, + ]}} + DB = {"files": {"a" * 40: {"path": "bios/held.fnt", "name": "held.fnt"}}, + "indexes": {"by_name": {"held.fnt": ["a" * 40]}, "by_md5": {}, + "by_crc32": {}, "by_sha256": {}, "by_path_suffix": {}}} + + def _gaps(self, platform: str) -> dict[str, dict]: + report = cross_reference(self.PROFILES, {}, self.DB, platform=platform) + return {g["name"]: g for g in report["emu"]["gap_details"]} + + def test_an_entry_the_platform_never_opens_is_not_a_gap(self): + self.assertNotIn("skipped.bin", self._gaps("plat")) + self.assertIn("skipped.bin", self._gaps("other")) + self.assertIn("skipped.bin", self._gaps("")) + + def test_bundled_follows_the_platform_package(self): + gaps = self._gaps("plat") + self.assertFalse(gaps["font.ttf"]["bundled"]) + self.assertTrue(gaps["held.fnt"]["bundled"]) + gaps = self._gaps("other") + self.assertTrue(gaps["font.ttf"]["bundled"]) + self.assertFalse(gaps["held.fnt"]["bundled"]) + + def test_a_held_bundled_file_is_not_announced_as_available(self): + import contextlib + import io + + report = cross_reference(self.PROFILES, {}, self.DB, platform="plat") + with contextlib.redirect_stdout(io.StringIO()) as out: + print_report(report) + text = out.getvalue() + self.assertIn("held.fnt [BUNDLED]", text) + self.assertIn("0 available", text) + self.assertIn("1 missing (need to be sourced)", text) + + def test_the_report_reads_the_shared_rules(self): + """One rule, one function: a local `bool(f.get("bundled"))` answered + differently from verify for every installed_on/not_bundled_on entry.""" + source = (REPO_ROOT / "scripts" / "cross_reference.py").read_text() + self.assertNotIn('bool(f.get("bundled"))', source) + self.assertIn("installed_with_emulator(f, platform)", source) + self.assertIn("unread_on(f, platform)", source) + + if __name__ == "__main__": unittest.main() From 2af9509ef42d3755ce7a531b9e675887cc010b91 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 09:56:07 +0200 Subject: [PATCH 2/9] fix: resolve declarations through one function verify resolved platform entries with common.resolve_local_file and put the slot override first, and the region pass added a destination hint. packresolve.resolve_declaration now serves the builder, the manifest, verify and the region pass: release assets cached for a file absent from bios/ count as shipped in all four, and a release asset keeps its answer over a slot override. --- scripts/generate_pack.py | 17 ++++++------- scripts/packextras.py | 10 +++++--- scripts/packresolve.py | 40 ++++++++++++++++++++++++++++++ scripts/slots.py | 14 ----------- scripts/verify.py | 14 ++++++----- tests/test_audit_regressions.py | 44 +++++++++++++++++++++++++++++++-- tests/test_region_held.py | 6 ++--- 7 files changed, 108 insertions(+), 37 deletions(-) diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index fa7b434a..fb17d4a9 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -936,19 +936,16 @@ def generate_pack( total_files += 1 continue - local_path, status = resolve_file( + local_path, status = resolve_declaration( file_entry, db, bios_dir, zip_contents, - data_dir_registry=data_registry, + data_registry, + slot_overrides.get(full_dest), offline=offline, ) - local_path, status = slots.apply_override( - slot_overrides.get(full_dest), local_path, status - ) - if status == "external": file_ext = os.path.splitext(file_entry["name"])[1] or "" with tempfile.NamedTemporaryFile( @@ -4243,14 +4240,15 @@ def generate_manifest( file_entry: dict, sys_id: str, dest: str, override: str | None ) -> tuple[str | None, str]: """The file served once the slot is settled, or None and why not.""" - local_path, status = slots.apply_override(override, *resolve_file( + local_path, status = resolve_declaration( file_entry, db, bios_dir, zip_contents, - data_dir_registry=data_registry, + data_registry, + override, offline=offline, - )) + ) if status == "not_found" and not reads_file_contents(verification_mode): found = agnostic_substitute(file_entry, sys_id, db, manifest_profiles) if found: @@ -4914,6 +4912,7 @@ from packresolve import ( # noqa: E402,F401 parse_hash_input, parse_hash_file, lookup_hashes, + resolve_declaration, resolve_file, ) diff --git a/scripts/packextras.py b/scripts/packextras.py index cf046dd9..ad31cf17 100644 --- a/scripts/packextras.py +++ b/scripts/packextras.py @@ -1229,14 +1229,18 @@ def _kept(entries: list[dict], required_only: bool) -> list[dict]: return [e for e in entries if e.get("required") is not False] -def _ships(config: dict, file_entry: dict, db: dict, dest: str) -> bool: +def _ships(config: dict, file_entry: dict, db: dict) -> bool: """Whether the builder packs this declaration: resolved, and not refused by a hash frontend. A dump whose md5 Recalbox would reject is left out of the pack, so it cannot win a regional slot over a file that ships. """ + from packresolve import resolve_declaration from validation import baseline_verdict - path, status = resolve_local_file(file_entry, db, dest_hint=dest) + # The builder's resolution: no destination hint, release assets included. + path, status = resolve_declaration( + file_entry, db, "bios", None, None, None, offline=True, + ) if not path: return False return baseline_verdict( @@ -1288,7 +1292,7 @@ def platform_region_groups( if dest: members.append((dest, file_entry.get("name", ""))) if held_out is not None and db is not None and _ships( - config, file_entry, db, dest + config, file_entry, db ): held_out.add(dest) diff --git a/scripts/packresolve.py b/scripts/packresolve.py index 1d54413b..a851291f 100644 --- a/scripts/packresolve.py +++ b/scripts/packresolve.py @@ -286,6 +286,46 @@ def resolve_file( return None, "not_found" +def apply_override( + override: str | None, local_path: str | None, status: str +) -> tuple[str | None, str]: + """The file a pack serves at a destination once its slot is settled. + + An entry the platform stores outside the collection, or a release asset + fetched for its declared hash, keeps its own answer. + """ + if override and status not in ("external", "release_asset"): + return override, "slot_arbitrated" + return local_path, status + + +def resolve_declaration( + file_entry: dict, + db: dict, + bios_dir: str, + zip_contents: dict | None, + data_dir_registry: dict | None, + override: str | None, + *, + offline: bool | None = None, +) -> tuple[str | None, str]: + """The file a platform pack serves for one of the platform's declarations. + + Read by the builder, the install manifest, verify and the region pass. + verify resolved with common.resolve_local_file and put the slot override + first: without the release-asset tier a gitignored file absent from bios/ + read as missing there while the pack shipped the cached asset. + """ + return apply_override(override, *resolve_file( + file_entry, + db, + bios_dir, + zip_contents, + data_dir_registry=data_dir_registry, + offline=offline, + )) + + def preferred_entries( pack_systems: dict, db: dict, diff --git a/scripts/slots.py b/scripts/slots.py index 6a5668bc..854718fc 100644 --- a/scripts/slots.py +++ b/scripts/slots.py @@ -578,20 +578,6 @@ def slot_choices( return choices -def apply_override( - override: str | None, local_path: str | None, status: str -) -> tuple[str | None, str]: - """The file a pack serves at a destination once its slot is settled. - - An entry the platform stores outside the collection, or a release asset - fetched for its declared hash, keeps its own answer. The builder and the - install manifest both read this. - """ - if override and status not in ("external", "release_asset"): - return override, "slot_arbitrated" - return local_path, status - - def _strongest(claims: list[Claim]) -> Claim: """The profile claim with the strongest proof, the first emulator breaking ties. diff --git a/scripts/verify.py b/scripts/verify.py index 09947723..8ece9575 100644 --- a/scripts/verify.py +++ b/scripts/verify.py @@ -1047,18 +1047,20 @@ def _resolve_for_platform( ) -> tuple[str | None, str]: """Resolve one platform entry the way the pack builder does. - A slot the arbitration decided wins over the resolver, and a frontend that + The builder's own resolution (packresolve.resolve_declaration), read from + disk and the large-file cache: verify never downloads. A frontend that never reads the bytes takes the filename-free core's own image. """ + from packresolve import resolve_declaration + override = slot_overrides.get( sanitize_pack_path(file_entry.get("destination", file_entry.get("name", ""))) ) - if override: - return override, "slot_arbitrated" - local_path, status = resolve_local_file( - file_entry, db, zip_contents, data_dir_registry=data_dir_registry + local_path, status = resolve_declaration( + file_entry, db, DEFAULT_BIOS_DIR, zip_contents, data_dir_registry, + override, offline=True, ) - if local_path is None and not reads_file_contents(mode): + if status == "not_found" and not reads_file_contents(mode): found = agnostic_substitute(file_entry, sys_id, db, platform_profiles) if found: return found[0], "agnostic_fallback" diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index d12efd68..c9d8c848 100644 --- a/tests/test_audit_regressions.py +++ b/tests/test_audit_regressions.py @@ -1207,7 +1207,7 @@ class OneBaselineVerdict(unittest.TestCase): ) def test_a_release_asset_keeps_its_answer(self): - from scripts.slots import apply_override + from scripts.packresolve import apply_override self.assertEqual( apply_override("bios/x.bin", "/cache/big.bin", "release_asset"), @@ -1229,9 +1229,49 @@ class OneBaselineVerdict(unittest.TestCase): body = ast.get_source_segment(source, function) or "" with self.subTest(function=name): self.assertIn("baseline_verdict(", body) - self.assertIn("slots.apply_override(", body) + self.assertIn("resolve_declaration(", body) self.assertNotIn('"slot_arbitrated"', body) + def test_verify_and_the_region_pass_resolve_as_the_builder(self): + """verify called common.resolve_local_file and put the slot override + first, the region pass passed a destination hint the builder never + gives: a gitignored file absent from bios/ read as missing in verify + while the pack shipped its cached release asset.""" + for module, name in (("verify.py", "_resolve_for_platform"), + ("packextras.py", "_ships")): + source = (ROOT / "scripts" / module).read_text(encoding="utf-8") + function = next( + node for node in ast.parse(source).body + if isinstance(node, ast.FunctionDef) and node.name == name + ) + body = ast.get_source_segment(source, function) or "" + with self.subTest(function=name): + self.assertIn("resolve_declaration(", body) + self.assertNotIn("resolve_local_file(", body) + self.assertNotIn('"slot_arbitrated"', body) + + def test_verify_finds_the_cached_release_asset(self): + # verify imports the scripts directory's modules by their bare names. + import packresolve + from scripts.verify import _resolve_for_platform + + with tempfile.TemporaryDirectory(dir=TMP_ROOT) as directory: + cached = Path(directory) / "BIG.PUP" + cached.write_bytes(b"large firmware") + sha1 = hashlib.sha1(cached.read_bytes()).hexdigest() + db = {"files": {sha1: {"path": f"{directory}/absent/BIG.PUP", + "name": "BIG.PUP", "sha1": sha1}}, + "indexes": {"by_name": {"BIG.PUP": [sha1]}}} + entry = {"name": "BIG.PUP", "sha1": sha1, "destination": "BIG.PUP"} + with mock.patch.object( + packresolve, "fetch_large_file", return_value=str(cached) + ): + found = _resolve_for_platform( + entry, "sys", db, {}, None, {"BIG.PUP": "bios/other.bin"}, + "md5", {}, + ) + self.assertEqual(found, (str(cached), "release_asset")) + class PackExclusionRegressions(unittest.TestCase): def test_slug_platform_core_requirement_uses_the_generated_destination(self): diff --git a/tests/test_region_held.py b/tests/test_region_held.py index 158471de..cb384cb4 100644 --- a/tests/test_region_held.py +++ b/tests/test_region_held.py @@ -41,9 +41,9 @@ class ARefusedDumpHoldsNothing(unittest.TestCase): "by_path_suffix": {}}, } entry = {"name": "us.bin", "md5": "0" * 32} - self.assertFalse(_ships({"verification_mode": "md5"}, entry, db, "us.bin")) - self.assertTrue(_ships({"verification_mode": "existence"}, entry, db, "us.bin")) - self.assertTrue(_ships({"verification_mode": "md5"}, {"name": "us.bin"}, db, "us.bin")) + self.assertFalse(_ships({"verification_mode": "md5"}, entry, db)) + self.assertTrue(_ships({"verification_mode": "existence"}, entry, db)) + self.assertTrue(_ships({"verification_mode": "md5"}, {"name": "us.bin"}, db)) if __name__ == "__main__": From 4ceca6acc579659ea723366b22fc2237184dc798 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 09:56:08 +0200 Subject: [PATCH 3/9] fix: credit platforms with the file the pack ships FileConsumers resolved platform declarations with their destination as a hint, which the builder, the manifest and verify never pass. RetroBat's 63 mame/hash lists and the curated gc-ntsc-12.bin credited another copy than the one the manifests install. --- scripts/cross_reference.py | 6 +++++- tests/test_file_consumers.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/scripts/cross_reference.py b/scripts/cross_reference.py index b9aaa16e..c94645ed 100644 --- a/scripts/cross_reference.py +++ b/scripts/cross_reference.py @@ -435,7 +435,11 @@ class FileConsumers: for name, config in configs.items(): for system_id, system in config.get("systems", {}).items(): for entry in system.get("files", []): - sha1 = self.sha1_of(entry, entry.get("destination", "")) + # No destination hint, as the builder, the manifest and + # verify resolve a declaration: with one, RetroBat's 63 + # mame/hash lists credited another revision than the one + # its manifest installs. + sha1 = self.sha1_of(entry) if sha1: self.platforms.setdefault(sha1, set()).add(name) self.platform_systems.setdefault(sha1, {}).setdefault( diff --git a/tests/test_file_consumers.py b/tests/test_file_consumers.py index 94b23f13..df11a77a 100644 --- a/tests/test_file_consumers.py +++ b/tests/test_file_consumers.py @@ -83,6 +83,36 @@ class ReadersResolve(unittest.TestCase): self.assertNotIn("ioquake3", et) +class PlatformReadersResolveAsThePack(unittest.TestCase): + """A hashless declaration resolves without its destination in the + builder, the manifest and verify. The site passed the destination and + credited RetroBat with another revision of its 63 mame/hash lists than + the one its manifest installs.""" + + def test_the_declared_file_is_the_one_the_pack_ships(self): + from packresolve import resolve_declaration + + first, first_entry = _collected("bios/Arcade/MAME/hash/apfm1000.xml", b"one") + other, other_entry = _collected("bios/Other/mame/hash/apfm1000.xml", b"two") + db = { + "files": {first: first_entry, other: other_entry}, + "indexes": { + "by_name": {"apfm1000.xml": [first, other]}, + "by_path_suffix": {"mame/hash/apfm1000.xml": [other], + "Other/mame/hash/apfm1000.xml": [other], + "MAME/hash/apfm1000.xml": [first], + "Arcade/MAME/hash/apfm1000.xml": [first]}, + }, + } + entry = {"name": "apfm1000.xml", "destination": "mame/hash/apfm1000.xml"} + shipped, _status = resolve_declaration(entry, db, "bios", None, None, None, + offline=True) + consumers = FileConsumers(db, data_dir_registry={}) + consumers.add_platforms({"plat": {"systems": {"s": {"files": [entry]}}}}) + self.assertEqual(shipped, first_entry["path"]) + self.assertEqual(consumers.platforms, {first: {"plat"}}) + + class ReadersFollowTheDataTrees(unittest.TestCase): """Dolphin names codehandler.bin without a hash. The pack takes it from data/dolphin-sys, and the site credited Dolphin and Triforce with the From 084dff84f43c5ae99194f5dbc1e77437666447c5 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 09:58:09 +0200 Subject: [PATCH 4/9] fix: list what the emulator pack writes verify --emulator listed only OK destinations, while the emulator pack also writes a copy whose hash the profile contradicts, loose or archive. pack_destinations now records the paths the replay places, and both branches resolve through packresolve.resolve_file as the pack does. --- scripts/verify.py | 22 +++++++++++---- tests/test_verify_emulator_evidence.py | 37 ++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/scripts/verify.py b/scripts/verify.py index 8ece9575..5fae7154 100644 --- a/scripts/verify.py +++ b/scripts/verify.py @@ -1816,6 +1816,7 @@ def verify_emulator( `system_ids` keeps the entries of those systems, as the system pack does. """ from packextras import expand_agnostic_entries + from packresolve import resolve_file from packpaths import ( _archive_destination, _register_path, @@ -1846,6 +1847,10 @@ def verify_emulator( file_severity: dict[str, str] = {} dest_to_name: dict[str, str] = {} data_dir_notices: list[str] = [] + # The paths the emulator pack writes a profile file to. A copy whose hash + # the profile contradicts is written too (UNTESTED here, a DISCREPANCY in + # the pack), so the status alone does not say what the pack holds. + packed_dests: set[str] = set() # The emulator pack withdraws these, and the report withdraws the same. region_drops: set[str] = set() @@ -1936,11 +1941,13 @@ def verify_emulator( archive = file_entry.get("archive") if archive and archive not in seen_archives: seen_archives.add(archive) - local_path, resolve_status = resolve_local_file( + local_path, resolve_status = resolve_file( archive_entry(archive, files), db, + DEFAULT_BIOS_DIR, zip_contents, data_dir_registry=data_registry, + offline=True, ) required = any( f.get("archive") == archive and f.get("required", True) @@ -1997,6 +2004,7 @@ def verify_emulator( ) elif local_path: occupy(dest, local_path, emu_name) + packed_dests.add(dest) dest_to_name[dest] = archive cur = result["status"] prev = file_status.get(dest) @@ -2023,13 +2031,18 @@ def verify_emulator( continue dest_hint = file_entry.get("path", "") - local_path, resolve_status = resolve_local_file( + # The emulator pack's resolution, release assets included. + local_path, resolve_status = resolve_file( {**with_archive_members(file_entry, files), "source_profile": emu_name}, db, + DEFAULT_BIOS_DIR, zip_contents, dest_hint=dest_hint, data_dir_registry=data_registry, + offline=True, ) + if resolve_status in ("not_found", "user_provided", "external"): + local_path = None name = file_entry.get("name", "") required = file_entry.get("required", True) hle = file_entry.get("hle_fallback", False) @@ -2159,6 +2172,7 @@ def verify_emulator( block_reason = f"the pack holds {owner}'s {other_dest} at this path" elif not block and packed is not None: occupy(dest, packed, emu_name) + packed_dests.add(dest) for result, required, hle in alternatives: if block_reason and result["status"] != Status.MISSING: result["status"] = Status.MISSING @@ -2226,9 +2240,7 @@ def verify_emulator( "total_files": total, # The paths the emulator pack fills with the profile's files, for the # pipeline's comparison with the pack built in memory. - "pack_destinations": sorted( - dest for dest, status in file_status.items() if status == Status.OK - ), + "pack_destinations": sorted(packed_dests), "severity_counts": counts, "status_counts": status_counts, "details": details, diff --git a/tests/test_verify_emulator_evidence.py b/tests/test_verify_emulator_evidence.py index c8a20e9e..b8f498c3 100644 --- a/tests/test_verify_emulator_evidence.py +++ b/tests/test_verify_emulator_evidence.py @@ -52,6 +52,23 @@ def _db(path: Path, name: str) -> dict: } +def _pack_files(root: Path, emulators: Path, db: dict) -> set[str]: + """The paths the emulator pack writes, built in memory as the pipeline does.""" + import generate_pack + + record = generate_pack.PackRecord() + cwd = os.getcwd() + os.chdir(root) + try: + generate_pack.generate_emulator_pack( + ["demo"], str(emulators), db, "bios", "", False, {}, offline=True, + record=record, + ) + finally: + os.chdir(cwd) + return set(record.files) + + class HashMismatchIsNotCoverage(unittest.TestCase): def setUp(self): self._tmp = tempfile.TemporaryDirectory() @@ -113,6 +130,17 @@ class HashMismatchIsNotCoverage(unittest.TestCase): self.assertIn("reason", detail) self.assertIn("hash", detail["reason"].lower()) + def test_the_contradicted_copy_the_pack_writes_is_a_pack_destination(self): + """The pack writes it with a DISCREPANCY, so verify's list of what + the pack holds names it too: step 5 compared the two and failed.""" + self._profile("f" * 32) + result = self._run() + self.assertEqual(result["pack_destinations"], ["shared.rom"]) + self.assertEqual( + set(result["pack_destinations"]), + _pack_files(self.root, self.emulators, self.db), + ) + def test_a_matching_hash_is_still_ok(self): self._profile(hashlib.md5(self.rom.read_bytes()).hexdigest()) detail = next( @@ -230,6 +258,15 @@ class ArchiveHashMismatchIsNotCoverage(unittest.TestCase): "an archive whose bytes contradict its declared hash counted as covered", ) + def test_the_contradicted_archive_the_pack_writes_is_a_pack_destination(self): + self._profile("f" * 32) + result = self._run() + self.assertEqual(result["pack_destinations"], ["romset.zip"]) + self.assertEqual( + set(result["pack_destinations"]), + _pack_files(self.root, self.emulators, self.db), + ) + def test_a_matching_container_hash_is_still_ok(self): self._profile(hashlib.md5(self.archive.read_bytes()).hexdigest()) self.assertEqual(set(self._statuses(self._run())), {verify.Status.OK}) From 3b60facd3426acee40e005678ae9d66595dee7fc Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 10:01:08 +0200 Subject: [PATCH 5/9] fix: count core copies under their own category The second pass, the archive-prefix pass and the agnostic scan built extras without source_name or source_path, so verify found no gap report entry for them and counted game data such as hiscore.dat as core files. Every extra now carries its entry's category and key, and verify reads the category from the extra the builder places. --- scripts/packextras.py | 30 ++++++++++++++++++++++-------- scripts/verify.py | 21 ++++++++------------- tests/test_pack_counts.py | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 21 deletions(-) diff --git a/scripts/packextras.py b/scripts/packextras.py index ad31cf17..9861f3aa 100644 --- a/scripts/packextras.py +++ b/scripts/packextras.py @@ -215,7 +215,7 @@ def hidden_extras( placed: set[str] | None = None, report: list[dict] | None = None, resolver: ExtraResolver | None = None, - placed_by: dict[str, tuple[str, str, str]] | None = None, + placed_kind: dict[str, str] | None = None, ) -> dict[tuple[str, str, str], str]: """Report entries the builder holds and still leaves out, with the reason. @@ -231,8 +231,10 @@ def hidden_extras( file left out. ``shipped`` maps the full destination of each platform file the pack carries to its local copy, or to None for a slot it keeps without one (user_provided, external). ``placed``, when given, - receives the full destination of each held extra the pack carries. - ``resolver`` is the builder's lookup. An extra it cannot resolve is + receives the full destination of each held extra the pack carries, and + ``placed_kind`` its category, carried by the extra itself: the second + pass, the archive-prefix pass and the agnostic scan copy files no gap + report entry names. ``resolver`` is the builder's lookup. An extra it cannot resolve is skipped and takes no path, as the builder skips it. """ from packpaths import _register_path, placement_block @@ -288,11 +290,8 @@ def hidden_extras( lower.add(full.lower()) if placed is not None and local: placed.add(full) - if placed_by is not None and local: - placed_by[full] = ( - fe.get("source_emulator", ""), fe.get("source_name", ""), - fe.get("source_path", ""), - ) + if placed_kind is not None and local: + placed_kind[full] = fe.get("category", "bios") return hidden @@ -469,6 +468,9 @@ def _agnostic_scan_extras( "hle_fallback": False, "source_emulator": profile.get("emulator", emu_name), "source_profile": emu_name, + "source_name": fname, + "source_path": f.get("path") or "", + "category": f.get("category", "bios"), "source_system": f.get("system"), "source_systems": list(profile.get("systems", [])), "region": f.get("region"), @@ -622,6 +624,9 @@ def _companion_extras( "hle_fallback": False, "source_emulator": profile.get("emulator", emu_name), "source_profile": emu_name, + "source_name": seed.get("name", ""), + "source_path": seed.get("path") or "", + "category": seed.get("category", "bios"), "source_system": seed.get("system"), "source_systems": list(profile.get("systems", [])), "region": seed.get("region"), @@ -734,6 +739,10 @@ def _archive_prefix_extras( "hle_fallback": False, "source_emulator": profile.get("emulator", emu_name), "source_profile": emu_name, + # Keyed as the gap report keys an archive: name and path alike. + "source_name": archive_name, + "source_path": archive_name, + "category": archive_entry.get("category", "bios"), "source_system": archive_entry.get("system"), "source_systems": list(profile.get("systems", [])), "region": archive_entry.get("region"), @@ -1032,6 +1041,10 @@ def _collect_emulator_extras( "hle_fallback": f.get("hle_fallback", False), "source_emulator": profile.get("emulator", emu_name), "source_profile": emu_name, + # The key the gap report gives the same entry. + "source_name": fname, + "source_path": raw, + "category": f.get("category", "bios"), "source_system": f.get("system"), "source_systems": list(profile.get("systems", [])), "region": f.get("region"), @@ -1161,6 +1174,7 @@ def _report_extra(u: dict, name: str, dest: str) -> dict: # the wrong regional variant from the report. "source_name": u.get("name", ""), "source_path": u.get("path") or "", + "category": u.get("category", "bios"), "source_system": u.get("system"), "source_systems": u.get("systems", []), "region": u.get("region"), diff --git a/scripts/verify.py b/scripts/verify.py index 5fae7154..d9643fbb 100644 --- a/scripts/verify.py +++ b/scripts/verify.py @@ -885,7 +885,7 @@ def _mark_hidden( region_drops: set[str] | None = None, report: list[dict] | None = None, resolver: object | None = None, - placed_by: dict[str, tuple[str, str, str]] | None = None, + placed_kind: dict[str, str] | None = None, ) -> set[str]: """What the builder holds but cannot place beside the files it packed. @@ -900,7 +900,7 @@ def _mark_hidden( config, emulators_dir, db, emu_profiles if emu_profiles is not None else load_emulator_profiles(emulators_dir), - shipped, target_cores, region_drops, placed, report, resolver, placed_by, + shipped, target_cores, region_drops, placed, report, resolver, placed_kind, ) for u in undeclared: reason = hidden.get( @@ -1333,7 +1333,7 @@ def verify_platform( _mark_unplaced(config, undeclared, profiles) from packextras import ExtraResolver - placed_by: dict[str, tuple[str, str, str]] = {} + placed_kind: dict[str, str] = {} placed = _mark_hidden( config, undeclared, emulators_dir, db, emu_profiles, occupied, target_cores, region_drops, report_for_builder, @@ -1341,20 +1341,15 @@ def verify_platform( db, DEFAULT_BIOS_DIR, zip_contents, data_dir_registry, validation_index, validation_owners, ), - placed_by, + placed_kind, ) # What the builder places from the cores, one per pack path, by category: # several entries claim one path (rom.key from three Amiga cores) and the - # builder ships one file there. - categories = { - (u.get("emulator", ""), u.get("name", ""), u.get("path") or ""): - u.get("category", "bios") - for u in report_for_builder - } + # builder ships one file there. The category travels on the extra: a copy + # the second pass makes has no gap report entry to look it up by. packed_extras = {"bios": 0, "game_data": 0} - for key in placed_by.values(): - kind = "game_data" if categories.get(key) == "game_data" else "bios" - packed_extras[kind] += 1 + for category in placed_kind.values(): + packed_extras["game_data" if category == "game_data" else "bios"] += 1 if not reads_file_contents(mode): # An existence frontend is satisfied by any file at the path, and the # builder fills a free platform destination with a core's file. diff --git a/tests/test_pack_counts.py b/tests/test_pack_counts.py index be858eff..b4141b24 100644 --- a/tests/test_pack_counts.py +++ b/tests/test_pack_counts.py @@ -517,5 +517,41 @@ class TheReleaseNotesReadTheRecord(unittest.TestCase): self.assertIn("scripts/release_notes.py", page) +class CoreCopiesKeepTheirCategory(unittest.TestCase): + """The second pass copies a file the platform declares to the path a core + reads it from. Its extra named no gap report entry, so verify looked its + category up by an empty key and counted RetroArch's six hiscore.dat + copies, game data, under core files.""" + + def test_a_copy_counts_as_its_entry_says(self): + from packextras import _collect_emulator_extras, hidden_extras + + with tempfile.TemporaryDirectory(dir=REPO_ROOT / "tmp") as tmp: + held = Path(tmp) / "x.dat" + held.write_bytes(b"data") + sha1 = hashlib.sha1(b"data").hexdigest() + config = {"platform": "P", "_key": "p", "cores": ["demo"], "systems": { + "s": {"files": [{"name": "x.dat", "destination": "x.dat"}]}}} + profiles = {"demo": { + "emulator": "Demo", "type": "libretro", "cores": ["demo"], + "systems": ["s"], + "files": [{"name": "x.dat", "path": "core/x.dat", + "category": "game_data"}], + }} + db = {"files": {sha1: {"path": str(held), "name": "x.dat", "sha1": sha1}}, + "indexes": {"by_name": {"x.dat": [sha1]}, "by_path_suffix": {}}} + (extra,) = _collect_emulator_extras( + config, "emulators", db, {"x.dat"}, "", profiles + ) + self.assertEqual( + (extra["source_name"], extra["source_path"], extra["category"]), + ("x.dat", "core/x.dat", "game_data"), + ) + kinds: dict[str, str] = {} + hidden_extras(config, "emulators", db, profiles, {"x.dat": str(held)}, + report=[], placed_kind=kinds) + self.assertEqual(kinds, {"core/x.dat": "game_data"}) + + if __name__ == "__main__": unittest.main() From 236f30860eb9bca814ab63dd92047a3e328a3add Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 10:08:48 +0200 Subject: [PATCH 6/9] fix: check core files where the builder puts them The pack integrity check rebuilt the core extras with no taken paths and tested presence alone, so it credited core files at paths the platform's own file holds and printed a core count the builder contradicted. It now replays the builder's placement through packextras.hidden_extras and compares each placed member with the copy the builder resolves. --- scripts/packextras.py | 7 +- scripts/packverify.py | 153 +++++++++++++++++++++------------ tests/test_pack_core_extras.py | 91 ++++++++++++++++++++ wiki/testing-guide.md | 2 +- 4 files changed, 194 insertions(+), 59 deletions(-) create mode 100644 tests/test_pack_core_extras.py diff --git a/scripts/packextras.py b/scripts/packextras.py index 9861f3aa..66f15b63 100644 --- a/scripts/packextras.py +++ b/scripts/packextras.py @@ -216,6 +216,7 @@ def hidden_extras( report: list[dict] | None = None, resolver: ExtraResolver | None = None, placed_kind: dict[str, str] | None = None, + placed_local: dict[str, str] | None = None, ) -> dict[tuple[str, str, str], str]: """Report entries the builder holds and still leaves out, with the reason. @@ -234,7 +235,9 @@ def hidden_extras( receives the full destination of each held extra the pack carries, and ``placed_kind`` its category, carried by the extra itself: the second pass, the archive-prefix pass and the agnostic scan copy files no gap - report entry names. ``resolver`` is the builder's lookup. An extra it cannot resolve is + report entry names. ``placed_local`` receives the local copy the builder + writes at each of those destinations, for the pack integrity check. + ``resolver`` is the builder's lookup. An extra it cannot resolve is skipped and takes no path, as the builder skips it. """ from packpaths import _register_path, placement_block @@ -292,6 +295,8 @@ def hidden_extras( placed.add(full) if placed_kind is not None and local: placed_kind[full] = fe.get("category", "bios") + if placed_local is not None and local: + placed_local[full] = local return hidden diff --git a/scripts/packverify.py b/scripts/packverify.py index 94c13046..a48d251f 100644 --- a/scripts/packverify.py +++ b/scripts/packverify.py @@ -10,10 +10,7 @@ from collections import Counter from auto_fetch import DEFAULT_BIOS_DIR from artifacts import _build_timestamp -from packextras import _collect_emulator_extras -from packextras import _detect_extras_prefix from packpaths import _has_path_conflict -from packpaths import _register_path from ziptools import build_zip_contents_index from ziptools import check_inside_zip from nativemode import digest_algorithm @@ -525,6 +522,81 @@ def _structural_errors(zf: zipfile.ZipFile, zip_set: set) -> list[str]: return errors +def _builder_extras( + config: dict, + emulators_dir: str, + db: dict, + emu_profiles: dict, + baseline_groups: dict[str, list[dict]], + base_dest: str, + is_flat: bool, + zip_set: set[str], + zip_lower: dict[str, str], + target_cores: set[str] | None, + region_drops: set[str], + data_registry: dict | None, +) -> dict[str, str]: + """{full destination: local copy} of each core extra the builder writes. + + The platform's paths come first, as in the builder: a destination the + pack holds whose declarations resolve to a file, or that the platform + keeps for a user_provided or external entry. One whose declarations + resolve to nothing is left free, since the builder then places a core's + file there. + """ + from packextras import ExtraResolver, hidden_extras + from packresolve import resolve_declaration + from validation import _build_validation_index, destination_owners + + shipped: dict[str, str | None] = {} + for expected, declarations in baseline_groups.items(): + if expected not in zip_set and expected.lower() not in zip_lower: + continue + unflat = ( + f"{base_dest}/{expected}" + if is_flat and base_dest and not expected.startswith(f"{base_dest}/") + else expected + ) + if any(fe.get("storage") in ("user_provided", "external") for fe in declarations): + shipped[unflat] = None + continue + local = next( + ( + found for fe in declarations + if (found := resolve_declaration( + fe, db, DEFAULT_BIOS_DIR, None, data_registry, None, offline=True, + )[0]) + ), + None, + ) + if local: + shipped[unflat] = local + platform_profiles = { + name: emu_profiles[name] for name in resolve_platform_cores(config, emu_profiles) + } + placed_local: dict[str, str] = {} + hidden_extras( + config, emulators_dir, db, emu_profiles, shipped, target_cores, + region_drops or None, + resolver=ExtraResolver( + db, DEFAULT_BIOS_DIR, build_zip_contents_index(db), data_registry, + _build_validation_index(platform_profiles), + destination_owners(platform_profiles), True, + ), + placed_local=placed_local, + ) + return placed_local + + +def _same_content(zf: zipfile.ZipFile, member: str, local: str) -> bool: + """Whether a pack member carries the bytes of a local copy, or the + members of an archive the builder normalized.""" + with open(local, "rb") as handle: + if zf.read(member) == handle.read(): + return True + return local.lower().endswith(".zip") and _same_members(zf, member, local) + + def verify_pack_against_platform( zip_path: str, platform_name: str, @@ -707,66 +779,33 @@ def verify_pack_against_platform( ): errors.append(member_errors[0]) - # 2. Core extras presence (files from emulator profiles, in repo) - # Mirror the pack builder's skip logic: only count files that - # can actually be resolved and don't have path conflicts. + # 2. Core extras: the builder's own placement replayed + # (packextras.hidden_extras, which verify reads too), after the + # platform files it wrote. Each placed extra must sit at its path + # with the bytes the builder resolved: an empty seen set and a + # presence test credited a core file at a path where the pack + # holds the platform's other dump. core_checked = 0 core_present = 0 if db is not None: - core_files = _collect_emulator_extras( - config, - emulators_dir, - db, - set(), - base_dest, - emu_profiles, - target_cores=target_cores, + placed_local = _builder_extras( + config, emulators_dir, db, emu_profiles, baseline_groups, + base_dest, is_flat, zip_set, zip_lower, target_cores, + region_drops, data_registry, ) - seen_conformance: set[str] = set(zip_set) - seen_parents: set[str] = set() - for n in zip_set: - parts = n.split("/") - for i in range(1, len(parts)): - seen_parents.add("/".join(parts[:i])) - extras_pfx = _detect_extras_prefix(config, base_dest) - for fe in core_files: - raw_dest = fe.get("destination", fe.get("name", "")) - dest = sanitize_pack_path(raw_dest) - if not dest: - continue - if region_drops and sanitize_pack_path(dest) in region_drops: - continue - # The builder's own placement: the prefix, then the flattening - # of a pack written at the BIOS root. A destination that - # already starts with the prefix (aethersx2's bios/) - # lands at the root of a flat pack, not under bios/. - if extras_pfx and not dest.startswith(f"{extras_pfx}/"): - full = f"{extras_pfx}/{dest}" - else: - full = dest - full = _flat(full, base_dest, is_flat) - # Skip path conflicts (same logic as pack builder) - if _has_path_conflict(full, seen_conformance, seen_parents): - continue - # Skip unresolvable files (game_data dirs, etc.) - local_path, status = resolve_file( - fe, - db, - "bios", - {}, - dest_hint=raw_dest, - data_dir_registry=data_registry, - offline=True, - ) - if status in ("not_found", "external", "user_provided"): - continue + for full, local in sorted(placed_local.items()): core_checked += 1 - if full in zip_set or full.lower() in zip_lower: - core_present += 1 - seen_conformance.add(full) - _register_path(full, seen_conformance, seen_parents) - else: + member = _flat(full, base_dest, is_flat) + member = member if member in zip_set else zip_lower.get(member.lower()) + if member is None: errors.append(f"core missing: {full}") + elif not _same_content(zf, member, local): + errors.append( + f"core file differs: {member} is not the {os.path.basename(local)}" + " the builder places there" + ) + else: + core_present += 1 checked = baseline_checked + core_checked present = baseline_present + core_present diff --git a/tests/test_pack_core_extras.py b/tests/test_pack_core_extras.py new file mode 100644 index 00000000..9cd50724 --- /dev/null +++ b/tests/test_pack_core_extras.py @@ -0,0 +1,91 @@ +"""The pack integrity check counts the core files the builder places. + +It rebuilt the core extras with an empty set of taken paths and tested +presence alone. On a Recalbox pack it credited 68 core files at a path the +platform's own file holds, 15 of them other bytes (fuse/trdos.rom, the PS1 +BIOS DuckStation reads), and printed a core count that contradicted the +builder's in the same run. +""" + +from __future__ import annotations + +import hashlib +import os +import sys +import tempfile +import unittest +import zipfile +from pathlib import Path + +import yaml + +REPO_ROOT = Path(__file__).resolve().parent.parent +sys.path.insert(0, str(REPO_ROOT / "scripts")) + +import packverify # noqa: E402 + + +def _held(files: dict, by_name: dict, path: str, payload: bytes) -> str: + Path(path).parent.mkdir(parents=True, exist_ok=True) + Path(path).write_bytes(payload) + sha1 = hashlib.sha1(payload).hexdigest() + name = Path(path).name + files[sha1] = {"path": path, "name": name, "sha1": sha1, + "md5": hashlib.md5(payload).hexdigest(), "size": len(payload)} + by_name.setdefault(name, []).append(sha1) + return sha1 + + +class CoreFilesAsTheBuilderPlacesThem(unittest.TestCase): + def setUp(self): + tmp = tempfile.TemporaryDirectory(dir=REPO_ROOT / "tmp") + self.addCleanup(tmp.cleanup) + previous = os.getcwd() + os.chdir(tmp.name) + self.addCleanup(os.chdir, previous) + files: dict = {} + by_name: dict = {} + _held(files, by_name, "bios/plat/trdos.rom", b"platform dump") + core_trdos = _held(files, by_name, "bios/core/trdos.rom", b"core dump") + _held(files, by_name, "bios/core/other.rom", b"core file") + self.db = {"files": files, "indexes": { + "by_name": by_name, "by_md5": {e["md5"]: s for s, e in files.items()}, + "by_path_suffix": {}, "by_crc32": {}, "by_sha256": {}}} + Path("platforms").mkdir() + config = {"platform": "P", "verification_mode": "existence", "cores": ["fuse"], + "systems": {"zx": {"files": [ + {"name": "trdos.rom", "destination": "fuse/trdos.rom"}]}}} + Path("platforms/p.yml").write_text(yaml.safe_dump(config), encoding="utf-8") + Path("emulators").mkdir() + self.profiles = {"fuse": { + "emulator": "fuse", "type": "libretro", "systems": ["zx"], "files": [ + {"name": "trdos.rom", "path": "fuse/trdos.rom", "sha1": core_trdos}, + {"name": "other.rom", "path": "core/other.rom"}, + ]}} + + def _check(self, other: bytes) -> tuple: + with zipfile.ZipFile("pack.zip", "w") as zf: + zf.writestr("fuse/trdos.rom", b"platform dump") + zf.writestr("core/other.rom", other) + return packverify.verify_pack_against_platform( + "pack.zip", "p", "platforms", self.db, emulators_dir="emulators", + emu_profiles=self.profiles, + ) + + def test_a_path_the_platform_file_holds_is_not_a_core_file(self): + ok, _checked, _present, errors, _bc, _bp, core_checked, core_present, _ex = ( + self._check(b"core file") + ) + self.assertTrue(ok, errors) + self.assertEqual((core_checked, core_present), (1, 1)) + + def test_other_bytes_at_a_core_path_are_an_error(self): + ok, _checked, _present, errors, *_rest = self._check(b"something else") + self.assertFalse(ok) + self.assertTrue( + any(e.startswith("core file differs: core/other.rom") for e in errors), errors + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/wiki/testing-guide.md b/wiki/testing-guide.md index da47320f..4f05b17a 100644 --- a/wiki/testing-guide.md +++ b/wiki/testing-guide.md @@ -38,7 +38,7 @@ Every module in `tests/`, by the area it guards. | Core regression suites | synthetic | `test_e2e`, `test_audit_regressions`, `test_tool_messages` | | File resolution and identity | synthetic + collection | `test_adler32_resolution`, `test_agnostic_scan`, `test_agnostic_scan_mode`, `test_agnostic_scope`, `test_agnostic_substitute`, `test_archive_choice`, `test_bios_companions`, `test_bundled_identity`, `test_casefold_cache`, `test_db_aliases`, `test_declared_names`, `test_derivations`, `test_directory_entries`, `test_extra_identity`, `test_foreign_tree`, `test_foreign_tree_names`, `test_frontend_digest`, `test_game_data_homonyms`, `test_hashing`, `test_identity`, `test_md5_pack_context`, `test_name_tiebreak`, `test_nec_filing`, `test_owner_resolution`, `test_path_suffix`, `test_resolution_evidence`, `test_shipped_content`, `test_twin_declarations`, `test_validated_choice`, `test_variant_choice`, `test_variant_identity`, `test_zipped_member` | | Verification, cross-reference and profile checks | synthetic + profiles | `test_crypto_aes`, `test_crypto_unique_data`, `test_crypto_verify`, `test_case_insensitive_platforms`, `test_checks_have_values`, `test_core_file_fills_declaration`, `test_cross_reference_sources`, `test_file_consumers`, `test_fileless_audit`, `test_gap_settlement`, `test_hidden_extras`, `test_launcher_layer`, `test_native_mode`, `test_no_case_collisions`, `test_path_conflict_status`, `test_path_null_copies`, `test_profile_checks`, `test_romm_hash_rule`, `test_shared_groups`, `test_standalone_mode`, `test_undeclared_dedup`, `test_unplaced_extras`, `test_validation_scope`, `test_verify_all_dir`, `test_verify_emulator_evidence`, `test_verify_listing`, `test_verify_pack_choice` | -| Packs and manifests | synthetic + real packs | `test_artifact_lock`, `test_atomic_artifacts`, `test_batocera_cores`, `test_deterministic_zip`, `test_emulator_pack_counts`, `test_finalize_pack_names`, `test_large_file_cache`, `test_libretro_layout`, `test_manifest_entry`, `test_pack_conformance_dirs`, `test_pack_counts`, `test_pack_destinations`, `test_pack_failures`, `test_pack_file_names`, `test_pack_grouping`, `test_pack_integrity`, `test_pack_member_identity`, `test_pack_readme`, `test_platform_inheritance`, `test_platform_keys`, `test_required_only_archives`, `test_required_only_twins`, `test_split_extras`, `test_split_names`, `test_split_pack`, `test_system_pack_extras`, `test_system_pack_parity`, `test_torrentzip`, `test_which_pack` | +| Packs and manifests | synthetic + real packs | `test_artifact_lock`, `test_atomic_artifacts`, `test_batocera_cores`, `test_deterministic_zip`, `test_emulator_pack_counts`, `test_finalize_pack_names`, `test_large_file_cache`, `test_libretro_layout`, `test_manifest_entry`, `test_pack_conformance_dirs`, `test_pack_core_extras`, `test_pack_counts`, `test_pack_destinations`, `test_pack_failures`, `test_pack_file_names`, `test_pack_grouping`, `test_pack_integrity`, `test_pack_member_identity`, `test_pack_readme`, `test_platform_inheritance`, `test_platform_keys`, `test_required_only_archives`, `test_required_only_twins`, `test_split_extras`, `test_split_names`, `test_split_pack`, `test_system_pack_extras`, `test_system_pack_parity`, `test_torrentzip`, `test_which_pack` | | Slots and regions | synthetic + profiles | `test_region`, `test_region_agnostic_scan`, `test_region_groups`, `test_region_held`, `test_region_platform_index`, `test_region_required_only`, `test_slot`, `test_slots`, `test_slots_shipped_file` | | Hardware targets and systems | synthetic + target files | `test_extract_targets`, `test_lakka_targets`, `test_libretro_slugs`, `test_system_ids`, `test_system_selection`, `test_target_names`, `test_target_scrapers`, `test_target_systems`, `test_target_verify` | | Installer and downloads | loopback server | `test_check_release`, `test_download`, `test_install`, `test_install_cores`, `test_install_experience`, `test_install_selection` | From 604ab3371b8dc8615494f62353d9df444fc6e9fb Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 10:09:51 +0200 Subject: [PATCH 7/9] fix: give a suffixed release its emulator's tree _foreign_tree compared owner tokens by equality, so cxbx-reloaded-2023 read bios/Other/cxbx-reloaded/ as another emulator's tree and the case-folded fallback served xemu's encrypted EEPROM. The leading whole words of a profile key now name its own tree too. Across every profile entry this moves cxbx-reloaded-2023's EEPROM.bin to the decrypted dump, turns eight name matches on the same files into path matches, and gives solarus-2.0.0 the Solarus tree's gamecontrollerdb.txt. --- scripts/common.py | 12 +++++++++- tests/test_foreign_tree.py | 45 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/scripts/common.py b/scripts/common.py index 35b53465..89f8daef 100644 --- a/scripts/common.py +++ b/scripts/common.py @@ -934,11 +934,21 @@ def _affinity_tokens(text: str) -> set[str]: def _owner_tokens(file_entry: dict) -> set[str]: - """Tokens naming the profile that asks for the file.""" + """Tokens naming the profile that asks for the file. + + A profile named after another with a suffix (cxbx-reloaded-2023, + mame4droid-2024) is a release of that emulator, and the tree named by + the shorter name is its own: cxbx-reloaded-2023 was served xemu's + encrypted EEPROM because bios/Other/cxbx-reloaded/ read as foreign. + Only whole leading words count, so dolphin-mmjr2 is not dolphin-mmjr. + """ tokens: set[str] = set() for owner in (file_entry.get("source_profile"), file_entry.get("source_emulator")): if owner: tokens |= _affinity_tokens(str(owner).replace(" ", "")) + profile = str(file_entry.get("source_profile") or "").casefold() + words = [word for word in re.split(r"[^0-9a-z]+", profile) if word] + tokens |= {"".join(words[:end]) for end in range(1, len(words))} return tokens diff --git a/tests/test_foreign_tree.py b/tests/test_foreign_tree.py index e4530590..4fba40a8 100644 --- a/tests/test_foreign_tree.py +++ b/tests/test_foreign_tree.py @@ -68,5 +68,50 @@ class ForeignTree(unittest.TestCase): self.assertNotEqual(status, "path_exact") +class AReleaseNamedAfterTheTree(unittest.TestCase): + """cxbx-reloaded-2023 asks for EEPROM.bin. Its emulator's tree holds the + decrypted dump Cxbx reads, xemu's aliases name the encrypted retail one + eeprom.bin. The tree read as foreign to the suffixed profile, so the + case-folded fallback served the encrypted dump.""" + + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self._cwd = os.getcwd() + os.chdir(self._tmp.name) + files = {} + for rel, payload in ( + ("bios/Other/cxbx-reloaded/EEPROM.bin", b"decrypted"), + ("bios/Microsoft/Xbox/eeprom.bin", b"encrypted"), + ): + path = Path(rel) + path.parent.mkdir(parents=True, exist_ok=True) + path.write_bytes(payload) + sha1 = hashlib.sha1(payload).hexdigest() + files[sha1] = {"path": rel, "name": path.name, "size": len(payload), + "sha1": sha1, "md5": hashlib.md5(payload).hexdigest(), + "sha256": hashlib.sha256(payload).hexdigest(), + "crc32": f"{len(files) + 1:08x}"} + self.db = {"files": files, "indexes": generate_db.build_indexes(files, {})} + + def tearDown(self): + os.chdir(self._cwd) + self._tmp.cleanup() + + def _resolve(self, owner: str, name: str): + entry = {"name": name, "source_profile": owner} + return resolve_local_file(entry, self.db, {}) + + def test_the_suffixed_release_reads_its_emulator_tree(self): + local, _status = self._resolve("cxbx-reloaded-2023", "EEPROM.bin") + self.assertEqual(local, "bios/Other/cxbx-reloaded/EEPROM.bin") + + def test_only_whole_leading_words_name_the_tree(self): + """dolphin-mmjr2 is not a release of dolphin-mmjr: mmjr2 is one word.""" + from common import _owner_tokens + + self.assertNotIn("dolphinmmjr", _owner_tokens({"source_profile": "dolphin-mmjr2"})) + self.assertIn("cxbxreloaded", _owner_tokens({"source_profile": "cxbx-reloaded-2023"})) + + if __name__ == "__main__": unittest.main() From 5004c4d8d5cc85c9fcf1b38070da529b8b0768df Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 10:14:09 +0200 Subject: [PATCH 8/9] fix: refuse an archive holding no declared member _holding_members kept every same-named copy when the best held nothing, so galgames.zip, a lone assembler listing, resolved by name for mame, groovymame and mame4droid-2024, and awbios.zip for mame4droid. Such a copy now resolves as hash_mismatch. mame2003_plus declared its contents by name only, while fileio.c:1031-1040 loads a member by CRC when the name is absent. The 61 members now carry the CRC their driver declares, so stvbios, megatech and the six ZN sets keep matching the archives the collection holds. --- emulators/mame2003_plus.yml | 60 +++++++++++++++++++++++++++++++++++- scripts/common.py | 4 +++ tests/test_archive_choice.py | 14 +++++++++ 3 files changed, 77 insertions(+), 1 deletion(-) diff --git a/emulators/mame2003_plus.yml b/emulators/mame2003_plus.yml index e8ce6872..a9139a0e 100644 --- a/emulators/mame2003_plus.yml +++ b/emulators/mame2003_plus.yml @@ -198,51 +198,69 @@ files: category: bios_zip system: snk-neogeo-mvs note: "MVS/AES BIOS, 15 variants including Universe BIOS 4.0" - source_ref: "src/drivers/neogeo.c:1677-1714" + source_ref: "src/drivers/neogeo.c:1677-1714, 1726-1734, 1803" contents: # 68K BIOS variants (one active at a time via dipswitch) - name: sp-s2.sp1 description: Europe MVS v2, default + crc32: "9036d879" - name: sp-s.sp1 description: Europe MVS v1 + crc32: "c7f2fa45" - name: usa_2slt.bin description: US MVS v2 + crc32: "e72943de" - name: sp-e.sp1 description: US MVS v1 + crc32: "2723a5b5" - name: asia-s3.rom description: Asia MVS v3 + crc32: "91b64be3" - name: vs-bios.rom description: Japan MVS v6 + crc32: "f0e8f27d" - name: sp-j2.sp1 description: Japan MVS v2 + crc32: "acede59c" - name: uni-bios_4_0.rom description: Universe BIOS 4.0, hack + crc32: "a7aab458" - name: uni-bios_3_3.rom description: Universe BIOS 3.3, hack + crc32: "24858466" - name: uni-bios_2_0.rom description: Universe BIOS 2.0, hack + crc32: "0c12c2ad" - name: uni-bios_1_3.rom description: Universe BIOS 1.3, hack + crc32: "b24b44a0" - name: uni-bios_1_1.rom description: Universe BIOS 1.1, hack + crc32: "5dda0d84" - name: uni-bios_1_0.rom description: Universe BIOS 1.0, hack + crc32: "0ce453a0" - name: neodebug.rom description: Debug MVS + crc32: "698ebb7d" - name: neo-epo.bin description: Asia AES + crc32: "d27a71f1" # Z80 sound BIOS (always required) - name: mame.sm1 description: Z80 sound BIOS size: 131072 + crc32: "97cf998b" # Fix tiles (always required) - name: sfix.sfx description: fix layer tiles size: 131072 + crc32: "354029fc" # Zoom table (always required) - name: mamelo.lo description: Y zoom control size: 65536 + crc32: "e09e253c" # --------------------------------------------------------- # IGS PGM (pgm.zip) @@ -257,12 +275,15 @@ files: - name: pgm_p01s.rom description: 68K BIOS size: 131072 + crc32: "e42b166e" - name: pgm_t01s.rom description: text tiles size: 2097152 + crc32: "1a7123a0" - name: pgm_m01s.rom description: samples size: 2097152 + crc32: "45ae7159" # --------------------------------------------------------- # Sega ST-V (stvbios.zip) @@ -276,16 +297,22 @@ files: contents: - name: epr19730.ic8 description: Japan + crc32: "d0e0889d" - name: mp17951a.s description: Japan alt + crc32: "2672f9d8" - name: mp17952a.s description: USA + crc32: "d1be2adf" - name: 20091.bin description: Japan alt 2 + crc32: "59ed40f4" - name: mp17953a.ic8 description: Taiwan + crc32: "a4c47570" - name: mp17954a.s description: Europe + crc32: "f7722da3" # --------------------------------------------------------- # Sega Mega Play (megaplay.zip) @@ -299,8 +326,10 @@ files: contents: - name: ep15294.ic2 description: Mega Play BIOS v1 + crc32: "aa8dc2d8" - name: megaplay.bin description: Mega Play BIOS v2 + crc32: "f97c68aa" # --------------------------------------------------------- # Sega Mega Tech (megatech.zip) @@ -315,6 +344,7 @@ files: - name: epr12664.20 description: Mega Tech BIOS size: 32768 + crc32: "f71e9526" # --------------------------------------------------------- # DECO Cassette (decocass.zip) @@ -328,20 +358,27 @@ files: contents: - name: v0a-.7e description: 6502 BIOS variant A + crc32: "3d33ac34" - name: rms8.cpu description: 6502 BIOS variant B + crc32: "23d929b7" - name: rms8.snd description: audio CPU size: 2048 + crc32: "b66b2c2a" - name: cass8041.bin description: MCU size: 1024 + crc32: "a6df18fd" - name: dsp8.3m description: PROM + crc32: "238fdb40" - name: dsp8.10d description: PROM + crc32: "3b5836b4" - name: rms8.j3 description: DRAM banking PROM + crc32: "51eef657" # --------------------------------------------------------- # Nintendo PlayChoice-10 (playch10.zip) @@ -356,18 +393,25 @@ files: - name: pch1-c.8t description: Z80 BIOS size: 16384 + crc32: "d52fa07a" - name: pch1-c.8p description: character ROM + crc32: "30c15e23" - name: pch1-c.8m description: character ROM + crc32: "c1232eee" - name: pch1-c.8k description: character ROM + crc32: "9acffb30" - name: 82s129.6f description: color PROM + crc32: "e5414ca3" - name: 82s129.6e description: color PROM + crc32: "a2625c6e" - name: 82s129.6d description: color PROM + crc32: "1213ebd4" # --------------------------------------------------------- # Nintendo Super System (nss.zip) @@ -382,12 +426,15 @@ files: - name: spc700.rom description: SPC700 boot ROM size: 64 + crc32: "38000b6b" - name: nss-c.dat description: NSS BIOS CPU size: 32768 + crc32: "a8e202b3" - name: nss-ic14.02 description: NSS BIOS CPU alt size: 32768 + crc32: "e06cb58f" # --------------------------------------------------------- # Super Kaneko Nova System (skns.zip) @@ -402,15 +449,19 @@ files: - name: sknsj1.u10 description: Japan BIOS size: 524288 + crc32: "7e2b836c" - name: sknse1.u10 description: Europe BIOS size: 524288 + crc32: "e2b9d7d1" - name: sknsa1.u10 description: Asia BIOS size: 524288 + crc32: "745e5212" - name: sknsu1.u10 description: USA BIOS size: 524288 + crc32: "384d21ec" # --------------------------------------------------------- # Konami GX (konamigx.zip) @@ -425,6 +476,7 @@ files: - name: 300a01.34k description: GX BIOS size: 131072 + crc32: "d5fa95f5" # --------------------------------------------------------- # ZN-1/ZN-2 PlayStation-based arcade boards @@ -443,6 +495,7 @@ files: - name: coh1002c.bin description: Capcom ZN-1 BIOS size: 524288 + crc32: "50033af6" # Capcom ZN-2 - name: cpzn2.zip @@ -455,6 +508,7 @@ files: - name: coh3002c.bin description: Capcom ZN-2 BIOS size: 524288 + crc32: "e860ea8b" # Taito FX-1 - name: taitofx1.zip @@ -467,6 +521,7 @@ files: - name: coh1002t.bin description: Taito FX-1 BIOS size: 524288 + crc32: "e3f23b6e" # Tecmo TPS - name: tps.zip @@ -479,6 +534,7 @@ files: - name: coh1002m.bin description: Tecmo TPS BIOS size: 524288 + crc32: "69ffbcb4" # Video System / Taito KN - name: sncwgltd.zip @@ -491,6 +547,7 @@ files: - name: coh1002v.bin description: KN BIOS size: 524288 + crc32: "5ff165f3" # Acclaim PSX - name: acpsx.zip @@ -503,6 +560,7 @@ files: - name: coh1000a.bin description: Acclaim PSX BIOS size: 524288 + crc32: "8d8d0764" # Century Electronics CVS - name: cvs.zip diff --git a/scripts/common.py b/scripts/common.py index 89f8daef..2d694d77 100644 --- a/scripts/common.py +++ b/scripts/common.py @@ -1094,6 +1094,10 @@ def _holding_members(paths: list[str], members: list[dict]) -> list[str]: scores = {path: held(path) for path in paths} best = max(scores.values(), default=(0, 0)) + if best == (0, 0): + # Not one declared member in any copy: the name is all that matches, + # and galgames.zip holding only an assembler listing is not the set. + return [] return [path for path in paths if scores[path] == best] diff --git a/tests/test_archive_choice.py b/tests/test_archive_choice.py index 046742d3..e6e0732e 100644 --- a/tests/test_archive_choice.py +++ b/tests/test_archive_choice.py @@ -316,6 +316,20 @@ class AnArchiveRankedByWhatItHolds(unittest.TestCase): for name in ("epr-21330.ic27", "epr-21332.ic27")]} self.assertEqual(self._resolve(entry, db), "bios/MAME/hod2bios.zip") + def test_a_copy_holding_no_declared_member_is_not_the_archive(self): + """galgames.zip held only an assembler listing and none of the six + ROMs MAME declares: the name step served it as the BIOS set.""" + from common import resolve_local_file + + db = self._db([("bios/MAME/hod2bios.zip", ["listing.lst"])]) + entry = {"name": "hod2bios.zip", "contents": [ + {"name": name, "crc32": format(zlib.crc32(self.roms[name]), "08x")} + for name in ("epr-21330.ic27", "epr-21332.ic27")]} + path, status = resolve_local_file(entry, db) + self.assertEqual(status, "hash_mismatch") + self.assertEqual(Path(path).relative_to(self.root).as_posix(), + "bios/MAME/hod2bios.zip") + def test_without_a_layout_a_superset_primary_wins(self): db = self._db([("bios/Arcade/hod2bios.zip", ["epr-21330.ic27"]), ("bios/MAME/hod2bios.zip", list(self.roms))]) From ab35bb407a87476be1b11974ce11da8b659198da Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 10:14:42 +0200 Subject: [PATCH 9/9] fix: keep the Apple II SCSI ROM from np2pi np2pi reads 0x4000 bytes of scsi.rom as the SCSI BIOS (cbus/scsiio.c: 208-233). The only 16 KB copy of that name is Clock Signal's Apple II card ROM, which the name step served and the RetroPie manifest installed. The entry now carries unsourceable: and np2pi falls back on its stub. --- emulators/np2pi.yml | 1 + tests/test_foreign_tree.py | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/emulators/np2pi.yml b/emulators/np2pi.yml index 06324efa..66d29cef 100644 --- a/emulators/np2pi.yml +++ b/emulators/np2pi.yml @@ -190,3 +190,4 @@ files: hle_fallback: true note: "Read when a SCSI disk is attached, 0x4000 bytes. Otherwise the built-in stub is copied." source_ref: "cbus/scsiio.c:208-233" + unsourceable: "PC-98 SCSI BIOS image the collection does not hold, and a same-named ROM of another machine must not stand in" diff --git a/tests/test_foreign_tree.py b/tests/test_foreign_tree.py index 4fba40a8..47056e55 100644 --- a/tests/test_foreign_tree.py +++ b/tests/test_foreign_tree.py @@ -113,5 +113,31 @@ class AReleaseNamedAfterTheTree(unittest.TestCase): self.assertIn("cxbxreloaded", _owner_tokens({"source_profile": "cxbx-reloaded-2023"})) +class AnotherMachinesRomIsNotServed(unittest.TestCase): + """np2pi reads a 16 KB PC-98 SCSI BIOS as scsi.rom. The only copy of that + size was Clock Signal's Apple II SCSI card ROM, and the name step served + it. Reads the repository.""" + + def test_np2pi_does_not_receive_the_apple_ii_rom(self): + import yaml + + from common import load_database + + if not (REPO_ROOT / "database.json").is_file(): + self.skipTest("database.json is absent") + profile = yaml.safe_load((REPO_ROOT / "emulators" / "np2pi.yml").read_text()) + entry = next(f for f in profile["files"] if f.get("name") == "scsi.rom") + previous = os.getcwd() + os.chdir(REPO_ROOT) + try: + db = load_database("database.json") + local, _status = resolve_local_file( + {**entry, "source_profile": "np2pi"}, db, {}, dest_hint=entry["path"] + ) + finally: + os.chdir(previous) + self.assertFalse(local and "Clock Signal" in local, local) + + if __name__ == "__main__": unittest.main()