From f0bab9766c21300c44d41470b3def5fb3469b719 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Tue, 6 Oct 2026 05:18:07 +0200 Subject: [PATCH] fix: tell same-name files apart by path in truth --- scripts/truth.py | 132 ++++++++++++++++++++++++++------------- tests/test_truth_diff.py | 60 ++++++++++++++++-- 2 files changed, 142 insertions(+), 50 deletions(-) diff --git a/scripts/truth.py b/scripts/truth.py index 9ad4429d..cf10fa4c 100644 --- a/scripts/truth.py +++ b/scripts/truth.py @@ -103,15 +103,27 @@ def _merge_file_into_system( emu_name: str, db: dict | None, ) -> None: - """Merge a file entry into a system's file list, deduplicating by name.""" + """Merge a file entry into a system's file list, deduplicating by name. + + A name is one file across cores, whatever path each core gives it. A + core that declares the same name under two paths declares two files: + Dolphin's three IPL.bin differ only by GC/USA, GC/EUR, GC/JAP. Under + one path or none, its declarations are revisions of one file. + """ files = system.setdefault("files", []) name_lower = file_entry["name"].lower() + path = str(file_entry.get("path") or "").casefold() - existing = None - for f in files: - if f["name"].lower() == name_lower: - existing = f - break + def other_path(f: dict) -> str: + return str(f.get("path") or "").casefold() + + same_name = [f for f in files if f["name"].lower() == name_lower] + existing = ( + next((f for f in same_name if path and other_path(f) == path), None) + or next((f for f in same_name if emu_name not in f.get("_cores", ())), None) + # Revisions accepted under one name and no path fill one slot. + or next((f for f in same_name if not path or not other_path(f)), None) + ) if existing is not None: existing["_cores"] = existing.get("_cores", set()) | {emu_name} @@ -418,7 +430,7 @@ def generate_platform_truth( def _match_renames( unmatched_truth: list[dict], unmatched_scraped: dict -) -> tuple[set[str], set[str]]: +) -> tuple[set[int], set]: """Pair files a platform renamed with the truth entry they came from. A platform is free to call a file whatever it likes -- Batocera ships @@ -427,51 +439,62 @@ def _match_renames( is one file under two names, and counting it as both missing and extra would report a gap that does not exist. - Returns the truth names and scraped keys that pair up. + Returns the positions in unmatched_truth and the scraped keys that pair + up: a name does not identify a truth entry, two can share it. """ # Hash-based fallback: detect platform renames (e.g. Batocera ROM → ROM1) # If an unmatched scraped file shares a hash with an unmatched truth file, # it's the same file under a different name: a platform rename, not a gap. - rename_matched_truth: set[str] = set() - rename_matched_scraped: set[str] = set() + rename_matched_truth: set[int] = set() + rename_matched_scraped: set = set() if unmatched_truth and unmatched_scraped: # Build hash → truth file index for unmatched truth files - truth_hash_index: dict[str, dict] = {} - for fe in unmatched_truth: + truth_hash_index: dict[str, int] = {} + for position, fe in enumerate(unmatched_truth): for h in ("sha1", "md5", "crc32"): val = fe.get(h) if val and isinstance(val, str): - truth_hash_index[val.lower()] = fe + truth_hash_index[val.lower()] = position for s_key, s_entry in unmatched_scraped.items(): for h in ("sha1", "md5", "crc32"): s_val = s_entry.get(h) if not s_val or not isinstance(s_val, str): continue - t_entry = truth_hash_index.get(s_val.lower()) - if t_entry is not None: + position = truth_hash_index.get(s_val.lower()) + if position is not None: # Rename detected. Count as matched - rename_matched_truth.add(t_entry["name"].lower()) + rename_matched_truth.add(position) rename_matched_scraped.add(s_key) break return rename_matched_truth, rename_matched_scraped -def _diff_system(truth_sys: dict, scraped_sys: dict) -> dict: - """Compare files between truth and scraped for a single system.""" - # Build truth index: name.lower() -> entry, alias.lower() -> entry - truth_index: dict[str, dict] = {} - for fe in truth_sys.get("files", []): - truth_index[fe["name"].lower()] = fe - for alias in fe.get("aliases", []): - truth_index[alias.lower()] = fe +def _hash_set(entry: dict) -> set[str]: + values: set[str] = set() + for h in ("sha1", "md5", "crc32"): + value = entry.get(h) or [] + values.update(str(v).lower() for v in (value if isinstance(value, list) else [value])) + return values - # Build scraped index: name.lower() -> entry - scraped_index: dict[str, dict] = {} - for fe in scraped_sys.get("files", []): - scraped_index[fe["name"].lower()] = fe + +def _diff_system(truth_sys: dict, scraped_sys: dict) -> dict: + """Compare files between truth and scraped for a single system. + + A name can stand for several files of one system (Dolphin's IPL.bin + under GC/USA, GC/EUR and GC/JAP), so each scraped entry is paired with + the same-named truth entry at its destination first, and an entry + never answers for two. + """ + truth_files = truth_sys.get("files", []) + truth_index: dict[str, list[int]] = {} + for position, fe in enumerate(truth_files): + for key in [fe["name"], *fe.get("aliases", [])]: + truth_index.setdefault(key.lower(), []).append(position) + + scraped_files = scraped_sys.get("files", []) missing: list[dict] = [] hash_mismatch: list[dict] = [] @@ -479,14 +502,35 @@ def _diff_system(truth_sys: dict, scraped_sys: dict) -> dict: extra_phantom: list[dict] = [] extra_unprofiled: list[dict] = [] - matched_truth_names: set[str] = set() + def _tail(value: object) -> str: + return str(value or "").replace("\\", "/").casefold() - # Compare scraped files against truth - for s_key, s_entry in scraped_index.items(): - t_entry = truth_index.get(s_key) - if t_entry is None: + matched: set[int] = set() + unmatched_scraped: dict[int, dict] = {} + for s_position, s_entry in enumerate(scraped_files): + candidates = [ + p for p in truth_index.get(s_entry["name"].lower(), []) if p not in matched + ] + if not candidates: + if s_entry["name"].lower() not in truth_index: + unmatched_scraped[s_position] = s_entry continue - matched_truth_names.add(t_entry["name"].lower()) + destination = _tail(s_entry.get("destination")) + s_hashes = _hash_set(s_entry) + + def rank(position: int) -> tuple[bool, bool, bool, bool]: + t = truth_files[position] + t_path = _tail(t.get("path")) + return ( + bool(t_path) and destination.endswith(t_path), + "/" in t_path and t_path.rsplit("/", 1)[0] == destination.rpartition("/")[0], + t["name"].lower() == s_entry["name"].lower(), + bool(s_hashes & _hash_set(t)), + ) + + t_position = max(candidates, key=rank) + matched.add(t_position) + t_entry = truth_files[t_position] # Hash comparison for h in ("sha1", "md5", "crc32"): @@ -525,23 +569,16 @@ def _diff_system(truth_sys: dict, scraped_sys: dict) -> dict: # Collect unmatched files from both sides unmatched_truth = [ - fe - for fe in truth_sys.get("files", []) - if fe["name"].lower() not in matched_truth_names + fe for position, fe in enumerate(truth_files) if position not in matched ] - unmatched_scraped = { - s_key: s_entry - for s_key, s_entry in scraped_index.items() - if s_key not in truth_index - } rename_matched_truth, rename_matched_scraped = _match_renames( unmatched_truth, unmatched_scraped ) # Truth files not matched (by name, alias, or hash) -> missing - for fe in unmatched_truth: - if fe["name"].lower() not in rename_matched_truth: + for position, fe in enumerate(unmatched_truth): + if position not in rename_matched_truth: missing.append( { "name": fe["name"], @@ -553,9 +590,14 @@ def _diff_system(truth_sys: dict, scraped_sys: dict) -> dict: # Scraped files not in truth -> extra coverage = truth_sys.get("_coverage", {}) has_unprofiled = bool(coverage.get("cores_unprofiled")) + # An archive declared once per ROM it holds (zipped_file) is still one + # file. + seen_extra: set[tuple[str, str]] = set() for s_key, s_entry in unmatched_scraped.items(): - if s_key in rename_matched_scraped: + key = (s_entry["name"].lower(), _tail(s_entry.get("destination"))) + if s_key in rename_matched_scraped or key in seen_extra: continue + seen_extra.add(key) entry = {"name": s_entry["name"]} if has_unprofiled: extra_unprofiled.append(entry) diff --git a/tests/test_truth_diff.py b/tests/test_truth_diff.py index f4730e24..e5673053 100644 --- a/tests/test_truth_diff.py +++ b/tests/test_truth_diff.py @@ -75,12 +75,62 @@ class ADiffThatComparedNothingFails(unittest.TestCase): self.assertIn("skip", proc.stderr) +class OneNameSeveralFiles(unittest.TestCase): + """Dolphin's three IPL.bin differ only by path; the model kept one.""" + + REGIONS = {"USA": "a" * 32, "EUR": "b" * 32, "JAP": "c" * 32} + + def _system(self) -> dict: + from truth import _merge_file_into_system + + system: dict = {} + for core in ("dolphin", "ishiiruka"): + for region, md5 in self.REGIONS.items(): + _merge_file_into_system( + system, + {"name": "IPL.bin", "path": f"GC/{region}/IPL.bin", "md5": md5}, + core, + None, + ) + _merge_file_into_system(system, {"name": "IPL.bin"}, "other", None) + return system + + def test_a_core_declaring_a_name_twice_declares_two_files(self): + files = self._system()["files"] + self.assertEqual( + sorted(f["path"] for f in files), + ["GC/EUR/IPL.bin", "GC/JAP/IPL.bin", "GC/USA/IPL.bin"], + ) + for entry in files: + self.assertEqual(entry["_cores"] - {"other"}, {"dolphin", "ishiiruka"}) + + def test_revisions_under_one_name_and_no_path_stay_one_file(self): + from truth import _merge_file_into_system + + system: dict = {} + for md5 in ("a" * 32, "b" * 32): + _merge_file_into_system( + system, {"name": "MT32_CONTROL.ROM", "md5": md5}, "dosbox", None + ) + self.assertEqual(len(system["files"]), 1) + + def test_the_diff_pairs_each_by_destination(self): + from truth import _diff_system + + scraped = {"files": [ + {"name": "IPL.bin", "destination": f"dolphin-emu/Sys/GC/{region}/IPL.bin", + "md5": md5} + for region, md5 in self.REGIONS.items() + ]} + self.assertEqual(_diff_system(self._system(), scraped), {}) + + class RenameMatching(unittest.TestCase): def test_a_shared_sha1_pairs_the_two_names(self): truth = [_entry("bios_CD_U.bin", sha1="a" * 40)] scraped = {"rom1.bin": _entry("ROM1.bin", sha1="a" * 40)} matched_truth, matched_scraped = _match_renames(truth, scraped) - self.assertEqual(matched_truth, {"bios_cd_u.bin"}) + self.assertEqual(matched_truth, {0}) self.assertEqual(matched_scraped, {"rom1.bin"}) def test_md5_and_crc32_pair_them_too(self): @@ -90,12 +140,12 @@ class RenameMatching(unittest.TestCase): truth = [_entry("a.bin", **{algorithm: value})] scraped = {"b.bin": _entry("b.bin", **{algorithm: value})} matched_truth, _ = _match_renames(truth, scraped) - self.assertEqual(matched_truth, {"a.bin"}) + self.assertEqual(matched_truth, {0}) def test_hash_comparison_ignores_case(self): truth = [_entry("a.bin", sha1="AbCdEf" + "0" * 34)] scraped = {"b.bin": _entry("b.bin", sha1="abcdef" + "0" * 34)} - self.assertEqual(_match_renames(truth, scraped)[0], {"a.bin"}) + self.assertEqual(_match_renames(truth, scraped)[0], {0}) def test_different_content_is_not_a_rename(self): truth = [_entry("a.bin", sha1="a" * 40)] @@ -135,14 +185,14 @@ class RenameMatching(unittest.TestCase): "d.bin": _entry("d.bin", sha1="c" * 40), } matched_truth, matched_scraped = _match_renames(truth, scraped) - self.assertEqual(matched_truth, {"a.bin", "c.bin"}) + self.assertEqual(matched_truth, {0, 1}) self.assertEqual(matched_scraped, {"b.bin", "d.bin"}) def test_an_unrelated_file_beside_a_rename_stays_unmatched(self): truth = [_entry("a.bin", sha1="a" * 40), _entry("lonely.bin", sha1="f" * 40)] scraped = {"b.bin": _entry("b.bin", sha1="a" * 40)} matched_truth, _ = _match_renames(truth, scraped) - self.assertEqual(matched_truth, {"a.bin"}) + self.assertEqual(matched_truth, {0}) self.assertNotIn("lonely.bin", matched_truth)