[PATCH] Fix the handling of set_const options pulled in from the boot cmdline

Martin Kolman mkolman at redhat.com
Thu May 22 14:44:23 UTC 2014



----- Original Message -----
> From: "David Shea" <dshea at redhat.com>
> To: anaconda-patches at lists.fedorahosted.org
> Sent: Wednesday, May 21, 2014 7:31:16 PM
> Subject: Re: [PATCH] Fix the handling of set_const options pulled in from	the boot cmdline
> 
> On 05/21/2014 01:23 PM, David Shea wrote:
> > We lost option.process when switching to argparse, so we need handle the
> > set_const case ourselves in parse_boot_cmdline.
> > ---
> >   pyanaconda/anaconda_argparse.py | 13 +++++++++++--
> >   1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/pyanaconda/anaconda_argparse.py
> > b/pyanaconda/anaconda_argparse.py
> > index ed7fa20..cfee657 100644
> > --- a/pyanaconda/anaconda_argparse.py
> > +++ b/pyanaconda/anaconda_argparse.py
> > @@ -189,12 +189,21 @@ class AnacondaArgumentParser(ArgumentParser):
> >               if option.nargs != 0 and val is None:
> >                   # nargs == 0 -> option does not take any values, skip it
> >                   continue  # TODO: emit a warning or something there?
> > -            if option.nargs == 0 and option.const is True and val in ("0",
> > "no", "off"):
> > +            if option.nargs == 0 and option.const:
> 
> So that snarky comment I left below shows I was thinking about
> option.const == False, but I didn't think it through all of the way. The
> conditional here should be option.const is not None.
Also an ACK from me !

I was actually also investigating this issue, but you managed to bet me to it. :)


On a related note:

There might be still some additional issues with boot option parsing without the process() method.

For example, if an option, let's call it "foo", is specified as integer and is passed 
to boot command line as foo=1, we will just set it to the "1" string in the namespace.
This might cause issues if some code expects the parsed option to have a specific type 
and for example tries to subtract it from a number.

I've been looking into this and the best solution seems to be to basically reassemble
something that looks like command line arguments from those boot options that we care about
(returning to the example, foo=1 on boot command line would be turned to ["--foo", "1"])
and running it through the parser. This would make sure the options have the expected values
and could also raise any relevant errors.

As argparse supports parser chaining (parser can take an existing Namespace instance) we could then 
feed the pre-populated namespace to the "real" CLI option parsing call and the result should be a
correct combined namespace.

That's what I have been able to come with & feedback is welcome! :)

> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> 


More information about the anaconda-patches mailing list