From bf206d5b42fab62ed1a820382c28472ff0fae120 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Fri, 6 Nov 2026 08:55:26 +0100 Subject: [PATCH] feat: slots names who declares a collided path A collision line named two files and neither the system declaring each nor the one the pack ships, so the reader went back to the platform file. find_collisions walks the systems in the builder's sorted order, takes the builder's preferred declaration, and the line and the JSON now name each system and the file packed. RetroDECK's bios/disk.rom reads as the CoCo ROM packed, the PC-88 one left out, as its manifest holds. --- scripts/slots.py | 58 +++++++++++++++++++++++++++++++++++---------- tests/test_slots.py | 17 +++++++++++++ 2 files changed, 63 insertions(+), 12 deletions(-) diff --git a/scripts/slots.py b/scripts/slots.py index 8e7f826cd..0b69507d3 100644 --- a/scripts/slots.py +++ b/scripts/slots.py @@ -675,14 +675,23 @@ class Collision: destination: str resolved: list[str] - # Each declaration with the file it resolved to, in the platform's order. + # Each declaration with the file it resolved to, in the builder's order. declarations: list[tuple[dict, str | None]] = field(default_factory=list) + # The system of each declaration, in the same order. + systems: list[str] = field(default_factory=list) + # The file the pack ships at the destination, as the builder decides it. + shipped: str | None = None def _collision_json(collision: Collision) -> dict: return { "destination": collision.destination, "resolved": list(collision.resolved), + "shipped": collision.shipped, + "declarations": [ + {"system": system, "name": entry.get("name", ""), "file": local} + for system, (entry, local) in zip(collision.systems, collision.declarations) + ], } @@ -702,19 +711,27 @@ def find_collisions( CoCo disk ROM at one bios/disk.rom. """ fold = bool(config.get("case_insensitive_fs", False)) - by_dest: dict[str, list[dict]] = {} - for system in (config.get("systems") or {}).values(): + systems = config.get("systems") or {} + # The builder walks the systems sorted and lets a hash-constrained + # declaration that resolves by content take the path: the same order + # and the same choice say which file ships. + preferred = set( + preferred_entries(systems, db, "bios", "", False, zip_contents, True).values() + ) + by_dest: dict[str, list[tuple[str, dict]]] = {} + for sys_id, system in sorted(systems.items()): 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, fold), []).append(entry) + by_dest.setdefault(_normalize(dest, fold), []).append((sys_id, entry)) collisions = [] - for key, entries in sorted(by_dest.items()): - if len(entries) < 2: + for key, claims in sorted(by_dest.items()): + if len(claims) < 2: continue + entries = [entry for _sys_id, entry in claims] resolved = [] declarations = [] for entry in entries: @@ -734,8 +751,18 @@ def find_collisions( # also names the first one's file. if any(_accepted_by_all(path, entries, db) for path in resolved): continue + shipped = next( + (local for entry, local in declarations if local and id(entry) in preferred), + resolved[0], + ) collisions.append( - Collision(destination=key, resolved=resolved, declarations=declarations) + Collision( + destination=key, + resolved=resolved, + declarations=declarations, + systems=[sys_id for sys_id, _entry in claims], + shipped=shipped, + ) ) return collisions @@ -863,11 +890,18 @@ def _same_file_family(paths: list[str]) -> bool: 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) - ) + """One line naming the destination, which system declares which file, + and the one the pack ships there.""" + owners: dict[str, list[str]] = {} + for system, (_entry, local) in zip(collision.systems, collision.declarations): + if local and system not in owners.setdefault(local, []): + owners[local].append(system) + parts = [ + f"{path} ({', '.join(owners[path])}) " + + ("packed" if path == collision.shipped else "left out") + for path in collision.resolved + ] + return f"{collision.destination}: " + ", ".join(parts) def format_conflict(conflict: Conflict) -> str: diff --git a/tests/test_slots.py b/tests/test_slots.py index 44afbe8e0..64dd87669 100644 --- a/tests/test_slots.py +++ b/tests/test_slots.py @@ -509,6 +509,23 @@ class TestSelfContradictingDestinations(unittest.TestCase): self.assertEqual(len(collisions), 1) self.assertEqual(len(collisions[0].resolved), 2) + def test_the_line_names_each_system_and_the_file_packed(self): + """The line named two files and neither the system that declares + each nor the one the pack ships: the reader went to the YAML.""" + config = {"systems": { + "zeta": {"files": [{"name": "IPL.bin", "destination": "disk.rom", + "md5": "m" * 32}]}, + "alpha": {"files": [{"name": "IPL.bin", "destination": "disk.rom", + "md5": "n" * 32}]}, + }} + collision = slots.find_collisions(config, REGIONS_DB)[0] + line = slots.format_collision(collision) + packed, left = collision.resolved + # The builder walks the systems sorted: alpha's declaration ships. + self.assertIn(f"{packed} (alpha) packed", line) + self.assertIn(f"{left} (zeta) left out", line) + self.assertEqual(collision.shipped, packed) + 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(