From 5ebb141dbf94568585a95d964aaf93958460c51f Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Fri, 9 Oct 2026 22:20:00 +0200 Subject: [PATCH] fix: score a losing twin against the shipped file --- .../resources}/RedumpDatabase.yaml | 0 scripts/verify.py | 81 +++++++++++++++++- tests/test_twin_declarations.py | 83 +++++++++++++++++++ 3 files changed, 161 insertions(+), 3 deletions(-) rename bios/Other/{armsx2 => emucorex/resources}/RedumpDatabase.yaml (100%) create mode 100644 tests/test_twin_declarations.py diff --git a/bios/Other/armsx2/RedumpDatabase.yaml b/bios/Other/emucorex/resources/RedumpDatabase.yaml similarity index 100% rename from bios/Other/armsx2/RedumpDatabase.yaml rename to bios/Other/emucorex/resources/RedumpDatabase.yaml diff --git a/scripts/verify.py b/scripts/verify.py index 55b806f3..94177870 100644 --- a/scripts/verify.py +++ b/scripts/verify.py @@ -84,6 +84,7 @@ from validation import ( DEFAULT_DB = "database.json" DEFAULT_PLATFORMS_DIR = "platforms" +DEFAULT_BIOS_DIR = "bios" # The repository's platforms wherever the script runs from, for library calls # that pass no directory. _REPO_PLATFORMS = os.path.join(os.path.dirname(__file__), "..", "platforms") @@ -715,6 +716,64 @@ def find_exclusion_notes( # Platform verification +def _twin_index( + verify_systems: dict, db: dict, base_dest: str, zip_contents: dict, + data_dir_registry: dict | None, +) -> tuple[dict[str, int], dict[int, tuple[str, dict]]]: + """Which declaration the pack ships at each destination, and every declaration by id.""" + from generate_pack import _preferred_entries + + preferred_entries = _preferred_entries( + verify_systems, db, DEFAULT_BIOS_DIR, base_dest, False, + zip_contents, data_dir_registry, True, + ) + winners = { + id(fe): (sid, fe) + for sid, system in verify_systems.items() + for fe in system.get("files", []) + } + return preferred_entries, winners + + +def _resolve_declaration( + file_entry: dict, sys_id: str, twins: tuple, db: dict, zip_contents: dict, + data_dir_registry: dict | None, slot_overrides: dict, mode: str, + platform_profiles: dict, +) -> tuple[str | None, str, str | None]: + """Resolve a declaration to the file the pack ships at its destination. + + When another declaration holds the destination, that one's file is + returned and the evidence reset: the winner's hash match is not this + declaration's, which is hashed against the shipped file. + """ + preferred_entries, winners, base_dest = twins + bare = sanitize_pack_path(file_entry.get("destination", file_entry.get("name", ""))) + preferred = preferred_entries.get(f"{base_dest}/{bare}" if base_dest else bare) + held_by, winner = (None, file_entry) + if preferred is not None and preferred != id(file_entry): + held_by, winner = winners[preferred] + local_path, resolve_status = _resolve_for_platform( + winner, held_by or sys_id, db, zip_contents, data_dir_registry, + slot_overrides, mode, platform_profiles, + ) + return local_path, "" if held_by else resolve_status, held_by + + +def _twin_unmet(result: dict, held_by: str | None) -> bool: + """Mark a declaration unmet at a path another declaration holds. + + The path is answered by the winner's file, which the pack counts once; + this declaration's failure is reported, never folded into the + destination's status. + """ + if held_by is None or result["status"] == Status.OK: + return False + result["discrepancy"] = ( + f"{result.get('name', '')} is not met at the path {held_by} holds" + ) + return True + + def _resolve_for_platform( file_entry: dict, sys_id: str, @@ -845,6 +904,15 @@ def verify_platform( region_groups, region_mod.build_region_index(profiles), regions ) + # A destination two systems declare with two hashes ships one file, the + # one the builder prefers. The other declaration is scored against that + # file, as the frontend will score it after installation: RetroDECK's + # xroar component hashes bios/disk.rom and finds the PC-88 ROM. + base_dest = config.get("base_destination", "") + preferred_entries, winners = _twin_index( + verify_systems, db, base_dest, zip_contents, data_dir_registry + ) + for sys_id, system in verify_systems.items(): for file_entry in system.get("files", []): if region_drops and ( @@ -854,9 +922,10 @@ def verify_platform( in region_drops ): continue - local_path, resolve_status = _resolve_for_platform( - file_entry, sys_id, db, zip_contents, data_dir_registry, - slot_overrides, mode, platform_profiles, + local_path, resolve_status, held_by = _resolve_declaration( + file_entry, sys_id, (preferred_entries, winners, base_dest), + db, zip_contents, data_dir_registry, slot_overrides, mode, + platform_profiles, ) destination = sanitize_pack_path( file_entry.get("destination", file_entry.get("name", "")) @@ -893,6 +962,8 @@ def verify_platform( validation_index, ) details.append(result) + if _twin_unmet(result, held_by): + continue # Aggregate by destination dest = file_entry.get("destination", file_entry.get("name", "")) @@ -1048,6 +1119,10 @@ def _print_detail_entries(details: list[dict], seen: set[str], verbose: bool) -> """Print UNTESTED, MISSING, and DISCREPANCY entries from verification details.""" for d in details: if d["status"] == Status.UNTESTED: + if d.get("discrepancy"): + # Reported below with the ground for it: a declaration + # unmet at a path another declaration holds. + continue key = f"{d['system']}/{d['name']}" if key in seen: continue diff --git a/tests/test_twin_declarations.py b/tests/test_twin_declarations.py new file mode 100644 index 00000000..10e39317 --- /dev/null +++ b/tests/test_twin_declarations.py @@ -0,0 +1,83 @@ +"""A destination two systems declare with two hashes ships one file. + +RetroDECK declares bios/disk.rom for pc88 (the N88SUB ROM) and for coco (the +CoCo disk ROM), and bios/cdibios.zip for two components with two md5. The +pack holds one file per path; verify resolved each declaration to its own +file and counted both OK, so three green tools described a pack whose +frontend hashes bios/disk.rom for xroar and finds the PC-88 ROM. +""" + +from __future__ import annotations + +import hashlib +import os +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")) + +import common # noqa: E402 +import generate_db # noqa: E402 +from verify import verify_platform # noqa: E402 + + +class TwinDeclarations(unittest.TestCase): + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self._cwd = os.getcwd() + os.chdir(self._tmp.name) + files, self.md5 = {}, {} + for rel, payload in ( + ("bios/NEC/PC-98/N88SUB.ROM", b"pc88 disk rom"), + ("bios/Tandy/CoCo/disk.rom", b"coco disk rom"), + ): + path = Path(rel) + path.parent.mkdir(parents=True, exist_ok=True) + path.write_bytes(payload) + sha1 = hashlib.sha1(payload).hexdigest() + self.md5[rel] = hashlib.md5(payload).hexdigest() + files[sha1] = { + "path": rel, "name": path.name, "size": len(payload), "sha1": sha1, + "md5": self.md5[rel], "sha256": hashlib.sha256(payload).hexdigest(), + "crc32": "00000001", + } + self.db = {"files": files, "indexes": generate_db.build_indexes(files, {})} + self.emulators = Path(self._tmp.name) / "emulators" + self.emulators.mkdir() + common._emulator_profiles_cache.clear() + + def tearDown(self): + common._emulator_profiles_cache.clear() + os.chdir(self._cwd) + self._tmp.cleanup() + + def test_the_losing_declaration_is_scored_against_the_shipped_file(self): + config = { + "platform": "Twins", + "verification_mode": "md5", + "base_destination": "bios", + "cores": [], + "systems": { + "pc88": {"files": [{"name": "N88SUB.ROM", "destination": "disk.rom", + "md5": self.md5["bios/NEC/PC-98/N88SUB.ROM"]}]}, + "coco": {"files": [{"name": "disk.rom", "destination": "disk.rom", + "md5": self.md5["bios/Tandy/CoCo/disk.rom"]}]}, + }, + } + report = verify_platform( + config, self.db, str(self.emulators), {}, supplemental_names=set() + ) + by_system = {d["system"]: d for d in report["details"] if d["name"] in ("N88SUB.ROM", "disk.rom")} + statuses = sorted(d["status"] for d in by_system.values()) + self.assertEqual(statuses.count("ok"), 1, by_system) + loser = next(d for d in by_system.values() if d["status"] != "ok") + self.assertIn("is not met at the path", loser.get("discrepancy", "")) + # The path counts once, for the file it ships; the loser is a detail. + self.assertEqual(report["status_counts"], {"ok": 1}) + + +if __name__ == "__main__": + unittest.main()