From aafedbb30baf521a49823473e839abc87556709d Mon Sep 17 00:00:00 2001 From: Adam Williamson Date: Mon, 15 Jul 2024 17:54:21 -0700 Subject: [PATCH] Accept but ignore the old --squashfs-only argument 01dd27a broke lorax due to some unspecified behavior in argparse when two different arguments write to the same target. It added a --rootfs-type argument with a string value and a default of "squashfs", and changed --squashfs-only to write the value "squashfs" to the same target (rootfs_type) if passed. Unfortunately, if you do this *with --squashfs-only first and --rootfs-type second*, then if neither --squashfs-only nor --rootfs-type is passed, the value of rootfs_type in the resulting args dict is None, not "squashfs" as expected. If you reverse the order of the args - so --rootfs-type is first and --squashfs-only is second - then if neither arg is passed, the value of rootfs_type comes out as "squashfs" as expected. So we *could* just swap the args in both places. However, this strikes me as fragile as it seems like this behaviour is not defined and could change unexpectedly. It seems safer, to me, to accept --squashfs-only but simply ignore it (since the default value of --rootfs-type is squashfs anyway, and if someone were to specify both, I think it's sane for --rootfs-type to 'win'). Weirdly, argparse doesn't seem to have a built-in way to specify a completely no-op argument, so let's just keep it as `store_true` like it used to be, and simply never use the value. Alternatively we could use parse_known_args instead of parse_args and then check that the only unknown arg was squashfs-only, but we'd have to do that in multiple places, this seems a bit simpler. Signed-off-by: Adam Williamson --- .github/workflows/example-livemedia-creator.yml | 2 -- src/pylorax/cmdline.py | 10 ++++------ 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/.github/workflows/example-livemedia-creator.yml b/.github/workflows/example-livemedia-creator.yml index bfdf612b2..53af027c5 100644 --- a/.github/workflows/example-livemedia-creator.yml +++ b/.github/workflows/example-livemedia-creator.yml @@ -44,14 +44,12 @@ jobs: ## Create the ISO - name: Create the custom ISO # --no-virt: Needed since we're in a container, no host CPU - # --squashfs-only: Just to speed things up, not required run: | livemedia-creator \ --ks "${{ inputs.kickstart_path }}" \ --no-virt \ --make-iso \ --iso-only \ - --squashfs-only \ --iso-name Fedora-custom-example.iso \ --project Fedora \ --volid "Fedora-${{ inputs.fedora_release }}" \ diff --git a/src/pylorax/cmdline.py b/src/pylorax/cmdline.py index 343e9f273..ad0e8323b 100644 --- a/src/pylorax/cmdline.py +++ b/src/pylorax/cmdline.py @@ -109,9 +109,8 @@ def lorax_parser(dracut_default=""): help="Do not verify SSL certificates") optional.add_argument("--dnfplugin", action="append", default=[], dest="dnfplugins", help="Enable a DNF plugin by name/glob, or * to enable all of them.") - optional.add_argument("--squashfs-only", action="store_const", const="squashfs", - default="squashfs", dest="rootfs_type", - help="Use a plain squashfs filesystem for the runtime.") + optional.add_argument("--squashfs-only", action="store_true", + help="Ignored, provided for backward compatibility.") optional.add_argument("--skip-branding", action="store_true", default=False, help="Disable automatic branding package selection. Use --installpkgs to add custom branding.") optional.add_argument("--rootfs-type", metavar="ROOTFSTYPE", default="squashfs", @@ -328,9 +327,8 @@ def lmc_parser(dracut_default=""): parser.add_argument("--releasever", default=DEFAULT_RELEASEVER, help="substituted for @VERSION@ in bootloader config files") parser.add_argument("--volid", default=None, help="volume id") - parser.add_argument("--squashfs-only", action="store_const", const="squashfs", - dest="rootfs_type", - help="Use a plain squashfs filesystem for the runtime.") + parser.add_argument("--squashfs-only", action="store_true", + help="Ignored, provided for backward compatibility.") parser.add_argument("--rootfs-type", metavar="ROOTFSTYPE", default="squashfs", help="Type of rootfs: %s" % ",".join(ROOTFSTYPES)) parser.add_argument("--timeout", default=None, type=int,