From b7a54b9d9b0b1e53104bb2f19877a5a1e1d9f3f8 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Thu, 22 Oct 2026 15:27:58 +0200 Subject: [PATCH] fix: collapse clone chains in the dedup map The clone map is cumulative. A run that removed sys68k_cpu21.zip and sys68k_cpu30x.zip as copies of other sets left the entries naming them canonical: their eight clones pointed at files that no longer existed and fell out of the manifests. Each clone now follows to the final canonical, and a loop is refused. --- _mame_clones.json | 24 ++++++------------------ scripts/dedup.py | 39 ++++++++++++++++++++++++++++++++++++++- tests/test_dedup.py | 28 +++++++++++++++++++++++++++- wiki/testing-guide.md | 2 +- 4 files changed, 72 insertions(+), 21 deletions(-) diff --git a/_mame_clones.json b/_mame_clones.json index 0ee30276..506b5c5f 100644 --- a/_mame_clones.json +++ b/_mame_clones.json @@ -66,7 +66,7 @@ "vrc5074.zip" ], "sha1": "17409b32b33cd1474b1aa1417dd3467b15589e16", - "total_copies": 35 + "total_copies": 34 }, "a3fdc.zip": { "clones": [ @@ -192,7 +192,7 @@ "bbc_opus3.zip" ], "sha1": "10dcceb968bb19c77b9544c89e471f4a7bf025ec", - "total_copies": 2 + "total_copies": 1 }, "buddha.zip": { "clones": [ @@ -600,13 +600,7 @@ }, "sys68k_cpu20.zip": { "clones": [ - "sys68k_cpu21.zip" - ], - "sha1": "e3d9b79fd99417535cad2a0751ceb90adb1b0ae5", - "total_copies": 1 - }, - "sys68k_cpu21.zip": { - "clones": [ + "sys68k_cpu21.zip", "sys68k_cpu21a.zip", "sys68k_cpu21b.zip", "sys68k_cpu21s.zip", @@ -614,23 +608,17 @@ "sys68k_cpu21yb.zip" ], "sha1": "e3d9b79fd99417535cad2a0751ceb90adb1b0ae5", - "total_copies": 5 + "total_copies": 6 }, "sys68k_cpu30.zip": { - "clones": [ - "sys68k_cpu30x.zip" - ], - "sha1": "47eb7d56111e587702724ba753bd60ed4b432566", - "total_copies": 1 - }, - "sys68k_cpu30x.zip": { "clones": [ "sys68k_cpu30be.zip", + "sys68k_cpu30x.zip", "sys68k_cpu30xa.zip", "sys68k_cpu30za.zip" ], "sha1": "47eb7d56111e587702724ba753bd60ed4b432566", - "total_copies": 3 + "total_copies": 4 }, "tms32030.zip": { "clones": [ diff --git a/scripts/dedup.py b/scripts/dedup.py index b7dbdd62..97a198a7 100644 --- a/scripts/dedup.py +++ b/scripts/dedup.py @@ -314,7 +314,7 @@ def deduplicate(bios_dir: str, dry_run: bool = False) -> dict: if isinstance(previous, dict): merged = dict(previous) merged.update(mame_clones) - mame_clones = merged + mame_clones = collapse_clone_chains(merged) if dry_run: print(f"\nWould write MAME clone map: {clone_path}") print( @@ -332,6 +332,43 @@ def deduplicate(bios_dir: str, dry_run: bool = False) -> dict: return results +def collapse_clone_chains(clone_map: dict[str, dict]) -> dict[str, dict]: + """Point every clone at a canonical archive that still exists. + + The map is cumulative. When a later run finds that a canonical archive is + itself a byte-identical copy of another and removes it, the entry that + named it as canonical would send its clones to a file that is gone: + sys68k_cpu21a pointed at sys68k_cpu21 after sys68k_cpu21 became a clone + of sys68k_cpu20. Each such entry's clones move to the final canonical. + """ + owner = { + clone: canonical + for canonical, entry in clone_map.items() + for clone in entry.get("clones", []) + } + + def final(name: str) -> str: + seen = {name} + while name in owner: + name = owner[name] + if name in seen: + raise ValueError(f"clone map loops through {name}") + seen.add(name) + return name + + collapsed: dict[str, dict] = {} + for canonical, entry in clone_map.items(): + target = final(canonical) + kept = collapsed.setdefault(target, {**clone_map.get(target, entry), "clones": []}) + names = set(kept["clones"]) | set(entry.get("clones", [])) + if target != canonical: + names.add(canonical) + names.discard(target) + kept["clones"] = sorted(names) + kept["total_copies"] = len(kept["clones"]) + return collapsed + + def main() -> None: parser = argparse.ArgumentParser(allow_abbrev=False, description="Deduplicate bios/ directory") parser.add_argument( diff --git a/tests/test_dedup.py b/tests/test_dedup.py index 02a1207d..d0badfba 100644 --- a/tests/test_dedup.py +++ b/tests/test_dedup.py @@ -341,5 +341,31 @@ class OnlyFilesSharingASizeAreHashed(_Bios): sorted(os.path.join(*p.split("/")) for p in ("A/one.bin", "B/one.bin"))) + +class CloneChainsCollapse(unittest.TestCase): + """A later run removed sys68k_cpu21.zip as a copy of sys68k_cpu20.zip + while the map still named it canonical for five clones: those clones + pointed at a file that no longer existed.""" + + def test_clones_follow_to_the_canonical_that_exists(self): + import dedup # noqa: PLC0415 + + chain = { + "cpu20.zip": {"clones": ["cpu21.zip"], "sha1": "a", "total_copies": 1}, + "cpu21.zip": {"clones": ["cpu21a.zip", "cpu21b.zip"], "sha1": "a", "total_copies": 2}, + } + self.assertEqual(dedup.collapse_clone_chains(chain), { + "cpu20.zip": {"clones": ["cpu21.zip", "cpu21a.zip", "cpu21b.zip"], + "sha1": "a", "total_copies": 3}, + }) + + def test_a_loop_is_refused(self): + import dedup # noqa: PLC0415 + + loop = {"a.zip": {"clones": ["b.zip"]}, "b.zip": {"clones": ["a.zip"]}} + with self.assertRaises(ValueError): + dedup.collapse_clone_chains(loop) + + if __name__ == "__main__": - unittest.main() + unittest.main() \ No newline at end of file diff --git a/wiki/testing-guide.md b/wiki/testing-guide.md index 28022bda..d30186e0 100644 --- a/wiki/testing-guide.md +++ b/wiki/testing-guide.md @@ -48,7 +48,7 @@ Every module in `tests/`, by the area it guards. |------|----------|---------| | Core regression suites | synthetic | `test_e2e`, `test_audit_regressions`, `test_tool_messages` | | File resolution and identity | synthetic + collection | `test_adler32_resolution`, `test_agnostic_scan`, `test_agnostic_scan_mode`, `test_agnostic_scope`, `test_agnostic_substitute`, `test_archive_choice`, `test_archive_recipes`, `test_bios_companions`, `test_bundled_identity`, `test_casefold_cache`, `test_db_aliases`, `test_declared_names`, `test_derivations`, `test_directory_entries`, `test_extra_identity`, `test_foreign_tree`, `test_foreign_tree_names`, `test_frontend_digest`, `test_game_data_homonyms`, `test_hashing`, `test_identity`, `test_md5_pack_context`, `test_name_tiebreak`, `test_nec_filing`, `test_owner_resolution`, `test_path_suffix`, `test_resolution_evidence`, `test_sega_filing`, `test_shipped_content`, `test_twin_declarations`, `test_validated_choice`, `test_variant_choice`, `test_variant_identity`, `test_zipped_member` | -| Verification, cross-reference and profile checks | synthetic + profiles | `test_case_insensitive_platforms`, `test_checks_have_values`, `test_core_file_fills_declaration`, `test_core_size_refusal`, `test_cross_reference_sources`, `test_crypto_aes`, `test_crypto_unique_data`, `test_crypto_verify`, `test_entry_categories`, `test_file_consumers`, `test_fileless_audit`, `test_flycast_layout`, `test_gap_settlement`, `test_hidden_extras`, `test_launcher_aliases`, `test_launcher_layer`, `test_native_mode`, `test_no_case_collisions`, `test_path_conflict_status`, `test_path_null_copies`, `test_profile_checks`, `test_profile_repositories`, `test_romm_hash_rule`, `test_shared_groups`, `test_standalone_links`, `test_standalone_mode`, `test_undeclared_dedup`, `test_unplaced_extras`, `test_validation_scope`, `test_verify_all_dir`, `test_verify_emulator_evidence`, `test_verify_listing`, `test_verify_pack_choice`, `test_verify_release_asset` | +| Verification, cross-reference and profile checks | synthetic + profiles | `test_cache_independent_counts`, `test_case_insensitive_platforms`, `test_checks_have_values`, `test_core_file_fills_declaration`, `test_core_size_refusal`, `test_cross_reference_sources`, `test_crypto_aes`, `test_crypto_unique_data`, `test_crypto_verify`, `test_entry_categories`, `test_file_consumers`, `test_fileless_audit`, `test_flycast_layout`, `test_gap_settlement`, `test_hidden_extras`, `test_launcher_aliases`, `test_launcher_layer`, `test_native_mode`, `test_no_case_collisions`, `test_path_conflict_status`, `test_path_null_copies`, `test_profile_checks`, `test_profile_repositories`, `test_romm_hash_rule`, `test_shared_groups`, `test_standalone_links`, `test_standalone_mode`, `test_undeclared_dedup`, `test_unplaced_extras`, `test_validation_scope`, `test_verify_all_dir`, `test_verify_emulator_evidence`, `test_verify_listing`, `test_verify_pack_choice`, `test_verify_release_asset` | | Packs and manifests | synthetic + real packs | `test_artifact_lock`, `test_atomic_artifacts`, `test_batocera_cores`, `test_deterministic_zip`, `test_emulator_pack_counts`, `test_finalize_pack_names`, `test_large_file_cache`, `test_libretro_layout`, `test_manifest_entry`, `test_minui_platform`, `test_no_game_data`, `test_pack_conformance_dirs`, `test_pack_core_extras`, `test_pack_counts`, `test_pack_destinations`, `test_pack_failures`, `test_pack_file_names`, `test_pack_grouping`, `test_pack_integrity`, `test_pack_member_identity`, `test_pack_readme`, `test_platform_inheritance`, `test_platform_keys`, `test_required_only_archives`, `test_required_only_twins`, `test_split_extras`, `test_split_names`, `test_split_pack`, `test_system_pack_extras`, `test_system_pack_parity`, `test_torrentzip`, `test_which_pack` | | Slots and regions | synthetic + profiles | `test_region`, `test_region_agnostic_scan`, `test_region_groups`, `test_region_held`, `test_region_platform_index`, `test_region_required_only`, `test_slot`, `test_slots`, `test_slots_shipped_file` | | Hardware targets and systems | synthetic + target files | `test_extract_targets`, `test_lakka_targets`, `test_libretro_slugs`, `test_system_ids`, `test_system_selection`, `test_target_names`, `test_target_scrapers`, `test_target_systems`, `test_target_verify` |