[anaconda][master][PATCH] Parse boot options before parsing CLI options (#1101341)
Vratislav Podzimek
vpodzime at redhat.com
Wed May 28 15:27:33 UTC 2014
On Wed, 2014-05-28 at 14:52 +0200, Martin Kolman wrote:
> On Tue, 2014-05-27 at 13:22 -0400, Anne Mulhern wrote:
> >
> >
> >
> > ----- Original Message -----
> > > From: "Martin Kolman" <mkolman at redhat.com>
> > > To: anaconda-patches at lists.fedorahosted.org
> > > Sent: Tuesday, May 27, 2014 9:43:29 AM
> > > Subject: [anaconda][master][PATCH] Parse boot options before parsing CLI options (#1101341)
> > >
> > > First parse all boot options relevant to Anaconda and only then
> > > parse CLI options (if any). This makes it possible for CLI options
> > > to override boot options, which could be useful for example
> > > when running Anaconda during a live installation.
> > >
> > > This also fixes issues with boot options that have a default value
> > > in the Anaconda argument parser - such options were previously ignored,
> > > resulting for example in the "text" boot option not triggering the text mode.
> > >
> > > Signed-off-by: Martin Kolman <mkolman at redhat.com>
> > > ---
> > > pyanaconda/anaconda_argparse.py | 35 +++++++++++++++++------------------
> > > 1 file changed, 17 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/pyanaconda/anaconda_argparse.py
> > > b/pyanaconda/anaconda_argparse.py
> > > index 35272fe..33b1f4e 100644
> > > --- a/pyanaconda/anaconda_argparse.py
> > > +++ b/pyanaconda/anaconda_argparse.py
> > > @@ -30,7 +30,7 @@ import fcntl
> > > import termios
> > > import struct
> > >
> > > -from argparse import ArgumentParser, ArgumentError, HelpFormatter
> > > +from argparse import ArgumentParser, ArgumentError, HelpFormatter, Namespace
> > >
> > > from pyanaconda.flags import BootArgs
> > >
> > > @@ -146,10 +146,10 @@ class AnacondaArgumentParser(ArgumentParser):
> > > self.deprecated_bootargs.append(arg)
> > > return option
> > >
> > > - def parse_boot_cmdline(self, boot_cmdline, namespace):
> > > + def parse_boot_cmdline(self, boot_cmdline):
> > > """
> > > - Parse the boot cmdline and set appropriate namespace according to
> > > - the options set by add_argument.
> > > + Parse the boot cmdline and create an appropriate Namespace instance
> > > + according to the option definitions set by add_argument.
> > >
> > > boot_cmdline can be given as a string (to be parsed by BootArgs), or
> > > a
> > > dict (or any object with .iteritems()) of {bootarg:value} pairs.
> > > @@ -158,17 +158,17 @@ class AnacondaArgumentParser(ArgumentParser):
> > > by default (/proc/cmdline, /run/initramfs/etc/cmdline,
> > > /etc/cmdline).
> > >
> > > If an option requires a value but the boot arg doesn't provide one,
> > > - we'll quietly not set anything.
> > > + we'll quietly not set anything in the Namespace. We also skip any
> > > boot options
> > > + that were not specified by add_argument as we don't care about them
> > > + (there will usually be quite a lot of them (rd.*, etc.).
> > >
> > > :param boot_cmdline: the Anaconda boot command line arguments
> > > :type boot_cmdline: string, dict or None
> > >
> > > - :param namespace: argparse Namespace instance
> > > - :type namespace: argparse Namespace
> > > -
> > > :returns: an argparse Namespace instance
> > > :rtype: Namespace
> > > """
> > > + namespace = Namespace()
> > > if boot_cmdline is None or type(boot_cmdline) is str:
> > > bootargs = BootArgs(boot_cmdline)
> > > else:
> > > @@ -182,12 +182,9 @@ class AnacondaArgumentParser(ArgumentParser):
> > > if option is None:
> > > # this boot option is unknown to Anaconda, skip it
> > > continue
> > > - if getattr(namespace, option.dest) is not None:
> > > - # if the option is already set on program command line,
> > > - # we ignore any boot options that might modify it
> > > - continue
> > > if option.nargs != 0 and val is None:
> > > - # nargs == 0 -> option does not take any values, skip it
> > > + # nargs == 0 -> the option expects one or more arguments but
> > > the
> > > + # boot option was not given any, so we skip it
> > > continue # TODO: emit a warning or something there?
> >
> > Is the TODO worth thinking about at this time as you're right here?
> Good point, might as well log a warning and get rid of the TODO when I'm
> at it. So fixing locally.
> >
> > > if option.nargs == 0 and option.const is not None:
> > > # nargs == 0 & constr == True -> store_true
> > > @@ -224,11 +221,13 @@ class AnacondaArgumentParser(ArgumentParser):
> > > :returns: an argparse Namespace instance
> > > :rtype: Namespace
> > > """
> > > - # parse arguments (if any) and return the resulting namespace
> > > - namespace = ArgumentParser.parse_args(self, args)
> > > - # now parse boot options (if any) and modify the namespace
> > > accordingly
> > > - namespace = self.parse_boot_cmdline(boot_cmdline, namespace)
> > > - # and return the resulting namespace
> > > + # parse boot options first
> > > + namespace = self.parse_boot_cmdline(boot_cmdline)
> > > + # parse CLI arguments (if any) and add them to the namespace
> > > + # created from parsing boot options, overriding any options
> > > + # with the same destination already present in the namespace
> > > + # NOTE: this basically means that CLI options override boot options
> >
> > I'ld leave out the note. When you say something "basically" does something,
> > you're implying that there are insidious, non-basic, ways in which
> > it does something else. If that's really true, but I don't think it is,
> > then the comment should really explain those instead.
> >
> > To be honest, I'ld leave out all the comments here. You shouldn't feel the
> > need to work so hard just to save me from having to read the parse_args
> > documentation.
> Well, I think the way how the boot option and CLI option parsing works &
> interacts is a bit too tricky to "get" from just reading the source code
> and therefore I think the additional documentation is needed here.
Agreed.
--
Vratislav Podzimek
Anaconda Rider | RHCE | Red Hat, Inc. | Brno - Czech Republic
More information about the anaconda-patches
mailing list