From 68bcef544d60ba52efeadbec0d2db60c1da436e4 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Mon, 5 Oct 2026 21:53:59 +0200 Subject: [PATCH] fix: give the install manifest the arbitrated file --- scripts/generate_pack.py | 27 +++++---------- scripts/slots.py | 40 ++++++++++++++++++++++ scripts/verify.py | 29 ++++------------ tests/test_slots.py | 74 ++++++++++++++++++++++++++++++++++++---- 4 files changed, 123 insertions(+), 47 deletions(-) diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index 56815bbc..5d709258 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -651,30 +651,15 @@ def generate_pack( from common import resolve_platform_cores validation_index = {} - slot_overrides: dict[str, str] = {} if emu_profiles: platform_profiles = { name: emu_profiles[name] for name in resolve_platform_cores(config, emu_profiles) } validation_index = _build_validation_index(platform_profiles) - # Where a source-verified profile contradicts the scraped baseline on - # one destination, the pack answers to the platform it is built for. - # In existence mode the frontend never reads the bytes, so serving the - # emulator's file satisfies both sides and nothing is traded away. - mode = config.get("verification_mode", "existence") - for conflict in slots.find_conflicts( - config, - platform_profiles, - 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: - slot_overrides[conflict.destination] = decision.winner.local_path + slot_overrides = slots.pack_overrides( + config, emu_profiles or {}, db, zip_contents, data_registry + ) # Filter systems by target if specified plat_cores = ( @@ -2967,6 +2952,9 @@ def generate_manifest( pack_only_sizes: list[int] = [] if data_registry is None: data_registry = load_data_dir_registry(platforms_dir) + slot_overrides = slots.pack_overrides( + config, emu_profiles, db, zip_contents, data_registry + ) def manifest_destination(full_destination: str) -> str: if base_dest and full_destination.startswith(f"{base_dest}/"): @@ -3062,6 +3050,9 @@ def generate_manifest( data_dir_registry=data_registry, offline=offline, ) + override = slot_overrides.get(full_dest) + if override: + local_path, status = override, "slot_arbitrated" if ( status == "hash_mismatch" and local_path diff --git a/scripts/slots.py b/scripts/slots.py index ae35cab3..a42d9e2c 100644 --- a/scripts/slots.py +++ b/scripts/slots.py @@ -264,6 +264,46 @@ class Decision: return self.reason == SERVES_BOTH +def pack_overrides( + config: dict, + profiles: dict, + db: dict, + zip_contents: dict | None = None, + data_dir_registry: dict | None = None, +) -> dict[str, str]: + """Full pack destination -> the file a platform pack serves there instead. + + Where a source-verified profile contradicts the scraped baseline on one + destination, the pack answers to the platform it is built for; in + existence mode the frontend never reads the bytes, so serving the + emulator's file satisfies both. The ZIP builder, the install manifest and + verify all read this: two of them deciding alone gave the one-line + installer different bytes than the ZIP. + """ + from common import resolve_platform_cores + + if not profiles: + return {} + platform_profiles = { + name: profiles[name] for name in resolve_platform_cores(config, profiles) + } + mode = config.get("verification_mode", "existence") + overrides: dict[str, str] = {} + for conflict in find_conflicts( + config, + platform_profiles, + db, + config.get("base_destination", ""), + {str(c) for c in config.get("standalone_cores", [])}, + zip_contents, + data_dir_registry, + ): + decision = arbitrate(conflict, mode) + if decision.serves_both and decision.winner.local_path: + overrides[conflict.destination] = decision.winner.local_path + return overrides + + def arbitrate(conflict: Conflict, mode: str, addressee: str = "platform") -> Decision: """Decide a contested destination for the pack being built. diff --git a/scripts/verify.py b/scripts/verify.py index f5012802..e8b7fd93 100644 --- a/scripts/verify.py +++ b/scripts/verify.py @@ -730,28 +730,13 @@ def verify_platform( ) 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", "") - arbitrated = { - name: emu_profiles[name] - for name in resolve_platform_cores(config, emu_profiles) - } - for conflict in slots.find_conflicts( - config, - arbitrated, - 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: - key = conflict.destination - if base_dest and key.startswith(f"{base_dest}/"): - key = key[len(base_dest) + 1:] - slot_overrides[key] = decision.winner.local_path + base_dest = config.get("base_destination", "") + slot_overrides = { + (key[len(base_dest) + 1:] if base_dest and key.startswith(f"{base_dest}/") else key): path + for key, path in slots.pack_overrides( + config, emu_profiles or {}, db, zip_contents, data_dir_registry + ).items() + } # Build HLE + validation indexes from emulator profiles profiles = ( diff --git a/tests/test_slots.py b/tests/test_slots.py index efe28173..be440b2c 100644 --- a/tests/test_slots.py +++ b/tests/test_slots.py @@ -203,12 +203,25 @@ class TestConflicts(unittest.TestCase): 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): + parameters = inspect.signature(slots.pack_overrides).parameters + self.assertIn("zip_contents", parameters) + self.assertIn("data_dir_registry", parameters) + + # Every consumer reads the one decision and hands its indexes over: + # the ZIP builder, the install manifest and the report. The manifest + # deciding nothing gave the installer other bytes than the ZIP. + for module, calls in ((generate_pack, 2), (verify, 1)): source = inspect.getsource(module) - call = source[source.index("slots.find_conflicts(") :][:400] with self.subTest(module=module.__name__): - self.assertIn("zip_contents", call) + self.assertNotIn("slots.arbitrate(", source) + self.assertNotIn("slots.find_conflicts(", source) + self.assertEqual(source.count("slots.pack_overrides("), calls) + for start in range(len(source)): + start = source.find("slots.pack_overrides(", start) + if start < 0: + break + self.assertIn("zip_contents", source[start:start + 200]) + start += 1 def test_a_rom_inside_a_romset_claims_nothing_of_its_own(self): """The archive occupies the destination, not the ROM it holds. @@ -299,9 +312,12 @@ class TestBuilderAndVerifierAgree(unittest.TestCase): verifier = Path(__file__).resolve().parents[1] / "scripts" / "verify.py" for source in (builder, verifier): text = source.read_text(encoding="utf-8") - self.assertIn("slots.find_conflicts(", text, source.name) - self.assertIn("slots.arbitrate(", text, source.name) - self.assertIn("decision.serves_both", text, source.name) + self.assertIn("slots.pack_overrides(", text, source.name) + self.assertNotIn("decision.serves_both", text, source.name) + self.assertIn( + "decision.serves_both", + (builder.parent / "slots.py").read_text(encoding="utf-8"), + ) def test_neither_reimplements_the_mode_test(self): # A local "if mode == md5" beside the override would drift from the @@ -470,5 +486,49 @@ class TestProvenEvidence(unittest.TestCase): ) + +class ManifestFollowsTheArbitration(unittest.TestCase): + """The installer is handed the file the ZIP carries on a contested slot.""" + + def test_retroarch_manifest_serves_every_override(self): + repo = Path(__file__).resolve().parents[1] + if not (repo / "database.json").is_file(): + self.skipTest("no database.json") + # The database names files relative to the repository root, and this + # module's fixtures move the working directory elsewhere. + previous = os.getcwd() + os.chdir(repo) + self.addCleanup(os.chdir, previous) + import generate_pack + from common import ( + build_zip_contents_index, + load_data_dir_registry, + load_database, + load_emulator_profiles, + load_platform_config, + ) + + db = load_database(str(repo / "database.json")) + config = load_platform_config("retroarch", str(repo / "platforms")) + profiles = load_emulator_profiles(str(repo / "emulators")) + overrides = slots.pack_overrides( + config, profiles, db, build_zip_contents_index(db), + load_data_dir_registry(str(repo / "platforms")), + ) + if not overrides: + self.skipTest("no contested slot the pack settles on its own") + manifest = generate_pack.generate_manifest( + "retroarch", str(repo / "platforms"), db, str(repo / "bios"), + str(repo / "platforms" / "_registry.yml"), + emulators_dir=str(repo / "emulators"), emu_profiles=profiles, offline=True, + ) + base = config.get("base_destination", "") + by_dest = {f["dest"]: f for f in manifest["files"]} + for destination, path in overrides.items(): + dest = destination[len(base) + 1:] if base else destination + with self.subTest(destination=destination): + self.assertEqual(by_dest[dest]["repo_path"], path) + + if __name__ == "__main__": unittest.main()