From 866f214e206855ba4b732832f7073be20c7142d2 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Fri, 16 Oct 2026 17:09:08 +0200 Subject: [PATCH] refactor: share the baseline verdict with the manifest --- scripts/generate_pack.py | 112 ++++++++++++-------------------- scripts/slots.py | 14 ++++ scripts/validation.py | 31 +++++++++ tests/test_audit_regressions.py | 57 ++++++++++++++++ 4 files changed, 145 insertions(+), 69 deletions(-) diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index 665034e7..d699715f 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -78,13 +78,11 @@ from nativemode import ( reads_file_contents, ) from validation import ( + baseline_verdict, existence_discrepancy, destination_owners, validated_choice, - inner_rom_check, - settle_mismatch, agnostic_substitute, - frontend_digest_matches, _build_validation_index, filter_files_by_mode, ) @@ -758,9 +756,9 @@ def generate_pack( offline=offline, ) - override = slot_overrides.get(full_dest) - if override and status not in ("external", "release_asset"): - local_path, status = override, "slot_arbitrated" + local_path, status = slots.apply_override( + slot_overrides.get(full_dest), local_path, status + ) if status == "external": file_ext = os.path.splitext(file_entry["name"])[1] or "" @@ -840,52 +838,26 @@ def generate_pack( file_reasons[dedup_key] = "not found" continue - # Batocera hashes the member, never the ZIP: a zipped_file entry - # is checked inside the archive whatever the outer resolution - # said, as verify does. An archive without the member passed. - inner_checked = bool(file_entry.get("zipped_file") and local_path) - if hash_mismatch_excludes_file(verification_mode) and ( - status == "hash_mismatch" or inner_checked + # The verdict the install manifest reads too: a member missing + # from a zipped_file archive, or a hash the frontend rejects. + status, refusal = baseline_verdict( + file_entry, local_path, status, verification_mode + ) + if refusal: + file_status[dedup_key] = "excluded" + file_reasons[dedup_key] = refusal + continue + if not hash_mismatch_excludes_file(verification_mode) and ( + status == "hash_mismatch" and local_path ): - zf_name = file_entry.get("zipped_file") - if zf_name and local_path: - last_result = inner_rom_check(file_entry, local_path) - zip_ok = last_result == "ok" - if zip_ok: - status = "zip_exact" - file_status.setdefault(dedup_key, "ok") - elif last_result == "not_in_zip": - file_status[dedup_key] = "excluded" - file_reasons[dedup_key] = f"{zf_name} not found inside ZIP" - elif last_result == "error": - file_status[dedup_key] = "excluded" - file_reasons[dedup_key] = "cannot read ZIP" - else: - file_status[dedup_key] = "excluded" - file_reasons[dedup_key] = ( - f"{zf_name} MD5 mismatch inside ZIP" - ) - if not zip_ok: - continue - elif frontend_digest_matches( - file_entry, local_path, digest_algorithm(verification_mode) - ): - status = "frontend_digest_exact" - file_status.setdefault(dedup_key, "ok") - else: - file_status[dedup_key] = "excluded" - file_reasons[dedup_key] = "hash mismatch" - continue - else: # Existence platforms accept any file at the declared path: # their code never reads the bytes. An upstream hash that # contradicts a local dump is reported, not acted on. - if status == "hash_mismatch" and local_path: - file_reasons.setdefault( - dedup_key, - existence_discrepancy(file_entry, local_path, platform_display), - ) - file_status.setdefault(dedup_key, "ok") + file_reasons.setdefault( + dedup_key, + existence_discrepancy(file_entry, local_path, platform_display), + ) + file_status.setdefault(dedup_key, "ok") # Emulator-level validation: informational only for platform packs. # Platform verification (existence/md5) is the authority for pack status. @@ -3558,18 +3530,17 @@ def generate_manifest( ) continue - override = slot_overrides.get(full_dest) - if override: - local_path, status = override, "slot_arbitrated" - else: - local_path, status = resolve_file( - file_entry, - db, - bios_dir, - zip_contents, - data_dir_registry=data_registry, - offline=offline, - ) + local_path, status = resolve_file( + file_entry, + db, + bios_dir, + zip_contents, + data_dir_registry=data_registry, + offline=offline, + ) + local_path, status = slots.apply_override( + slot_overrides.get(full_dest), local_path, status + ) if status == "not_found" and not reads_file_contents(verification_mode): found = agnostic_substitute(file_entry, sys_id, db, manifest_profiles) if found: @@ -3577,15 +3548,18 @@ def generate_manifest( 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", "")) - status = settle_mismatch(file_entry, local_path, status, verification_mode) - # An existence platform never reads the bytes, so a declared - # hash the local dump contradicts is not a reason to withhold - # the file. Hash platforms would reject it, so they omit it. - if status in ("not_found", "external") or ( - status == "hash_mismatch" - and hash_mismatch_excludes_file(verification_mode) - ): - record_omission(full_dest, file_entry, sys_id, status, None) + # The builder's verdict: a hash platform omits what its + # frontend would reject, a zipped_file archive included when + # the member is missing whatever the outer resolution said. + # An existence platform never reads the bytes. + status, refusal = baseline_verdict( + file_entry, local_path, status, verification_mode + ) + if refusal or status in ("not_found", "external"): + record_omission( + full_dest, file_entry, sys_id, + "hash_mismatch" if refusal else status, None, + ) continue # The variant the pack ships when an emulator rejects the # resolved file: the installer must place the same bytes. diff --git a/scripts/slots.py b/scripts/slots.py index d5ca771e..f8d9abd9 100644 --- a/scripts/slots.py +++ b/scripts/slots.py @@ -366,6 +366,20 @@ def pack_overrides( return overrides +def apply_override( + override: str | None, local_path: str | None, status: str +) -> tuple[str | None, str]: + """The file a pack serves at a destination once its slot is settled. + + An entry the platform stores outside the collection, or a release asset + fetched for its declared hash, keeps its own answer. The builder and the + install manifest both read this. + """ + if override and status not in ("external", "release_asset"): + return override, "slot_arbitrated" + return local_path, status + + def _strongest(claims: list[Claim]) -> Claim: """The profile claim with the strongest proof; the first emulator breaks ties. diff --git a/scripts/validation.py b/scripts/validation.py index 8e1a5da3..f6e7761d 100644 --- a/scripts/validation.py +++ b/scripts/validation.py @@ -721,6 +721,37 @@ def settle_mismatch( return status +def baseline_verdict( + file_entry: dict, local_path: str | None, status: str, verification_mode: str +) -> tuple[str, str | None]: + """The status a platform entry ships under, or why a hash platform refuses it. + + Batocera hashes the member, never the ZIP: a zipped_file entry is checked + inside the archive whatever the outer resolution said, as verify does. A + frontend that hashes one digest accepts a file whose other declared + hashes disagree. The pack builder and the install manifest both read + this, so the one-line installer never places a file the pack refused. + Existence platforms never read the bytes: nothing is refused here. + """ + if not hash_mismatch_excludes_file(verification_mode): + return status, None + member = file_entry.get("zipped_file") + if member and local_path: + result = inner_rom_check(file_entry, local_path) + if result == "ok": + return "zip_exact", None + if result == "not_in_zip": + return status, f"{member} not found inside ZIP" + if result == "error": + return status, "cannot read ZIP" + return status, f"{member} MD5 mismatch inside ZIP" + if status != "hash_mismatch": + return status, None + if frontend_digest_matches(file_entry, local_path, digest_algorithm(verification_mode)): + return "frontend_digest_exact", None + return status, "hash mismatch" + + def destination_owners(profiles: dict) -> dict[str, list[tuple[str, str]]]: """For each file name, the path tail each emulator reads it under.""" owners: dict[str, list[tuple[str, str]]] = {} diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index ee415721..136fdf6c 100644 --- a/tests/test_audit_regressions.py +++ b/tests/test_audit_regressions.py @@ -708,6 +708,63 @@ class ResolverRegressions(unittest.TestCase): self.assertEqual(status, "hash_mismatch") +class OneBaselineVerdict(unittest.TestCase): + """The builder and the manifest each wrote the platform baseline. + + The builder looked inside a zipped_file archive whatever the outer + resolution said; the manifest only after a hash_mismatch. An archive + found by name without the member Batocera checks left the pack and + stayed in the manifest. A slot override replaced a release asset in the + manifest and not in the pack. + """ + + def _archive(self, directory: Path, member: str) -> str: + path = directory / "dragon32.zip" + with zipfile.ZipFile(path, "w") as archive: + archive.writestr(member, b"rom") + return str(path) + + def test_a_missing_member_is_refused_whatever_the_outer_status(self): + from scripts.validation import baseline_verdict + + entry = {"name": "dragon32.zip", "zipped_file": "d32.rom", "md5": ""} + with tempfile.TemporaryDirectory(dir=TMP_ROOT) as directory: + path = self._archive(Path(directory), "other.rom") + status, refusal = baseline_verdict(entry, path, "name_exact", "md5") + self.assertEqual(refusal, "d32.rom not found inside ZIP") + # An existence frontend never opens it. + self.assertEqual( + baseline_verdict(entry, path, "name_exact", "existence"), + ("name_exact", None), + ) + + def test_a_release_asset_keeps_its_answer(self): + from scripts.slots import apply_override + + self.assertEqual( + apply_override("bios/x.bin", "/cache/big.bin", "release_asset"), + ("/cache/big.bin", "release_asset"), + ) + self.assertEqual( + apply_override("bios/x.bin", "bios/y.bin", "name_exact"), + ("bios/x.bin", "slot_arbitrated"), + ) + + def test_the_builder_and_the_manifest_read_the_same_rules(self): + source = (ROOT / "scripts" / "generate_pack.py").read_text(encoding="utf-8") + tree = ast.parse(source) + for name in ("generate_pack", "generate_manifest"): + function = next( + node for node in tree.body + if isinstance(node, ast.FunctionDef) and node.name == name + ) + body = ast.get_source_segment(source, function) or "" + with self.subTest(function=name): + self.assertIn("baseline_verdict(", body) + self.assertIn("slots.apply_override(", body) + self.assertNotIn('"slot_arbitrated"', body) + + class PackExclusionRegressions(unittest.TestCase): def test_slug_platform_core_requirement_uses_the_generated_destination(self): with tempfile.TemporaryDirectory(dir=TMP_ROOT) as directory: