diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index b5f7a1a6..55c40deb 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -316,6 +316,7 @@ def _narrowings( target_name: str | None, one_per_slot: bool, required_only: bool, + standalone: bool = False, ) -> list[tuple[str, str]]: """Every dimension that narrows a pack, as (filename tag, plain label). @@ -342,6 +343,8 @@ def _narrowings( ) if one_per_slot: applied.append(("_OnePerSlot", "one BIOS per system and region")) + if standalone: + applied.append(("_Standalone", "standalone mode files only")) if required_only: applied.append(("_Required", "required files only")) return applied @@ -2168,8 +2171,13 @@ def generate_emulator_pack( # ZIP naming display_names = [p.get("emulator", n).replace(" ", "") for n, p in selected] - region_tag_str = f"_{region_mod.region_tag(regions)}" if regions else "" - zip_name = "_".join(display_names) + f"{region_tag_str}_BIOS_Pack.zip" + narrow_tags = "".join( + tag + for tag, _label in _narrowings( + "full", regions, None, False, required_only, standalone + ) + ) + zip_name = "_".join(display_names) + f"{narrow_tags}_BIOS_Pack.zip" zip_path = os.path.join(output_dir, zip_name) os.makedirs(output_dir, exist_ok=True) @@ -2532,8 +2540,17 @@ def generate_split_packs( """Generate split packs (one ZIP per system or manufacturer).""" config = load_platform_config(platform_name, platforms_dir) platform_display = config.get("platform", platform_name) - source_tag = {"platform": "_Platform", "truth": "_Truth"}.get(source, "") - split_dir = os.path.join(output_dir, f"{platform_display.replace(' ', '_')}{source_tag}_Split") + # The split directory carries the same tags as the packs inside it, or two + # differently narrowed runs write into one another. + split_tags = "".join( + tag + for tag, _label in _narrowings( + source, regions, target_name, one_per_slot, required_only + ) + ) + split_dir = os.path.join( + output_dir, f"{platform_display.replace(' ', '_')}{split_tags}_Split" + ) os.makedirs(split_dir, exist_ok=True) systems = config.get("systems", {}) @@ -2929,19 +2946,19 @@ def _run_manifest_mode( target_name=args.target, offline=args.offline, ) - source_suffix = {"platform": "_platform", "truth": "_truth"}.get(source, "") - req_suffix = "_required" if required_only else "" - rgn = getattr(args, "regions", None) - region_suffix = ( - f"_{region_mod.region_tag(rgn).lower()}" if rgn else "" - ) - target_suffix = ( - f"_{_target_tag(args.target).lower()}" if args.target else "" + narrow_suffix = "".join( + tag.lower() + for tag, _label in _narrowings( + source, + getattr(args, "regions", None), + args.target, + False, + required_only, + ) ) out_path = os.path.join( args.output_dir, - f"{representative}{source_suffix}{region_suffix}" - f"{target_suffix}{req_suffix}.json", + f"{representative}{narrow_suffix}.json", ) _write_manifest_if_changed(out_path, manifest) print( @@ -2954,8 +2971,7 @@ def _run_manifest_mode( if alias_plat != representative: alias_path = os.path.join( args.output_dir, - f"{alias_plat}{source_suffix}{region_suffix}" - f"{target_suffix}{req_suffix}.json", + f"{alias_plat}{narrow_suffix}.json", ) alias_manifest = dict(manifest) alias_manifest["platform"] = alias_plat @@ -3141,17 +3157,19 @@ def _run_platform_packs( load_platform_config(p, args.platforms_dir).get("platform", p) for p in group_platforms ] - source_tag = {"platform": "_Platform", "truth": "_Truth"}.get(source, "") - region_values = getattr(args, "regions", None) - rgn_tag = ( - f"_{region_mod.region_tag(region_values)}" - if region_values - else "" + narrow_tags = "".join( + tag + for tag, _label in _narrowings( + source, + getattr(args, "regions", None), + args.target, + args.one_per_slot, + required_only, + ) ) - req_tag = "_Required" if required_only else "" combined = ( "_".join(n.replace(" ", "") for n in all_names) - + f"{ver_tag}{source_tag}{rgn_tag}{req_tag}_BIOS_Pack.zip" + + f"{ver_tag}{narrow_tags}_BIOS_Pack.zip" ) new_path = os.path.join(os.path.dirname(zip_path), combined) if new_path != zip_path: @@ -3301,6 +3319,8 @@ def main(): parser.error(str(exc)) if args.manifest_targets: parser.error("--region is incompatible with --manifest-targets") + if args.one_per_slot and args.manifest_targets: + parser.error("--one-per-slot is incompatible with --manifest-targets") # Quick-exit modes: --verify-packs alone = verify existing packs only # Combined with --all-variants, generation runs first then verify diff --git a/tests/test_e2e.py b/tests/test_e2e.py index 2935f0c5..66ea49b9 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -5714,6 +5714,66 @@ struct BurnDriver BurnDrvneogeo = { self.assertIn("PACK TYPE: Narrowed", readme) self.assertIn(label, readme) + def test_no_mode_swallows_a_narrowing_flag(self): + """Every mode either honours a narrowing flag or refuses it. + + Silently ignoring one is how --manifest-targets accepted --region and + wrote seven unfiltered manifests, and how --manifest accepted + --one-per-slot. A mode that cannot apply a flag must say so. + """ + import subprocess + + repo = os.path.join(os.path.dirname(__file__), "..") + matrix = [ + (["--manifest-targets"], ["--region", "us"], "refuse"), + (["--manifest-targets"], ["--one-per-slot"], "refuse"), + (["--platform", "retroarch", "--manifest"], ["--one-per-slot"], "refuse"), + (["--emulator", "duckstation"], ["--one-per-slot"], "refuse"), + (["--system", "sony-playstation"], ["--one-per-slot"], "refuse"), + (["--platform", "retroarch", "--from-md5", "d8f1"], ["--region", "us"], + "refuse"), + (["--platform", "retroarch", "--from-md5", "d8f1"], ["--one-per-slot"], + "refuse"), + ] + for mode, flag, expected in matrix: + with self.subTest(mode=mode, flag=flag): + result = subprocess.run( + [sys.executable, "scripts/generate_pack.py", *mode, *flag, + "--output-dir", os.path.join(self.root, "modecheck")], + capture_output=True, text=True, cwd=repo, timeout=120, + ) + combined = result.stdout + result.stderr + self.assertNotEqual( + result.returncode, 0, + f"{' '.join(mode + flag)} was accepted and ignored:\n{combined}", + ) + self.assertIn("error:", combined) + + def test_standalone_mode_names_itself(self): + """--emulator --standalone ships a different file set; without a tag it + overwrote the pack built without it.""" + from generate_pack import _narrowings + + applied = _narrowings("full", None, None, False, False, standalone=True) + self.assertEqual([tag for tag, _l in applied], ["_Standalone"]) + + def test_every_name_builder_uses_the_same_list(self): + """Four places name a pack or a manifest. None may assemble tags by + hand: that is how --one-per-slot reached --split unnamed.""" + import inspect + + import generate_pack + + source = inspect.getsource(generate_pack) + for hand_rolled in ( + 'req_tag = "_Required"', + 'source_tag = {"platform": "_Platform"', + 'req_suffix = "_required"', + 'slot_tag = "_OnePerSlot"', + ): + self.assertNotIn(hand_rolled, source, hand_rolled) + self.assertGreaterEqual(source.count("_narrowings("), 5) + def test_a_full_pack_is_not_announced_as_narrowed(self): from generate_pack import _build_readme, _narrowings diff --git a/tests/test_region.py b/tests/test_region.py index c5827492..f1e25646 100644 --- a/tests/test_region.py +++ b/tests/test_region.py @@ -419,5 +419,36 @@ class TestSchemaMatchesModule(unittest.TestCase): self.assertEqual(set(node["items"]["enum"]), set(region.REGIONS)) +class TestVerifyModesHonourRegion(unittest.TestCase): + """verify.py must apply --region in every mode, or refuse it. + + It silently ignored the flag in --emulator and --system mode: the report + was identical with and without, so a filtered pack could not be checked. + """ + + def _run(self, *args: str) -> str: + import subprocess + + repo = os.path.join(os.path.dirname(__file__), "..") + return subprocess.run( + [sys.executable, "scripts/verify.py", *args], + capture_output=True, text=True, cwd=repo, timeout=900, + ).stdout + + def test_emulator_mode_narrows(self): + plain = self._run("--emulator", "duckstation") + filtered = self._run("--emulator", "duckstation", "--region", "us") + if "duckstation" not in plain: + self.skipTest("duckstation profile not present") + self.assertNotEqual(plain, filtered) + + def test_system_mode_narrows(self): + plain = self._run("--system", "sony-playstation") + filtered = self._run("--system", "sony-playstation", "--region", "us") + if not plain.strip(): + self.skipTest("system not present") + self.assertNotEqual(plain, filtered) + + if __name__ == "__main__": unittest.main() diff --git a/wiki/advanced-usage.md b/wiki/advanced-usage.md index f7b873c3..357a34f4 100644 --- a/wiki/advanced-usage.md +++ b/wiki/advanced-usage.md @@ -158,8 +158,9 @@ selects by hash. `pipeline.py` never passes it, so the released packs stay complete. Every dimension that removes files appears in the output filename and in the -pack README, because both are built from the same list (`_narrowings` in -`generate_pack.py`). A pack narrowed three ways is called +pack README, because the five places that name an artefact (the pack, the +`--split` rename, the `_Split` directory, the grouped-alias rename and the +manifest) all build from one list (`_narrowings` in `generate_pack.py`). A pack narrowed three ways is called `Recalbox_10.0.8_NorthAmerica_OnePerSlot_Required_BIOS_Pack.zip` and opens with a `PACK TYPE: Narrowed` block listing the three. Adding a new way to narrow a pack means adding it to that list, so it cannot reach users nameless or