Skip to content

Make error logged/raised from has_option_in_help more accurate#2943

Merged
schaefi merged 1 commit intoOSInside:mainfrom
AdamWill:option-in-help-correct-error
Feb 3, 2026
Merged

Make error logged/raised from has_option_in_help more accurate#2943
schaefi merged 1 commit intoOSInside:mainfrom
AdamWill:option-in-help-correct-error

Conversation

@AdamWill
Copy link
Copy Markdown
Contributor

@AdamWill AdamWill commented Feb 2, 2026

If I'm following this code correctly, we're not raising here because we "could not parse" the output. We're raising because we parsed it and it did not have the flag we were looking for.

If I'm following this code correctly, we're not raising here
because we "could not parse" the output. We're raising because
we parsed it and it did not have the flag we were looking for.

Signed-off-by: Adam Williamson <awilliam@redhat.com>
AdamWill added a commit to AdamWill/kiwi that referenced this pull request Feb 3, 2026
In OSInside#2921 , based on the
commit message and the fact that the code uses an `if`, it's
pretty clear the use of `dracut --printconfig` is meant to be
conditional. But setting the misleadingly-named
`raise_on_error` to `True` means it isn't. `raise_on_error`
makes `has_option_in_help` raise an exception if it *fails* -
doesn't find the specified flag - not if it *errors*. Since it
does not handle the exception, this code crashes if dracut does
not have the argument:
https://koji.fedoraproject.org/koji/taskinfo?taskID=141808928

DEBUG util.py:459:  [ ERROR   ]: 06:03:28 | KiwiCommandCapabilitiesError: Could not parse dracut output

I've sent OSInside#2943 to make the
message less of a lie, and this makes us stop crashing if the
arg isn't available.

Signed-off-by: Adam Williamson <awilliam@redhat.com>
AdamWill added a commit to AdamWill/kiwi that referenced this pull request Feb 3, 2026
In OSInside#2921 , based on the
commit message and the fact that the code uses an `if`, it's
pretty clear the use of `dracut --printconfig` is meant to be
conditional. But setting the misleadingly-named
`raise_on_error` to `True` means it isn't. `raise_on_error`
makes `has_option_in_help` raise an exception if it *fails* -
doesn't find the specified flag - not if it *errors*. Since it
does not handle the exception, this code crashes if dracut does
not have the argument:
https://koji.fedoraproject.org/koji/taskinfo?taskID=141808928

DEBUG util.py:459:  [ ERROR   ]: 06:03:28 | KiwiCommandCapabilitiesError: Could not parse dracut output

I've sent OSInside#2943 to make the
message less of a lie, and this makes us stop crashing if the
arg isn't available.

Signed-off-by: Adam Williamson <awilliam@redhat.com>
AdamWill added a commit to AdamWill/kiwi that referenced this pull request Feb 3, 2026
In OSInside#2921 , based on the
commit message and the fact that the code uses an `if`, it's
pretty clear the use of `dracut --printconfig` is meant to be
optional. But setting the misleadingly-named `raise_on_error`
to `True` means it isn't. `raise_on_error` makes
`has_option_in_help` raise an exception if it *fails* - doesn't
find the specified flag - not if it *errors*. Since it does not
handle the exception, this code crashes if dracut does not have
the argument:
https://koji.fedoraproject.org/koji/taskinfo?taskID=141808928

DEBUG util.py:459:  [ ERROR   ]: 06:03:28 | KiwiCommandCapabilitiesError: Could not parse dracut output

I've sent OSInside#2943 to make the
message less of a lie, and this makes us stop crashing if the
arg isn't available.

Signed-off-by: Adam Williamson <awilliam@redhat.com>
@AdamWill
Copy link
Copy Markdown
Contributor Author

AdamWill commented Feb 3, 2026

Hmm, so there's history here. Before #2159 , this did behave as the message implies - the exception was only raised if the command exited non-zero. However, in #2159 @schaefi changed it, on the basis that some commands might return non-zero when printing a usage message. He kept the exception raising behavior around but changed it to "raise an exception if the argument isn't present", not "raise an exception if the command exited non-zero"; he updated the docstring to describe this accurately, but not the exception message itself.

I'm not sure if the behavior is actually useful as it currently exists? Maybe we should just get rid of it? Only one thing sets this to True, and it appears to be a bug - #2944 changes it to False.

@schaefi
Copy link
Copy Markdown
Collaborator

schaefi commented Feb 3, 2026

@AdamWill Thanks I like the new message 👍

@schaefi schaefi self-requested a review February 3, 2026 08:58
@schaefi
Copy link
Copy Markdown
Collaborator

schaefi commented Feb 3, 2026

changed it, on the basis that some commands might return non-zero when printing a usage message. He kept the exception raising behavior around but changed it to "raise an exception if the argument isn't present", not "raise an exception if the command exited non-zero";

right I remember that setfiles behaved this way. As kiwi's Command instance raises an exception if the command returns with a non zero exit code we only had the chance to let it go and parse whatever we get on any channel. This lousy style allowed us to parse some output but also does not improve the message on real errors. In addition to your change I also added #2945

@schaefi schaefi merged commit a799aea into OSInside:main Feb 3, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants