From 2a4dd59bb2680fb8ab214bdc108b9b5d223cbaab Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Sun, 6 Sep 2026 23:28:27 +0200 Subject: [PATCH] fix: narrow pack conformance by hardware target --- scripts/generate_pack.py | 84 ++++++++++++---------------------------- scripts/packverify.py | 21 ++++++++-- scripts/pipeline.py | 5 +++ tests/test_e2e.py | 63 ++++++++++++++++++------------ 4 files changed, 87 insertions(+), 86 deletions(-) diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index 97bbac07..750a66d8 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -2149,6 +2149,9 @@ def _run_verify_packs(args): emu_profiles=verify_profiles, regions=verify_regions, data_registry=verify_data_registry, + target_cores=_target_cores_for( + platform_name, args.target, args.platforms_dir + ), ) ok, errors = result[0], result[3] bl_checked, bl_present = result[4], result[5] @@ -2283,6 +2286,7 @@ def _run_platform_packs( skip_conformance=skip_conf, data_registry=data_registry, regions=getattr(args, "regions", None), + target_name=args.target, ) if args.split: for entry in os.listdir(args.output_dir): @@ -2422,12 +2426,10 @@ def main(): # Combined with --all-variants, generation runs first then verify if args.verify_packs and not args.all_variants: # This mode checks packs already on disk against the platform's own - # list. It reads the region priority above and nothing else, so a + # list, narrowed by the region priority above and by --target. A # narrowing flag it cannot honour is refused rather than dropped: a - # dropped flag answers about an artifact the caller did not name, and - # an unknown target name reads as accepted. + # dropped flag answers about an artifact the caller did not name. for flag, given in ( - ("--target", args.target), ("--one-per-slot", args.one_per_slot), ("--required-only", args.required_only), ("--source", args.source != "full"), @@ -3087,71 +3089,33 @@ def inject_manifest(zip_path: str, manifest: dict) -> None: -_TARGET_TAGS_CACHE: set[str] | None = None +def _target_cores_for( + platform_name: str, target_name: str | None, platforms_dir: str +) -> set[str] | None: + """Cores a target leaves available, or None when no target was asked for. - -def _known_target_tags(platforms_dir: str = "platforms") -> set[str]: - """Every filename tag a hardware target can put in a pack name. - - A target tag is derived from the target's own name, so unlike the other - narrowings it cannot be a literal list. + None means no narrowing, which is what an unfiltered pack expects. """ - global _TARGET_TAGS_CACHE - if _TARGET_TAGS_CACHE is not None: - return _TARGET_TAGS_CACHE - tags: set[str] = set() - targets_dir = os.path.join(platforms_dir, "targets") - if os.path.isdir(targets_dir): - for entry in sorted(os.listdir(targets_dir)): - if not entry.endswith(".yml") or entry.startswith("_"): - continue - try: - with open(os.path.join(targets_dir, entry)) as fh: - data = yaml_load(fh) or {} - except OSError as exc: - print(f"warning: cannot read {entry}: {exc}", file=sys.stderr) - continue - for name in (data.get("targets") or {}): - tags.add(f"_{_target_tag(str(name))}") - # A pack is named after what the caller typed, and an alias is a name a - # caller may type: --target switch names the pack _Switch while the target - # file only knows nintendo-switch. - overrides_path = os.path.join(targets_dir, "_overrides.yml") - if os.path.exists(overrides_path): - try: - with open(overrides_path) as fh: - overrides = yaml_load(fh) or {} - except OSError as exc: - print(f"warning: cannot read _overrides.yml: {exc}", file=sys.stderr) - overrides = {} - for platform in overrides.values(): - if not isinstance(platform, dict): - continue - for target in (platform.get("targets") or {}).values(): - if not isinstance(target, dict): - continue - for alias in target.get("aliases") or []: - tags.add(f"_{_target_tag(str(alias))}") - _TARGET_TAGS_CACHE = tags - return tags + if not target_name: + return None + cache, _kept = build_target_cores_cache( + [platform_name], target_name, platforms_dir + ) + return cache.get(platform_name) def _narrows_contents(pack_name: str) -> bool: """True when a pack holds fewer files than the platform declares. - A source-restricted, required-only or target-filtered build is narrower by - design, so the full platform expectation does not apply to it and - conformance is skipped. Without the target tag here, every targeted pack - was checked against the platform's whole system list and reported the - systems the target itself had removed as missing. - Region is not listed: the region filter is passed to the check itself. + A source-restricted or required-only build is narrower by design, so the + full platform expectation does not apply to it and conformance is skipped. + Region is not listed: the region filter is passed to the check itself, and + a hardware target is passed the same way. """ - if any( + return any( tag in pack_name for tag in ("_Platform_", "_Truth_", "_Required", "_OnePerSlot") - ): - return True - return any(f"{tag}_" in pack_name for tag in _known_target_tags()) + ) def verify_and_finalize_packs( @@ -3161,6 +3125,7 @@ def verify_and_finalize_packs( skip_conformance: bool = False, data_registry: dict | None = None, regions: list[str] | None = None, + target_name: str | None = None, ) -> bool: """Verify all packs, inject manifests, generate SHA256SUMS. @@ -3227,6 +3192,7 @@ def verify_and_finalize_packs( db=db, regions=regions, data_registry=data_registry, + target_cores=_target_cores_for(pname, target_name, platforms_dir), ) status = "OK" if p_ok else "FAILED" exclusion_note = ( diff --git a/scripts/packverify.py b/scripts/packverify.py index 97e99980..c9110668 100644 --- a/scripts/packverify.py +++ b/scripts/packverify.py @@ -19,6 +19,7 @@ from ziptools import check_inside_zip from nativemode import digest_algorithm from nativemode import hash_mismatch_excludes_file import hashlib +from common import filter_systems_by_target from common import load_emulator_profiles from common import load_platform_config import os @@ -26,6 +27,7 @@ from packextras import platform_region_groups from nativemode import reads_file_contents import region as region_mod from packresolve import resolve_file +from common import resolve_platform_cores from common import sanitize_pack_path import zipfile def verify_pack( @@ -449,11 +451,14 @@ def verify_pack_against_platform( emu_profiles: dict | None = None, regions: list[str] | None = None, data_registry: dict | None = None, + target_cores: set[str] | None = None, ) -> tuple[bool, int, int, list[str], int, int, int, int, int]: """Verify a pack ZIP against its platform config and core requirements. A region priority list narrows the expectation to what the builder would - have packed, using the same selection function. + have packed, using the same selection function. A hardware target narrows + it the same way and for the same reason: the systems it removes are not + missing from the pack, they were never asked for. Checks: 1. Every baseline file declared by the platform exists in the ZIP @@ -477,12 +482,21 @@ def verify_pack_against_platform( if emu_profiles is None: emu_profiles = load_emulator_profiles(emulators_dir) + expected_systems = config.get("systems", {}) + if target_cores is not None: + expected_systems = filter_systems_by_target( + expected_systems, + emu_profiles, + target_cores, + resolve_platform_cores(config, emu_profiles), + ) + region_drops: set[str] = set() if regions: region_index = region_mod.build_region_index(emu_profiles) region_groups, _extra_dests = platform_region_groups( config, - config.get("systems", {}), + expected_systems, emulators_dir, db, base_dest, @@ -520,7 +534,7 @@ def verify_pack_against_platform( for i in range(1, len(parts)): zip_parents.add("/".join(parts[:i])) baseline_groups: dict[str, list[dict]] = {} - for _sys_id, system in config.get("systems", {}).items(): + for _sys_id, system in expected_systems.items(): for fe in system.get("files", []): dest = sanitize_pack_path(fe.get("destination", fe.get("name", ""))) if not dest: @@ -600,6 +614,7 @@ def verify_pack_against_platform( set(), base_dest, emu_profiles, + target_cores=target_cores, ) seen_conformance: set[str] = set(zip_set) seen_parents: set[str] = set() diff --git a/scripts/pipeline.py b/scripts/pipeline.py index a4bd8eef..acd94804 100644 --- a/scripts/pipeline.py +++ b/scripts/pipeline.py @@ -532,6 +532,11 @@ def main(): ] if args.include_archived: integrity_cmd.append("--include-archived") + # Step 4 built with the target, so step 6 has to check against the + # same expectation. Without it a targeted run reported every system + # the target removed as missing from a pack that was exactly right. + if args.target: + integrity_cmd.extend(["--target", args.target]) ok, _ = run(integrity_cmd, "6/8 pack integrity") results["pack_integrity"] = ok all_ok = all_ok and ok diff --git a/tests/test_e2e.py b/tests/test_e2e.py index 2b6bd722..9bc83fa2 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -5814,13 +5814,10 @@ struct BurnDriver BurnDrvneogeo = { "refuse"), (["--platform", "retroarch", "--from-md5", "d8f1"], ["--one-per-slot"], "refuse"), - # --verify-packs returns before the argument checks run, so it used - # to accept all four of these -- an unknown target name included -- - # and answer about the pack sitting in the output directory. - (["--platform", "retroarch", "--verify-packs"], ["--target", "switch"], - "refuse"), - (["--platform", "retroarch", "--verify-packs"], ["--target", "no-such-xyz"], - "refuse"), + # --verify-packs returns before the argument checks run, so it + # used to accept these and answer about the pack sitting in the + # output directory. --target is honoured instead of refused: the + # check narrows by it. (["--platform", "retroarch", "--verify-packs"], ["--one-per-slot"], "refuse"), (["--platform", "retroarch", "--verify-packs"], ["--required-only"], @@ -5842,27 +5839,45 @@ struct BurnDriver BurnDrvneogeo = { ) self.assertIn("error:", combined) - def test_a_targeted_pack_skips_full_platform_conformance(self): - """A target-filtered pack is narrower, so the full expectation is off. + def test_conformance_narrows_by_target_like_it_does_by_region(self): + """A targeted pack is checked against the targeted expectation. - The tag was missing from the skip list, so every targeted pack was - checked against the platform's whole system list and reported the - systems the target itself had removed as missing: a correct build - exited non-zero on hundreds of files it was never asked to carry. + --target narrowed the build and nothing else: the conformance stage + held every targeted pack to the platform's whole system list and + core set, so a correct build exited non-zero on hundreds of files it + was never asked to carry. Region was already passed to the check; + target has to travel the same way, which is why the pack is not in + the skip list. """ - from generate_pack import _narrowings, _narrows_contents + import inspect + import packverify + from generate_pack import _narrows_contents, _narrowings + + source = inspect.getsource(packverify.verify_pack_against_platform) + self.assertIn( + "target_cores", + inspect.signature(packverify.verify_pack_against_platform).parameters, + "the pack verifier takes no target, so it cannot narrow by one", + ) + self.assertIn( + "filter_systems_by_target(", + source, + "the baseline expectation is not narrowed by the target", + ) + self.assertIn( + "target_cores=target_cores", + source, + "the core-extras expectation is not narrowed by the target", + ) + + # And the pack must not be skipped instead: skipping checks nothing. for target in ("nintendo-switch", "switch", "rpi4"): - with self.subTest(target=target): - applied = _narrowings("full", None, target, False, False) - self.assertEqual(len(applied), 1, target) - tag = applied[0][0] - self.assertTrue( - _narrows_contents(f"Platform_1.0{tag}_BIOS_Pack.zip"), - f"a pack built for {target} would be held to the full list", - ) - - self.assertFalse(_narrows_contents("Platform_1.0_BIOS_Pack.zip")) + tag = _narrowings("full", None, target, False, False)[0][0] + self.assertFalse( + _narrows_contents(f"Platform_1.0{tag}_BIOS_Pack.zip"), + f"a pack built for {target} would skip conformance entirely", + ) def test_standalone_mode_names_itself(self): """--emulator --standalone ships a different file set; without a tag it