From dbc8d63318c26be8adb66580ce5a0c14e0dd2b84 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Tue, 6 Oct 2026 04:19:40 +0200 Subject: [PATCH] fix: one agnostic substitute rule for pack and verify --- scripts/generate_pack.py | 60 +++++++++++++++---------------- scripts/packverify.py | 2 +- scripts/validation.py | 41 ++++++++++++++++++++- scripts/verify.py | 6 ++++ tests/test_agnostic_substitute.py | 60 +++++++++++++++++++++++++++++++ tests/test_e2e.py | 17 +++++++-- 6 files changed, 152 insertions(+), 34 deletions(-) create mode 100644 tests/test_agnostic_substitute.py diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index 0ed46303..bdfd9b69 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -28,6 +28,7 @@ import zipfile from pathlib import Path sys.path.insert(0, os.path.dirname(__file__)) +from common import resolve_platform_cores as _platform_cores from common import ( apply_target_overrides, artifact_lock, @@ -73,6 +74,7 @@ from nativemode import ( reads_file_contents, ) from validation import ( + agnostic_substitute, _build_validation_index, check_file_validation, filter_files_by_mode, @@ -600,6 +602,10 @@ def generate_pack( config = load_platform_config(platform_name, platforms_dir) if zip_contents is None: zip_contents = {} + if emu_profiles is None: + # The pipeline always passes them; a caller that does not must get the + # same pack, not one built without validation, slots or agnostic cores. + emu_profiles = load_emulator_profiles(emulators_dir) verification_mode = config.get("verification_mode", "existence") platform_display = config.get("platform", platform_name) @@ -641,6 +647,7 @@ def generate_pack( from common import resolve_platform_cores validation_index = {} + platform_profiles: dict[str, dict] = {} if emu_profiles: platform_profiles = { name: emu_profiles[name] @@ -806,38 +813,16 @@ def generate_pack( continue if status == "not_found": - # Agnostic fallback: if an agnostic core covers this system, - # find any matching file in the DB - by_name = db.get("indexes", {}).get("by_name", {}) + # A filename-agnostic core takes any image of its system, + # renamed; only an existence frontend accepts that. files_db = db.get("files", {}) agnostic_path = None agnostic_resolved = False - if emu_profiles: - for _emu_key, _emu_prof in emu_profiles.items(): - if _emu_prof.get("bios_mode") != "agnostic": - continue - if sys_id not in set(_emu_prof.get("systems", [])): - continue - for _ef in _emu_prof.get("files", []): - ef_name = _ef.get("name", "") - for _sha1 in by_name.get(ef_name, []): - _entry = files_db.get(_sha1, {}) - _path = _entry.get("path", "") - if _path: - _prefix = _path.rsplit("/", 1)[0] + "/" - for _s, _e in files_db.items(): - if _e.get("path", "").startswith(_prefix): - if size_fits(_ef, _e.get("size", 0)): - if os.path.exists(_e["path"]): - local_path = _e["path"] - agnostic_path = _prefix - agnostic_resolved = True - break - break - if agnostic_resolved: - break - if agnostic_resolved: - break + if not reads_file_contents(verification_mode): + found = agnostic_substitute(file_entry, sys_id, db, platform_profiles) + if found: + local_path, agnostic_path = found + agnostic_resolved = True if agnostic_resolved and local_path: # Write rename README @@ -2968,6 +2953,10 @@ def generate_manifest( base_dest = config.get("base_destination", "") case_insensitive = config.get("case_insensitive_fs", False) verification_mode = config.get("verification_mode", "existence") + manifest_profiles = { + name: emu_profiles[name] + for name in _platform_cores(config, emu_profiles) + } # Load registry for install metadata registry: dict = {} @@ -3001,6 +2990,7 @@ def generate_manifest( # Sizes of the files the pack carries and the installer cannot fetch: the # collection does not index them, a data directory cache answered. pack_only_sizes: list[int] = [] + rename_notes: set[str] = set() if data_registry is None: data_registry = load_data_dir_registry(platforms_dir) slot_overrides = slots.pack_overrides( @@ -3104,6 +3094,13 @@ def generate_manifest( override = slot_overrides.get(full_dest) if override: local_path, status = override, "slot_arbitrated" + if status == "not_found" and not reads_file_contents(verification_mode): + found = agnostic_substitute(file_entry, sys_id, db, manifest_profiles) + if found: + local_path, status = found[0], "agnostic_fallback" + if os.path.basename(local_path) != file_entry.get("name", ""): + # The pack explains the rename in a note of its own. + rename_notes.add(file_entry.get("name", "")) if ( status == "hash_mismatch" and local_path @@ -3228,7 +3225,10 @@ def generate_manifest( "standalone_copies": standalone_copies, "total_files": len(manifest_files), "total_size": total_size, - "pack_files": len(manifest_files) + len(data_sizes) + len(PACK_DOCUMENTS), + "pack_files": ( + len(manifest_files) + len(data_sizes) + len(PACK_DOCUMENTS) + + len(rename_notes) + ), "pack_size": total_size + sum(data_sizes), "total_omitted": len(omitted_by_destination), "omitted_files": sorted( diff --git a/scripts/packverify.py b/scripts/packverify.py index 5f2bf153..1df6199e 100644 --- a/scripts/packverify.py +++ b/scripts/packverify.py @@ -72,7 +72,7 @@ def verify_pack( if info.is_dir(): continue name = info.filename - if name.startswith("INSTRUCTIONS_") or name in ( + if name.startswith(("INSTRUCTIONS_", "RENAMED_")) or name in ( "manifest.json", "README.txt", ): diff --git a/scripts/validation.py b/scripts/validation.py index 7c44402e..5b568294 100644 --- a/scripts/validation.py +++ b/scripts/validation.py @@ -9,7 +9,7 @@ from __future__ import annotations import os -from common import compute_hashes +from common import compute_hashes, size_fits from hashing import parse_md5_list # Validation types that require console-specific cryptographic keys. @@ -392,3 +392,42 @@ def find_validated_variant( if check_file_validation(path, fname, validation_index, bios_dir) is None: return path return None + + +def agnostic_substitute( + file_entry: dict, sys_id: str, db: dict, platform_profiles: dict[str, dict] +) -> tuple[str, str] | None: + """(path, directory) of a held file a filename-agnostic core can boot. + + A core with `bios_mode: agnostic` (PCSX2 picks any image in its BIOS + folder) is served by any image of the right size, renamed to what the + platform declares. That only satisfies a frontend that checks existence: + a digest frontend compares the bytes. Only the platform's own cores are + asked, and the pack and verify read this one answer. An entry that + declares a content hash names one file: no substitute stands for it. + """ + if any(file_entry.get(h) for h in ("sha1", "md5", "sha256", "crc32")): + return None + by_name = db.get("indexes", {}).get("by_name", {}) + files_db = db.get("files", {}) + for profile in platform_profiles.values(): + if profile.get("bios_mode") != "agnostic": + continue + if sys_id not in set(profile.get("systems", [])): + continue + for entry in profile.get("files", []): + for sha1 in by_name.get(entry.get("name", ""), []): + path = files_db.get(sha1, {}).get("path", "") + if not path: + continue + prefix = path.rsplit("/", 1)[0] + "/" + for candidate in files_db.values(): + held = candidate.get("path", "") + if ( + held.startswith(prefix) + and size_fits(entry, candidate.get("size", 0)) + and os.path.exists(held) + ): + return held, prefix + break + return None diff --git a/scripts/verify.py b/scripts/verify.py index d4da377c..5283f1c1 100644 --- a/scripts/verify.py +++ b/scripts/verify.py @@ -67,6 +67,7 @@ from nativemode import ( reads_file_contents, ) from validation import ( + agnostic_substitute, _build_validation_index, _parse_validation, build_ground_truth, @@ -760,6 +761,7 @@ def verify_platform( # cpc6128.rom against cap32's 32K one) has nothing to say about a # RetroArch pack. plat_cores = resolve_platform_cores(config, profiles) + platform_profiles = {name: profiles[name] for name in plat_cores} validation_index = _build_validation_index( {name: profiles[name] for name in plat_cores} ) @@ -827,6 +829,10 @@ def verify_platform( ) if override: local_path, resolve_status = override, "slot_arbitrated" + if not reads_file_contents(mode) and local_path is None: + found = agnostic_substitute(file_entry, sys_id, db, platform_profiles) + if found: + local_path, resolve_status = found[0], "agnostic_fallback" if not reads_file_contents(mode): result = verify_entry_existence( file_entry, diff --git a/tests/test_agnostic_substitute.py b/tests/test_agnostic_substitute.py new file mode 100644 index 00000000..3c6cb00a --- /dev/null +++ b/tests/test_agnostic_substitute.py @@ -0,0 +1,60 @@ +"""Pack, verify and manifest answer the same way for a filename-agnostic core. + +The pack alone renamed any PS2 image into a missing slot, even on an md5 +platform whose frontend then rejects it, and counted it OK; verify called +the slot missing and the manifest omitted it. +""" + +from __future__ import annotations + +import re +import sys +import tempfile +import unittest +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(REPO_ROOT / "scripts")) + +from validation import agnostic_substitute # noqa: E402 + + +class AgnosticSubstitute(unittest.TestCase): + def test_only_the_platform_cores_answer(self): + with tempfile.TemporaryDirectory(dir=REPO_ROOT / "tmp") as tmp: + held = Path(tmp) / "ps2" / "scph39001.bin" + held.parent.mkdir() + held.write_bytes(b"x" * 16) + db = { + "files": {"a": {"path": str(held), "name": "scph39001.bin", "size": 16}}, + "indexes": {"by_name": {"scph39001.bin": ["a"]}}, + } + agnostic = { + "bios_mode": "agnostic", + "systems": ["sony-playstation-2"], + "files": [{"name": "scph39001.bin", "min_size": 8}], + } + self.assertEqual( + agnostic_substitute({}, "sony-playstation-2", db, {"pcsx2": agnostic}), + (str(held), f"{held.parent}/"), + ) + self.assertIsNone(agnostic_substitute({}, "sony-playstation-2", db, {})) + self.assertIsNone(agnostic_substitute({}, "nintendo-nes", db, {"pcsx2": agnostic})) + self.assertIsNone( + agnostic_substitute({"md5": "0" * 32}, "sony-playstation-2", db, {"pcsx2": agnostic}) + ) + + def test_every_reader_calls_it_behind_the_existence_gate(self): + builder = (REPO_ROOT / "scripts" / "generate_pack.py").read_text(encoding="utf-8") + verify = (REPO_ROOT / "scripts" / "verify.py").read_text(encoding="utf-8") + self.assertEqual(builder.count("agnostic_substitute(file_entry, sys_id"), 2) + self.assertEqual(verify.count("agnostic_substitute(file_entry, sys_id"), 1) + for source in (builder, verify): + for call in re.finditer(r"agnostic_substitute\(file_entry, sys_id", source): + window = source[max(0, call.start() - 300):call.start()] + self.assertIn("reads_file_contents", window) + self.assertNotIn('_emu_prof.get("bios_mode")', builder) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_e2e.py b/tests/test_e2e.py index 9f7f1e17..eba45804 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -848,6 +848,13 @@ class TestE2E(unittest.TestCase): r = check_inside_zip(self.files["bad_inner.zip"]["path"], "inner.rom", "wrong") self.assertEqual(r, "untested") + def test_20b_agnostic_core_fills_existence_slots(self): + """An agnostic core on the platform serves a missing slot, as the pack does.""" + config = load_platform_config("test_existence", self.platforms_dir) + result = verify_platform(config, self.db, self.emulators_dir) + c = result["severity_counts"] + self.assertEqual(c[Severity.OK], result["total_files"]) + def test_13_check_inside_zip_not_found(self): r = check_inside_zip(self.files["missing_inner.zip"]["path"], "nope.rom", "abc") self.assertEqual(r, "not_in_zip") @@ -859,7 +866,13 @@ class TestE2E(unittest.TestCase): def test_20_verify_existence_platform(self): config = load_platform_config("test_existence", self.platforms_dir) - result = verify_platform(config, self.db, self.emulators_dir) + # Without the filename-agnostic core, which would serve the two + # missing slots under existence (test_20b covers that case). + profiles = { + k: v for k, v in load_emulator_profiles(self.emulators_dir).items() + if v.get("bios_mode") != "agnostic" + } + result = verify_platform(config, self.db, self.emulators_dir, emu_profiles=profiles) c = result["severity_counts"] total = result["total_files"] # 2 present (1 req + 1 opt), 2 missing (1 req WARNING + 1 opt INFO) @@ -3704,7 +3717,7 @@ class TestE2E(unittest.TestCase): zip_names = { n for n in zf.namelist() - if not n.startswith("INSTRUCTIONS_") + if not n.startswith(("INSTRUCTIONS_", "RENAMED_")) and n != "manifest.json" and n != "README.txt" }