From 9a0501d9d30959435117fe61c90121436857a21a Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 10:50:03 +0200 Subject: [PATCH] fix: count an uncached release asset as fetched verify resolved offline, so a release asset absent from bios/ and the cache read as missing while an online build fetched and packed it. verify now reads the database record of the registered path and counts such a declaration present, listed as fetched at build time. --offline keeps the old count, and the offline pipeline passes it to its verify step. --- scripts/packresolve.py | 64 ++++++++++++++ scripts/pipeline.py | 4 + scripts/verify.py | 53 +++++++++++- tests/test_verify_release_asset.py | 131 +++++++++++++++++++++++++++++ wiki/testing-guide.md | 2 +- wiki/tools.md | 1 + 6 files changed, 251 insertions(+), 4 deletions(-) create mode 100644 tests/test_verify_release_asset.py diff --git a/scripts/packresolve.py b/scripts/packresolve.py index 16756ab4..d21ec1f1 100644 --- a/scripts/packresolve.py +++ b/scripts/packresolve.py @@ -139,6 +139,70 @@ def _verified_asset( return None +def run_is_offline(offline: bool | None = None) -> bool: + """Whether this run fetches nothing: --offline, or RETROBIOS_OFFLINE.""" + return bool( + (_OFFLINE if offline is None else offline) + or os.environ.get("RETROBIOS_OFFLINE") + ) + + +_RECORDS_BY_PATH: list[tuple[dict, int, dict[str, dict]]] = [] + + +def _records_by_path(db: dict) -> dict[str, dict]: + """The database records by path, built once per database.""" + files = db.get("files", {}) + if _RECORDS_BY_PATH: + cached, size, index = _RECORDS_BY_PATH[0] + if cached is files and size == len(files): + return index + index = {record.get("path"): record for record in files.values()} + _RECORDS_BY_PATH[:] = [(files, len(files), index)] + return index + + +def pending_release_asset( + file_entry: dict, db: dict, registered: list[str] | None = None, +) -> str | None: + """The release asset an online build fetches for an entry, if one does. + + The registered path whose database record carries what the entry + declares, read without a download: verify never fetches, and a release + asset the cache lacks was reported missing while an online build packed + it. ``registered`` is the .gitignore registry (load_registered_paths). + """ + from largefiles import asset_candidates, asset_names, load_registered_paths + + name = file_entry.get("name", "") + if not name or name.endswith("/") or file_entry.get("type") == "directory": + return None + declared = _declared_hashes(file_entry) + if file_entry.get("unsourceable") and not any(declared.values()): + return None + if file_entry.get("zipped_file") and not any(declared.values()): + return None + if registered is None: + registered = load_registered_paths() + assets = set(asset_candidates(name, registered)) + if not assets: + return None + records = _records_by_path(db) + declared_size = file_entry.get("size") + for path, asset in sorted(asset_names(registered).items()): + record = records.get(path) + if asset not in assets or record is None: + continue + if isinstance(declared_size, int) and record.get("size") != declared_size: + continue + if all( + not values or str(record.get(field, "")).lower() in values + for field, values in declared.items() + ): + return path + return None + + # Digest length in hex digits -> kind. 29 is the MD5 Batocera writes with # its last three digits cut, matched by prefix as the resolver does. _HASH_KINDS = {8: "crc32", 29: "md5", 32: "md5", 40: "sha1", 64: "sha256"} diff --git a/scripts/pipeline.py b/scripts/pipeline.py index 9f89b4cd..cc5fa99a 100644 --- a/scripts/pipeline.py +++ b/scripts/pipeline.py @@ -873,6 +873,10 @@ def main() -> None: verify_cmd.append("--include-archived") if args.target: verify_cmd.extend(["--target", args.target]) + if args.offline: + # The packs of step 4 fetch no release asset: a missing one is + # missing on both sides. + verify_cmd.append("--offline") ok, verify_output = run(verify_cmd, STEP_LABELS["verify"]) results["verify"] = ok all_ok = all_ok and ok diff --git a/scripts/verify.py b/scripts/verify.py index 61eab4fe..5fe2688d 100644 --- a/scripts/verify.py +++ b/scripts/verify.py @@ -149,6 +149,11 @@ class Status: MISSING = "missing" +# A declaration whose file is a release asset the cache does not hold yet: +# an online build fetches it, verify reads the database record instead. +RELEASE_ASSET_PENDING = "release_asset_pending" + + class Severity: CRITICAL = "critical" # required file missing or bad hash (Recalbox RED) WARNING = "warning" # optional missing or hash mismatch (Recalbox YELLOW) @@ -1075,7 +1080,7 @@ def _resolve_for_platform( disk and the large-file cache: verify never downloads. A frontend that never reads the bytes takes the filename-free core's own image. """ - from packresolve import resolve_declaration + from packresolve import pending_release_asset, resolve_declaration, run_is_offline override = slot_overrides.get( sanitize_pack_path(file_entry.get("destination", file_entry.get("name", ""))) @@ -1084,6 +1089,12 @@ def _resolve_for_platform( file_entry, db, DEFAULT_BIOS_DIR, zip_contents, data_dir_registry, override, offline=True, ) + if status in ("not_found", "hash_mismatch") and not run_is_offline(): + # Not in the cache, and the release holds a copy that answers the + # declaration: an online build fetches and packs it (resolve_file). + asset = pending_release_asset(file_entry, db) + if asset: + return asset, RELEASE_ASSET_PENDING if status == "not_found" and not reads_file_contents(mode): found = agnostic_substitute(file_entry, sys_id, db, platform_profiles) if found: @@ -1234,7 +1245,23 @@ def verify_platform( file_entry.get("destination", file_entry.get("name", "")) ) stored = _storage_verdict(file_entry, emulator_pack=False) - if stored is not None: + if resolve_status == RELEASE_ASSET_PENDING: + # Counted as an online build counts it, which fetches the + # asset. Under --offline the builder cannot, and neither + # does this report (_resolve_for_platform). + result = { + "name": file_entry.get("name", ""), + "required": file_entry.get("required", True), + "status": Status.OK, + "release_asset": local_path, + "reason": "release asset, fetched when the pack is built", + } + packed_order.append(( + sys_id, f"{base_dest}/{destination}" if base_dest else destination, None, + )) + shipped_keys.add(_dest_key(file_entry, sys_id)) + local_path = None + elif stored is not None: result = stored local_path = None # The builder keeps the slot: instructions for a user_provided @@ -1744,6 +1771,12 @@ def print_platform_result( if missing: parts.append(f"{missing} missing") print(f"{label}: {', '.join(parts)} [{mode}]") + fetched = sorted({d["name"] for d in result["details"] if d.get("release_asset")}) + if fetched: + # Counted present: an online build downloads them. --offline counts + # them missing, as an offline build leaves them out. + shown = ", ".join(fetched[:10]) + (f" (+{len(fetched) - 10})" if len(fetched) > 10 else "") + print(f" Release assets fetched when the pack is built ({len(fetched)}): {shown}") fallbacks = result.get("region_fallbacks") or [] if fallbacks: from packreadme import _group_label @@ -2440,6 +2473,7 @@ def _refuse_listing_narrowings( ("--strict", args.strict), ("--verbose", args.verbose), ("--destinations", args.destinations), + ("--offline", args.offline), ): if on: parser.error(f"{flag} is incompatible with {listing}") @@ -2457,6 +2491,8 @@ def _refuse_mode_flags(args: argparse.Namespace, parser: argparse.ArgumentParser "--target is incompatible with --emulator and --system"), (args.destinations and not (args.platform or args.all), "--destinations requires --platform or --all"), + (args.offline and not (args.platform or args.all), + "--offline requires --platform or --all"), (args.destinations and getattr(args, "region", None), "--destinations describes the full pack and is incompatible with --region"), ): @@ -2536,6 +2572,11 @@ def _main(held: contextlib.ExitStack) -> None: help="Show emulator ground truth details", ) parser.add_argument("--json", action="store_true", help="JSON output") + parser.add_argument( + "--offline", action="store_true", + help="Report as an offline build packs: a release asset the cache " + "lacks is missing. Without it, it counts as fetched at build time", + ) parser.add_argument( "--destinations", metavar="FILE", help="With --platform or --all, also write the paths each pack holds " @@ -2552,6 +2593,10 @@ def _main(held: contextlib.ExitStack) -> None: work_from_repo_root(args, parser, ["db", "platforms_dir", "emulators_dir"]) if args.platform: require_platform(args.platform, args.platforms_dir) + if args.offline: + import packresolve + + packresolve.set_offline(True) requested_regions: list[str] = [] if getattr(args, "region", None): @@ -2696,7 +2741,9 @@ def _json_details(details: list[dict], verbose: bool) -> list[dict]: used to be accepted beside --json and dropped, the output byte for byte the same, so a script asking for the passing entries got nothing. """ - return list(details) if verbose else [d for d in details if d["status"] != Status.OK] + return list(details) if verbose else [ + d for d in details if d["status"] != Status.OK or d.get("release_asset") + ] def _exit_if_strict(strict: bool, results: list[dict]) -> None: diff --git a/tests/test_verify_release_asset.py b/tests/test_verify_release_asset.py new file mode 100644 index 00000000..f8c69c3b --- /dev/null +++ b/tests/test_verify_release_asset.py @@ -0,0 +1,131 @@ +"""verify counts a release asset the cache lacks as an online build does. + +verify never downloads. A declaration served by a release asset of the +large-files release, absent from bios/ and from .cache/large/, read as +missing there while an online build fetched it and packed it, so the two +disagreed on every checkout without the cache (CI among them). The +database record of the registered path says whether the asset answers the +declaration. Under --offline the builder fetches nothing, and verify then +counts the file missing as before. +""" + +from __future__ import annotations + +import hashlib +import os +import sys +import tempfile +import unittest +import zipfile +from pathlib import Path +from unittest import mock + +import yaml + +REPO_ROOT = Path(__file__).resolve().parent.parent +sys.path.insert(0, str(REPO_ROOT / "scripts")) + +import common # noqa: E402 +import generate_db # noqa: E402 +import generate_pack as builder # noqa: E402 +import packresolve # noqa: E402 +from verify import Status, verify_platform # noqa: E402 + +PAYLOAD = b"a firmware image the repository does not commit" +SHA1 = hashlib.sha1(PAYLOAD).hexdigest() +REGISTERED = ["bios/Big/big.bin"] + + +class ReleaseAssetNotCached(unittest.TestCase): + mode = "existence" + + def setUp(self): + self._tmp = tempfile.TemporaryDirectory(dir=REPO_ROOT / "tmp") + self._cwd = os.getcwd() + os.chdir(self._tmp.name) + for d in ("emulators", "platforms", "bios/Big"): + Path(d).mkdir(parents=True) + # Indexed by the database, gitignored, so absent from a checkout. + files = {SHA1: { + "path": REGISTERED[0], "name": "big.bin", "size": len(PAYLOAD), + "sha1": SHA1, "md5": hashlib.md5(PAYLOAD).hexdigest(), + "sha256": hashlib.sha256(PAYLOAD).hexdigest(), "crc32": "00000001", + }} + self.db = {"files": files, "indexes": generate_db.build_indexes(files, {})} + self.config = { + "platform": "Big", "verification_mode": self.mode, + "systems": {"sys": {"files": [{ + "name": "big.bin", "destination": "big.bin", "required": True, + "sha1": SHA1, "md5": hashlib.md5(PAYLOAD).hexdigest(), + }]}}, + } + Path("platforms/big.yml").write_text(yaml.dump(self.config)) + Path("platforms/_registry.yml").write_text( + yaml.dump({"platforms": {"big": {"status": "active"}}}) + ) + common._platform_config_cache.clear() + common._emulator_profiles_cache.clear() + patches = ( + mock.patch("largefiles.load_registered_paths", return_value=REGISTERED), + mock.patch.dict(os.environ, {"RETROBIOS_OFFLINE": ""}), + ) + for patch in patches: + patch.start() + self.addCleanup(patch.stop) + self.addCleanup(packresolve.set_offline, False) + packresolve.set_offline(False) + + def tearDown(self): + common._platform_config_cache.clear() + os.chdir(self._cwd) + self._tmp.cleanup() + + def _entry(self) -> dict: + report = verify_platform(self.config, self.db, "emulators", {}) + return next(d for d in report["details"] if d["name"] == "big.bin") + + def test_counted_as_fetched_when_the_pack_is_built(self): + entry = self._entry() + self.assertEqual(entry["status"], Status.OK) + self.assertEqual(entry["release_asset"], REGISTERED[0]) + + def test_offline_counts_it_missing(self): + packresolve.set_offline(True) + self.assertEqual(self._entry()["status"], Status.MISSING) + + def test_an_asset_with_other_content_is_not_counted(self): + self.config["systems"]["sys"]["files"][0]["sha1"] = "0" * 40 + self.assertEqual(self._entry()["status"], Status.MISSING) + + def test_the_online_build_packs_it(self): + cache = Path("cache/big.bin") + cache.parent.mkdir() + cache.write_bytes(PAYLOAD) + with mock.patch.object( + packresolve, "fetch_large_file", return_value=str(cache) + ): + zip_path = builder.generate_pack( + "big", "platforms", self.db, "bios", "out", + emulators_dir="emulators", emu_profiles={}, offline=False, + ) + with zipfile.ZipFile(zip_path) as zf: + self.assertEqual(zf.read("big.bin"), PAYLOAD) + self.assertEqual(self._entry()["status"], Status.OK) + + +class ReleaseAssetNotCachedMd5(ReleaseAssetNotCached): + mode = "md5" + + +class OfflinePipelineVerifiesOffline(unittest.TestCase): + """The offline pipeline builds packs without release assets: its verify + step has to count them the same way, or step 5 compares two answers.""" + + def test_verify_step_gets_offline(self): + source = (REPO_ROOT / "scripts" / "pipeline.py").read_text() + block = source[source.index("verify_cmd = ["):source.index("ok, verify_output = run(")] + self.assertIn('verify_cmd.append("--offline")', block) + + +if __name__ == "__main__": + unittest.main() diff --git a/wiki/testing-guide.md b/wiki/testing-guide.md index 1312a04b..ef2392a8 100644 --- a/wiki/testing-guide.md +++ b/wiki/testing-guide.md @@ -39,7 +39,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_core_size_refusal`, `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` | +| 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_core_size_refusal`, `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`, `test_verify_release_asset` | | 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` | diff --git a/wiki/tools.md b/wiki/tools.md index e3a61246..eec2ebfb 100644 --- a/wiki/tools.md +++ b/wiki/tools.md @@ -125,6 +125,7 @@ python scripts/verify.py --system atari-lynx # single system python scripts/verify.py --platform retroarch --target switch # filter by hardware python scripts/verify.py --platform recalbox --region us # regional priority list python scripts/verify.py --all --destinations tmp/paths.json # also write the paths each pack holds +python scripts/verify.py --all --offline # a release asset the cache lacks counts missing, as in an offline build python scripts/verify.py --list-emulators # list all emulators python scripts/verify.py --list-systems # list all systems python scripts/verify.py --platform retroarch --list-targets # list available targets