diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index fa7b434a..dba57a5e 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -2433,7 +2433,7 @@ def _validate_args(args: argparse.Namespace, parser: argparse.ArgumentParser) -> parser.error("--split requires --platform or --all") if args.split and has_emulator: parser.error("--split is incompatible with --emulator") - if args.group_by != "system" and not args.split: + if args.group_by is not None and not args.split: parser.error("--group-by requires --split") if args.target and not (has_platform or has_all): parser.error("--target requires --platform or --all") @@ -2450,7 +2450,9 @@ def _validate_args(args: argparse.Namespace, parser: argparse.ArgumentParser) -> # A mode either applies a narrowing flag or refuses it. Swallowing one # answers about an artifact the caller did not ask for, and the name on # disk then asserts a narrowing the contents do not carry. - source_given = getattr(args, "source", "full") != "full" + # --source and --group-by default to None so a typed default value is + # seen: compared with "full", `--emulator X --source full` went through. + source_given = getattr(args, "source", None) is not None if source_given and (has_emulator or (has_system and not has_platform)): parser.error("--source requires --platform or --all") if has_from_md5: @@ -2920,7 +2922,7 @@ def _refuse_for_all_variants(args: argparse.Namespace, parser: argparse.Argument # or --required-only beside it would be overridden, and the modes that # build one pack (emulator, system, hashes) never read it. for flag, given in ( - ("--source", args.source != "full"), + ("--source", args.source is not None), ("--required-only", args.required_only), ("--emulator", args.emulator), ("--from-md5", args.from_md5 or args.from_md5_file), @@ -3029,7 +3031,7 @@ def _given_flags(args: argparse.Namespace) -> set[str]: "--system": args.system, "--standalone": args.standalone, "--split": args.split, - "--group-by": args.group_by != "system", + "--group-by": args.group_by is not None, "--manifest": args.manifest, "--manifest-targets": args.manifest_targets, "--verify-packs": args.verify_packs, @@ -3037,7 +3039,7 @@ def _given_flags(args: argparse.Namespace) -> set[str]: "--from-md5": args.from_md5, "--from-md5-file": args.from_md5_file, "--required-only": args.required_only, - "--source": args.source != "full", + "--source": args.source is not None, "--target": args.target, "--region": args.region, "--one-per-slot": args.one_per_slot, @@ -3142,9 +3144,9 @@ def _main(held: contextlib.ExitStack) -> None: parser.add_argument( "--source", choices=["platform", "truth", "full"], - default="full", + default=None, help="File source: platform (YAML only), truth (emulator profiles), full " - "(both)", + "(both, the default)", ) parser.add_argument( "--all-variants", @@ -3157,7 +3159,7 @@ def _main(held: contextlib.ExitStack) -> None: parser.add_argument( "--group-by", choices=["system", "manufacturer"], - default="system", + default=None, help="Grouping for --split (default: system)", ) parser.add_argument("--target", "-t", help="Hardware target (e.g., switch, rpi4)") @@ -3257,6 +3259,9 @@ def _main(held: contextlib.ExitStack) -> None: return _validate_args(args, parser) + # Unset until every refusal has run, so a typed default value is seen. + args.source = args.source or "full" + args.group_by = args.group_by or "system" # Hash lookup / pack mode # A platform run may refresh the trees below, under the exclusive lock: diff --git a/scripts/pipeline.py b/scripts/pipeline.py index 02971049..d66e2256 100644 --- a/scripts/pipeline.py +++ b/scripts/pipeline.py @@ -560,13 +560,18 @@ def _refuse_unapplied_flags( --all-variants builds every source itself, so a --source beside it is dropped; --skip-packs builds none, so both are. """ - if args.all_variants and args.source != "full": + # --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. + source_given = args.source is not None + if args.all_variants and source_given: parser.error("--source is incompatible with --all-variants") if args.skip_packs: - for flag, on in (("--source", args.source != "full"), + for flag, on in (("--source", source_given), ("--all-variants", args.all_variants)): if on: parser.error(f"{flag} is incompatible with --skip-packs") + if args.source is None: + args.source = "full" def _truth_steps(args: argparse.Namespace, results: dict) -> bool: @@ -661,9 +666,9 @@ def main() -> None: ) parser.add_argument("--target", "-t", help="Hardware target (e.g., switch, rpi4)") parser.add_argument( - "--source", choices=["platform", "truth", "full"], default="full", + "--source", choices=["platform", "truth", "full"], default=None, help="packs to build: the platform list, the profiles, or both (default: " - "%(default)s)", + "full)", ) parser.add_argument( "--all-variants", action="store_true", diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index d12efd68..85e7343b 100644 --- a/tests/test_audit_regressions.py +++ b/tests/test_audit_regressions.py @@ -3890,6 +3890,8 @@ class EveryToolRefusesAFlagItWouldDrop(unittest.TestCase): ["--offline", "--skip-packs", "--source", "truth"], ["--offline", "--skip-packs", "--all-variants"], ["--offline", "--all-variants", "--source", "platform"], + ["--offline", "--all-variants", "--source", "full"], + ["--offline", "--skip-packs", "--source", "full"], ], } diff --git a/tests/test_e2e.py b/tests/test_e2e.py index f0d158bb..a169d30c 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -6016,6 +6016,18 @@ struct BurnDriver BurnDrvneogeo = { "refuse"), (["--platform", "retroarch", "--all-variants"], ["--required-only"], "refuse"), + # The default value typed out is still the flag: compared with its + # default, `--source full` and `--group-by system` went through. + (["--platform", "retroarch", "--all-variants"], ["--source", "full"], + "refuse"), + (["--manifest-targets"], ["--source", "full"], "refuse"), + (["--emulator", "handy"], ["--source", "full"], "refuse"), + (["--platform", "retroarch", "--from-md5", "d8f1"], + ["--source", "full"], "refuse"), + (["--platform", "retroarch", "--offline"], ["--group-by", "system"], + "refuse"), + (["--platform", "retroarch", "--verify-packs"], ["--group-by", "system"], + "refuse"), (["--emulator", "duckstation"], ["--all-variants"], "refuse"), (["--system", "sony-playstation"], ["--all-variants"], "refuse"), (["--emulator", "duckstation", "--verify-packs"], ["--all-variants"],