From 012bf2fec275010dc5ca5f6917b4cdaaeb799087 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Mon, 5 Oct 2026 21:36:14 +0200 Subject: [PATCH] fix: one rule for the variant an emulator accepts --- scripts/generate_pack.py | 18 ++++++- scripts/packresolve.py | 49 ------------------- scripts/validation.py | 71 +++++++++++++++++++++++++++ scripts/verify.py | 93 ++---------------------------------- tests/test_variant_choice.py | 89 ++++++++++++++++++++++++++++++++++ 5 files changed, 181 insertions(+), 139 deletions(-) create mode 100644 tests/test_variant_choice.py diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index 22e1c2e9..cedfd162 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -76,6 +76,7 @@ from validation import ( _build_validation_index, check_file_validation, filter_files_by_mode, + find_validated_variant, ) yaml = require_yaml() @@ -960,12 +961,13 @@ def generate_pack( ) if check: reason, emus_list = check - better = _find_candidate_satisfying_both( + better = find_validated_variant( file_entry, db, local_path, validation_index, bios_dir, + platform_digest=digest_algorithm(verification_mode), ) if better: local_path = better @@ -1270,6 +1272,7 @@ def generate_emulator_pack( # ZIP naming display_names = [p.get("emulator", n).replace(" ", "") for n, p in selected] + validation_index = _build_validation_index(dict(selected)) narrow_tags = "".join( tag for tag, _label in _narrowings( @@ -1480,6 +1483,18 @@ def generate_emulator_pack( missing_files.append(fe["name"]) continue + # The file verify --emulator credits is the one shipped: a + # dump the core's own check rejects gives way to a held one + # it accepts (azahar's otp.bin, dolphin's dsp_rom.bin). + if check_file_validation( + local_path, fe["name"], validation_index, bios_dir + ): + better = find_validated_variant( + fe, db, local_path, validation_index, bios_dir + ) + if better: + local_path = better + # SHA1 dedup: skip if same physical file AND same destination # (but allow same file to be packed under different destinations, # e.g., IPL.bin in GC/USA/ and GC/EUR/ from same source) @@ -3447,7 +3462,6 @@ from packresolve import ( # noqa: E402,F401 parse_hash_input, parse_hash_file, lookup_hashes, - _find_candidate_satisfying_both, resolve_file, ) diff --git a/scripts/packresolve.py b/scripts/packresolve.py index 9c0fcaef..5cedb844 100644 --- a/scripts/packresolve.py +++ b/scripts/packresolve.py @@ -8,7 +8,6 @@ from __future__ import annotations import re _HEX_RE = re.compile(r"\b([0-9a-fA-F]{8,40})\b") -from validation import check_file_validation from largefiles import fetch_large_file import os from hashing import parse_md5_list @@ -117,54 +116,6 @@ def lookup_hashes( pass print(f" In repo: {'YES' if in_repo else 'NO'}") -def _find_candidate_satisfying_both( - file_entry: dict, - db: dict, - local_path: str, - validation_index: dict, - bios_dir: str, -) -> str | None: - """Search for a repo file that satisfies both platform MD5 and emulator validation. - - When the current file passes platform verification but fails emulator checks, - search all candidates with the same name for one that passes both. - Returns a better path, or None if no upgrade found. - """ - fname = file_entry.get("name", "") - if not fname: - return None - entry = validation_index.get(fname) - if not entry: - return None - - md5_expected = file_entry.get("md5", "") - md5_set = ( - {m.strip().lower() for m in md5_expected.split(",") if m.strip()} - if md5_expected - else set() - ) - - by_name = db.get("indexes", {}).get("by_name", {}) - files_db = db.get("files", {}) - - for sha1 in by_name.get(fname, []): - candidate = files_db.get(sha1, {}) - path = candidate.get("path", "") - if ( - not path - or not os.path.exists(path) - or os.path.realpath(path) == os.path.realpath(local_path) - ): - continue - # Must still satisfy platform MD5 - if md5_set and candidate.get("md5", "").lower() not in md5_set: - continue - # Check emulator validation - reason = check_file_validation(path, fname, validation_index, bios_dir) - if reason is None: - return path - return None - def resolve_file( file_entry: dict, db: dict, diff --git a/scripts/validation.py b/scripts/validation.py index 798ea4df..7c44402e 100644 --- a/scripts/validation.py +++ b/scripts/validation.py @@ -10,6 +10,7 @@ from __future__ import annotations import os from common import compute_hashes +from hashing import parse_md5_list # Validation types that require console-specific cryptographic keys. # verify.py cannot reproduce these -size checks still apply if combined. @@ -321,3 +322,73 @@ def filter_files_by_mode(files: list[dict], standalone: bool) -> list[dict]: continue result.append(f) return result + + +def find_validated_variant( + file_entry: dict, + db: dict, + current_path: str, + validation_index: dict, + bios_dir: str = "bios", + platform_digest: str | None = None, +) -> str | None: + """A held file the emulator's own checks accept, in place of current_path. + + Candidates come first from the hashes the emulator declares, which finds + a dump stored under another name, then from the files sharing the name. + platform_digest is the hash the frontend compares ("md5", "sha1") or None + when it reads no bytes: a candidate must then also carry a value the + entry declares, since the frontend would reject anything else. The + report and the packs read this one function, so the file a report counts + as satisfying the emulator is the file a pack ships. + """ + fname = file_entry.get("name", "") + if not fname or fname not in validation_index: + return None + accepted: set[str] = set() + if platform_digest: + if file_entry.get("zipped_file"): + return None + declared = file_entry.get(platform_digest) or "" + if platform_digest == "md5": + accepted = set(parse_md5_list(declared)) + elif isinstance(declared, str) and declared: + accepted = {declared.lower()} + elif isinstance(declared, list): + accepted = {str(value).lower() for value in declared} + + files_db = db.get("files", {}) + indexes = db.get("indexes", {}) + current_real = os.path.realpath(current_path) + seen: set[str] = set() + + def candidates(): + expected = validation_index[fname] + for hash_type, index_key in ( + ("sha1", None), ("md5", "by_md5"), ("crc32", "by_crc32"), ("sha256", "by_sha256"), + ): + for value in expected.get(hash_type) or []: + if index_key is None: + yield value + continue + found = indexes.get(index_key, {}).get(value) + if isinstance(found, str): + yield found + elif isinstance(found, list): + yield from found + yield from indexes.get("by_name", {}).get(fname, []) + + for sha1 in candidates(): + entry = files_db.get(sha1) or {} + path = entry.get("path", "") + if not path or not os.path.exists(path): + continue + real = os.path.realpath(path) + if real == current_real or real in seen: + continue + seen.add(real) + if accepted and str(entry.get(platform_digest, "")).lower() not in accepted: + continue + if check_file_validation(path, fname, validation_index, bios_dir) is None: + return path + return None diff --git a/scripts/verify.py b/scripts/verify.py index 3baa89e5..e496d035 100644 --- a/scripts/verify.py +++ b/scripts/verify.py @@ -71,6 +71,7 @@ from validation import ( build_ground_truth, check_file_validation, filter_files_by_mode, + find_validated_variant, ) DEFAULT_DB = "database.json" @@ -125,7 +126,7 @@ def verify_entry_existence( reason, emus_list = check suppressed = False if db: - better = _find_best_variant( + better = find_validated_variant( file_entry, db, local_path, validation_index, ) if better: @@ -700,91 +701,6 @@ def find_exclusion_notes( # Platform verification -def _find_best_variant( - file_entry: dict, - db: dict, - current_path: str, - validation_index: dict, -) -> str | None: - """Search for a repo file that passes emulator validation. - - Two-pass search: - 1. Hash lookup, using the emulator's expected hashes (sha1, md5, sha256, - crc32) to find candidates directly in the DB indexes. This finds - variants stored under different filenames (e.g. megacd2_v200_eu.bin - for bios_CD_E.bin). - 2. Name lookup, checking all files sharing the same name (aliases, - .variants/ with name-based suffixes). - - If any candidate on disk passes ``check_file_validation``, the - discrepancy is suppressed: the repo has what the emulator needs. - """ - fname = file_entry.get("name", "") - if not fname or fname not in validation_index: - return None - - files_db = db.get("files", {}) - current_real = os.path.realpath(current_path) - seen_paths: set[str] = set() - - def _try_candidate(sha1: str) -> str | None: - candidate = files_db.get(sha1, {}) - path = candidate.get("path", "") - if not path or not os.path.exists(path): - return None - rp = os.path.realpath(path) - if rp == current_real or rp in seen_paths: - return None - seen_paths.add(rp) - if check_file_validation(path, fname, validation_index) is None: - return path - return None - - # Pass 1: hash-based lookup from emulator expected values - ventry = validation_index[fname] - indexes = db.get("indexes", {}) - for hash_type, db_index_key in ( - ("sha1", None), - ("md5", "by_md5"), - ("crc32", "by_crc32"), - ("sha256", "by_sha256"), - ): - expected = ventry.get(hash_type) - if not expected: - continue - if db_index_key is None: - # SHA1 is the primary key of files_db - for h in expected: - if h in files_db: - result = _try_candidate(h) - if result: - return result - continue - db_index = indexes.get(db_index_key, {}) - for h in expected: - entries = db_index.get(h) - if not entries: - continue - if isinstance(entries, list): - for sha1 in entries: - result = _try_candidate(sha1) - if result: - return result - elif isinstance(entries, str): - result = _try_candidate(entries) - if result: - return result - - # Pass 2: name-based lookup (aliases, .variants/ with same filename) - by_name = db.get("indexes", {}).get("by_name", {}) - for sha1 in by_name.get(fname, []): - result = _try_candidate(sha1) - if result: - return result - - return None - - def verify_platform( config: dict, db: dict, @@ -941,11 +857,12 @@ def verify_platform( ) if check: reason, emus_list = check - better = _find_best_variant( + better = find_validated_variant( file_entry, db, local_path, validation_index, + platform_digest=digest_algorithm(mode), ) if not better: emus = ", ".join(emus_list) @@ -1514,7 +1431,7 @@ def verify_emulator( check = check_file_validation(local_path, name, validation_index) if check: reason, _emus = check - better = _find_best_variant( + better = find_validated_variant( file_entry, db, local_path, validation_index, ) if better: diff --git a/tests/test_variant_choice.py b/tests/test_variant_choice.py new file mode 100644 index 00000000..57eb8e4d --- /dev/null +++ b/tests/test_variant_choice.py @@ -0,0 +1,89 @@ +"""One rule picks the held file that satisfies the emulator. + +verify credited a variant found through the emulator's hashes while the +platform pack only searched by name and kept the platform's md5: the report +called ATARIOSB.ROM satisfied while the pack shipped the dump atari800 +rejects, and `verify --emulator azahar` credited an otp.bin the emulator pack +never swapped in. Both sides now read find_validated_variant. +""" + +from __future__ import annotations + +import hashlib +import re +import sys +import tempfile +import unittest +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(REPO_ROOT / "scripts")) + +from validation import find_validated_variant # noqa: E402 + + +class FindValidatedVariant(unittest.TestCase): + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self.addCleanup(self._tmp.cleanup) + root = Path(self._tmp.name) + self.bad = root / "rom.bin" + self.bad.write_bytes(b"x" * 8) + self.good = root / "other_name.bin" + self.good.write_bytes(b"y" * 16) + self.alias = root / "v" / "rom.bin" + self.alias.parent.mkdir() + self.alias.write_bytes(b"z" * 16) + + def entry(path: Path) -> tuple[str, dict]: + data = path.read_bytes() + sha1 = hashlib.sha1(data).hexdigest() + return sha1, {"path": str(path), "name": path.name, "sha1": sha1, + "md5": hashlib.md5(data).hexdigest(), "size": len(data)} + + self.files = dict(entry(p) for p in (self.bad, self.good, self.alias)) + self.good_sha1 = hashlib.sha1(self.good.read_bytes()).hexdigest() + self.db = {"files": self.files, "indexes": { + "by_name": {"rom.bin": [s for s, e in self.files.items() if e["name"] == "rom.bin"]}, + "by_md5": {e["md5"]: s for s, e in self.files.items()}, + }} + from validation import _build_validation_index + + self.by_hash = _build_validation_index({"emu": {"files": [ + {"name": "rom.bin", "validation": ["size", "sha1"], "size": 16, + "sha1": self.good_sha1}]}}) + self.by_size = _build_validation_index({"emu": {"files": [ + {"name": "rom.bin", "validation": ["size"], "size": 16}]}}) + + def test_a_dump_held_under_another_name_is_found_by_the_emulator_hash(self): + found = find_validated_variant({"name": "rom.bin"}, self.db, str(self.bad), self.by_hash) + self.assertEqual(found, str(self.good)) + + def test_a_digest_platform_only_takes_a_value_its_entry_declares(self): + alias_md5 = hashlib.md5(self.alias.read_bytes()).hexdigest() + entry = {"name": "rom.bin", "md5": alias_md5} + found = find_validated_variant( + entry, self.db, str(self.bad), self.by_size, platform_digest="md5") + self.assertEqual(found, str(self.alias)) + entry = {"name": "rom.bin", "md5": "0" * 32} + self.assertIsNone(find_validated_variant( + entry, self.db, str(self.bad), self.by_size, platform_digest="md5")) + + +class OneImplementation(unittest.TestCase): + """The report and the builders call the same finder and keep no copy.""" + + def test_no_module_carries_its_own_variant_search(self): + for name in ("verify.py", "generate_pack.py", "packresolve.py"): + source = (REPO_ROOT / "scripts" / name).read_text(encoding="utf-8") + with self.subTest(module=name): + self.assertNotRegex(source, r"def _find_best_variant|def _find_candidate_satisfying_both") + for name in ("verify.py", "generate_pack.py"): + source = (REPO_ROOT / "scripts" / name).read_text(encoding="utf-8") + with self.subTest(module=name): + self.assertGreaterEqual(len(re.findall(r"find_validated_variant\(", source)), 2 + if name == "generate_pack.py" else 3) + + +if __name__ == "__main__": + unittest.main()