[master/rhel7] Add platform specific group selection (#884385)

Vratislav Podzimek vpodzime at redhat.com
Thu Jul 24 06:13:49 UTC 2014


On Wed, 2014-07-23 at 08:54 -0700, Brian C. Lane wrote:
> On Wed, Jul 23, 2014 at 09:41:58AM +0200, Vratislav Podzimek wrote:
> > On Tue, 2014-07-22 at 14:10 -0700, Brian C. Lane wrote:
> 
> > > +def get_platform_groupid():
> > > +    """ Return a platform group id string
> > > +
> > > +        This runs systemd-detect-virt and if the result is not 'none' it
> > > +        prefixes the lower case result with "platform-" for use as a group id.
> > > +
> > > +        :returns: str
> > I think this should be ':rtype: str'
> 
> oops, right.
> 
> > 
> > > +    """
> > > +    try:
> > > +        platform = execWithCapture("systemd-detect-virt", []).strip()
> > > +    except (IOError, AttributeError):
> > > +        return ""
> > > +
> > > +    if platform == "none":
> > > +        return ""
> > > +
> > > +    return "platform-"+platform.lower()
> > Please add spaces here  ^^^
> 
> will do.
> 
> > 
> > > diff --git a/pyanaconda/packaging/__init__.py b/pyanaconda/packaging/__init__.py
> > > index c368e0d..d235183 100644
> > > --- a/pyanaconda/packaging/__init__.py
> > > +++ b/pyanaconda/packaging/__init__.py
> > > @@ -725,6 +725,17 @@ class PackagePayload(Payload):
> > >          else:
> > >              self.rpmMacros.append(('__file_context_path', '%{nil}'))
> > >  
> > > +        # Add platform specific group
> > > +        groupid = iutil.get_platform_groupid()
> > > +        if groupid and groupid  in self.groups:
> > Extra space                 here ^^
> > 
> > > +            if isinstance(groups, list):
> > Looking at the code 'groups' can be either a list or None so I think
> > 'if groups:'
> > would be a nicer check here. And it could be anything that has the
> > 'append' method that way.
> 
> 'if groups' won't catch the case where an empty list is passed in and if
> None is passed you can't pass a new object back out. groups is supposed
> to be a list so I don't see any benefit in expanding it to cover other
> things with append (what is there? set() doesn't have it and neither
> does dict).
It could still be 'if groups is not None', but you are right. I forgot
that anything derived from the list class would work with the
isinstance() check.

-- 
Vratislav Podzimek

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




More information about the anaconda-patches mailing list