From 1455e0423ce542e1c062ed20371a86c0eb6380e5 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:02:39 +0200 Subject: [PATCH] fix: arbitrate on the evidence the builder uses --- scripts/generate_pack.py | 2 ++ scripts/slots.py | 61 ++++++++++++++++++++++++++++++++++------ scripts/verify.py | 16 ++++++----- tests/test_slots.py | 31 ++++++++++++++++++++ 4 files changed, 95 insertions(+), 15 deletions(-) diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index 7e5f2134..3666146c 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -621,6 +621,8 @@ def generate_pack( db, base_dest, {str(c) for c in config.get("standalone_cores", [])}, + zip_contents, + data_registry, ): decision = slots.arbitrate(conflict, mode) if decision.serves_both and decision.winner.local_path: diff --git a/scripts/slots.py b/scripts/slots.py index efecda9a..0fa9c4bc 100644 --- a/scripts/slots.py +++ b/scripts/slots.py @@ -20,6 +20,8 @@ from dataclasses import dataclass, field import nativemode from common import ( + build_zip_contents_index, + load_data_dir_registry, resolution_is_hash_exact, resolve_local_file, ) @@ -86,7 +88,13 @@ def build_claim_index(claims: list[Claim]) -> dict[str, list[Claim]]: return index -def platform_claims(config: dict, db: dict, base_dest: str = "") -> list[Claim]: +def platform_claims( + config: dict, + db: dict, + base_dest: str = "", + zip_contents: dict | None = None, + data_dir_registry: dict | None = None, +) -> list[Claim]: """What the platform YAML says belongs at each of its destinations.""" claims: list[Claim] = [] for system in (config.get("systems") or {}).values(): @@ -97,7 +105,13 @@ def platform_claims(config: dict, db: dict, base_dest: str = "") -> list[Claim]: if not dest: continue full = f"{base_dest}/{dest}" if base_dest else dest - local, status = resolve_local_file(entry, db, dest_hint=dest) + local, status = resolve_local_file( + entry, + db, + zip_contents, + dest_hint=dest, + data_dir_registry=data_dir_registry, + ) claims.append( Claim( origin="platform", @@ -116,6 +130,8 @@ def profile_claims( db: dict, base_dest: str = "", standalone_cores: set[str] | None = None, + zip_contents: dict | None = None, + data_dir_registry: dict | None = None, ) -> list[Claim]: """What each emulator profile says belongs at each destination it names. @@ -157,7 +173,13 @@ def profile_claims( if not dest: continue full = f"{base_dest}/{dest}" if base_dest else dest - local, status = resolve_local_file(entry, db, dest_hint=dest) + local, status = resolve_local_file( + entry, + db, + zip_contents, + dest_hint=dest, + data_dir_registry=data_dir_registry, + ) claims.append( Claim( origin="profile", @@ -178,6 +200,8 @@ def find_conflicts( db: dict, base_dest: str = "", standalone_cores: set[str] | None = None, + zip_contents: dict | None = None, + data_dir_registry: dict | None = None, ) -> list[Conflict]: """Destinations where a proven profile claim contradicts what ships. @@ -185,7 +209,9 @@ def find_conflicts( asserts nothing about content and cannot contradict anything. """ by_dest: dict[str, Claim] = {} - for claim in platform_claims(config, db, base_dest): + for claim in platform_claims( + config, db, base_dest, zip_contents, data_dir_registry + ): by_dest.setdefault(_normalize(claim.destination), claim) # Grouped before judging: a profile may declare several revisions that are @@ -193,7 +219,9 @@ def find_conflicts( # is agreement, not contradiction. Only a destination where no profile # claim at all matches what ships is a disagreement. by_slot: dict[str, list[Claim]] = {} - for claim in profile_claims(profiles, db, base_dest, standalone_cores): + for claim in profile_claims( + profiles, db, base_dest, standalone_cores, zip_contents, data_dir_registry + ): key = _normalize(claim.destination) platform = by_dest.get(key) if platform is None or not platform.is_proven or not claim.is_proven: @@ -295,7 +323,12 @@ def _collision_json(collision: Collision) -> dict: } -def find_collisions(config: dict, db: dict) -> list[Collision]: +def find_collisions( + config: dict, + db: dict, + zip_contents: dict | None = None, + data_dir_registry: dict | None = None, +) -> list[Collision]: """Destinations a platform declares twice and resolves two ways. One path holds one file, so whichever declaration the builder reaches @@ -321,7 +354,11 @@ def find_collisions(config: dict, db: dict) -> list[Collision]: resolved = [] for entry in entries: local, _ = resolve_local_file( - entry, db, dest_hint=entry.get("destination", "") + entry, + db, + zip_contents, + dest_hint=entry.get("destination", ""), + data_dir_registry=data_dir_registry, ) if local and local not in resolved: resolved.append(local) @@ -376,12 +413,17 @@ def scan_platform( config = load_platform_config(platform, platforms_dir) keys = resolve_platform_cores(config, profiles) relevant = {k: profiles[k] for k in keys if k in profiles} + # The same evidence the builder and the verifier resolve with: without + # the ZIP index and the data-directory registry the arbitration judged + # with less than the tools that read its verdict. return find_conflicts( config, relevant, db, config.get("base_destination", ""), {str(c) for c in config.get("standalone_cores", [])}, + build_zip_contents_index(db), + load_data_dir_registry(platforms_dir), ) @@ -428,7 +470,10 @@ def main() -> int: if conflicts: found[name] = conflicts collisions = find_collisions( - load_platform_config(name, args.platforms_dir), db + load_platform_config(name, args.platforms_dir), + db, + build_zip_contents_index(db), + load_data_dir_registry(args.platforms_dir), ) if collisions: collided[name] = collisions diff --git a/scripts/verify.py b/scripts/verify.py index 782d74ca..93d62fec 100644 --- a/scripts/verify.py +++ b/scripts/verify.py @@ -797,6 +797,13 @@ def verify_platform( # The builder settles a destination claimed by both layers; this must read # the same decision, or the two tools describe different packs. + has_zipped = any( + fe.get("zipped_file") + for sys in config.get("systems", {}).values() + for fe in sys.get("files", []) + ) + zip_contents = build_zip_contents_index(db) if has_zipped else {} + slot_overrides: dict[str, str] = {} if emu_profiles: base_dest = config.get("base_destination", "") @@ -810,6 +817,8 @@ def verify_platform( db, base_dest, {str(c) for c in config.get("standalone_cores", [])}, + zip_contents, + data_dir_registry, ): decision = slots.arbitrate(conflict, mode) if decision.serves_both and decision.winner.local_path: @@ -818,13 +827,6 @@ def verify_platform( key = key[len(base_dest) + 1:] slot_overrides[key] = decision.winner.local_path - has_zipped = any( - fe.get("zipped_file") - for sys in config.get("systems", {}).values() - for fe in sys.get("files", []) - ) - zip_contents = build_zip_contents_index(db) if has_zipped else {} - # Build HLE + validation indexes from emulator profiles profiles = ( emu_profiles diff --git a/tests/test_slots.py b/tests/test_slots.py index b4fc46a0..fc48e1f8 100644 --- a/tests/test_slots.py +++ b/tests/test_slots.py @@ -179,6 +179,37 @@ class TestConflicts(unittest.TestCase): slots.find_conflicts(self._config("m" * 32), profile, REGIONS_DB), [] ) + def test_the_arbitration_reads_the_same_evidence_as_the_builder(self): + """A verdict decided on less evidence than its consumers apply. + + slots resolved without the ZIP index and without the data-directory + registry, while generate_pack and verify pass both, so an entry only + a ZIP member or a data directory can satisfy looked unproven here and + proven there. + """ + import inspect + + import generate_pack + import verify + + for function in ( + slots.platform_claims, + slots.profile_claims, + slots.find_conflicts, + slots.find_collisions, + ): + parameters = inspect.signature(function).parameters + with self.subTest(function=function.__name__): + self.assertIn("zip_contents", parameters) + self.assertIn("data_dir_registry", parameters) + + # And both consumers hand theirs over rather than letting it default. + for module in (generate_pack, verify): + source = inspect.getsource(module) + call = source[source.index("slots.find_conflicts(") :][:400] + with self.subTest(module=module.__name__): + self.assertIn("zip_contents", call) + def test_a_rom_inside_a_romset_claims_nothing_of_its_own(self): """The archive occupies the destination, not the ROM it holds.