diff --git a/scripts/packextras.py b/scripts/packextras.py index 9861f3aa..66f15b63 100644 --- a/scripts/packextras.py +++ b/scripts/packextras.py @@ -216,6 +216,7 @@ def hidden_extras( report: list[dict] | None = None, resolver: ExtraResolver | None = None, placed_kind: dict[str, str] | None = None, + placed_local: dict[str, str] | None = None, ) -> dict[tuple[str, str, str], str]: """Report entries the builder holds and still leaves out, with the reason. @@ -234,7 +235,9 @@ def hidden_extras( receives the full destination of each held extra the pack carries, and ``placed_kind`` its category, carried by the extra itself: the second pass, the archive-prefix pass and the agnostic scan copy files no gap - report entry names. ``resolver`` is the builder's lookup. An extra it cannot resolve is + report entry names. ``placed_local`` receives the local copy the builder + writes at each of those destinations, for the pack integrity check. + ``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 @@ -292,6 +295,8 @@ def hidden_extras( placed.add(full) if placed_kind is not None and local: placed_kind[full] = fe.get("category", "bios") + if placed_local is not None and local: + placed_local[full] = local return hidden diff --git a/scripts/packverify.py b/scripts/packverify.py index 94c13046..a48d251f 100644 --- a/scripts/packverify.py +++ b/scripts/packverify.py @@ -10,10 +10,7 @@ from collections import Counter from auto_fetch import DEFAULT_BIOS_DIR from artifacts import _build_timestamp -from packextras import _collect_emulator_extras -from packextras import _detect_extras_prefix from packpaths import _has_path_conflict -from packpaths import _register_path from ziptools import build_zip_contents_index from ziptools import check_inside_zip from nativemode import digest_algorithm @@ -525,6 +522,81 @@ def _structural_errors(zf: zipfile.ZipFile, zip_set: set) -> list[str]: return errors +def _builder_extras( + config: dict, + emulators_dir: str, + db: dict, + emu_profiles: dict, + baseline_groups: dict[str, list[dict]], + base_dest: str, + is_flat: bool, + zip_set: set[str], + zip_lower: dict[str, str], + target_cores: set[str] | None, + region_drops: set[str], + data_registry: dict | None, +) -> dict[str, str]: + """{full destination: local copy} of each core extra the builder writes. + + The platform's paths come first, as in the builder: a destination the + pack holds whose declarations resolve to a file, or that the platform + keeps for a user_provided or external entry. One whose declarations + resolve to nothing is left free, since the builder then places a core's + file there. + """ + from packextras import ExtraResolver, hidden_extras + from packresolve import resolve_declaration + from validation import _build_validation_index, destination_owners + + shipped: dict[str, str | None] = {} + for expected, declarations in baseline_groups.items(): + if expected not in zip_set and expected.lower() not in zip_lower: + continue + unflat = ( + f"{base_dest}/{expected}" + if is_flat and base_dest and not expected.startswith(f"{base_dest}/") + else expected + ) + if any(fe.get("storage") in ("user_provided", "external") for fe in declarations): + shipped[unflat] = None + continue + local = next( + ( + found for fe in declarations + if (found := resolve_declaration( + fe, db, DEFAULT_BIOS_DIR, None, data_registry, None, offline=True, + )[0]) + ), + None, + ) + if local: + shipped[unflat] = local + platform_profiles = { + name: emu_profiles[name] for name in resolve_platform_cores(config, emu_profiles) + } + placed_local: dict[str, str] = {} + hidden_extras( + config, emulators_dir, db, emu_profiles, shipped, target_cores, + region_drops or None, + resolver=ExtraResolver( + db, DEFAULT_BIOS_DIR, build_zip_contents_index(db), data_registry, + _build_validation_index(platform_profiles), + destination_owners(platform_profiles), True, + ), + placed_local=placed_local, + ) + return placed_local + + +def _same_content(zf: zipfile.ZipFile, member: str, local: str) -> bool: + """Whether a pack member carries the bytes of a local copy, or the + members of an archive the builder normalized.""" + with open(local, "rb") as handle: + if zf.read(member) == handle.read(): + return True + return local.lower().endswith(".zip") and _same_members(zf, member, local) + + def verify_pack_against_platform( zip_path: str, platform_name: str, @@ -707,66 +779,33 @@ def verify_pack_against_platform( ): errors.append(member_errors[0]) - # 2. Core extras presence (files from emulator profiles, in repo) - # Mirror the pack builder's skip logic: only count files that - # can actually be resolved and don't have path conflicts. + # 2. Core extras: the builder's own placement replayed + # (packextras.hidden_extras, which verify reads too), after the + # platform files it wrote. Each placed extra must sit at its path + # with the bytes the builder resolved: an empty seen set and a + # presence test credited a core file at a path where the pack + # holds the platform's other dump. core_checked = 0 core_present = 0 if db is not None: - core_files = _collect_emulator_extras( - config, - emulators_dir, - db, - set(), - base_dest, - emu_profiles, - target_cores=target_cores, + placed_local = _builder_extras( + config, emulators_dir, db, emu_profiles, baseline_groups, + base_dest, is_flat, zip_set, zip_lower, target_cores, + region_drops, data_registry, ) - seen_conformance: set[str] = set(zip_set) - seen_parents: set[str] = set() - for n in zip_set: - parts = n.split("/") - for i in range(1, len(parts)): - seen_parents.add("/".join(parts[:i])) - extras_pfx = _detect_extras_prefix(config, base_dest) - for fe in core_files: - raw_dest = fe.get("destination", fe.get("name", "")) - dest = sanitize_pack_path(raw_dest) - if not dest: - continue - if region_drops and sanitize_pack_path(dest) in region_drops: - continue - # The builder's own placement: the prefix, then the flattening - # of a pack written at the BIOS root. A destination that - # already starts with the prefix (aethersx2's bios/) - # lands at the root of a flat pack, not under bios/. - if extras_pfx and not dest.startswith(f"{extras_pfx}/"): - full = f"{extras_pfx}/{dest}" - else: - full = dest - full = _flat(full, base_dest, is_flat) - # Skip path conflicts (same logic as pack builder) - if _has_path_conflict(full, seen_conformance, seen_parents): - continue - # Skip unresolvable files (game_data dirs, etc.) - local_path, status = resolve_file( - fe, - db, - "bios", - {}, - dest_hint=raw_dest, - data_dir_registry=data_registry, - offline=True, - ) - if status in ("not_found", "external", "user_provided"): - continue + for full, local in sorted(placed_local.items()): core_checked += 1 - if full in zip_set or full.lower() in zip_lower: - core_present += 1 - seen_conformance.add(full) - _register_path(full, seen_conformance, seen_parents) - else: + member = _flat(full, base_dest, is_flat) + member = member if member in zip_set else zip_lower.get(member.lower()) + if member is None: errors.append(f"core missing: {full}") + elif not _same_content(zf, member, local): + errors.append( + f"core file differs: {member} is not the {os.path.basename(local)}" + " the builder places there" + ) + else: + core_present += 1 checked = baseline_checked + core_checked present = baseline_present + core_present diff --git a/tests/test_pack_core_extras.py b/tests/test_pack_core_extras.py new file mode 100644 index 00000000..9cd50724 --- /dev/null +++ b/tests/test_pack_core_extras.py @@ -0,0 +1,91 @@ +"""The pack integrity check counts the core files the builder places. + +It rebuilt the core extras with an empty set of taken paths and tested +presence alone. On a Recalbox pack it credited 68 core files at a path the +platform's own file holds, 15 of them other bytes (fuse/trdos.rom, the PS1 +BIOS DuckStation reads), and printed a core count that contradicted the +builder's in the same run. +""" + +from __future__ import annotations + +import hashlib +import os +import sys +import tempfile +import unittest +import zipfile +from pathlib import Path + +import yaml + +REPO_ROOT = Path(__file__).resolve().parent.parent +sys.path.insert(0, str(REPO_ROOT / "scripts")) + +import packverify # noqa: E402 + + +def _held(files: dict, by_name: dict, path: str, payload: bytes) -> str: + Path(path).parent.mkdir(parents=True, exist_ok=True) + Path(path).write_bytes(payload) + sha1 = hashlib.sha1(payload).hexdigest() + name = Path(path).name + files[sha1] = {"path": path, "name": name, "sha1": sha1, + "md5": hashlib.md5(payload).hexdigest(), "size": len(payload)} + by_name.setdefault(name, []).append(sha1) + return sha1 + + +class CoreFilesAsTheBuilderPlacesThem(unittest.TestCase): + def setUp(self): + tmp = tempfile.TemporaryDirectory(dir=REPO_ROOT / "tmp") + self.addCleanup(tmp.cleanup) + previous = os.getcwd() + os.chdir(tmp.name) + self.addCleanup(os.chdir, previous) + files: dict = {} + by_name: dict = {} + _held(files, by_name, "bios/plat/trdos.rom", b"platform dump") + core_trdos = _held(files, by_name, "bios/core/trdos.rom", b"core dump") + _held(files, by_name, "bios/core/other.rom", b"core file") + self.db = {"files": files, "indexes": { + "by_name": by_name, "by_md5": {e["md5"]: s for s, e in files.items()}, + "by_path_suffix": {}, "by_crc32": {}, "by_sha256": {}}} + Path("platforms").mkdir() + config = {"platform": "P", "verification_mode": "existence", "cores": ["fuse"], + "systems": {"zx": {"files": [ + {"name": "trdos.rom", "destination": "fuse/trdos.rom"}]}}} + Path("platforms/p.yml").write_text(yaml.safe_dump(config), encoding="utf-8") + Path("emulators").mkdir() + self.profiles = {"fuse": { + "emulator": "fuse", "type": "libretro", "systems": ["zx"], "files": [ + {"name": "trdos.rom", "path": "fuse/trdos.rom", "sha1": core_trdos}, + {"name": "other.rom", "path": "core/other.rom"}, + ]}} + + def _check(self, other: bytes) -> tuple: + with zipfile.ZipFile("pack.zip", "w") as zf: + zf.writestr("fuse/trdos.rom", b"platform dump") + zf.writestr("core/other.rom", other) + return packverify.verify_pack_against_platform( + "pack.zip", "p", "platforms", self.db, emulators_dir="emulators", + emu_profiles=self.profiles, + ) + + def test_a_path_the_platform_file_holds_is_not_a_core_file(self): + ok, _checked, _present, errors, _bc, _bp, core_checked, core_present, _ex = ( + self._check(b"core file") + ) + self.assertTrue(ok, errors) + self.assertEqual((core_checked, core_present), (1, 1)) + + def test_other_bytes_at_a_core_path_are_an_error(self): + ok, _checked, _present, errors, *_rest = self._check(b"something else") + self.assertFalse(ok) + self.assertTrue( + any(e.startswith("core file differs: core/other.rom") for e in errors), errors + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/wiki/testing-guide.md b/wiki/testing-guide.md index da47320f..4f05b17a 100644 --- a/wiki/testing-guide.md +++ b/wiki/testing-guide.md @@ -38,7 +38,7 @@ Every module in `tests/`, by the area it guards. | Core regression suites | synthetic | `test_e2e`, `test_audit_regressions`, `test_tool_messages` | | File resolution and identity | synthetic + collection | `test_adler32_resolution`, `test_agnostic_scan`, `test_agnostic_scan_mode`, `test_agnostic_scope`, `test_agnostic_substitute`, `test_archive_choice`, `test_bios_companions`, `test_bundled_identity`, `test_casefold_cache`, `test_db_aliases`, `test_declared_names`, `test_derivations`, `test_directory_entries`, `test_extra_identity`, `test_foreign_tree`, `test_foreign_tree_names`, `test_frontend_digest`, `test_game_data_homonyms`, `test_hashing`, `test_identity`, `test_md5_pack_context`, `test_name_tiebreak`, `test_nec_filing`, `test_owner_resolution`, `test_path_suffix`, `test_resolution_evidence`, `test_shipped_content`, `test_twin_declarations`, `test_validated_choice`, `test_variant_choice`, `test_variant_identity`, `test_zipped_member` | | Verification, cross-reference and profile checks | synthetic + profiles | `test_crypto_aes`, `test_crypto_unique_data`, `test_crypto_verify`, `test_case_insensitive_platforms`, `test_checks_have_values`, `test_core_file_fills_declaration`, `test_cross_reference_sources`, `test_file_consumers`, `test_fileless_audit`, `test_gap_settlement`, `test_hidden_extras`, `test_launcher_layer`, `test_native_mode`, `test_no_case_collisions`, `test_path_conflict_status`, `test_path_null_copies`, `test_profile_checks`, `test_romm_hash_rule`, `test_shared_groups`, `test_standalone_mode`, `test_undeclared_dedup`, `test_unplaced_extras`, `test_validation_scope`, `test_verify_all_dir`, `test_verify_emulator_evidence`, `test_verify_listing`, `test_verify_pack_choice` | -| Packs and manifests | synthetic + real packs | `test_artifact_lock`, `test_atomic_artifacts`, `test_batocera_cores`, `test_deterministic_zip`, `test_emulator_pack_counts`, `test_finalize_pack_names`, `test_large_file_cache`, `test_libretro_layout`, `test_manifest_entry`, `test_pack_conformance_dirs`, `test_pack_counts`, `test_pack_destinations`, `test_pack_failures`, `test_pack_file_names`, `test_pack_grouping`, `test_pack_integrity`, `test_pack_member_identity`, `test_pack_readme`, `test_platform_inheritance`, `test_platform_keys`, `test_required_only_archives`, `test_required_only_twins`, `test_split_extras`, `test_split_names`, `test_split_pack`, `test_system_pack_extras`, `test_system_pack_parity`, `test_torrentzip`, `test_which_pack` | +| Packs and manifests | synthetic + real packs | `test_artifact_lock`, `test_atomic_artifacts`, `test_batocera_cores`, `test_deterministic_zip`, `test_emulator_pack_counts`, `test_finalize_pack_names`, `test_large_file_cache`, `test_libretro_layout`, `test_manifest_entry`, `test_pack_conformance_dirs`, `test_pack_core_extras`, `test_pack_counts`, `test_pack_destinations`, `test_pack_failures`, `test_pack_file_names`, `test_pack_grouping`, `test_pack_integrity`, `test_pack_member_identity`, `test_pack_readme`, `test_platform_inheritance`, `test_platform_keys`, `test_required_only_archives`, `test_required_only_twins`, `test_split_extras`, `test_split_names`, `test_split_pack`, `test_system_pack_extras`, `test_system_pack_parity`, `test_torrentzip`, `test_which_pack` | | Slots and regions | synthetic + profiles | `test_region`, `test_region_agnostic_scan`, `test_region_groups`, `test_region_held`, `test_region_platform_index`, `test_region_required_only`, `test_slot`, `test_slots`, `test_slots_shipped_file` | | Hardware targets and systems | synthetic + target files | `test_extract_targets`, `test_lakka_targets`, `test_libretro_slugs`, `test_system_ids`, `test_system_selection`, `test_target_names`, `test_target_scrapers`, `test_target_systems`, `test_target_verify` | | Installer and downloads | loopback server | `test_check_release`, `test_download`, `test_install`, `test_install_cores`, `test_install_experience`, `test_install_selection` |