From 959934d43cf82599a3228e4d897814b5a46c6f3e Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 05:18:53 +0200 Subject: [PATCH] refactor: compute the gap report once per build The region grouping and phase 2 of each pack and manifest called find_undeclared_files with the same arguments: 30 reports for the 15 manifests of --all --manifest, 2.8 s, now 15 and 1.7 s. The 15 manifests are byte-identical before and after. --- scripts/generate_pack.py | 30 +++++++++++++++++++++++++++++- scripts/packextras.py | 33 +++++++++++++++++++++++++++------ tests/test_directory_entries.py | 31 +++++++++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 7 deletions(-) diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index 8aa52869..6c2a5b51 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -546,6 +546,7 @@ def _select_variants( regions: list[str] | None, one_per_slot: bool, required_only: bool = False, + undeclared: list[dict] | None = None, ) -> tuple[set, list, list]: """Which regional and slot alternatives this pack leaves out. @@ -574,6 +575,7 @@ def _select_variants( include_all=(source == "truth"), required_only=required_only, held_out=held, + undeclared=undeclared, ) if regions: region_index = region_mod.platform_region_index( @@ -796,9 +798,20 @@ def generate_pack( # dedicated extras pass with an empty seen set: it only needs group # membership, never destinations, and a superset of members is the safe # direction. Runs only when --region is given. + # The gap report both extras passes read, computed once: the region + # grouping and the core complement below ask it the same question. + undeclared = ( + undeclared_report( + config, emulators_dir, db, emu_profiles, target_cores, + include_all=(source == "truth"), + ) + if source != "platform" and precomputed_extras is None + else None + ) region_drops, region_fallbacks, slot_undecidable = _select_variants( config, pack_systems, emulators_dir, db, base_dest, emu_profiles, target_cores, source, regions, one_per_slot, required_only, + undeclared=undeclared, ) with _writing_pack(zip_path) as zf: @@ -1060,6 +1073,7 @@ def generate_pack( _collect_emulator_extras( config, emulators_dir, db, set(), base_dest, emu_profiles, target_cores=target_cores, include_all=(source == "truth"), + undeclared=undeclared, ), system_filter, ) @@ -1073,6 +1087,7 @@ def generate_pack( emu_profiles, target_cores=target_cores, include_all=(source == "truth"), + undeclared=undeclared, ) core_count = 0 @@ -3888,6 +3903,7 @@ def _manifest_selection( source: str, required_only: bool, regions: list[str] | None, + undeclared: list[dict] | None = None, ) -> tuple[set[str], SelectionIndex]: """Destinations a region list removes, and the groups the installer replays. @@ -3909,6 +3925,7 @@ def _manifest_selection( required_only=required_only, extras_out=extras, held_out=held, + undeclared=undeclared, ) region_index = region_mod.platform_region_index( config, emu_profiles, target_cores @@ -4100,9 +4117,18 @@ def generate_manifest( ), } + # Read by the region grouping and by phase 2 with the same arguments. + undeclared = ( + undeclared_report( + config, emulators_dir, db, emu_profiles, target_cores, + include_all=(source == "truth"), + ) + if source != "platform" + else None + ) region_drops, selection = _manifest_selection( config, pack_systems, emulators_dir, db, base_dest, emu_profiles, - target_cores, source, required_only, regions, + target_cores, source, required_only, regions, undeclared=undeclared, ) # Phase 1: baseline files @@ -4296,6 +4322,7 @@ def generate_manifest( unplaceable=unplaceable, baseline_claims=baseline_claims, absent=absent, + undeclared=undeclared, ) else: core_files = [] @@ -4802,6 +4829,7 @@ from packextras import ( # noqa: E402,F401 emulator_region_drops, _emulator_region_group, expand_agnostic_entries, + undeclared_report, ) diff --git a/scripts/packextras.py b/scripts/packextras.py index e32dab34..4d68af1c 100644 --- a/scripts/packextras.py +++ b/scripts/packextras.py @@ -793,6 +793,26 @@ class ExtraPlacement: return self.slug_based and self.flat and "/" in dest +def undeclared_report( + config: dict, + emulators_dir: str, + db: dict, + emu_profiles: dict | None, + target_cores: set[str] | None = None, + include_all: bool = False, +) -> list[dict]: + """The gap report _collect_emulator_extras reads, for a caller that + collects the extras more than once with these arguments: the region + grouping and the pack or manifest phase 2 each computed it in full.""" + from verify import find_undeclared_files + + return find_undeclared_files( + config, emulators_dir, db, emu_profiles, target_cores=target_cores, + include_all=include_all, + declared_names=platform_declarations(config, db, enrich=False), + ) + + def _collect_emulator_extras( config: dict, emulators_dir: str, @@ -840,8 +860,6 @@ def _collect_emulator_extras( Works for ANY platform (RetroArch, Batocera, Recalbox, etc.) """ - from verify import find_undeclared_files - profiles = ( emu_profiles if emu_profiles is not None @@ -858,9 +876,8 @@ def _collect_emulator_extras( yaml_names = platform_declarations(config, db, enrich=False) if undeclared is None: - undeclared = find_undeclared_files( - config, emulators_dir, db, emu_profiles, target_cores=target_cores, - include_all=include_all, declared_names=yaml_names, + undeclared = undeclared_report( + config, emulators_dir, db, emu_profiles, target_cores, include_all ) # A destination two cores claim goes to the one that names its content: # hypseus-singe asks for gamecontrollerdb.txt by name alone and took the @@ -1153,6 +1170,7 @@ def platform_region_groups( required_only: bool = False, extras_out: list[dict] | None = None, held_out: set[str] | None = None, + undeclared: list[dict] | None = None, ) -> tuple[dict[str, list[tuple[str, str]]], dict[tuple[str, str, str], str]]: """Group a platform's pack candidates the way region filtering reads them. @@ -1165,7 +1183,9 @@ def platform_region_groups( withdraw from its report exactly what the builder withdraws from the pack: grouping the declared files here and the core extras there would let the two answer differently on the same request. `extras_out` receives the - core extras that were grouped, for the install manifest. + core extras that were grouped, for the install manifest. ``undeclared`` + is the gap report when the caller holds it (undeclared_report, with the + same target_cores and include_all). """ groups: dict[str, list[tuple[str, str]]] = {} for sys_id, system in systems.items(): @@ -1199,6 +1219,7 @@ def platform_region_groups( include_all=include_all, claimants=extra_dests, absent=absent, + undeclared=undeclared, ) held = {id(extra) for extra in extras} spellings = system_spellings(systems) diff --git a/tests/test_directory_entries.py b/tests/test_directory_entries.py index 1b3ce115..ef817563 100644 --- a/tests/test_directory_entries.py +++ b/tests/test_directory_entries.py @@ -14,6 +14,7 @@ import json import sys import tempfile import unittest +from unittest import mock import zipfile from pathlib import Path @@ -247,6 +248,36 @@ class PackCarriesADirectory(DirectoryFixture): self.assertTrue(self.EXPECTED <= dests, sorted(dests)) json.dumps(manifest) + def test_the_gap_report_is_computed_once_per_build(self): + """The region grouping and phase 2 each ran find_undeclared_files with + the same arguments: 30 reports for 15 manifests.""" + import verify + + real = verify.find_undeclared_files + calls: list[int] = [] + + def counted(*a, **kw): + calls.append(1) + return real(*a, **kw) + + profiles = self._profiles() + out = self.root / "counted" + out.mkdir() + with mock.patch.object(verify, "find_undeclared_files", counted): + builder.generate_manifest( + "demo", str(self.platforms), self._db(), str(self.bios), + str(self.platforms / "_registry.yml"), + emulators_dir=str(self.emulators), emu_profiles=profiles, + offline=True, + ) + self.assertEqual(len(calls), 1) + builder.generate_pack( + "demo", str(self.platforms), self._db(), str(self.bios), str(out), + emulators_dir=str(self.emulators), emu_profiles=profiles, + offline=True, regions=["north-america"], + ) + self.assertEqual(len(calls), 2) + def test_an_emulator_pack_ships_the_tree(self): self._profiles() out = self.root / "emu"