From d7e7ec13ae2390d86e0ae69b57bd7bbde59638d4 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Tue, 6 Oct 2026 09:08:33 +0200 Subject: [PATCH] fix: install the variant the pack ships --- scripts/generate_pack.py | 43 ++++++-------- scripts/validation.py | 69 ++++++++++++++++++++++ tests/test_validated_choice.py | 104 +++++++++++++++++++++++++++++++++ 3 files changed, 191 insertions(+), 25 deletions(-) create mode 100644 tests/test_validated_choice.py diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index e3bde6b1..d0a36842 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -73,6 +73,8 @@ from nativemode import ( reads_file_contents, ) from validation import ( + destination_owners, + validated_choice, inner_rom_check, settle_mismatch, agnostic_substitute, @@ -643,6 +645,7 @@ def generate_pack( for name in resolve_platform_cores(config, emu_profiles) } validation_index = _build_validation_index(platform_profiles) + validation_owners = destination_owners(platform_profiles) slot_overrides = slots.pack_overrides( config, emu_profiles or {}, db, zip_contents, data_registry ) @@ -904,33 +907,15 @@ def generate_pack( # Platform verification (existence/md5) is the authority for pack status. # Emulator checks are supplementary -logged but don't downgrade. # When a discrepancy is found, try to find a file satisfying both. - if ( - file_status.get(dedup_key) == "ok" - and local_path - and validation_index - ): - fname = file_entry.get("name", "") - check = check_file_validation( - local_path, fname, validation_index, bios_dir + if file_status.get(dedup_key) == "ok": + local_path, disagreement = validated_choice( + file_entry, local_path, db, validation_index, bios_dir, + digest_algorithm(verification_mode), dest, validation_owners, ) - if check: - reason, emus_list = check - better = find_validated_variant( - file_entry, - db, - local_path, - validation_index, - bios_dir, - platform_digest=digest_algorithm(verification_mode), + if disagreement: + file_reasons.setdefault( + dedup_key, f"{platform_display} says OK but {disagreement}" ) - if better: - local_path = better - else: - emus = ", ".join(emus_list) - file_reasons.setdefault( - dedup_key, - f"{platform_display} says OK but {emus} says {reason}", - ) if already_packed: continue @@ -3039,6 +3024,8 @@ def generate_manifest( name: emu_profiles[name] for name in _platform_cores(config, emu_profiles) } + manifest_validation = _build_validation_index(manifest_profiles) + manifest_owners = destination_owners(manifest_profiles) # Load registry for install metadata registry: dict = {} @@ -3179,6 +3166,12 @@ def generate_manifest( ): record_omission(full_dest, file_entry, sys_id, status, None) continue + # The variant the pack ships when an emulator rejects the + # resolved file: the installer must place the same bytes. + local_path, _disagreement = validated_choice( + file_entry, local_path, db, manifest_validation, bios_dir, + digest_algorithm(verification_mode), dest, manifest_owners, + ) # Get SHA1 and size. The installer fetches by hash, so record # the copy this repo holds: an upstream hash carried by no diff --git a/scripts/validation.py b/scripts/validation.py index a088a4c4..16f9cf08 100644 --- a/scripts/validation.py +++ b/scripts/validation.py @@ -490,3 +490,72 @@ def settle_mismatch( ): return "frontend_digest_exact" return status + + +def destination_owners(profiles: dict) -> dict[str, list[tuple[str, str]]]: + """For each file name, the path tail each emulator reads it under.""" + owners: dict[str, list[tuple[str, str]]] = {} + for emu_name, profile in profiles.items(): + for f in profile.get("files", []): + name = str(f.get("name") or "") + if not name: + continue + for declared in {f.get("path"), f.get("standalone_path")} - {None, ""} or {name}: + owners.setdefault(name.lower(), []).append( + (str(declared).replace("\\", "/").lower(), emu_name) + ) + return owners + + +def _owners_of(destination: str, name: str, owners: dict) -> set[str]: + """Emulators whose declared path is the longest tail of the destination.""" + destination = destination.replace("\\", "/").lower() + matching = [ + (tail, emu) for tail, emu in owners.get(name.lower(), []) + if destination == tail or destination.endswith("/" + tail) + ] + if not matching: + return set() + longest = max(len(tail) for tail, _emu in matching) + return {emu for tail, emu in matching if len(tail) == longest} + + +def validated_choice( + file_entry: dict, + local_path: str | None, + db: dict, + validation_index: dict, + bios_dir: str, + platform_digest: str | None, + destination: str = "", + owners: dict | None = None, +) -> tuple[str | None, str | None]: + """The file a platform destination ships, and the disagreement if any. + + A frontend's own check can pass while an emulator it runs rejects the + file; a held variant both accept replaces it. The pack and the install + manifest both read this, so they name the same file. A destination + another emulator declares more precisely is that emulator's: ZEsarUX's + 48 KB cpc6128.rom check has no say over ep128emu/roms/cpc6128.rom. + """ + if not local_path or not validation_index: + return local_path, None + name = file_entry.get("name", "") + rules = validation_index.get(name) + if rules and owners and destination: + owning = _owners_of(destination, name, owners) + if owning and not owning & set(rules["emulators"]): + return local_path, None + check = check_file_validation( + local_path, file_entry.get("name", ""), validation_index, bios_dir + ) + if not check: + return local_path, None + better = find_validated_variant( + file_entry, db, local_path, validation_index, bios_dir, + platform_digest=platform_digest, + ) + if better: + return better, None + reason, emulators = check + return local_path, f"{', '.join(emulators)} says {reason}" diff --git a/tests/test_validated_choice.py b/tests/test_validated_choice.py new file mode 100644 index 00000000..4a3a0bd5 --- /dev/null +++ b/tests/test_validated_choice.py @@ -0,0 +1,104 @@ +"""The pack and the install manifest place the same bytes at a destination. + +The builder replaced a file an emulator rejects by a held variant both +accept; the manifest kept the first resolution. install.py then placed the +PS2 ROM2.BIN at galaksija/ROM2.BIN where the pack carried the Galaksija ROM, +across 14 destinations on six platforms. +""" + +from __future__ import annotations + +import ast +import hashlib +import sys +import tempfile +import unittest +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent +sys.path.insert(0, str(REPO_ROOT / "scripts")) + +from validation import ( # noqa: E402 + _build_validation_index, + destination_owners, + validated_choice, +) + + +class ValidatedChoice(unittest.TestCase): + def test_a_rejected_file_gives_way_to_an_accepted_variant(self): + with tempfile.TemporaryDirectory(dir=REPO_ROOT / "tmp") as tmp: + wrong = Path(tmp) / "ps2" / "ROM2.BIN" + right = Path(tmp) / "galaksija" / "ROM2.BIN" + wrong.parent.mkdir() + right.parent.mkdir() + wrong.write_bytes(b"p" * 64) + right.write_bytes(b"g" * 4096) + db = { + "files": { + hashlib.sha1(p.read_bytes()).hexdigest(): { + "path": str(p), + "name": "ROM2.BIN", + "size": p.stat().st_size, + "md5": hashlib.md5(p.read_bytes()).hexdigest(), + } + for p in (wrong, right) + }, + "indexes": {"by_name": {}, "by_md5": {}}, + } + db["indexes"]["by_name"]["ROM2.BIN"] = list(db["files"]) + index = _build_validation_index({ + "galaksija": { + "emulator": "galaksija", + "files": [{"name": "ROM2.BIN", "size": 4096, "validation": ["size"]}], + } + }) + chosen, disagreement = validated_choice( + {"name": "ROM2.BIN"}, str(wrong), db, index, tmp, None + ) + self.assertEqual(chosen, str(right)) + self.assertIsNone(disagreement) + + def test_a_destination_another_emulator_owns_is_not_judged(self): + """ZEsarUX's 48 KB cpc6128.rom check gave ep128emu the composite image.""" + profiles = { + "zesarux": {"files": [ + {"name": "cpc6128.rom", "size": 49152, "validation": ["size"]}, + ]}, + "ep128emu": {"files": [ + {"name": "cpc6128.rom", "path": "ep128emu/roms/cpc6128.rom"}, + ]}, + } + index = _build_validation_index(profiles) + owners = destination_owners(profiles) + with tempfile.TemporaryDirectory(dir=REPO_ROOT / "tmp") as tmp: + rom = Path(tmp) / "cpc6128.rom" + rom.write_bytes(b"c" * 32768) + db = {"files": {}, "indexes": {"by_name": {}, "by_md5": {}}} + for destination, judged in ( + ("ep128emu/roms/cpc6128.rom", False), + ("cpc6128.rom", True), + ): + with self.subTest(destination=destination): + _chosen, disagreement = validated_choice( + {"name": "cpc6128.rom"}, str(rom), db, index, tmp, None, + destination, owners, + ) + self.assertEqual(disagreement is not None, judged) + + def test_pack_and_manifest_both_read_it(self): + tree = ast.parse((REPO_ROOT / "scripts" / "generate_pack.py").read_text(encoding="utf-8")) + callers = { + node.name + for node in ast.walk(tree) + if isinstance(node, ast.FunctionDef) + and any( + isinstance(call, ast.Call) and getattr(call.func, "id", None) == "validated_choice" + for call in ast.walk(node) + ) + } + self.assertLessEqual({"generate_pack", "generate_manifest"}, callers) + + +if __name__ == "__main__": + unittest.main()