diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index 56975f37..fa5533ff 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -2392,14 +2392,7 @@ def _run_manifest_mode( with open(registry_path) as _rf: registry = yaml_load(_rf) or {} - if args.all_variants: - variants = [ - ("full", False), ("full", True), - ("platform", False), ("platform", True), - ("truth", False), ("truth", True), - ] - else: - variants = [(args.source, args.required_only)] + variants = ALL_VARIANTS if args.all_variants else ((args.source, args.required_only),) failed: list[str] = [] for source, required_only in variants: @@ -2516,8 +2509,14 @@ def _run_verify_packs(args: argparse.Namespace) -> None: verify_regions = getattr(args, "regions", None) verify_profiles = load_emulator_profiles(args.emulators_dir) verify_data_registry = load_data_dir_registry(args.platforms_dir) + # After an --all-variants build every variant is named, so none of the + # six packs the run wrote passes unmentioned. expected = _expected_pack_names( - platforms, args.platforms_dir, verify_regions, args.target + platforms, + args.platforms_dir, + verify_regions, + args.target, + ALL_VARIANTS if args.all_variants else (("full", False),), ) on_disk = ( set(os.listdir(args.output_dir)) if os.path.isdir(args.output_dir) else set() @@ -2544,6 +2543,16 @@ def _run_verify_packs(args: argparse.Namespace) -> None: # A group pack and a pack built for the platform alone can both be # present, and each one is a pack of this platform. for pack_name in found: + if _narrows_contents(pack_name): + # Only reached after an --all-variants build, whose stage 1 + # re-hashed every member against the collection above. The + # platform's list describes the full pack, so a narrowed one + # is not compared to it. + print( + f" {platform_name} [{pack_name}]: hashes checked during the build; " + "narrowed by design, so not compared to the platform's full list" + ) + continue zip_path = os.path.join(args.output_dir, pack_name) result = verify_pack_against_platform( zip_path, @@ -2582,6 +2591,14 @@ def _run_verify_packs(args: argparse.Namespace) -> None: sys.exit(1) +# The source x required combinations --all-variants builds, in build order. +ALL_VARIANTS: tuple[tuple[str, bool], ...] = ( + ("full", False), ("full", True), + ("platform", False), ("platform", True), + ("truth", False), ("truth", True), +) + + def _pack_label(source: str, required_only: bool) -> str: """The variant tag pipeline.parse_pack_counts reads after a pack name.""" return f"[source={source}, required]" if required_only else f"[source={source}]" @@ -2640,14 +2657,7 @@ def _run_platform_packs( system_filter: list[str] | None, ) -> None: """Generate ZIP packs for platform groups and verify.""" - if args.all_variants: - variants = [ - ("full", False), ("full", True), - ("platform", False), ("platform", True), - ("truth", False), ("truth", True), - ] - else: - variants = [(args.source, args.required_only)] + variants = ALL_VARIANTS if args.all_variants else ((args.source, args.required_only),) failed: list[str] = [] for source, required_only in variants: @@ -4485,22 +4495,31 @@ def _expected_pack_names( platforms_dir: str, regions: list[str] | None, target_name: str | None, + variants: tuple[tuple[str, bool], ...] = (("full", False),), ) -> dict[str, set[str]]: """Exact pack file names --verify-packs may check, per platform. Rebuilt from the name builder, never matched by substring: a substring picks the full pack when a regional or targeted one was asked for, or a narrowed pack when the full one was. A platform answers to its own name - and to the name of the group it shares a pack with. + and to the name of the group it shares a pack with. ``variants`` lists + the (source, required_only) combinations the run built. """ - def narrow_tags(platform_name: str) -> str: + def narrow_tags(platform_name: str) -> list[str]: named = _platform_target(platform_name, target_name, platforms_dir) - return "".join( - tag for tag, _label in _narrowings("full", regions, named, False, False) - ) + return [ + "".join( + tag + for tag, _label in _narrowings(source, regions, named, False, required) + ) + for source, required in variants + ] names: dict[str, set[str]] = { - p: {f"{_platform_pack_stem([p], p, platforms_dir)}{narrow_tags(p)}_BIOS_Pack.zip"} + p: { + f"{_platform_pack_stem([p], p, platforms_dir)}{tags}_BIOS_Pack.zip" + for tags in narrow_tags(p) + } for p in platforms } # Groups depend on which platforms a run built: --all alone or with @@ -4523,13 +4542,11 @@ def _expected_pack_names( ): if len(group) < 2: continue - name = ( - f"{_platform_pack_stem(group, representative, platforms_dir)}" - f"{narrow_tags(representative)}_BIOS_Pack.zip" - ) - for p in group: - if p in names: - names[p].add(name) + stem = _platform_pack_stem(group, representative, platforms_dir) + for tags in narrow_tags(representative): + for p in group: + if p in names: + names[p].add(f"{stem}{tags}_BIOS_Pack.zip") return names diff --git a/tests/test_verify_pack_choice.py b/tests/test_verify_pack_choice.py index 1898de8a..0de0a352 100644 --- a/tests/test_verify_pack_choice.py +++ b/tests/test_verify_pack_choice.py @@ -53,6 +53,7 @@ class VerifyPackChoice(unittest.TestCase): "output_dir": str(self.out), "target": None, "regions": [], + "all_variants": False, } values.update(overrides) return argparse.Namespace(**values) @@ -93,6 +94,27 @@ class VerifyPackChoice(unittest.TestCase): self._run(self._args()) self.assertEqual(ctx.exception.code, 1) + def test_all_variants_names_every_variant_it_built(self): + """After --all-variants the check opened the full pack only: the five + narrowed packs the same run wrote passed without a line.""" + import contextlib + import io + + names = [] + for source, required in self.gp.ALL_VARIANTS: + tags = "".join( + tag for tag, _ in self.gp._narrowings(source, None, None, False, required) + ) + names.append(f"{self.stem}{tags}_BIOS_Pack.zip") + self._touch(names[-1]) + out = io.StringIO() + with contextlib.redirect_stdout(out): + checked = self._run(self._args(all_variants=True)) + self.assertEqual(checked, [("retroarch", f"{self.stem}_BIOS_Pack.zip")]) + printed = out.getvalue() + for name in names[1:]: + self.assertIn(f"[{name}]: hashes checked during the build", printed) + def test_include_archived_reaches_the_archived_pack(self): stem = self.gp._platform_pack_stem(["retropie"], "retropie", "platforms") self._touch(f"{stem}_BIOS_Pack.zip") diff --git a/wiki/tools.md b/wiki/tools.md index 8164c5d4..ebe0395e 100644 --- a/wiki/tools.md +++ b/wiki/tools.md @@ -229,7 +229,9 @@ would have loaded. - `--db`, `--platforms-dir`, `--emulators-dir`: read another database or source tree instead of the repository's - `--source {platform,truth,full}`: select file source (platform YAML only, emulator profiles only, or both) -- `--all-variants`: generate all 6 combinations of source x required_only +- `--all-variants`: generate all 6 combinations of source x required_only. With + `--verify-packs`, the full pack is compared to the platform's list and each + narrowed variant is named with the hash check the build ran on its members - `--refresh-data`: force re-download all data directories before packing - `--no-refresh-data`: pack the data directories as cached, still fetching release assets (what `pipeline.py` does after its own refresh step)