From 3183ad982c1561736a98e8625f4eae684aee05ab Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Tue, 6 Oct 2026 09:32:32 +0200 Subject: [PATCH] fix: read gaps by normalized system, sha256, file names --- scripts/cross_reference.py | 120 ++++++++++++++------------ scripts/generate_site.py | 50 ++++------- tests/test_cross_reference_sources.py | 59 +++++++++++++ tests/test_e2e.py | 6 +- 4 files changed, 145 insertions(+), 90 deletions(-) create mode 100644 tests/test_cross_reference_sources.py diff --git a/scripts/cross_reference.py b/scripts/cross_reference.py index a9848ac6..020c6cbe 100644 --- a/scripts/cross_reference.py +++ b/scripts/cross_reference.py @@ -20,6 +20,7 @@ from pathlib import Path sys.path.insert(0, os.path.dirname(__file__)) from common import ( + _norm_system_id, get_mame_clone_map, list_registered_platforms, load_database, @@ -44,6 +45,9 @@ def load_platform_files( """Collect declared filenames + data_directories per system. Restricted to *platforms* when given, otherwise every registered platform. + Keyed by the normalized system ID: a profile writes `sega-megacd` where + RetroArch writes `sega-mega-cd`, and an exact match called 924 declared + files undeclared. """ declared = {} platform_data_dirs = {} @@ -55,42 +59,47 @@ def load_platform_files( for fe in system.get("files", []): name = fe.get("name", "") if name: - declared.setdefault(sys_id, set()).add(name) + declared.setdefault(_norm_system_id(sys_id), set()).add(name) for dd in system.get("data_directories", []): ref = dd.get("ref", "") if ref: - platform_data_dirs.setdefault(sys_id, set()).add(ref) + platform_data_dirs.setdefault(_norm_system_id(sys_id), set()).add(ref) return declared, platform_data_dirs def _build_supplemental_index( data_root: str = "data", bios_root: str = "bios" ) -> set[str]: - """Build a set of filenames and directory names in data/ and inside bios/ ZIPs.""" + """Build a set of filenames and directory names in data/ and inside bios/ ZIPs. + + A directory is indexed only with its trailing slash: a bare name matched a + file entry of the same name, and BasiliskII's required `ROM` read as held + because cpcemu keeps a directory called ROM. + """ names: set[str] = set() root_path = Path(data_root) if root_path.is_dir(): for fpath in root_path.rglob("*"): if fpath.name.startswith("."): continue - names.add(fpath.name) - names.add(fpath.name.lower()) + if fpath.is_file(): + names.add(fpath.name) + names.add(fpath.name.lower()) + else: + names.add(fpath.name + "/") + names.add(fpath.name.lower() + "/") if fpath.is_dir(): # Also index relative path from data/subdir/ for directory entries parts = fpath.relative_to(root_path).parts if len(parts) > 1: rel = "/".join(parts[1:]) - names.add(rel) names.add(rel + "/") - names.add(rel.lower()) names.add(rel.lower() + "/") bios_path = Path(bios_root) if bios_path.is_dir(): # Index directory names for directory-type entries (e.g., "nestopia/samples/moepro/") for dpath in bios_path.rglob("*"): if dpath.is_dir() and not dpath.name.startswith("."): - names.add(dpath.name) - names.add(dpath.name.lower()) names.add(dpath.name + "/") names.add(dpath.name.lower() + "/") import zipfile @@ -168,15 +177,54 @@ def _resolve_source( by_name_lower[canonical.lower()] ): return "bios" - # data/ supplemental index + # data/ supplemental index: a directory entry looks for a directory, a + # file entry for a file. if data_names: - if fname in data_names or key in data_names: - return "data" - if basename and (basename in data_names or basename.lower() in data_names): + is_directory = fname.endswith("/") or (file_entry or {}).get("type") == "directory" + looked_up = [fname, key] + ([basename, basename.lower()] if basename else []) + if is_directory: + looked_up = [name.rstrip("/") + "/" for name in looked_up] + if any(name in data_names for name in looked_up): return "data" return None +def entry_source(f: dict, index: dict) -> str | None: + """Where the collection holds a profile entry, or None. + + By name, path and alias, then by any hash the entry declares. The gap + report and the site pages read this one function, so a file is held or + missing in the same way on both. + """ + by_name, by_name_lower = index["by_name"], index["by_name_lower"] + data_names, by_path_suffix = index.get("data_names"), index["by_path_suffix"] + db_files = index["db_files"] + fname = f.get("name", "") + path_field = f.get("path") or "" + for candidate in [fname, *([path_field] if path_field != fname else []), + *(f.get("aliases") or [])]: + if not candidate: + continue + source = _resolve_source( + candidate, by_name, by_name_lower, data_names, by_path_suffix, f, db_files, + ) + if source is not None: + return source + + def values(field: str) -> list[str]: + raw = f.get(field) or [] + return [str(v).lower() for v in (raw if isinstance(raw, list) else [raw]) if v] + + held = ( + any(index["by_md5"].get(v) for v in parse_md5_list(f.get("md5"))) + or any(v in db_files for v in values("sha1")) + # rust_dos names its SC-55 ROMs by sha256 alone + or any(v in index["by_sha256"] for v in values("sha256")) + or any(index["by_crc32"].get(v) for v in values("crc32")) + ) + return "bios" if held else None + + def _resolve_archive_source( archive_name: str, by_name: dict[str, list], @@ -207,10 +255,7 @@ def _cross_reference_profile( """ by_name = index["by_name"] by_name_lower = index["by_name_lower"] - by_md5 = index["by_md5"] - by_crc32 = index["by_crc32"] by_path_suffix = index["by_path_suffix"] - db_files = index["db_files"] data_names = index["data_names"] all_declared = index["all_declared"] emu_files = profile.get("files", []) @@ -225,7 +270,7 @@ def _cross_reference_profile( else: platform_names = set() for sys_id in systems: - platform_names.update(declared.get(sys_id, set())) + platform_names.update(declared.get(_norm_system_id(sys_id), set())) gaps = [] covered = [] @@ -339,44 +384,7 @@ def _cross_reference_profile( if storage in ("release", "large_file"): source = "large_file" else: - source = _resolve_source( - fname, by_name, by_name_lower, data_names, by_path_suffix, - f, db_files, - ) - if source is None: - path_field = f.get("path", "") - if path_field and path_field != fname: - source = _resolve_source( - path_field, by_name, by_name_lower, - data_names, by_path_suffix, f, db_files, - ) - # Try the alternate names the emulator accepts, like - # resolve_local_file does - if source is None: - for alias in f.get("aliases") or []: - source = _resolve_source( - alias, by_name, by_name_lower, - data_names, by_path_suffix, f, db_files, - ) - if source is not None: - break - # Try MD5 hash match - if source is None: - for md5_val in parse_md5_list(f.get("md5")): - if by_md5.get(md5_val): - source = "bios" - break - # Try SHA1 hash match - if source is None: - raw_sha1 = f.get("sha1", "") - sha1_values = raw_sha1 if isinstance(raw_sha1, list) else [raw_sha1] - if any(value and value in db_files for value in sha1_values): - source = "bios" - # Try CRC32 hash match - if source is None: - crc32 = str(f.get("crc32", "")).lower() - if crc32 and by_crc32.get(crc32): - source = "bios" + source = entry_source(f, index) if source is None: source = "missing" @@ -442,6 +450,7 @@ def cross_reference( by_name_lower = {k.lower(): k for k in by_name} by_md5 = db.get("indexes", {}).get("by_md5", {}) by_crc32 = db.get("indexes", {}).get("by_crc32", {}) + by_sha256 = db.get("indexes", {}).get("by_sha256", {}) by_path_suffix = db.get("indexes", {}).get("by_path_suffix", {}) db_files = db.get("files", {}) report = {} @@ -451,6 +460,7 @@ def cross_reference( "by_name_lower": by_name_lower, "by_md5": by_md5, "by_crc32": by_crc32, + "by_sha256": by_sha256, "by_path_suffix": by_path_suffix, "db_files": db_files, "data_names": data_names, diff --git a/scripts/generate_site.py b/scripts/generate_site.py index 7283493f..4162a8f8 100644 --- a/scripts/generate_site.py +++ b/scripts/generate_site.py @@ -525,7 +525,7 @@ def build_emulator_gap_report( The gap analysis page and the published gaps export must not compute this twice and drift; both read this one report. """ - from common import expand_platform_declared_names + from common import _norm_system_id, expand_platform_declared_names from cross_reference import cross_reference as run_cross_reference all_declared: set[str] = set() @@ -538,7 +538,7 @@ def build_emulator_gap_report( for fe in system.get("files", []): fname = fe.get("name", "") if fname: - declared.setdefault(sys_id, set()).add(fname) + declared.setdefault(_norm_system_id(sys_id), set()).add(fname) unique_profiles = { k: v @@ -2217,46 +2217,30 @@ def _availability_check(db: dict, data_names): missing while the gap report called it held would describe a different collection on two pages of the same site. """ - from cross_reference import _resolve_source + from cross_reference import entry_source by_name = db.get("indexes", {}).get("by_name", {}) by_name_lower = {k.lower(): k for k in by_name} by_path_suffix = db.get("indexes", {}).get("by_path_suffix", {}) - by_md5 = db.get("indexes", {}).get("by_md5", {}) - db_files = db.get("files", {}) + + index = { + "by_name": by_name, + "by_name_lower": by_name_lower, + "by_path_suffix": by_path_suffix, + "by_md5": db.get("indexes", {}).get("by_md5", {}), + "by_sha256": db.get("indexes", {}).get("by_sha256", {}), + "by_crc32": db.get("indexes", {}).get("by_crc32", {}), + "db_files": db.get("files", {}), + "data_names": data_names, + } def _file_available(f: dict) -> bool: """Check if a file is available using the same resolution as cross_reference.""" - fname = f.get("name", "") - if not fname: + if not f.get("name"): return False - storage = f.get("storage", "") - if storage in ("release", "large_file"): + if f.get("storage", "") in ("release", "large_file"): return True - src = _resolve_source( - fname, by_name, by_name_lower, data_names, by_path_suffix, - f, db_files, - ) - if src is not None: - return True - path_field = f.get("path", "") - if path_field and path_field != fname: - src = _resolve_source( - path_field, by_name, by_name_lower, data_names, - by_path_suffix, f, db_files, - ) - if src is not None: - return True - md5_raw = f.get("md5", "") - if md5_raw: - for md5_val in parse_md5_list(md5_raw): - if by_md5.get(md5_val): - return True - # A profile lists several sha1 when the code accepts several dumps. - sha1 = f.get("sha1") or [] - if any(value in db_files for value in ([sha1] if isinstance(sha1, str) else sha1)): - return True - return False + return entry_source(f, index) is not None return _file_available diff --git a/tests/test_cross_reference_sources.py b/tests/test_cross_reference_sources.py new file mode 100644 index 00000000..27697eba --- /dev/null +++ b/tests/test_cross_reference_sources.py @@ -0,0 +1,59 @@ +"""The gap report reads the collection the way the resolver does. + +Three ways it called held files missing, or a missing file held: system IDs +compared verbatim (sega-megacd against sega-mega-cd), no sha256 in the hash +fallback (rust_dos names its SC-55 ROMs by sha256 alone), and directory names +indexed as files (BasiliskII's required ROM read as held because cpcemu keeps +a directory called ROM). +""" + +from __future__ import annotations + +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 cross_reference import ( # noqa: E402 + _build_supplemental_index, + cross_reference, + entry_source, +) + + +def _index(**over) -> dict: + base = { + "by_name": {}, "by_name_lower": {}, "by_path_suffix": {}, "by_md5": {}, + "by_sha256": {}, "by_crc32": {}, "db_files": {}, "data_names": set(), + } + base.update(over) + return base + + +class GapReportSources(unittest.TestCase): + def test_a_sha256_alone_finds_the_file(self): + entry = {"name": "wave1.bin", "sha256": "AB" * 32} + self.assertEqual(entry_source(entry, _index(by_sha256={"ab" * 32: "s"})), "bios") + + def test_a_directory_does_not_stand_for_a_file(self): + with tempfile.TemporaryDirectory(dir=REPO_ROOT / "tmp") as tmp: + (Path(tmp) / "bios" / "cpcemu" / "ROM").mkdir(parents=True) + names = _build_supplemental_index(str(Path(tmp) / "data"), str(Path(tmp) / "bios")) + self.assertIsNone(entry_source({"name": "ROM"}, _index(data_names=names))) + directory = {"name": "ROM", "type": "directory"} + self.assertEqual(entry_source(directory, _index(data_names=names)), "data") + + def test_a_system_spelled_otherwise_is_still_declared(self): + profiles = {"gpgx": {"emulator": "gpgx", "systems": ["sega-megacd"], + "files": [{"name": "bios_CD_U.bin"}]}} + declared = {"megacd": {"bios_CD_U.bin"}} # sega-mega-cd, normalized + db = {"files": {}, "indexes": {}} + report = cross_reference(profiles, declared, db) + self.assertEqual(report["gpgx"]["gaps"], 0) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_e2e.py b/tests/test_e2e.py index 0f15d59f..4193752a 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -5562,11 +5562,13 @@ struct BurnDriver BurnDrvneogeo = { with open(os.path.join(plat_dir, f"{name}.yml"), "w") as f: yaml.dump(cfg, f) + # Keyed by the normalized system ID, as profiles spell systems + # differently from platforms. everything, _ = load_platform_files(plat_dir) - self.assertEqual(everything["shared-system"], {"a.bin", "b.bin"}) + self.assertEqual(everything["sharedsystem"], {"a.bin", "b.bin"}) only_alpha, _ = load_platform_files(plat_dir, ["alpha"]) - self.assertEqual(only_alpha["shared-system"], {"a.bin"}) + self.assertEqual(only_alpha["sharedsystem"], {"a.bin"}) def test_227_manifest_injected_compressed(self):