From 4ba935374eaf985ac7e29b12317fdc0240e9e40b Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 06:47:44 +0200 Subject: [PATCH] fix: keep pack order outside contested paths Ranking contested core claims sorted the whole gap report, which reordered every manifest. Only the claims on one contested path move now, grouped where the first of them stood, and two spellings of a path are one group where the filesystem folds case (RetroBat's 64DD_IPL_DEV.bin). An entry without a destination (EmuDeck's system:md5 whitelist) no longer counts as a pack path in verify, and step 5 prints one status for counts and paths together. A full offline run compared all 12 archives and 930 emulator packs path by path, every one equal. --- scripts/packextras.py | 38 +++++++++++++++++++++++++-------- scripts/pipeline.py | 1 + scripts/verify.py | 9 ++++++-- tests/test_pack_destinations.py | 14 ++++++++++++ 4 files changed, 51 insertions(+), 11 deletions(-) diff --git a/scripts/packextras.py b/scripts/packextras.py index 6a2b2d02..f5c8f4cb 100644 --- a/scripts/packextras.py +++ b/scripts/packextras.py @@ -869,10 +869,11 @@ def _collect_emulator_extras( # BIOS root before Solarus, which pins its own by sha1, and fbalpha2012's # hashless hiscore.dat took RetroDECK's over mame2015's sha1. Profile # order decides only between claims of equal proof. - rank = _ClaimRanks(undeclared, placement, db) - undeclared = sorted( - undeclared, key=lambda u: (-rank.of_entry(u), not _names_content(u)) + undeclared = sorted(undeclared, key=lambda u: not _names_content(u)) + rank = _ClaimRanks( + undeclared, placement, db, bool(config.get("case_insensitive_fs", False)) ) + undeclared = rank.ordered(undeclared) extras = [] seen_dests: set[str] = set(seen) # Destination -> the extra that took it, to record later claimants on. @@ -1056,8 +1057,12 @@ class _ClaimRanks: where two claims meet: a destination one core asks for has no contest. """ - def __init__(self, undeclared: list[dict], placement: ExtraPlacement, db: dict) -> None: + def __init__( + self, undeclared: list[dict], placement: ExtraPlacement, db: dict, fold: bool, + ) -> None: self._db = db + # Two spellings of one path are one file where the filesystem folds case. + self._key = (lambda dest: dest.lower()) if fold else (lambda dest: dest) self._placed: dict[int, tuple[str, str] | None] = {} counts: dict[str, int] = {} for u in undeclared: @@ -1067,14 +1072,13 @@ class _ClaimRanks: ) self._placed[id(u)] = placed if placed is not None: - counts[placed[1]] = counts.get(placed[1], 0) + 1 + len( - u.get("other_claims", []) - ) + key = self._key(placed[1]) + counts[key] = counts.get(key, 0) + 1 + len(u.get("other_claims", [])) self._contested = {dest for dest, n in counts.items() if n > 1} self._cache: dict[tuple, int] = {} def of_extra(self, extra: dict, full_dest: str) -> int: - if full_dest not in self._contested: + if self._key(full_dest) not in self._contested: return 0 key = (extra.get("source_profile"), extra.get("source_name"), extra.get("source_path"), full_dest) @@ -1093,9 +1097,25 @@ class _ClaimRanks: ) return self._cache[key] + def ordered(self, undeclared: list[dict]) -> list[dict]: + """The entries in their order, the claims on one contested path + reordered among themselves by proof. Nothing else moves, so the + pack and the manifest keep their order.""" + first: dict[str, int] = {} + keys: list[tuple[int, int, int]] = [] + for position, u in enumerate(undeclared): + placed = self._placed.get(id(u)) + if placed is None or self._key(placed[1]) not in self._contested: + keys.append((position, 0, position)) + continue + group = first.setdefault(self._key(placed[1]), position) + keys.append((group, -self.of_entry(u), position)) + order = sorted(range(len(undeclared)), key=keys.__getitem__) + return [undeclared[i] for i in order] + def of_entry(self, u: dict) -> int: placed = self._placed.get(id(u)) - if placed is None or placed[1] not in self._contested: + if placed is None or self._key(placed[1]) not in self._contested: return 0 name = u.get("archive") or u["name"] return max( diff --git a/scripts/pipeline.py b/scripts/pipeline.py index c6b235c1..ee3c4e62 100644 --- a/scripts/pipeline.py +++ b/scripts/pipeline.py @@ -919,6 +919,7 @@ def main() -> None: # Counts can agree while the paths differ, both ways at once. ok = check_pack_destinations(destinations_file, pack_output) and ok ok = check_emulator_packs() and ok + print(f"--- 5/8 consistency check, counts and paths: {'OK' if ok else 'FAILED'} ---") results["consistency"] = ok all_ok = all_ok and ok else: diff --git a/scripts/verify.py b/scripts/verify.py index 68eb6087..477af66f 100644 --- a/scripts/verify.py +++ b/scripts/verify.py @@ -904,8 +904,10 @@ def _builder_placement( The builder walks the systems in name order and skips a path that a written file blocks as a directory, or the reverse: RetroArch declares SGB1.sfc and SGB1.sfc/program.rom, and only the first is in the pack. - Returns (path -> local copy written there, path -> copy or None for each - slot kept, a user_provided or external one included). + An entry without a destination (EmuDeck's system:md5 whitelist) is + judged but takes no path. Returns (path -> local copy written there, + path -> copy or None for each slot kept, a user_provided or external one + included). """ from packpaths import _has_path_conflict, _register_path @@ -915,6 +917,9 @@ def _builder_placement( folded: set[str] = set() parents: set[str] = set() for _sys_id, full, local in sorted(packed_order, key=lambda item: item[0]): + # EmuDeck's system:md5 entries name no file: the builder writes none. + if not full or full.endswith("/"): + continue if ( full in occupied or (case_insensitive and full.lower() in folded) diff --git a/tests/test_pack_destinations.py b/tests/test_pack_destinations.py index 98402346..d5bbd80b 100644 --- a/tests/test_pack_destinations.py +++ b/tests/test_pack_destinations.py @@ -108,6 +108,20 @@ class AUserProvidedSlot(_Fixture): self.assertEqual(report["pack_destinations"], []) +class AnEntryWithoutAFileTakesNoPath(unittest.TestCase): + """EmuDeck's system:md5 entries name no file. verify credited the bare + base destination as a pack path, and the comparison failed on ''.""" + + def test_it_is_left_out(self): + from verify import _builder_placement + + shipped, occupied = _builder_placement( + [("s", "bios/", None), ("s", "", "x"), ("s", "bios/a.bin", "bios/a.bin")], False + ) + self.assertEqual(shipped, {"bios/a.bin": "bios/a.bin"}) + self.assertEqual(set(occupied), {"bios/a.bin"}) + + class ThePipelineComparesPaths(unittest.TestCase): """Equal counts, other paths: the comparison fails and names both sides."""