[PATCH 1/2] Add the -A option to makeupdates to allow easy extraction of multiple RPMs

Vratislav Podzimek vpodzime at redhat.com
Tue Feb 10 08:24:20 UTC 2015


On Mon, 2015-02-02 at 09:25 -0500, Anne Mulhern wrote:
> 
> 
> 
> 
> ----- Original Message -----
> > From: "Vratislav Podzimek" <vpodzime at redhat.com>
> > To: anaconda-patches at lists.fedorahosted.org
> > Sent: Sunday, February 1, 2015 7:21:15 PM
> > Subject: [PATCH 1/2] Add the -A option to makeupdates to allow easy extraction	of multiple RPMs
> > 
> > This is very useful if we need to test a new version of some tool/library
> > that
> > has multiple RPMs (as e.g. libblockdev with all its plugins).
> > 
> > Signed-off-by: Vratislav Podzimek <vpodzime at redhat.com>
> > ---
> >  scripts/makeupdates | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> > 
> > diff --git a/scripts/makeupdates b/scripts/makeupdates
> > index ab420e3..0c8195a 100755
> > --- a/scripts/makeupdates
> > +++ b/scripts/makeupdates
> > @@ -978,6 +978,10 @@ def main():
> >                          dest='add_rpms', metavar='PATH_TO_RPM', default=[],
> >                          help='add contents of RPMs to the updates image')
> >  
> > +    parser.add_argument('-A', '--add-dir-rpms', action='append', type=str,
> > nargs='+',
> > +                        dest='add_dir_rpms', metavar='PATH_TO_RPM_DIR',
> > default=[],
> > +                        help='add contents of RPMs from the directory to the
> > updates image')
> > +
> >      parser.add_argument('-f', '--fetch', action='store', type=str,
> >      metavar="ARCH",
> >                          help='autofetch new dependencies from Koji for
> >                          ARCH')
> >  
> > @@ -1025,6 +1029,21 @@ def main():
> >      if args.po:
> >          copyTranslations(updates, cwd, builddir)
> >  
> > +    if args.add_dir_rpms:
> > +        # as using -A or --add-dir-rpms mutltiple times adds a new list to
> > the
> > +        # list instead of extending the original list, we need to
> > "normalize" to
> > +        # a one-level list of strings
> > +        rpm_dirs = []
> > +        for item in args.add_dir_rpms:
> > +            if isinstance(item, list):
> > +                rpm_dirs.extend(item)
> > +            else:
> > +                rpm_dirs.append(item)
> > +
> 
> This duplicates the code for add_rpms, but that code is incorrect in treating
> the add_rpms list as if it might be heterogenous; its only elements are lists.
> 
> >>> z = arser.parse_args("--add rpm1 --add rpm2 --add rpm3 rpm4 rpm5".split())
> >>> z.add_rpms
> [['rpm1'], ['rpm2'], ['rpm3', 'rpm4', 'rpm5']]
> 
> So both lists can be flattened a lot more simply, like below.
> 
> >>> [r for l in z.add_rpms for r in l]
> ['rpm1', 'rpm2', 'rpm3', 'rpm4', 'rpm5']
Great, thanks for the "research". A wanted to deal with the -A option as
fast as possible because it was really only required for my libblockdev
testing. I'll change this into a better patch (set).

> 
> The _really_ proper thing would be to define a custom Action to use instead of
> append, something like:
> 
> class Extend(argparse.Action):
> 
>      def __call__(self, parser, namespace, values, option_string=None):
>          setattr(namespace, self.dest, getattr(namespace, self.dest) + values)
> 
> I think, and use it for both add_rpms and add_rpm_dirs.
>  
> It almost seems like an extend action should be part of argparse package;
> I'm trying out an RFE on Python bugs.
Once that's in, we should definitely switch to that action, but I don't
think we need to have it defined in our code in the meantime.

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list