fix: arbitrate slots on the file the pack ships

scripts/generate_pack.py

@@ -55,7 +55,6 @@ from common import (
     MANUFACTURER_PREFIXES,
     parse_md5_list,
     require_yaml,
-    resolution_is_hash_exact,
     ProfileSelectionError,
     resolve_local_file,
     sanitize_pack_path,
@@ -435,86 +434,6 @@ def _report_pack_counts(
             f"{core_count} from cores), {', '.join(parts)} [{verification_mode}]"
         )
 
-def _preferred_entries(
-    pack_systems: dict,
-    db: dict,
-    bios_dir: str,
-    base_dest: str,
-    required_only: bool,
-    zip_contents,
-    offline,
-) -> dict[str, int]:
-    """Which declaration wins when several claim the same destination.
-
-    A platform can declare one destination in more than one system, bare
-    in one and hash-constrained in another. First-come dedup would let
-    the bare entry pack whatever answers to the name, so a constrained
-    sibling that resolves by content claims the destination instead.
-    The choice is about the baseline and reads no data directory, for the
-    pack, the manifest and verify alike: verify alone passed the registry
-    and could crown a declaration only a data/ cache satisfies.
-    """
-    preferred_entries: dict[str, int] = {}
-    dest_entries: dict[str, list[dict]] = {}
-    for sys_id, system in sorted(pack_systems.items()):
-        for file_entry in system.get("files", []):
-            if required_only and file_entry.get("required") is False:
-                continue
-            dest = sanitize_pack_path(
-                file_entry.get("destination", file_entry.get("name", ""))
-            )
-            if not dest:
-                continue
-            full = f"{base_dest}/{dest}" if base_dest else dest
-            dest_entries.setdefault(full, []).append(file_entry)
-    for full, entries in dest_entries.items():
-        if len(entries) < 2:
-            continue
-        constrained = [
-            fe
-            for fe in entries
-            if fe.get("md5") or fe.get("sha1") or fe.get("zipped_file")
-        ]
-        if not constrained:
-            continue
-        resolved = [
-            (fe, *resolve_file(
-                fe,
-                db,
-                bios_dir,
-                zip_contents,
-                offline=offline,
-            ))
-            for fe in constrained
-        ]
-        members = [fe for fe in entries if fe.get("zipped_file")]
-        if members:
-            # One archive declared once per ROM it must hold: the archive
-            # to ship is the one the most declarations accept, not the first
-            # that answers one of them (adam_fdc.zip: a one-member MAME set
-            # answered first, the eight-member set sat beside it).
-            def accepted(path: str | None) -> int:
-                if not path or not path.endswith(".zip"):
-                    return -1
-                return sum(inner_rom_check(fe, path) == "ok" for fe in members)
-
-            fe, path, _st = max(resolved, key=lambda item: accepted(item[1]))
-            if accepted(path) > 0:
-                preferred_entries[full] = id(fe)
-                continue
-        best = None
-        for fe, _lp, _st in resolved:
-            if _lp and _st == "md5_exact":
-                best = fe
-                break
-            if best is None and _lp and resolution_is_hash_exact(_st):
-                best = fe
-        if best is not None:
-            preferred_entries[full] = id(best)
-
-    return preferred_entries
-
-
 def _select_variants(
     config: dict,
     pack_systems: dict,
@@ -4069,6 +3988,7 @@ from packverify import (  # noqa: E402,F401
 # facade name would keep saying False after set_offline() changed the flag.
 from packresolve import (  # noqa: E402,F401
     _detect_hash_type,
+    preferred_entries as _preferred_entries,
     parse_hash_input,
     parse_hash_file,
     lookup_hashes,

scripts/packresolve.py

@@ -10,7 +10,13 @@ import re
 _HEX_RE = re.compile(r"\b([0-9a-fA-F]{8,40})\b")
 from largefiles import fetch_large_file
 import os
-from common import declared_hash_verdict, resolve_local_file
+from common import (
+    declared_hash_verdict,
+    resolution_is_hash_exact,
+    resolve_local_file,
+    sanitize_pack_path,
+)
+from validation import inner_rom_check
 # CLI-wide network policy.  Public helpers can override it explicitly, while
 # every CLI mode inherits --offline (including large-file release fallback).
 _OFFLINE = False
@@ -220,3 +226,83 @@ def resolve_file(
         return path, status
 
     return None, "not_found"
+
+
+def preferred_entries(
+    pack_systems: dict,
+    db: dict,
+    bios_dir: str,
+    base_dest: str,
+    required_only: bool,
+    zip_contents,
+    offline,
+) -> dict[str, int]:
+    """Which declaration wins when several claim the same destination.
+
+    A platform can declare one destination in more than one system, bare
+    in one and hash-constrained in another. First-come dedup would let
+    the bare entry pack whatever answers to the name, so a constrained
+    sibling that resolves by content claims the destination instead.
+    The choice is about the baseline and reads no data directory, for the
+    pack, the manifest and verify alike: verify alone passed the registry
+    and could crown a declaration only a data/ cache satisfies.
+    """
+    preferred_entries: dict[str, int] = {}
+    dest_entries: dict[str, list[dict]] = {}
+    for sys_id, system in sorted(pack_systems.items()):
+        for file_entry in system.get("files", []):
+            if required_only and file_entry.get("required") is False:
+                continue
+            dest = sanitize_pack_path(
+                file_entry.get("destination", file_entry.get("name", ""))
+            )
+            if not dest:
+                continue
+            full = f"{base_dest}/{dest}" if base_dest else dest
+            dest_entries.setdefault(full, []).append(file_entry)
+    for full, entries in dest_entries.items():
+        if len(entries) < 2:
+            continue
+        constrained = [
+            fe
+            for fe in entries
+            if fe.get("md5") or fe.get("sha1") or fe.get("zipped_file")
+        ]
+        if not constrained:
+            continue
+        resolved = [
+            (fe, *resolve_file(
+                fe,
+                db,
+                bios_dir,
+                zip_contents,
+                offline=offline,
+            ))
+            for fe in constrained
+        ]
+        members = [fe for fe in entries if fe.get("zipped_file")]
+        if members:
+            # One archive declared once per ROM it must hold: the archive
+            # to ship is the one the most declarations accept, not the first
+            # that answers one of them (adam_fdc.zip: a one-member MAME set
+            # answered first, the eight-member set sat beside it).
+            def accepted(path: str | None) -> int:
+                if not path or not path.endswith(".zip"):
+                    return -1
+                return sum(inner_rom_check(fe, path) == "ok" for fe in members)
+
+            fe, path, _st = max(resolved, key=lambda item: accepted(item[1]))
+            if accepted(path) > 0:
+                preferred_entries[full] = id(fe)
+                continue
+        best = None
+        for fe, _lp, _st in resolved:
+            if _lp and _st == "md5_exact":
+                best = fe
+                break
+            if best is None and _lp and resolution_is_hash_exact(_st):
+                best = fe
+        if best is not None:
+            preferred_entries[full] = id(best)
+
+    return preferred_entries

scripts/slots.py

@@ -28,6 +28,7 @@ from common import (
     resolve_local_file,
     runs_standalone,
 )
+from packresolve import preferred_entries
 from validation import outside_gap_scope
 
 # A profile entry can prove a slot without declaring a hash: Dolphin names no
@@ -67,9 +68,14 @@ class Conflict:
         return sorted({c.emulator for c in self.profile_claims if c.emulator})
 
 
-def _normalize(destination: str) -> str:
-    """Comparable form of a destination, since layers write it differently."""
-    return destination.strip().strip("/").replace("\\", "/").casefold()
+def _normalize(destination: str, fold: bool = True) -> str:
+    """Comparable form of a destination, since layers write it differently.
+
+    Case is folded only where the platform's filesystem folds it: on Linux
+    RetroDECK ships bios/DISK.ROM and bios/disk.rom side by side.
+    """
+    cleaned = destination.strip().strip("/").replace("\\", "/")
+    return cleaned.casefold() if fold else cleaned
 
 
 def build_claim_index(claims: list[Claim]) -> dict[str, list[Claim]]:
@@ -236,16 +242,32 @@ def find_conflicts(
     Only proven claims are compared. A claim resolved by filename alone
     asserts nothing about content and cannot contradict anything.
     """
+    fold = bool(config.get("case_insensitive_fs", False))
+    # The builder's own choice among declarations sharing a destination,
+    # read from the same function: the most accepting archive for zipped_file
+    # members, then md5_exact. A rule of its own here compared profiles with
+    # a file the pack does not ship (Batocera adam_fdc.zip).
+    shipped = set(
+        preferred_entries(
+            config.get("systems") or {}, db, "bios", base_dest, False,
+            zip_contents, True,
+        ).values()
+    )
     by_dest: dict[str, Claim] = {}
     for claim in platform_claims(
         config, db, base_dest, zip_contents, data_dir_registry
     ):
-        key = _normalize(claim.destination)
+        key = _normalize(claim.destination, fold)
         held = by_dest.get(key)
-        # The pack ships the declaration a hash proves (_preferred_entries),
-        # so a bare sibling met first must not stand for the destination:
-        # it would hide the contradiction with the file actually shipped.
-        if held is None or _claim_rank(claim) > _claim_rank(held):
+        if held is not None and id(held.entry) in shipped:
+            continue
+        # A bare sibling met first must not stand for the destination: it
+        # would hide the contradiction with the file actually shipped.
+        if (
+            held is None
+            or id(claim.entry) in shipped
+            or _claim_rank(claim) > _claim_rank(held)
+        ):
             by_dest[key] = claim
 
     # Grouped before judging: a profile may declare several revisions that are
@@ -257,7 +279,7 @@ def find_conflicts(
         profiles, db, base_dest, standalone_cores, zip_contents, data_dir_registry,
         config=config,
     ):
-        key = _normalize(claim.destination)
+        key = _normalize(claim.destination, fold)
         platform = by_dest.get(key)
         if platform is None or not platform.is_proven or not claim.is_proven:
             continue
@@ -446,6 +468,7 @@ def find_collisions(
     upstream list carries. RetroDECK aims a PC-88 disk subsystem ROM and a
     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():
         for entry in system.get("files") or []:
@@ -453,7 +476,7 @@ def find_collisions(
                 continue
             dest = entry.get("destination") or entry.get("name") or ""
             if dest:
-                by_dest.setdefault(_normalize(dest), []).append(entry)
+                by_dest.setdefault(_normalize(dest, fold), []).append(entry)
 
     collisions = []
     for key, entries in sorted(by_dest.items()):

tests/test_slots_shipped_file.py

@@ -0,0 +1,92 @@
+"""Slot arbitration models the file the pack ships at a destination.
+
+slots.py chose the platform's file by a rule of its own and folded case on
+every platform. The builder keeps case on Linux frontends and picks, among
+declarations sharing a destination, the archive most zipped_file members
+accept, then the md5_exact one. RetroDECK's MSX ``DISK.ROM`` was reported as
+colliding with ``disk.rom``, which ships beside it, and Batocera's
+``adam_fdc.zip`` was taken for the one-member MAME set.
+Reads the repository: skips when the database or the files are absent.
+"""
+
+from __future__ import annotations
+
+import copy
+import hashlib
+import os
+import sys
+import unittest
+from pathlib import Path
+
+REPO_ROOT = Path(__file__).resolve().parents[1]
+sys.path.insert(0, str(REPO_ROOT / "scripts"))
+
+from common import load_database, load_platform_config  # noqa: E402
+import slots  # noqa: E402
+
+MAME_ADAM = "bios/Arcade/MAME/adam_fdc.zip"
+MSX_DISK = "bios/Microsoft/MSX/DISK.ROM"
+
+
+def _needs(*paths: str) -> None:
+    missing = [p for p in ("database.json", *paths) if not (REPO_ROOT / p).exists()]
+    if missing:
+        raise unittest.SkipTest(f"repository data absent: {missing}")
+
+
+class SlotsReadTheShippedFile(unittest.TestCase):
+    def setUp(self):
+        previous = os.getcwd()
+        os.chdir(REPO_ROOT)
+        self.addCleanup(os.chdir, previous)
+
+    def test_case_kept_where_the_filesystem_keeps_it(self):
+        _needs(MSX_DISK)
+        db = load_database("database.json")
+        config = load_platform_config("retrodeck")
+        self.assertFalse(config.get("case_insensitive_fs"))
+        listed = {
+            path
+            for collision in slots.find_collisions(config, db)
+            for path in collision.resolved
+        }
+        self.assertNotIn(MSX_DISK, listed)
+
+        folded = copy.deepcopy(config)
+        folded["case_insensitive_fs"] = True
+        listed = {
+            path
+            for collision in slots.find_collisions(folded, db)
+            for path in collision.resolved
+        }
+        self.assertIn(MSX_DISK, listed)
+
+    def test_profile_judged_against_the_builders_archive(self):
+        _needs(MAME_ADAM)
+        db = load_database("database.json")
+        config = load_platform_config("batocera")
+        # The declaration the one-member MAME set answers comes first: the
+        # YAML's order must not decide which archive is believed to ship,
+        # the builder takes the one most members accept.
+        files = config["systems"]["adam"]["files"]
+        files.sort(key=lambda fe: not str(fe.get("zipped_file", "")).startswith("adam disk"))
+        md5 = hashlib.md5((REPO_ROOT / MAME_ADAM).read_bytes()).hexdigest()
+        profiles = {
+            "probe": {
+                "emulator": "probe",
+                "type": "libretro",
+                "files": [
+                    {"name": "adam_fdc.zip", "md5": md5, "system": "coleco-adam"}
+                ],
+            }
+        }
+        conflicts = slots.find_conflicts(
+            config, profiles, db, config.get("base_destination", "")
+        )
+        shipped = {c.destination: c.platform_claim.local_path for c in conflicts}
+        self.assertIn("bios/adam_fdc.zip", shipped)
+        self.assertNotEqual(shipped["bios/adam_fdc.zip"], MAME_ADAM)
+
+
+if __name__ == "__main__":
+    unittest.main()