diff --git a/scripts/slots.py b/scripts/slots.py index 98add5ef..5f0c5602 100644 --- a/scripts/slots.py +++ b/scripts/slots.py @@ -245,6 +245,75 @@ def format_decision(decision: Decision) -> str: ) +@dataclass +class Collision: + """One destination a platform fills with two different files.""" + + destination: str + resolved: list[str] + + +def find_collisions(config: dict, db: dict) -> list[Collision]: + """Destinations a platform declares twice and resolves two ways. + + One path holds one file, so whichever declaration the builder reaches + first decides in silence. Declaring an archive several times to name its + inner ROMs is the documented zipped_file pattern and resolves to one + archive; two declarations landing on two files is a contradiction the + upstream list carries. RetroDECK aims a PC-88 disk subsystem ROM and a + CoCo disk ROM at one bios/disk.rom. + """ + by_dest: dict[str, list[dict]] = {} + for system in (config.get("systems") or {}).values(): + for entry in system.get("files") or []: + if not isinstance(entry, dict): + continue + dest = entry.get("destination") or entry.get("name") or "" + if dest: + by_dest.setdefault(_normalize(dest), []).append(entry) + + collisions = [] + for key, entries in sorted(by_dest.items()): + if len(entries) < 2: + continue + resolved = [] + for entry in entries: + local, _ = resolve_local_file( + entry, db, dest_hint=entry.get("destination", "") + ) + if local and local not in resolved: + resolved.append(local) + if len(resolved) > 1 and not _same_file_family(resolved): + collisions.append(Collision(destination=key, resolved=resolved)) + return collisions + + +def _same_file_family(paths: list[str]) -> bool: + """Whether the paths are one file and its own pinned variants. + + A platform list built on an older romset pins bytes the primary no longer + carries, which the repository keeps side by side under .variants. That is + the documented multi-version case, not two machines fighting for a path. + """ + canonical = set() + for path in paths: + if "/.variants/" in path: + directory, _, name = path.rpartition("/") + directory = directory[: -len("/.variants")] + name = name.rsplit(".", 1)[0] + path = f"{directory}/{name}" + canonical.add(path.casefold()) + return len(canonical) == 1 + + +def format_collision(collision: Collision) -> str: + """One line naming the destination and the files fighting for it.""" + return ( + f"{collision.destination}: declared for " + + " and for ".join(collision.resolved) + ) + + def format_conflict(conflict: Conflict) -> str: """One line per conflict, naming both answers and who gave them.""" emus = ", ".join(conflict.emulators) or "profile" @@ -305,10 +374,16 @@ def main() -> int: ) found: dict[str, list[Conflict]] = {} + collided: dict[str, list[Collision]] = {} for name in names: conflicts = scan_platform(name, profiles, db, args.platforms_dir) if conflicts: found[name] = conflicts + collisions = find_collisions( + load_platform_config(name, args.platforms_dir), db + ) + if collisions: + collided[name] = collisions if args.json: print( @@ -345,13 +420,17 @@ def main() -> int: f"\n{total} contested destinations. {fixable} the pack settles on " f"its own, {total - fixable} rest on an upstream declaration." ) + for platform, collisions in collided.items(): + print(f"{platform}: {len(collisions)} destinations declared twice") + for collision in collisions: + print(f" {format_collision(collision)}") if not args.strict and total: print( "Reported, not failed: the remainder needs the upstream list " "corrected, which no build can do. Use --strict to gate on them." ) - return 1 if (found and args.strict) else 0 + return 1 if ((found or collided) and args.strict) else 0 if __name__ == "__main__": diff --git a/tests/test_slots.py b/tests/test_slots.py index 1440d1fe..687d92c1 100644 --- a/tests/test_slots.py +++ b/tests/test_slots.py @@ -263,6 +263,61 @@ class TestBuilderAndVerifierAgree(unittest.TestCase): self.assertNotIn('verification_mode") == "md5"', window, name) +class TestSelfContradictingDestinations(unittest.TestCase): + """One path holds one file, whatever the upstream list says.""" + + def _config(self, *entries: dict) -> dict: + return {"systems": {"console": {"files": list(entries)}}} + + def test_two_declarations_landing_on_two_files_are_reported(self): + config = self._config( + {"name": "IPL.bin", "destination": "disk.rom", "md5": "m" * 32}, + {"name": "IPL.bin", "destination": "disk.rom", "md5": "n" * 32}, + ) + collisions = slots.find_collisions(config, REGIONS_DB) + self.assertEqual(len(collisions), 1) + self.assertEqual(len(collisions[0].resolved), 2) + + def test_one_archive_named_by_its_inner_roms_is_not_a_collision(self): + # The documented zipped_file pattern: several md5s, one archive. + config = self._config( + {"name": "IPL.bin", "destination": "a.zip", "md5": "m" * 32, + "zipped_file": "one.bin"}, + {"name": "IPL.bin", "destination": "a.zip", "md5": "m" * 32, + "zipped_file": "two.bin"}, + ) + self.assertEqual(slots.find_collisions(config, REGIONS_DB), []) + + def test_a_primary_and_its_pinned_variant_are_one_family(self): + self.assertTrue( + slots._same_file_family( + ["bios/M/C/rom.zip", "bios/M/C/.variants/rom.zip.abcd1234"] + ) + ) + + def test_two_machines_are_not_one_family(self): + self.assertFalse( + slots._same_file_family( + ["bios/Microsoft/MSX/DISK.ROM", "bios/Tandy/CoCo/disk.rom"] + ) + ) + + def test_a_destination_declared_once_is_never_a_collision(self): + config = self._config( + {"name": "IPL.bin", "destination": "disk.rom", "md5": "m" * 32} + ) + self.assertEqual(slots.find_collisions(config, REGIONS_DB), []) + + def test_the_line_names_every_file_claiming_the_path(self): + config = self._config( + {"name": "IPL.bin", "destination": "disk.rom", "md5": "m" * 32}, + {"name": "IPL.bin", "destination": "disk.rom", "md5": "n" * 32}, + ) + line = slots.format_collision(slots.find_collisions(config, REGIONS_DB)[0]) + self.assertIn("USA/IPL.bin", line) + self.assertIn("JAP/IPL.bin", line) + + class TestProvenEvidence(unittest.TestCase): """What counts as proof that a claim is about content, not about a name."""