]> git.ipfire.org Git - thirdparty/mkosi.git/commitdiff
Stop using nargs="?" for options
authorDaan De Meyer <daan.j.demeyer@gmail.com>
Thu, 13 Feb 2025 13:52:34 +0000 (14:52 +0100)
committerDaan De Meyer <daan.j.demeyer@gmail.com>
Thu, 13 Feb 2025 18:33:49 +0000 (19:33 +0100)
This allows us to get rid of the ambiguity when parsing the verb which
could be interpreted as the argument of the previous option by argparse.

We also have to change the argument parser plumbing to only allow specifying
short options without an argument.

mkosi/completion.py
mkosi/config.py
mkosi/resources/completion.bash
mkosi/resources/man/mkosi.1.md
mkosi/resources/man/mkosi.news.7.md
tests/__init__.py
tests/test_extension.py

index 9f80180c5601979013df796a42a427044a1b6543..3f2fd3455ae8e0cf358609bcd40113bfe1e5335b 100644 (file)
@@ -63,7 +63,6 @@ class CompletionItem:
     short: Optional[str]
     long: Optional[str]
     help: Optional[str]
-    nargs: Union[str, int]
     choices: list[str]
     compgen: CompGen
 
@@ -76,7 +75,6 @@ def collect_completion_arguments() -> list[CompletionItem]:
             short=next((s for s in action.option_strings if not s.startswith("--")), None),
             long=next((s for s in action.option_strings if s.startswith("--")), None),
             help=action.help,
-            nargs=action.nargs or 0,
             choices=[str(c) for c in action.choices] if action.choices is not None else [],
             compgen=CompGen.from_action(action),
         )
@@ -93,7 +91,6 @@ def collect_completion_arguments() -> list[CompletionItem]:
             short=setting.short,
             long=setting.long,
             help=setting.help,
-            nargs=setting.nargs or 1,
             choices=[str(c) for c in setting.choices] if setting.choices is not None else [],
             compgen=CompGen.default,
         )
@@ -123,12 +120,6 @@ def finalize_completion_bash(options: list[CompletionItem], resources: Path) ->
         c.write(to_bash_array("_mkosi_options", options_by_key.keys()))
         c.write("\n\n")
 
-        nargs = to_bash_hasharray(
-            "_mkosi_nargs", {optname: v.nargs for optname, v in options_by_key.items()}
-        )
-        c.write(nargs)
-        c.write("\n\n")
-
         choices = to_bash_hasharray(
             "_mkosi_choices",
             {optname: " ".join(v.choices) for optname, v in options_by_key.items() if v.choices},
@@ -174,8 +165,7 @@ def finalize_completion_fish(options: list[CompletionItem], resources: Path) ->
                 c.write(f"-s {option.short.lstrip('-')} ")
             if option.long:
                 c.write(f"-l {option.long.lstrip('-')} ")
-            if isinstance(option.nargs, int) and option.nargs > 0:
-                c.write("-r ")
+            c.write("-r ")
             if option.choices:
                 c.write('-a "')
                 c.write(" ".join(option.choices))
index 23a1519d6fb1584950ff4dd0d78a5fa3f20c1092..120292d4af3ff4b9a6d42d91ae5a348098b0469f 100644 (file)
@@ -1517,7 +1517,6 @@ class ConfigSetting(Generic[T]):
     long: str = ""
     choices: Optional[list[str]] = None
     metavar: Optional[str] = None
-    nargs: Optional[str] = None
     const: Optional[Any] = None
     help: Optional[str] = None
 
@@ -2405,7 +2404,6 @@ SETTINGS: list[ConfigSetting[Any]] = [
     ConfigSetting(
         dest="repository_key_check",
         metavar="BOOL",
-        nargs="?",
         section="Distribution",
         default=True,
         parse=config_parse_boolean,
@@ -2415,7 +2413,6 @@ SETTINGS: list[ConfigSetting[Any]] = [
     ConfigSetting(
         dest="repository_key_fetch",
         metavar="BOOL",
-        nargs="?",
         section="Distribution",
         default_factory_depends=("distribution", "tools_tree", "tools_tree_distribution"),
         default_factory=config_default_repository_key_fetch,
@@ -2481,7 +2478,6 @@ SETTINGS: list[ConfigSetting[Any]] = [
     ConfigSetting(
         dest="compress_output",
         metavar="ALG",
-        nargs="?",
         section="Output",
         parse=config_parse_compression,
         default_factory=config_default_compression,
@@ -2538,7 +2534,6 @@ SETTINGS: list[ConfigSetting[Any]] = [
     ),
     ConfigSetting(
         dest="split_artifacts",
-        nargs="?",
         section="Output",
         parse=config_parse_artifact_output_list,
         default=ArtifactOutput.compat_no(),
@@ -2565,7 +2560,6 @@ SETTINGS: list[ConfigSetting[Any]] = [
     ConfigSetting(
         dest="overlay",
         metavar="BOOL",
-        nargs="?",
         section="Output",
         parse=config_parse_boolean,
         help="Only output the additions on top of the given base trees",
@@ -2638,7 +2632,6 @@ SETTINGS: list[ConfigSetting[Any]] = [
     ConfigSetting(
         dest="with_recommends",
         metavar="BOOL",
-        nargs="?",
         section="Content",
         parse=config_parse_boolean,
         help="Install recommended packages",
@@ -2646,7 +2639,6 @@ SETTINGS: list[ConfigSetting[Any]] = [
     ConfigSetting(
         dest="with_docs",
         metavar="BOOL",
-        nargs="?",
         section="Content",
         parse=config_parse_boolean,
         default=True,
@@ -2779,7 +2771,6 @@ SETTINGS: list[ConfigSetting[Any]] = [
     ConfigSetting(
         dest="bootable",
         metavar="FEATURE",
-        nargs="?",
         section="Content",
         parse=config_parse_feature,
         match=config_match_feature,
@@ -2852,7 +2843,6 @@ SETTINGS: list[ConfigSetting[Any]] = [
     ConfigSetting(
         dest="microcode_host",
         metavar="BOOL",
-        nargs="?",
         section="Content",
         parse=config_parse_boolean,
         default=False,
@@ -2911,7 +2901,6 @@ SETTINGS: list[ConfigSetting[Any]] = [
     ConfigSetting(
         dest="kernel_modules_initrd",
         metavar="BOOL",
-        nargs="?",
         section="Content",
         parse=config_parse_boolean,
         default=True,
@@ -3016,8 +3005,8 @@ SETTINGS: list[ConfigSetting[Any]] = [
     ConfigSetting(
         dest="autologin",
         short="-a",
+        const=True,
         metavar="BOOL",
-        nargs="?",
         section="Content",
         parse=config_parse_boolean,
         help="Enable root autologin",
@@ -3025,7 +3014,6 @@ SETTINGS: list[ConfigSetting[Any]] = [
     ConfigSetting(
         dest="make_initrd",
         metavar="BOOL",
-        nargs="?",
         section="Content",
         parse=config_parse_boolean,
         help="Make sure the image can be used as an initramfs",
@@ -3033,7 +3021,6 @@ SETTINGS: list[ConfigSetting[Any]] = [
     ConfigSetting(
         dest="ssh",
         metavar="BOOL",
-        nargs="?",
         section="Content",
         parse=config_parse_boolean,
         help="Set up SSH access from the host to the final image via 'mkosi ssh'",
@@ -3050,7 +3037,6 @@ SETTINGS: list[ConfigSetting[Any]] = [
     ConfigSetting(
         dest="secure_boot",
         metavar="BOOL",
-        nargs="?",
         section="Validation",
         parse=config_parse_boolean,
         help="Sign the resulting kernel/initrd image for UEFI SecureBoot",
@@ -3207,7 +3193,6 @@ SETTINGS: list[ConfigSetting[Any]] = [
     ConfigSetting(
         dest="checksum",
         metavar="BOOL",
-        nargs="?",
         section="Validation",
         parse=config_parse_boolean,
         help="Write SHA256SUMS file",
@@ -3215,7 +3200,6 @@ SETTINGS: list[ConfigSetting[Any]] = [
     ConfigSetting(
         dest="sign",
         metavar="BOOL",
-        nargs="?",
         section="Validation",
         parse=config_parse_boolean,
         help="Write and sign SHA256SUMS file",
@@ -3240,8 +3224,6 @@ SETTINGS: list[ConfigSetting[Any]] = [
         parse=config_make_path_parser(constants=("default",)),
         paths=("mkosi.tools",),
         help="Look up programs to execute inside the given tree",
-        nargs="?",
-        const="default",
         scope=SettingScope.universal,
     ),
     ConfigSetting(
@@ -3339,7 +3321,7 @@ SETTINGS: list[ConfigSetting[Any]] = [
     ConfigSetting(
         dest="incremental",
         short="-i",
-        nargs="?",
+        const=Incremental.yes,
         section="Build",
         parse=config_make_enum_parser_with_boolean(Incremental, yes=Incremental.yes, no=Incremental.no),
         default=Incremental.no,
@@ -3425,7 +3407,6 @@ SETTINGS: list[ConfigSetting[Any]] = [
     ConfigSetting(
         dest="use_subvolumes",
         metavar="FEATURE",
-        nargs="?",
         section="Build",
         parse=config_parse_feature,
         help="Use btrfs subvolumes for faster directory operations where possible",
@@ -3465,7 +3446,6 @@ SETTINGS: list[ConfigSetting[Any]] = [
     ),
     ConfigSetting(
         dest="build_sources_ephemeral",
-        nargs="?",
         section="Build",
         parse=config_make_enum_parser_with_boolean(
             BuildSourcesEphemeral, yes=BuildSourcesEphemeral.yes, no=BuildSourcesEphemeral.no
@@ -3496,9 +3476,7 @@ SETTINGS: list[ConfigSetting[Any]] = [
     ConfigSetting(
         dest="with_tests",
         short="-T",
-        long="--without-tests",
-        nargs="?",
-        const="no",
+        const=False,
         section="Build",
         parse=config_parse_boolean,
         default=True,
@@ -3508,7 +3486,6 @@ SETTINGS: list[ConfigSetting[Any]] = [
     ConfigSetting(
         dest="with_network",
         metavar="BOOL",
-        nargs="?",
         section="Build",
         parse=config_parse_boolean,
         help="Run build and postinst scripts with network access (instead of private network)",
@@ -3578,7 +3555,6 @@ SETTINGS: list[ConfigSetting[Any]] = [
             "If specified, the container/VM is run with a temporary snapshot of the output "
             "image that is removed immediately when the container/VM terminates"
         ),
-        nargs="?",
     ),
     ConfigSetting(
         dest="credentials",
@@ -3700,7 +3676,6 @@ SETTINGS: list[ConfigSetting[Any]] = [
     ConfigSetting(
         dest="console",
         metavar="MODE",
-        nargs="?",
         section="Runtime",
         parse=config_make_enum_parser(ConsoleMode),
         help="Configure the virtual machine console mode to use",
@@ -3732,7 +3707,6 @@ SETTINGS: list[ConfigSetting[Any]] = [
         dest="kvm",
         name="KVM",
         metavar="FEATURE",
-        nargs="?",
         section="Runtime",
         parse=config_parse_feature,
         help="Configure whether to use KVM or not",
@@ -3743,7 +3717,6 @@ SETTINGS: list[ConfigSetting[Any]] = [
         dest="vsock",
         name="VSock",
         metavar="FEATURE",
-        nargs="?",
         section="Runtime",
         parse=config_parse_feature,
         help="Configure whether to use vsock or not",
@@ -3766,7 +3739,6 @@ SETTINGS: list[ConfigSetting[Any]] = [
         dest="tpm",
         name="TPM",
         metavar="FEATURE",
-        nargs="?",
         section="Runtime",
         parse=config_parse_feature,
         help="Configure whether to use a virtual tpm or not",
@@ -3777,7 +3749,6 @@ SETTINGS: list[ConfigSetting[Any]] = [
         dest="cdrom",
         name="CDROM",
         metavar="BOOLEAN",
-        nargs="?",
         section="Runtime",
         parse=config_parse_boolean,
         help="Attach the image as a CD-ROM to the virtual machine",
@@ -3787,7 +3758,6 @@ SETTINGS: list[ConfigSetting[Any]] = [
     ConfigSetting(
         dest="removable",
         metavar="BOOLEAN",
-        nargs="?",
         section="Runtime",
         parse=config_parse_boolean,
         help="Attach the image as a removable drive to the virtual machine",
@@ -4060,6 +4030,7 @@ def create_argument_parser(chdir: bool = True) -> argparse.ArgumentParser:
     parser.add_argument(
         "verb",
         type=Verb,
+        nargs="?",
         choices=list(Verb),
         default=Verb.build,
         help=argparse.SUPPRESS,
@@ -4084,16 +4055,25 @@ def create_argument_parser(chdir: bool = True) -> argparse.ArgumentParser:
             group = parser.add_argument_group(f"{s.section} configuration options")
             last_section = s.section
 
+        if s.short and s.const is not None:
+            group.add_argument(  # type: ignore
+                s.short,
+                metavar="",
+                dest=s.dest,
+                const=s.const,
+                help="",
+                action="store_const",
+                default=argparse.SUPPRESS,
+            )
+
         for long in [s.long, *s.compat_longs]:
-            opts = [s.short, long] if s.short and long == s.long else [long]
+            opts = [s.short, long] if s.short and long == s.long and s.const is None else [long]
 
             group.add_argument(  # type: ignore
                 *opts,
                 dest=s.dest,
                 choices=s.choices,
                 metavar=s.metavar,
-                nargs=s.nargs,  # type: ignore
-                const=s.const,
                 help=s.help if long == s.long else argparse.SUPPRESS,
                 action=ConfigAction,
             )
@@ -4137,9 +4117,6 @@ class ConfigAction(argparse.Action):
     ) -> None:
         assert option_string is not None
 
-        if values is None and self.nargs == "?":
-            values = self.const or "yes"
-
         s = SETTINGS_LOOKUP_BY_DEST[self.dest]
 
         if values is None or isinstance(values, str):
@@ -4548,25 +4525,6 @@ def parse_config(
 ) -> tuple[Args, tuple[Config, ...]]:
     argv = list(argv)
 
-    # Make sure the verb command gets explicitly passed. Insert a -- before the positional verb argument
-    # otherwise it might be considered as an argument of a parameter with nargs='?'. For example mkosi -i
-    # summary would be treated as -i=summary.
-    for verb in Verb:
-        try:
-            v_i = argv.index(verb.value)
-        except ValueError:
-            continue
-
-        # Hack to make sure mkosi -C build works.
-        if argv[v_i - 1] in ("-C", "--directory"):
-            continue
-
-        if v_i > 0 and argv[v_i - 1] != "--":
-            argv.insert(v_i, "--")
-        break
-    else:
-        argv += ["--", "build"]
-
     context = ParseContext(resources)
 
     # The "image" field does not directly map to a setting but is required to determine some default values
index 593ec849a95b334b98034ebf542ba2e5bc2784f0..8e4fb07967fa93e91293c9b2471e426191d6a692 100644 (file)
@@ -11,7 +11,7 @@ _mkosi_compgen_dirs() {
 
 _mkosi_completion() {
     local -a _mkosi_options
-    local -A _mkosi_nargs _mkosi_choices _mkosi_compgen _mkosi_verbs
+    local -A _mkosi_choices _mkosi_compgen _mkosi_verbs
     local -i curword_idx verb_seen
 
 ##VARIABLEDEFINITIONS##
@@ -27,7 +27,6 @@ _mkosi_completion() {
     elif [[ "$completing_word_preceding" =~ ^- ]]  # the previous word was an option
     then
         current_option="${completing_word_preceding}"
-        current_option_nargs="${_mkosi_nargs[${current_option}]}"
         current_option_choices="${_mkosi_choices[${current_option}]}"
         current_option_compgen="${_mkosi_compgen[${current_option}]}"
 
@@ -43,12 +42,8 @@ _mkosi_completion() {
                   < <(compgen -W "${current_option_choices}" -- "${completing_word}")
 
         # if this (maybe) takes arguments, we'll just fall back to files
-        if [[ "${current_option_nargs}" == "?" ]] || ((current_option_nargs > 0))
-        then
-            readarray -t COMPREPLY -O "${#COMPREPLY[@]}" \
-                      < <(_mkosi_compgen_files "${completing_word}")
-            return
-        fi
+        readarray -t COMPREPLY -O "${#COMPREPLY[@]}" \
+                    < <(_mkosi_compgen_files "${completing_word}")
     fi
 
     # the preceding word wasn't an option or one that doesn't take arguments,
index e0e21526ce24b23fdd1cad8201d8f18d1d84c5bb..72fdb367c5e4aeda963e9fe1dec80b92126c622e 100644 (file)
@@ -1733,7 +1733,7 @@ boolean argument: either `1`, `yes`, or `true` to enable, or `0`, `no`,
 :   Space-delimited list of additional arguments to pass when invoking
     **qemu**.
 
-`Ephemeral=`, `--ephemeral`
+`Ephemeral=`, `--ephemeral=`
 :   When used with the `shell`, `boot`, or `vm` verbs, this option runs the specified verb on a temporary
     snapshot of the output image that is removed immediately when the container terminates. Taking the
     temporary snapshot is more efficient on file systems that support reflinks natively (**btrfs** or **xfs**)
index 208f44d4dae1a5ada8d5a2b6e0da38e3ab1da802..96a6f967da81143c42350922579195cca0579761 100644 (file)
   or `mkosi shell`) should now be delimited from regular options using
   `--`. Options passed after the verb without using the `--` delimiter
   are now interpreted as regular mkosi options.
+- Boolean options specified on the command line now always expect a
+  boolean argument. For example, `--repository-key-check` needs to
+  become `--repository-key-check=yes`. The reason for this change is to
+  remove ambiguity when parsing e.g. `--repository-key-check build`
+  where `build` would be interpreted as the argument for
+  `--repository-key-check` whereas now it'll be properly interpreted as
+  the verb.
 - Teach `--verity` a new `hash` value, which skips the verity signature
   partition for extension / portable images. To align the possible values,
   `yes` is renamed to `signed`.
index 004276b14155c786b0d84b764037e58dfe363446..98a0aec29e56a87ed4b7760938cdfedd29d9adeb 100644 (file)
@@ -122,7 +122,7 @@ class Image:
             "boot",
             [
                 "--runtime-build-sources=no",
-                "--ephemeral",
+                "--ephemeral=yes",
                 "--register=no",
                 *options,
             ],
@@ -145,7 +145,7 @@ class Image:
                 # TODO: Drop once both Hyper-V bugs are fixed in Github Actions.
                 "--qemu-args=-cpu max,pcid=off",
                 "--ram=2G",
-                "--ephemeral",
+                "--ephemeral=yes",
                 "--register=no",
                 *options,
             ],
index 23ac0443d387089744ed35c64c8c55542148afe4..59f1f1a24611dc6d6df45a49d5d19572050637f5 100644 (file)
@@ -23,7 +23,7 @@ def test_extension(config: ImageConfig, format: OutputFormat) -> None:
                     "",
                     "--incremental=no",
                     "--base-tree", Path(image.output_dir) / "image",
-                    "--overlay",
+                    "--overlay=yes",
                     "--package=lsof",
                     f"--format={format}",
                 ]