[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