diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index b41606dba..3f50f1f5f 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -133,14 +133,17 @@ class PackRecord: """A pack build kept in memory: the paths it writes, no archive. ``members`` is every path written, ``files`` the ones the build counts as - the profile's files (archives and loose files, not trees or notes). The - pipeline builds every emulator pack this way and compares ``files`` with - what verify --emulator credits, without reading or compressing a byte. + the profile's files (archives and loose files, not trees or notes), and + ``trees`` the members a platform pack takes from its data directories. + The pipeline builds every emulator pack and every narrowed platform pack + this way and compares them with what verify credits, without reading or + compressing a byte. """ def __init__(self) -> None: self.members: list[str] = [] self.files: list[str] = [] + self.trees: list[str] = [] def _add_pack_member(zf: zipfile.ZipFile, source_path: str, arcname: str) -> None: @@ -473,6 +476,8 @@ def _pack_data_directories( required_only=required_only, ): _add_pack_member(zf, src, _flat(full, base_dest, flatten)) + if isinstance(zf, PackRecord): + zf.trees.append(_flat(full, base_dest, flatten)) added += 1 return added @@ -677,6 +682,7 @@ def generate_pack( offline: bool | None = None, pack_name: str | None = None, no_game_data: bool = False, + record: PackRecord | None = None, ) -> str | None: """Generate a ZIP pack for a platform. @@ -687,6 +693,9 @@ def generate_pack( its group, and a name built from every system of a hundred-system group is longer than a file name can be. + ``record`` builds the pack in memory (PackRecord): no archive is written, + no external file fetched, and the paths are kept on it. + Returns the path to the generated ZIP, or None on failure. """ config = load_platform_config(platform_name, platforms_dir) @@ -719,7 +728,8 @@ def generate_pack( platform_name, platforms_dir, narrowings, system_filter, pack_name ), ) - os.makedirs(output_dir, exist_ok=True) + if record is None: + os.makedirs(output_dir, exist_ok=True) # Case-insensitive dedup only for platforms targeting Windows/macOS. # Linux-only platforms (Batocera, Recalbox, RetroDECK, Lakka, RomM) @@ -806,7 +816,10 @@ def generate_pack( undeclared=undeclared, ) - with _writing_pack(zip_path) as zf: + with ( + contextlib.nullcontext(record) if record is not None + else _writing_pack(zip_path) + ) as zf: if source != "truth": for sys_id, system in sorted(pack_systems.items()): for file_entry in system.get("files", []): @@ -927,7 +940,7 @@ def generate_pack( if case_insensitive: seen_lower.add(dedup_key.lower()) try: - if download_external(file_entry, tmp_path): + if record is None and download_external(file_entry, tmp_path): extract = file_entry.get("extract", False) if extract and tmp_path.endswith(".zip"): _extract_zip_to_archive(tmp_path, _flat(full_dest, base_dest, flatten), zf) @@ -1245,6 +1258,13 @@ def _extract_zip_to_archive( ) -> None: """Extract contents of a source ZIP into target ZIP under dest_prefix.""" with zipfile.ZipFile(source_zip, "r") as src: + if isinstance(target_zf, PackRecord): + target_zf.members.extend( + f"{dest_prefix}/{name}" if dest_prefix else name + for name in map(sanitize_pack_path, src.namelist()) + if name and not name.endswith("/") + ) + return for info in src.infolist(): if info.is_dir(): continue diff --git a/scripts/pipeline.py b/scripts/pipeline.py index e4300b034..27ca67df9 100644 --- a/scripts/pipeline.py +++ b/scripts/pipeline.py @@ -22,7 +22,8 @@ Steps, numbered as the run prints them: 4c --manifest-targets per-target core lists 4d bundled_only.py --strict copies only an emulator install provides 4e generate_games_index.py install/games/: what the games-on-card mode reads - 5 consistency check verify == pack: counts, paths, emulator and system packs + 5 consistency check verify == pack: counts, paths, emulator and system packs, + narrowed platform packs (see check_narrowed_packs) 6 --verify-packs pack integrity, extraction and hashes 7 generate_readme.py README.md and CONTRIBUTING.md generate_installer_doc.py run samples and tables of wiki/installer.md @@ -609,6 +610,165 @@ def check_emulator_packs( return all_ok and systems_ok +# How each _narrowings() dimension is compared by check_narrowed_packs, or why +# it is not. A test reads the builder's signature against these two tables, so +# a new narrowing fails it until it is compared or a reason is written here. +NARROWINGS_COMPARED = frozenset({"regions", "target_name", "required_only", "no_game_data"}) +NARROWINGS_NOT_COMPARED = { + "standalone": "an emulator pack mode, compared by check_emulator_packs", + "source": "verify has no platform-only or truth-only report to compare with", + "one_per_slot": "verify has no slot choice to compare with", +} + + +def _matrix_coverage(full: bool, count: int) -> range | list[int]: + """Which of *count* variants of one dimension a run compares. + + The full matrix takes every one. The default takes the first, the last + and one in between, which reaches each code path the dimension takes + (an empty selection, a single system, a large one) without its cost. + """ + if full or count <= 3: + return range(count) + return sorted({0, count // 2, count - 1}) + + +def narrowed_variants( + platform: str, config: dict, profiles: dict, platforms_dir: str, full: bool, +) -> list[tuple[str, dict]]: + """(label, builder arguments) of each narrowed pack compared for a platform. + + Every region a file of the platform's cores declares, every hardware + target, every system, --no-game-data and --required-only. Without + *full*, regions, targets and systems are sampled (_matrix_coverage). + """ + import region as region_mod # noqa: PLC0415 + from common import list_available_targets # noqa: PLC0415 + + regions = sorted({ + r for entry in region_mod.platform_region_index(config, profiles).values() + for r in entry["regions"] + }) + targets = [t["name"] for t in list_available_targets(platform, platforms_dir)] + systems = sorted(config.get("systems") or {}) + variants: list[tuple[str, dict]] = [ + ("--required-only", {"required_only": True}), + ("--no-game-data", {"no_game_data": True}), + ] + variants += [ + (f"--region {regions[i]}", {"regions": [regions[i]]}) + for i in _matrix_coverage(full, len(regions)) + ] + variants += [ + (f"--target {targets[i]}", {"target_name": targets[i]}) + for i in _matrix_coverage(full, len(targets)) + ] + variants += [ + (f"--system {systems[i]}", {"system_filter": [systems[i]]}) + for i in _matrix_coverage(full, len(systems)) + ] + return variants + + +def check_narrowed_packs( + emulators_dir: str = "emulators", platforms_dir: str = "platforms", + db_path: str = "database.json", include_archived: bool = False, + full: bool = False, +) -> bool: + """Build each narrowed platform pack in memory and compare it with verify. + + The paths the pack writes against the ones verify credits for the same + narrowing, as check_pack_destinations compares the full packs. Both sides + run offline: a release asset the cache lacks is missing on each. + + Coverage: --required-only and --no-game-data for every platform, and + --region, --target and --system. Measured on 2026-11-05, the full matrix + is 1,150 packs and took 25 minutes, one to two seconds a pack, most + of it platform-wide work (slot overrides, the gap report) that each build + redoes. A run therefore takes the first, middle and last region, target + and system of each platform, 155 packs in about 3 minutes: each value of + a dimension runs the same code, so three reach every path a narrowing + takes (the empty end, one system, a large one). *full* (--full-narrowings) + compares every value. NARROWINGS_NOT_COMPARED says why --source and + --one-per-slot are not compared. + """ + import generate_pack # noqa: PLC0415 + import packresolve # noqa: PLC0415 + import verify # noqa: PLC0415 + from common import ( # noqa: PLC0415 + build_zip_contents_index, + group_identical_platforms, + list_registered_platforms, + load_database, + load_emulator_profiles, + load_platform_config, + load_target_config, + ) + + print(f"\n--- {step_title(STEP_LABELS['consistency'])}, narrowed packs ---") + db = load_database(db_path) + zip_contents = build_zip_contents_index(db) + profiles = load_emulator_profiles(emulators_dir) + registry = load_data_dir_registry(platforms_dir) + groups = group_identical_platforms( + list_registered_platforms(platforms_dir, include_archived=include_archived), + platforms_dir, + ) + was_offline = packresolve._OFFLINE + packresolve.set_offline(True) + all_ok = True + compared = 0 + try: + for _members, platform in groups: + config = load_platform_config(platform, platforms_dir) + for label, narrowing in narrowed_variants( + platform, config, profiles, platforms_dir, full, + ): + target = narrowing.get("target_name") + target_cores = ( + load_target_config(platform, target, platforms_dir) if target else None + ) + record = generate_pack.PackRecord() + with contextlib.redirect_stdout(io.StringIO()), \ + contextlib.redirect_stderr(io.StringIO()): + built = generate_pack.generate_pack( + platform, platforms_dir, db, "bios", "", + emulators_dir=emulators_dir, zip_contents=zip_contents, + data_registry=registry, emu_profiles=profiles, + target_cores=target_cores, offline=True, record=record, + **narrowing, + ) + report = verify.verify_platform( + load_platform_config(platform, platforms_dir), db, + emulators_dir, profiles, target_cores=target_cores, + data_dir_registry=registry, + **{k: v for k, v in narrowing.items() if k != "target_name"}, + ) if built else None + compared += 1 + name = f"{platform} {label}" + if report is None: + # The builder declines a system no core of the platform runs. + print(f" {name}: the builder wrote no pack") + all_ok = False + continue + credited = set(report["pack_destinations"]) + trees = set(record.trees) - credited + written = { + m for m in record.members + if not _PACK_NOTES.search(m) and m not in trees + } + lines = _set_difference_lines(name, credited, written) + if lines: + print("\n".join(lines)) + all_ok = False + finally: + packresolve.set_offline(was_offline) + scope = "full matrix" if full else "sampled" + print(f" {compared} narrowed platform packs compared ({scope}), " + f"{'all equal' if all_ok else 'differences above'}") + return all_ok + + class _Skipped: """A step that did not run. Truthy, so it never fails the pipeline, and distinct, so the summary does not report it as work that was done.""" @@ -629,7 +789,8 @@ def _refuse_unapplied_flags( """A flag the run would drop is refused before any step. --all-variants builds every source itself, so a --source beside it is - dropped; --skip-packs builds none, so both are. + dropped; --skip-packs builds none, so both are, and step 5 does not run + for --full-narrowings to widen. """ # --source defaults to None so a typed `--source full` is seen: compared # with its default value, it went through beside the modes that refuse it. @@ -638,7 +799,8 @@ def _refuse_unapplied_flags( parser.error("--source is incompatible with --all-variants") if args.skip_packs: for flag, on in (("--source", source_given), - ("--all-variants", args.all_variants)): + ("--all-variants", args.all_variants), + ("--full-narrowings", args.full_narrowings)): if on: parser.error(f"{flag} is incompatible with --skip-packs") if args.source is None: @@ -768,6 +930,11 @@ def main() -> None: help="Build the six source x required packs of each platform; " "not with --source or --skip-packs", ) + parser.add_argument( + "--full-narrowings", action="store_true", + help="Step 5 compares every region, target and system of each " + "platform, not a sample of three (about 25 minutes)", + ) parser.add_argument( "--check-buildbot", action="store_true", @@ -1136,6 +1303,9 @@ def main() -> None: # Counts can agree while the paths differ, both ways at once. ok = check_pack_destinations(destinations_file, pack_output) and ok ok = check_emulator_packs() and ok + ok = check_narrowed_packs( + include_archived=args.include_archived, full=args.full_narrowings, + ) and ok print( f"--- {step_title(STEP_LABELS['consistency'])}, counts and paths: " f"{'OK' if ok else 'FAILED'} ---" diff --git a/scripts/verify.py b/scripts/verify.py index f9e47d8c6..147a8f80d 100644 --- a/scripts/verify.py +++ b/scripts/verify.py @@ -948,6 +948,7 @@ def _mark_hidden( resolver: object | None = None, placed_kind: dict[str, str] | None = None, system_ids: list[str] | None = None, + required_only: bool = False, ) -> set[str]: """What the builder holds but cannot place beside the files it packed. @@ -963,7 +964,7 @@ def _mark_hidden( emu_profiles if emu_profiles is not None else load_emulator_profiles(emulators_dir), shipped, target_cores, region_drops, placed, report, resolver, placed_kind, - system_ids=system_ids, + system_ids=system_ids, required_only=required_only, ) for u in undeclared: reason = hidden.get( @@ -1146,6 +1147,7 @@ def verify_platform( regions: list[str] | None = None, system_filter: list[str] | None = None, no_game_data: bool = False, + required_only: bool = False, ) -> dict: """Verify all BIOS files for a platform, including cross-reference gaps. @@ -1156,6 +1158,9 @@ def verify_platform( same list would carry, using the same selection function as the builder. ``system_filter`` does the same for a pack built with --system: the systems it keeps and the core extras those systems own. + ``required_only`` reports on the pack built with --required-only: the + declarations, trees and core extras marked ``required: false`` are left + out before any of them takes a path, as the builder leaves them out. """ left_out: list[str] = [] if no_game_data: @@ -1221,6 +1226,16 @@ def verify_platform( validation_owners = destination_owners(platform_profiles) verify_systems = platform_scope_systems(config, profiles, target_cores, system_filter) + if required_only: + verify_systems = { + sys_id: { + **system, + "files": [ + fe for fe in system.get("files", []) if fe.get("required") is not False + ], + } + for sys_id, system in verify_systems.items() + } details = [] file_status: dict[str, str] = {} @@ -1252,6 +1267,7 @@ def verify_platform( config.get("base_destination", ""), profiles, target_cores=target_cores, + required_only=required_only, held_out=held, ) region_index = region_mod.platform_region_index(config, profiles, target_cores) @@ -1445,6 +1461,8 @@ def verify_platform( from packextras import report_owned_by_systems undeclared = report_owned_by_systems(undeclared, system_filter) + if required_only: + undeclared = [u for u in undeclared if u.get("required") is not False] if region_drops: undeclared = [ u @@ -1467,6 +1485,7 @@ def verify_platform( ), placed_kind, system_filter, + required_only, ) # What the builder places from the cores, one per pack path, by category: # several entries claim one path (rom.key from three Amiga cores) and the diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index 94317c432..294adfe62 100644 --- a/tests/test_audit_regressions.py +++ b/tests/test_audit_regressions.py @@ -4141,6 +4141,7 @@ class EveryToolRefusesAFlagItWouldDrop(unittest.TestCase): "scripts/pipeline.py": [ ["--offline", "--skip-packs", "--source", "truth"], ["--offline", "--skip-packs", "--all-variants"], + ["--offline", "--skip-packs", "--full-narrowings"], ["--offline", "--all-variants", "--source", "platform"], ["--offline", "--all-variants", "--source", "full"], ["--offline", "--skip-packs", "--source", "full"], diff --git a/tests/test_pack_destinations.py b/tests/test_pack_destinations.py index 0cbfb8adb..6d15a718f 100644 --- a/tests/test_pack_destinations.py +++ b/tests/test_pack_destinations.py @@ -122,6 +122,95 @@ class AnEntryWithoutAFileTakesNoPath(unittest.TestCase): self.assertEqual(set(occupied), {"bios/a.bin"}) +class ARequiredOnlyPack(_Fixture): + """verify had no --required-only: nothing compared the pack to a report.""" + + def test_verify_credits_what_the_pack_writes(self): + from common import load_emulator_profiles, load_platform_config + from generate_pack import PackRecord, generate_pack + from verify import verify_platform + + (self.emulators / "coreemu.yml").write_text(yaml.safe_dump({ + "emulator": "CoreEmu", "type": "libretro", "systems": ["sys"], + "files": [{"name": "core.bin", "required": False}], + })) + self._platform( + [{"name": "sgb.sfc", "destination": "SGB1.sfc"}, + {"name": "program.rom", "destination": "program.rom", "required": False}], + cores=["coreemu"], + ) + profiles = load_emulator_profiles(str(self.emulators)) + record = PackRecord() + with contextlib.redirect_stdout(io.StringIO()): + zip_path = generate_pack( + "paths", str(self.platforms), self.db, str(self.bios), + str(self.root / "out"), emulators_dir=str(self.emulators), + emu_profiles=profiles, offline=True, required_only=True, + ) + generate_pack( + "paths", str(self.platforms), self.db, str(self.bios), "", + emulators_dir=str(self.emulators), emu_profiles=profiles, + offline=True, required_only=True, record=record, + ) + with zipfile.ZipFile(zip_path) as zf: + members = set(zf.namelist()) + # The record names what the archive holds, nothing more or less. + self.assertEqual(set(record.members), members) + report = verify_platform( + load_platform_config("paths", str(self.platforms)), self.db, + str(self.emulators), profiles, required_only=True, + ) + self.assertEqual(set(report["pack_destinations"]), {"SGB1.sfc"}) + self.assertEqual(members - {"README.txt"}, {"SGB1.sfc"}) + + +class EveryNarrowingIsCompared(unittest.TestCase): + """A way to narrow a pack that step 5 does not build is a way for the pack + and verify to drift with nothing to say so.""" + + def test_each_dimension_of_the_name_builder_is_accounted_for(self): + import inspect + + import pipeline + from generate_pack import _narrowings + + dimensions = set(inspect.signature(_narrowings).parameters) + compared = set(pipeline.NARROWINGS_COMPARED) + exempt = set(pipeline.NARROWINGS_NOT_COMPARED) + self.assertFalse(compared & exempt) + self.assertEqual(dimensions, compared | exempt, + "compare the new narrowing in check_narrowed_packs, " + "or write in NARROWINGS_NOT_COMPARED why it is not") + + def test_the_variants_reach_every_compared_dimension(self): + import pipeline + from common import load_emulator_profiles, load_platform_config + + platforms = str(REPO_ROOT / "platforms") + config = load_platform_config("retroarch", platforms) + profiles = load_emulator_profiles(str(REPO_ROOT / "emulators")) + for full in (False, True): + with self.subTest(full=full): + variants = pipeline.narrowed_variants( + "retroarch", config, profiles, platforms, full, + ) + used = {key for _label, narrowing in variants for key in narrowing} + self.assertEqual(used, pipeline.NARROWINGS_COMPARED | {"system_filter"}) + systems = [n for _l, n in variants if "system_filter" in n] + self.assertEqual(len(systems), len(config["systems"])) + + def test_the_sample_takes_both_ends_and_the_middle(self): + import pipeline + + self.assertEqual(list(pipeline._matrix_coverage(False, 2)), [0, 1]) + self.assertEqual(pipeline._matrix_coverage(False, 11), [0, 5, 10]) + self.assertEqual(list(pipeline._matrix_coverage(True, 11)), list(range(11))) + + def test_the_run_compares_them(self): + source = (REPO_ROOT / "scripts" / "pipeline.py").read_text(encoding="utf-8") + self.assertIn("ok = check_narrowed_packs(", source) + + class ThePipelineComparesPaths(unittest.TestCase): """Equal counts, other paths: the comparison fails and names both sides.""" diff --git a/wiki/tools.md b/wiki/tools.md index 5e4359e12..43d0f33ba 100644 --- a/wiki/tools.md +++ b/wiki/tools.md @@ -52,7 +52,7 @@ Pipeline steps: | 4c | Generate target manifests | `--skip-packs` | | 4d | Collected copies only an emulator install provides (`bundled_only.py --strict`, gates the run) | - | | 4e | Games index: what the installer reads for the games on a card (`generate_games_index.py`, fails under its coverage floor) | - (reads the committed install lists) | -| 5 | Consistency check: counts, paths per archive, every emulator and system pack | `--skip-packs`, or no full pack built (`--source platform` or `truth`) | +| 5 | Consistency check: counts, paths per archive, every emulator and system pack, the narrowed platform packs | `--skip-packs`, or no full pack built (`--source platform` or `truth`) | | 6 | Pack integrity (extract + hash) | `--skip-packs`, or no full pack built | | 7 | Generate README and CONTRIBUTING, then the installer page's tables and run samples (`generate_installer_doc.py`) | `--skip-docs` | | 8 | Generate site | `--skip-docs` | @@ -69,6 +69,7 @@ Pipeline flags: | `--target TARGET` | filter verify, packs and truth by hardware target | | `--source {platform,truth,full}` | pack file source, passed through to `generate_pack.py`. Refused with `--skip-packs` | | `--all-variants` | build the 6 source x required combinations. Refused with `--skip-packs` and with `--source` | +| `--full-narrowings` | step 5 compares every region, target and system of each platform instead of three of each. Refused with `--skip-packs` | | `--check-buildbot` | run step 2b. Additive, and ignored under `--offline` | | `--with-truth` | run steps 2c and 2d | | `--with-export` | run steps 2c, 2d and 2e | @@ -95,6 +96,16 @@ alone, run `python scripts/check_buildbot_system.py`. profile in each build, built in memory against `verify --emulator`, and every system pack that gathers several profiles against `verify --system`. Equal counts once hid four RetroArch paths credited and never written. +- **Each narrowed platform pack is compared too.** Step 5 builds in memory + the `--required-only` and `--no-game-data` pack of every platform, and its + `--region`, `--target` and `--system` packs, against verify with the same + narrowing. The full matrix is 1,150 packs and took 25 minutes on 2026-11-05, + so a run compares the first, the middle and the last region, target and + system of each platform (155 packs, about 3 minutes), which reaches every + code path each narrowing takes. `--full-narrowings` compares them all. + `--source` and `--one-per-slot` are not compared: verify has no report of + a platform-only, truth-only or one-per-slot pack. A test fails when a new + narrowing reaches the pack name without being compared or exempted. - **Generators write through `write_if_changed()`.** A new timestamp alone rewrites nothing, so a diff on a generated file always means content. - **One run per output directory.** Pack generation takes a lock on