refactor: share the core file lookup with verify

verify replayed the builder's placement of core files with a bare name lookup, without the archive index, the data directories or the core's own choice of copy. It compared other bytes than the pack's (blueMSX's MSX2P.rom from bios/ against the data tree's copy) and let an unresolved file take a path the builder leaves free. ExtraResolver now holds the builder's lookup and both call it.

scripts/generate_pack.py

@@ -1049,6 +1049,10 @@ def generate_pack(
           )
 
       core_count = 0
+      extra_resolver = ExtraResolver(
+          db, bios_dir, zip_contents, data_registry, validation_index,
+          validation_owners, offline,
+      )
       for fe in core_files:
           if required_only and fe.get("required") is False:
               continue
@@ -1078,26 +1082,13 @@ def generate_pack(
           ):
               continue
 
-          dest_hint = fe.get("destination", "")
-          local_path, status = resolve_file(
-              fe,
-              db,
-              bios_dir,
-              zip_contents,
-              dest_hint=dest_hint,
-              data_dir_registry=data_registry,
-              offline=offline,
-          )
-          if status in ("not_found", "external", "user_provided") or not local_path:
-              continue
           # The emulator's own check picks the copy, as for a declared file
           # and in the emulator pack: the name led RetroArch's GC/dsp_rom.bin
           # to Dolphin's obsolete v0.3.1 free ROM, beside Nintendo's coef
           # ROM, a pair Dolphin warns about at every start.
-          local_path, disagreement = validated_choice(
-              fe, local_path, db, validation_index, bios_dir, None, dest,
-              validation_owners,
-          )
+          local_path, status, disagreement = extra_resolver.resolve(fe, dest)
+          if not local_path:
+              continue
           if disagreement:
               core_discrepancies.append(f"{full_dest}: {disagreement}")
           if status == "hash_mismatch" and not _matches_an_alternative(
@@ -4672,6 +4663,7 @@ from packpaths import (  # noqa: E402,F401
     placement_block,
 )
 from packextras import (  # noqa: E402,F401
+    ExtraResolver,
     _detect_extras_prefix,
     _detect_slug_structure,
     _map_emulator_to_slug,

scripts/packextras.py

@@ -147,6 +147,63 @@ def native_slug_systems(config: dict) -> set[str]:
     }
 
 
+def _file_sha1(path: str) -> str | None:
+    """sha1 of a copy the database does not index, a data tree's file."""
+    from hashing import compute_hashes
+
+    try:
+        return compute_hashes(path, frozenset({"sha1"}))["sha1"]
+    except OSError:
+        return None
+
+
+class ExtraResolver:
+    """How the builder finds the copy of a core's file it packs.
+
+    The builder and verify's replay of its placement must look the file up
+    the same way: by the storage-aware resolver with the archive index and
+    the data directories, then through the core's own check. verify once
+    looked up by name alone and compared other bytes than the pack's
+    (blueMSX's MSX2P.rom from bios/ against the data tree's copy).
+    """
+
+    def __init__(
+        self,
+        db: dict,
+        bios_dir: str,
+        zip_contents: dict | None,
+        data_registry: dict | None,
+        validation_index: dict,
+        validation_owners: dict | None,
+        offline: bool | None = True,
+    ) -> None:
+        self.db = db
+        self.bios_dir = bios_dir
+        self.zip_contents = zip_contents
+        self.data_registry = data_registry
+        self.validation_index = validation_index
+        self.validation_owners = validation_owners
+        self.offline = offline
+
+    def resolve(self, fe: dict, dest: str) -> tuple[str | None, str, str]:
+        """(local copy or None, resolver status, the core's disagreement)."""
+        from packresolve import resolve_file
+        from validation import validated_choice
+
+        local_path, status = resolve_file(
+            fe, self.db, self.bios_dir, self.zip_contents,
+            dest_hint=fe.get("destination", ""),
+            data_dir_registry=self.data_registry, offline=self.offline,
+        )
+        if status in ("not_found", "external", "user_provided") or not local_path:
+            return None, status, ""
+        local_path, disagreement = validated_choice(
+            fe, local_path, self.db, self.validation_index, self.bios_dir, None,
+            dest, self.validation_owners,
+        )
+        return local_path, status, disagreement
+
+
 def hidden_extras(
     config: dict,
     emulators_dir: str,
@@ -157,6 +214,7 @@ def hidden_extras(
     region_drops: set[str] | None = None,
     placed: set[str] | None = None,
     report: list[dict] | None = None,
+    resolver: ExtraResolver | None = None,
 ) -> dict[tuple[str, str, str], str]:
     """Report entries the builder holds and still leaves out, with the reason.
 
@@ -172,6 +230,8 @@ def hidden_extras(
     file left out. ``shipped`` maps the full destination of each platform
     file the pack carries to its local copy. ``placed``, when given,
     receives the full destination of each held extra the pack carries.
+    ``resolver`` is the builder's lookup. An extra it cannot resolve is
+    skipped and takes no path, as the builder skips it.
     """
     from packpaths import _register_path, placement_block
 
@@ -202,10 +262,15 @@ def hidden_extras(
             # Another core's claim on the same path, settled by the claims
             # the collector records: not a file left out.
             continue
-        local, _status = resolve_local_file(
-            fe, db, dest_hint=fe.get("destination", "")
-        )
-        held = sha1_by_path.get(local) if local else None
+        if resolver is not None:
+            local, _status, _disagreement = resolver.resolve(fe, dest)
+            if not local:
+                continue
+        else:
+            local, _status = resolve_local_file(
+                fe, db, dest_hint=fe.get("destination", "")
+            )
+        held = (sha1_by_path.get(local) or _file_sha1(local)) if local else None
         if reason == "case_twin" and held and content.get(full.lower()) == held:
             continue
         if reason is not None:

scripts/verify.py

@@ -823,6 +823,7 @@ def _mark_hidden(
     target_cores: set[str] | None,
     region_drops: set[str] | None = None,
     report: list[dict] | None = None,
+    resolver: object | None = None,
 ) -> set[str]:
     """What the builder holds but cannot place beside the files it packed.
 
@@ -837,7 +838,7 @@ def _mark_hidden(
         config, emulators_dir, db,
         emu_profiles if emu_profiles is not None
         else load_emulator_profiles(emulators_dir),
-        shipped, target_cores, region_drops, placed, report,
+        shipped, target_cores, region_drops, placed, report, resolver,
     )
     for u in undeclared:
         reason = hidden.get(
@@ -1197,9 +1198,15 @@ def verify_platform(
             not in region_drops
         ]
     _mark_unplaced(config, undeclared, profiles)
+    from packextras import ExtraResolver
+
     placed = _mark_hidden(
         config, undeclared, emulators_dir, db, emu_profiles, shipped, target_cores,
         region_drops, report_for_builder,
+        ExtraResolver(
+            db, DEFAULT_BIOS_DIR, zip_contents, data_dir_registry, validation_index,
+            validation_owners,
+        ),
     )
     if not reads_file_contents(mode):
         # An existence frontend is satisfied by any file at the path, and the

tests/test_audit_regressions.py

@@ -2711,6 +2711,9 @@ class ARegionDroppedExtraBlocksNothing(unittest.TestCase):
         with tempfile.TemporaryDirectory(dir=TMP_ROOT) as directory:
             other = Path(directory) / "FOO.ROM"
             other.write_bytes(b"other bytes")
+            first = Path(directory) / "foo.rom"
+            first.write_bytes(b"first bytes")
+            copies = {"foo.rom": str(first), "FOO.ROM": str(other)}
             extras = [
                 {"name": "foo.rom", "destination": "foo.rom", "source_emulator": "a"},
                 {"name": "FOO.ROM", "destination": "FOO.ROM", "source_emulator": "b"},
@@ -2721,7 +2724,8 @@ class ARegionDroppedExtraBlocksNothing(unittest.TestCase):
                 with mock.patch.object(packextras, "_collect_emulator_extras",
                                        return_value=extras), \
                         mock.patch.object(packextras, "resolve_local_file",
-                                          return_value=(str(other), "name_exact")):
+                                          side_effect=lambda fe, *a, **k: (
+                                              copies[fe["name"]], "name_exact")):
                     return packextras.hidden_extras(
                         config, "emulators", {"files": {}}, {}, {}, None, region_drops
                     )

tests/test_hidden_extras.py

@@ -154,5 +154,38 @@ class TheGapReportIsComputedOnce(unittest.TestCase):
         self.assertEqual(counted.call_count, 1)
 
 
+class TheReplayResolvesAsTheBuilder(unittest.TestCase):
+    """verify replayed the builder's placement with a bare name lookup: it
+    compared other bytes than the pack's, and an extra it could not resolve
+    still took its path and blocked the next one the builder places."""
+
+    def test_an_unresolved_extra_blocks_nothing(self):
+        from unittest import mock
+
+        import packextras
+
+        extras = [
+            {"destination": "box.bin/inner.rom", "name": "inner.rom",
+             "source_emulator": "a", "source_name": "inner.rom", "source_path": ""},
+            {"destination": "box.bin", "name": "box.bin",
+             "source_emulator": "b", "source_name": "box.bin", "source_path": ""},
+        ]
+
+        class Resolver:
+            def resolve(self, fe, dest):
+                return (None, "not_found", "") if fe["name"] == "inner.rom" else (
+                    "bios/box.bin", "sha1_exact", ""
+                )
+
+        placed: set[str] = set()
+        with mock.patch.object(packextras, "_collect_emulator_extras", return_value=extras):
+            hidden = packextras.hidden_extras(
+                {"systems": {}}, "emulators", {"files": {}}, {}, {},
+                placed=placed, resolver=Resolver(),
+            )
+        self.assertEqual(hidden, {})
+        self.assertEqual(placed, {"box.bin"})
+
+
 if __name__ == "__main__":
     unittest.main()