From 55ac3162f56bb040ed61fa8ff22935c8486d2937 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Fri, 6 Nov 2026 03:13:57 +0100 Subject: [PATCH] docs: name the slot function the pack reads architecture.md said the pack and verify decide through slots.arbitrate(), while a test forbids them to call it: they read slot_choices() through pack_overrides(). build_claim_index, the name-keyed index the module docstring described, had no caller outside its own tests. --- scripts/slots.py | 33 +++++++---------------------- tests/test_doc_pages.py | 6 ++++++ tests/test_slots.py | 47 +++++++++++++++++++---------------------- wiki/architecture.md | 7 +++--- 4 files changed, 40 insertions(+), 53 deletions(-) diff --git a/scripts/slots.py b/scripts/slots.py index 3680f7c69..e9b45708b 100644 --- a/scripts/slots.py +++ b/scripts/slots.py @@ -7,11 +7,14 @@ module existed nothing compared the two: the builder resolves the platform entry first and drops the profile entry on a bare filename match, so a profile that names the right file for a slot could never win and never even be heard. -The index is keyed by destination AND by name, the name carrying the union of -what every same-named entry claims. Dolphin declares three IPL.bin separated -only by their path, so a name-only key merges them, and a path-only key answers -nothing when a candidate is known by name alone. Ambiguity resolves to the -union and is never discarded, the same rule region.py already applies. +Claims are keyed by destination, folded to one case only where the platform's +filesystem folds it. Dolphin declares three IPL.bin separated only by their +path, and each is its own slot. A profile claim resting on a bare filename +proves nothing about content and contests no slot. + +The pack builder, the install manifest and verify read slot_choices() through +pack_overrides(). arbitrate() states the same decision for one conflict and +feeds the report this module prints. """ from __future__ import annotations @@ -125,26 +128,6 @@ def _normalize(destination: str, fold: bool = True) -> str: return cleaned.casefold() if fold else cleaned -def build_claim_index(claims: list[Claim]) -> dict[str, list[Claim]]: - """Index claims by destination and by name, the name carrying the union. - - A lookup by full destination answers for one slot. A lookup by bare name - answers with every slot that shares it, which is what a candidate known - only by its filename needs in order not to be dropped. - """ - index: dict[str, list[Claim]] = {} - for claim in claims: - keys = {_normalize(claim.destination)} - if claim.name: - keys.add(_normalize(claim.name)) - base = claim.destination.rsplit("/", 1)[-1] - if base: - keys.add(_normalize(base)) - for key in keys - {""}: - index.setdefault(key, []).append(claim) - return index - - def platform_claims( config: dict, db: dict, diff --git a/tests/test_doc_pages.py b/tests/test_doc_pages.py index 3054134d4..ee225c127 100644 --- a/tests/test_doc_pages.py +++ b/tests/test_doc_pages.py @@ -1738,6 +1738,12 @@ class ArchitectureStatesTheArbitration(unittest.TestCase): section = page[page.index("### When both layers claim one destination"):] section = section[:section.index("\n## ")] self.assertIn("`slots.arbitrate()`", section) + # The pack, the manifest and verify decide through slot_choices, and + # a test in test_slots forbids them to call arbitrate: the page names + # the function they read, not only the one the report prints. + self.assertIn("`slots.slot_choices()`", section) + self.assertIn("`slots.pack_overrides()`", section) + self.assertNotIn("both decide through `slots.arbitrate()`", section) source = inspect.getsource(slots.arbitrate) # The three outcomes the section describes. for outcome in ("ADDRESSEE", "SERVES_BOTH", "FRONTEND_CHECKS_CONTENT"): diff --git a/tests/test_slots.py b/tests/test_slots.py index d4dabab91..70e98ca40 100644 --- a/tests/test_slots.py +++ b/tests/test_slots.py @@ -75,31 +75,6 @@ def tearDownModule(): shutil.rmtree(_ROOT, ignore_errors=True) -class TestClaimIndex(unittest.TestCase): - """Keyed by destination and by name, the name carrying the union.""" - - def _claims(self): - return [ - slots.Claim("profile", "GC/USA/IPL.bin", "IPL.bin", emulator="dolphin"), - slots.Claim("profile", "GC/JAP/IPL.bin", "IPL.bin", emulator="dolphin"), - ] - - def test_destination_key_separates_same_named_entries(self): - index = slots.build_claim_index(self._claims()) - self.assertEqual(len(index["gc/usa/ipl.bin"]), 1) - self.assertEqual(len(index["gc/jap/ipl.bin"]), 1) - - def test_bare_name_answers_with_the_union(self): - index = slots.build_claim_index(self._claims()) - self.assertEqual(len(index["ipl.bin"]), 2) - - def test_lookup_ignores_case_and_leading_separator(self): - index = slots.build_claim_index( - [slots.Claim("platform", "/BIOS/Disk.rom", "Disk.rom")] - ) - self.assertIn("bios/disk.rom", index) - - class TestConflicts(unittest.TestCase): """A proven profile claim contradicting what the baseline would ship.""" @@ -411,6 +386,28 @@ class TestArbitration(unittest.TestCase): # The hashed declaration keeps the file the frontend checks. self.assertEqual(overrides, {"IPL.bin": "bios/Console/GC/JAP/IPL.bin"}) + def test_the_report_and_the_pack_decide_alike(self): + """The report prints arbitrate(), the pack, the manifest and verify + read slot_choices(): both must name the same file for a slot.""" + for mode in ("existence", "md5"): + config = { + "verification_mode": mode, + "cores": ["dolphin"], + "systems": {"console": {"files": [ + {"name": "IPL.bin", "destination": "GC/JAP/IPL.bin", "md5": "m" * 32}, + ]}}, + } + profiles = {"dolphin": {"type": "libretro", "systems": ["console"], "files": [ + {"name": "IPL.bin", "path": "GC/JAP/IPL.bin"}, + ]}} + with self.subTest(mode=mode): + [conflict] = slots.find_conflicts(config, profiles, REGIONS_DB) + decision = slots.arbitrate(conflict, mode) + served = slots.pack_overrides(config, profiles, REGIONS_DB).get( + "GC/JAP/IPL.bin", conflict.platform_claim.local_path + ) + self.assertEqual(served, decision.winner.local_path) + def test_the_reported_line_gives_the_ground_for_the_decision(self): kept = slots.format_decision(slots.arbitrate(self._conflict(), "md5")) self.assertIn("verifies content", kept) diff --git a/wiki/architecture.md b/wiki/architecture.md index 215b16c8c..eb768e95b 100644 --- a/wiki/architecture.md +++ b/wiki/architecture.md @@ -196,9 +196,10 @@ whoever asked for it: - A pack built with `--emulator` answers to that emulator, and takes its file. `scripts/slots.py` prints each contested destination and the reason for its -winner (`python scripts/slots.py --platform retrobat`). `generate_pack.py` and -`verify.py` both decide through `slots.arbitrate()`, so the pack and the report -agree. +winner (`python scripts/slots.py --platform retrobat`), as `slots.arbitrate()` +decides it. `generate_pack.py`, the install manifest and `verify.py` all read +`slots.slot_choices()` through `slots.pack_overrides()`, which applies the same +order, so the pack, the installer and the report agree. A platform YAML is regenerated by its scraper and never edited by hand: an edit would be lost at the next scrape, and the file would stop describing the