diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index 7ee6fbee..c709700b 100644 --- a/scripts/generate_pack.py +++ b/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, diff --git a/scripts/packextras.py b/scripts/packextras.py index ec8a4a9c..2c2f49d0 100644 --- a/scripts/packextras.py +++ b/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: diff --git a/scripts/verify.py b/scripts/verify.py index 02d40761..46808562 100644 --- a/scripts/verify.py +++ b/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 diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index b9674ba1..20ee27c7 100644 --- a/tests/test_audit_regressions.py +++ b/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 ) diff --git a/tests/test_hidden_extras.py b/tests/test_hidden_extras.py index a97b1fdf..66a92d33 100644 --- a/tests/test_hidden_extras.py +++ b/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()