cli: Only allow some arguments if composefs-backend is true#1990
cli: Only allow some arguments if composefs-backend is true#1990Johan-Liebert1 wants to merge 1 commit intobootc-dev:mainfrom
Conversation
github-actions
bot
added
area/install
Currently the cli accepts `--insecure`, `--bootloader`, `--uki-addon` for ostree installs as well, which is not quite right as in the best case scenario they won't do anything and in the worst case scenario they might leave the install in undefined state Also, update the bootloader docs to specify systemd-boot is only available for composefs backend Xref: bootc-dev#1989 Signed-off-by: Johan-Liebert1 <pragyanpoudyal41999@gmail.com>
Johan-Liebert1
force-pushed
the
cfs-options-fix
branch
from
4a28be2 to
9dcdc2f
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly restricts the --insecure, --bootloader, and --uki-addon CLI arguments to be used only with --composefs-backend. This is a good improvement as it leverages clap's validation capabilities, providing earlier and clearer feedback to the user. The removal of the now-redundant manual validation code simplifies the implementation. The accompanying documentation update is also helpful. I have one minor suggestion to further improve the clarity of the documentation.
| If bootupd is not present in the input container image, then systemd-boot will be used | ||
| by default (except on s390x). |
There was a problem hiding this comment.
This sentence could be slightly confusing. While it's true that systemd-boot is the fallback if bootupd is not present, for ostree-based installs this will result in an error. It might be clearer to explicitly state that this default behavior applies to composefs installs to avoid any ambiguity.
For example:
For composefs-based installations, if `bootupd` is not present in the input container image, then `systemd-boot` will be used by default (except on s390x).| If bootupd is not present in the input container image, then systemd-boot will be used | |
| by default (except on s390x). | |
| For composefs-based installations, if `bootupd` is not present in the input container image, then systemd-boot will be used | |
| by default (except on s390x). |
1 participant
Currently the cli accepts
--insecure,--bootloader,--uki-addonfor ostree installs as well, which is not quite right as in the best case scenario they won't do anything and in the worst case scenario they might leave the install in undefined stateAlso, update the bootloader docs to specify systemd-boot is only
available for composefs backend
Xref: #1989