From c05d29d941e3ff578e980e46c21324d871449cc3 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 05:32:19 +0200 Subject: [PATCH] fix: a held name must carry the declared hash entry_source took a name or path hit before reading the hashes the entry declares, so the gap report and the site counted as held a file whose only homonym has other bytes: zquestclassic's ending.mid and gameover.mid matched Hydra Castle Labyrinth's. A name or path hit now counts only when its record carries one of the declared hashes, and the declared content anywhere in bios/ comes before a data/ copy. Measured over every profile entry: 254 change, 248 bundled ones (report and site only), three unsourceable SC-55 revisions now read missing, three data hits read bios by hash. --- scripts/cross_reference.py | 42 ++++++++++++++++++++++----- tests/test_cross_reference_sources.py | 16 ++++++++++ 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/scripts/cross_reference.py b/scripts/cross_reference.py index 171ea85d..25096341 100644 --- a/scripts/cross_reference.py +++ b/scripts/cross_reference.py @@ -178,21 +178,28 @@ def _resolve_source( or ``None`` (not available anywhere). A name hit whose size contradicts a size the emulator verifies is not the - file, so it does not count as available. + file, so it does not count as available. Nor is one whose bytes carry + none of the hashes the entry declares: a name never wins over a declared + hash, and zquestclassic's ending.mid read as held because Hydra Castle + Labyrinth has another file of that name. """ + declared = _declared_content(file_entry or {}) - def _name_hit(name: str) -> bool: - """Whether the files indexed under *name* can be this entry.""" + def _can_be(sha1s: list | str) -> bool: if not file_entry or db_files is None: return True - sha1s = by_name.get(name) or [] if not isinstance(sha1s, list): sha1s = [sha1s] return any( name_match_size_ok(file_entry, db_files.get(sha1, {}).get("size")) + and _record_carries(sha1, db_files.get(sha1, {}), declared) for sha1 in sha1s ) + def _name_hit(name: str) -> bool: + """Whether the files indexed under *name* can be this entry.""" + return _can_be(by_name.get(name) or []) + # bios/ via database.json by_name if fname in by_name and _name_hit(fname): return "bios" @@ -209,7 +216,7 @@ def _resolve_source( ): return "bios" # bios/ via by_path_suffix (regional variants) - if by_path_suffix and fname in by_path_suffix: + if by_path_suffix and fname in by_path_suffix and _can_be(by_path_suffix[fname]): return "bios" # bios/ under the canonical MAME set name, as resolve_local_file does: # a renamed archive is held once, under the name the dedup kept. @@ -252,6 +259,23 @@ def _data_hit( _HASH_FIELDS = ("sha1", "md5", "sha256", "crc32") +def _record_carries(sha1: str, record: dict, declared: dict[str, set[str]]) -> bool: + """Whether a database record has one of the declared hashes, or none is declared.""" + if not declared: + return True + actual = { + "sha1": sha1.lower(), + **{field: str(record.get(field) or "").lower() for field in ("md5", "sha256", "crc32")}, + } + return any( + actual[field] and ( + actual[field] in values + or (field == "md5" and any(len(v) < 32 and actual[field].startswith(v) for v in values)) + ) + for field, values in declared.items() + ) + + def _declared_content(file_entry: dict) -> dict[str, set[str]]: """The hashes an entry declares, lowercased, by algorithm.""" declared: dict[str, set[str]] = {} @@ -444,6 +468,7 @@ def entry_source(f: dict, index: dict) -> str | None: {"indexes": {"by_path_suffix": by_path_suffix}}, path_field or fname ): return "bios" + found = None for candidate in [fname, *([path_field] if path_field != fname else []), *(f.get("aliases") or [])]: if not candidate: @@ -452,12 +477,15 @@ def entry_source(f: dict, index: dict) -> str | None: candidate, by_name, by_name_lower, data_names, by_path_suffix, f, db_files, index.get("bios_dir"), ) - if source is not None: + if source == "bios": return source + found = found or source def values(field: str) -> list[str]: return parse_hash_list(f.get(field)) + # The declared content anywhere in bios/ comes before a data/ copy, as in + # the resolver: the hash steps run before the data directories. 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")) @@ -465,7 +493,7 @@ def entry_source(f: dict, index: dict) -> str | None: 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 + return "bios" if held else found def _resolve_archive_source( diff --git a/tests/test_cross_reference_sources.py b/tests/test_cross_reference_sources.py index d3596012..0f39afdd 100644 --- a/tests/test_cross_reference_sources.py +++ b/tests/test_cross_reference_sources.py @@ -38,6 +38,22 @@ class GapReportSources(unittest.TestCase): entry = {"name": "wave1.bin", "sha256": "AB" * 32} self.assertEqual(entry_source(entry, _index(by_sha256={"ab" * 32: "s"})), "bios") + def test_a_name_never_wins_over_a_declared_hash(self): + """zquestclassic pins ending.mid by sha1, the collection holds only + Hydra Castle Labyrinth's ending.mid: the report called it held.""" + other = "9" * 40 + index = _index( + by_name={"ending.mid": [other]}, by_name_lower={"ending.mid": "ending.mid"}, + by_path_suffix={"data/midi/ending.mid": [other]}, + db_files={other: {"path": "bios/hcl/ending.mid", "md5": "1" * 32, "size": 9}}, + ) + pinned = {"name": "ending.mid", "sha1": "5" * 40} + self.assertIsNone(entry_source(pinned, index)) + self.assertIsNone(entry_source({**pinned, "path": "data/midi/ending.mid"}, index)) + self.assertEqual(entry_source({"name": "ending.mid"}, index), "bios") + self.assertEqual(entry_source({"name": "ending.mid", "sha1": other}, index), "bios") + self.assertEqual(entry_source({"name": "ending.mid", "md5": "1" * 32}, index), "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)