Re: [gentoo-portage-dev] [PATCH] Add an emaint module that can scan for failed merges and that can fix failed merges.

[Pavel Kazakov]

On 02/19/2014 02:50 PM, Brian Dolbec wrote:
> 
> Really
> ?  don't use os.system()  It is nearly obsolete.  use subprocess.call()
> or Popen().  call() is a direct sub.  Use Popen for piping output...
> 

Good point, I'll switch over.

> Also, it would be better to call emerge with all pkgs in one command.
> I know it will rarely be more than 1, maybe 2 but, emerge is slow
> enough just intitializing.

Yep, makes sense.

> You might also want to turn off the progressbar for fix unless you
> intend to pipe emerge's output and hide it.  I might be inclined to
> just run emerge in minimum output mode.  It will display what it is
> doing and any more failed builds/merges.  I know I had the other
> modules output the data without displaying and let the cli handle any
> display.  We should be able to do similar to the progressbar, and
> connect the emerge stdout pipe to a display code.  That way any other
> frontend could do with the output what they like.

I'll disable the progressbar for fix(), and look more into minimizing
emerge output.

Regards,
Pavel

Re: [gentoo-portage-dev] [PATCH] Add an emaint module that can scan for failed merges and that can fix failed merges.

[Pavel Kazakov]

Hey Alec,

Thanks for the comments--good stuff.

On 02/19/2014 02:32 PM, Alec Warner wrote:
> On Wed, Feb 19, 2014 at 12:31 PM, Pavel Kazakov  > wrote:
> 
> ---
>  pym/portage/emaint/modules/merges/__init__.py | 20 
>  pym/portage/emaint/modules/merges/merges.py   | 70
> +++
>  2 files changed, 90 insertions(+)
>  create mode 100644 pym/portage/emaint/modules/merges/__init__.py
>  create mode 100644 pym/portage/emaint/modules/merges/merges.py
> 
> diff --git a/pym/portage/emaint/modules/merges/__init__.py
> b/pym/portage/emaint/modules/merges/__init__.py
> new file mode 100644
> index 000..2cd79af
> --- /dev/null
> +++ b/pym/portage/emaint/modules/merges/__init__.py
> @@ -0,0 +1,20 @@
> +# Copyright 2005-2014 Gentoo Foundation
> +# Distributed under the terms of the GNU General Public License v2
> +
> +"""Scan for failed merges and fix them.
> +"""
> 
> 
> """Scan for failed merges fix them."""
All the other modules put the second """ on a newline, so I wanted to be
consistent. Should I still make the change?
>  
> 
> +
> +
> +module_spec = {
> +   'name': 'merges',
> +   'description': __doc__,
> +   'provides':{
> 
> 
> 'module1' ?
All the other modules did it this way, so I wanted consistency.

>  
> 
> +   'module1': {
> +   'name': "merges",
> +   'class': "MergesHandler",
> +   'description': __doc__,
> +   'functions': ['check', 'fix',],
> +   'func_desc': {}
> +   }
> +   }
> +   }
> diff --git a/pym/portage/emaint/modules/merges/merges.py
> b/pym/portage/emaint/modules/merges/merges.py
> new file mode 100644
> index 000..b243082
> --- /dev/null
> +++ b/pym/portage/emaint/modules/merges/merges.py
> @@ -0,0 +1,70 @@
> +# Copyright 2005-2014 Gentoo Foundation
> +# Distributed under the terms of the GNU General Public License v2
> +
> +import portage
> +from portage import os
> +from portage.const import PRIVATE_PATH, VDB_PATH
> +from portage.util import writemsg
> +
> +import shutil
> +import sys
> +
> +if sys.hexversion >= 0x300:
> +   # pylint: disable=W0622
> +   long = int
> 
> 
> What is this little guy? Can we just do this in a library someplace?
Checks if python version is greater than or equal to 3, but I probably
don't even need it since I'm not using longs.
>  
> 
> +
> +class MergesHandler(object):
> +
> +   short_desc = "Remove failed merges"
> +
> 
> 
>  @staticmethod decorator? 
Yeah, I copied legacy emaint code from other modules, but I can switch.
> 
> +   def name():
> +   return "merges"
> +   name = staticmethod(name)
> +
> +   def __init__(self):
> 
> 
> Generally you want to be able to change these later, so you might do
> something like:
> 
> def __init__(self, eroot=None, vardb=None, vardb_path=None):
>   self._eroot = error or portage.settings['EROOT']
>   ... and so forth.
>   
>   Also..why can't self._vardb_path be queried from the vardb?
To be honest, I noticed the other modules assigning such variables to
self, but they could all just as easily be calculated without needing to
be instance member variables.
>  
> 
> +   self._eroot = portage.settings['EROOT']
> +   self._vardb = portage.db[self._eroot]["vartree"].dbapi
> +   self._vardb_path = os.path.join(self._eroot,
> VDB_PATH) + os.path.sep
> +
> +   def can_progressbar(self, func):
> +   return True
> +
> +   def _failed_packages(self, onProgress):
> +   for cat in os.listdir(self._vardb_path):
> 
>   os.listdir(os.path.join(...)) ?
Bad habits die hard ;/. I need to update the rest of my path
concatenation to use os.path.join().
> 
> +   pkgs = os.listdir(self._vardb_path + cat)
> +   maxval = len(pkgs)
> +   for i, pkg in enumerate(pkgs):
> +   if onProgress:
> +   onProgress(maxval, i+1)
> +
> +   if '-MERGING-' in pkg:
> +   yield cat + os.path.sep + pkg
> +
> +   def check(self, **kwargs):
> +   onProgress = kwargs.get('onProgress', None)
> +   failed_pkgs = []
> +   for pkg in self._failed_packages(onProgress):
> +   failed_pkgs.append(pkg)
> +
> +   errors = ["'%s' failed to merge." % x for x in
> failed_pkgs]
> +   return errors
> +
> +   

Re: [gentoo-portage-dev] [PATCH] Add an emaint module that can scan for failed merges and that can fix failed merges.

[Alec Warner]

On Wed, Feb 19, 2014 at 2:50 PM, Brian Dolbec  wrote:

> On Wed, 19 Feb 2014 12:31:44 -0800
> Pavel Kazakov  wrote:
>
> > ---
> >  pym/portage/emaint/modules/merges/__init__.py | 20 
> >  pym/portage/emaint/modules/merges/merges.py   | 70
> +++
> >  2 files changed, 90 insertions(+)
> >  create mode 100644 pym/portage/emaint/modules/merges/__init__.py
> >  create mode 100644 pym/portage/emaint/modules/merges/merges.py
> >
> > diff --git a/pym/portage/emaint/modules/merges/__init__.py
> b/pym/portage/emaint/modules/merges/__init__.py
> > new file mode 100644
> > index 000..2cd79af
> > --- /dev/null
> > +++ b/pym/portage/emaint/modules/merges/__init__.py
> > @@ -0,0 +1,20 @@
> > +# Copyright 2005-2014 Gentoo Foundation
> > +# Distributed under the terms of the GNU General Public License v2
> > +
> > +"""Scan for failed merges and fix them.
> > +"""
> > +
> > +
> > +module_spec = {
> > + 'name': 'merges',
> > + 'description': __doc__,
> > + 'provides':{
> > + 'module1': {
> > + 'name': "merges",
> > + 'class': "MergesHandler",
> > + 'description': __doc__,
> > + 'functions': ['check', 'fix',],
> > + 'func_desc': {}
> > + }
> > + }
> > + }
> > diff --git a/pym/portage/emaint/modules/merges/merges.py
> b/pym/portage/emaint/modules/merges/merges.py
> > new file mode 100644
> > index 000..b243082
> > --- /dev/null
> > +++ b/pym/portage/emaint/modules/merges/merges.py
> > @@ -0,0 +1,70 @@
> > +# Copyright 2005-2014 Gentoo Foundation
> > +# Distributed under the terms of the GNU General Public License v2
> > +
> > +import portage
> > +from portage import os
> > +from portage.const import PRIVATE_PATH, VDB_PATH
> > +from portage.util import writemsg
> > +
> > +import shutil
> > +import sys
> > +
> > +if sys.hexversion >= 0x300:
> > + # pylint: disable=W0622
> > + long = int
> > +
> > +class MergesHandler(object):
> > +
> > + short_desc = "Remove failed merges"
> > +
> > + def name():
> > + return "merges"
> > + name = staticmethod(name)
> > +
> > + def __init__(self):
> > + self._eroot = portage.settings['EROOT']
> > + self._vardb = portage.db[self._eroot]["vartree"].dbapi
> > + self._vardb_path = os.path.join(self._eroot, VDB_PATH) +
> os.path.sep
> > +
> > + def can_progressbar(self, func):
> > + return True
> > +
> > + def _failed_packages(self, onProgress):
> > + for cat in os.listdir(self._vardb_path):
> > + pkgs = os.listdir(self._vardb_path + cat)
> > + maxval = len(pkgs)
> > + for i, pkg in enumerate(pkgs):
> > + if onProgress:
> > + onProgress(maxval, i+1)
> > +
> > + if '-MERGING-' in pkg:
> > + yield cat + os.path.sep + pkg
> > +
> > + def check(self, **kwargs):
> > + onProgress = kwargs.get('onProgress', None)
> > + failed_pkgs = []
> > + for pkg in self._failed_packages(onProgress):
> > + failed_pkgs.append(pkg)
> > +
> > + errors = ["'%s' failed to merge." % x for x in failed_pkgs]
> > + return errors
> > +
> > + def fix(self, **kwargs):
> > + onProgress = kwargs.get('onProgress', None)
> > + tracking_path = os.path.join(self._eroot, PRIVATE_PATH,
> 'failed-merges');
> > + try:
> > + with open(tracking_path, 'w') as tracking_file:
> > + for failed_pkg in
> self._failed_packages(onProgress):
> > +
> tracking_file.write(failed_pkg + '\n')
> > + pkg_path =
> self._vardb_path + failed_pkg
> > + # Delete failed merge
> directory
> > + # XXX: Would be a good
> idea to attempt try removing
> > + # package contents to
> prevent orphaned files
> > + shutil.rmtree(pkg_path)
> > + # Re-emerge package
> > + pkg_name = '=' +
> failed_pkg.replace('-MERGING-', '')
> > +
> features='FEATURES="-collision-detect -protect-owned"'
> > + emerge_cmd="emerge
> --verbose --oneshot --complete-graph=y"
> > + os.system('%s %s %s' %
> (features, emerge_cmd, pkg_name))
> > + except Exception as ex:
> > + writemsg('Unable to fix failed package: %s' %
> str(ex))
>
>
> Really
> ?  don't use os.system()  It is nearly obsolete.  use subprocess.call()
> or Popen

Re: [gentoo-portage-dev] [PATCH] Add an emaint module that can scan for failed merges and that can fix failed merges.

[Alec Warner]

On Wed, Feb 19, 2014 at 3:20 PM, Brian Dolbec  wrote:

> On Wed, 19 Feb 2014 14:32:02 -0800
> Alec Warner  wrote:
>
> > On Wed, Feb 19, 2014 at 12:31 PM, Pavel Kazakov  >wrote:
> >
> > > ---
> > >  pym/portage/emaint/modules/merges/__init__.py | 20 
> > >  pym/portage/emaint/modules/merges/merges.py   | 70
> > > +++
> > >  2 files changed, 90 insertions(+)
> > >  create mode 100644 pym/portage/emaint/modules/merges/__init__.py
> > >  create mode 100644 pym/portage/emaint/modules/merges/merges.py
> > >
> > > diff --git a/pym/portage/emaint/modules/merges/__init__.py
> > > b/pym/portage/emaint/modules/merges/__init__.py
> > > new file mode 100644
> > > index 000..2cd79af
> > > --- /dev/null
> > > +++ b/pym/portage/emaint/modules/merges/__init__.py
> > > @@ -0,0 +1,20 @@
> > > +# Copyright 2005-2014 Gentoo Foundation
> > > +# Distributed under the terms of the GNU General Public License v2
> > > +
> > > +"""Scan for failed merges and fix them.
> > > +"""
> > >
> >
> Correct ^^
>
> > """Scan for failed merges fix them."""
>
> No, your grammar is wrong
>

I didn't mean grammar, I meant.

"""foo."""

not

"""foo.
"""


>
> >
> >
> > > +
> > > +
> > > +module_spec = {
> > > +   'name': 'merges',
> > > +   'description': __doc__,
> > > +   'provides':{
> > >
> >
> > 'module1' ?
> >
>
> It is just an identifier, not used other than to group together
> everything for that module.  The plugin system can handle multiple
> sub-modules within the same general module directory.  See the
>  emaint/modules/move/__init__.py. Move supplies 2 submodules.
>
> The new websync sync module (emerge-webrsync replacement) we started
> roughing in also has 2 sub-modules, the 1st will be the module that
> runs the original bash script.
> The 2nd will be the new python converted code to replace it.
>
> >
> > > +   'module1': {
> > > +   'name': "merges",
> > > +   'class': "MergesHandler",
> > > +   'description': __doc__,
> > > +   'functions': ['check', 'fix',],
> > > +   'func_desc': {}
> > > +   }
> > > +   }
> > > +   }
> > > diff --git a/pym/portage/emaint/modules/merges/merges.py
> > > b/pym/portage/emaint/modules/merges/merges.py
> > > new file mode 100644
> > > index 000..b243082
> > > --- /dev/null
> > > +++ b/pym/portage/emaint/modules/merges/merges.py
> > > @@ -0,0 +1,70 @@
> > > +# Copyright 2005-2014 Gentoo Foundation
> > > +# Distributed under the terms of the GNU General Public License v2
> > > +
> > > +import portage
> > > +from portage import os
> > > +from portage.const import PRIVATE_PATH, VDB_PATH
> > > +from portage.util import writemsg
> > > +
> > > +import shutil
> > > +import sys
> > > +
> > > +if sys.hexversion >= 0x300:
> > > +   # pylint: disable=W0622
> > > +   long = int
> > >
> >
> > What is this little guy? Can we just do this in a library someplace?
> >
> >
> > > +
> > > +class MergesHandler(object):
> > > +
> > > +   short_desc = "Remove failed merges"
> > > +
> > >
> >
> >  @staticmethod decorator?
>
> Yeah, that is copying legacy emaint code from the original module
> classes.
>
>
> >
> > > +   def name():
> > > +   return "merges"
> > > +   name = staticmethod(name)
> > > +
> > > +   def __init__(self):
> > >
> >
> > Generally you want to be able to change these later, so you might do
> > something like:
> >
> > def __init__(self, eroot=None, vardb=None, vardb_path=None):
> >   self._eroot = error or portage.settings['EROOT']
> >   ... and so forth.
> >
>
> The emaint code was not setup to handle init variable assignments.
> None of the original module classes used any.  The plugin system
> doesn't care.  But the TaskHandler class in main.py would need to be
> modified.  Also, all modules must use the same method of initializing,
> regardless whether they need it or not.  In the new sync modules all
> relevant data is passed in using kwargs, then it saves any info it
> needs.
>

So use kwargs here?


>
> >   Also..why can't self._vardb_path be queried from the vardb?
> >
> >
> > > +   self._eroot = portage.settings['EROOT']
> > > +   self._vardb = portage.db[self._eroot]["vartree"].dbapi
> > > +   self._vardb_path = os.path.join(self._eroot, VDB_PATH)
> +
> > > os.path.sep
> > > +
> > > +   def can_progressbar(self, func):
> > > +   return True
> > > +
> > > +   def _failed_packages(self, onProgress):
> > > +   for cat in os.listdir(self._vardb_path):
> > >
> >   os.listdir(os.path.join(...)) ?
> >
> > > +   pkgs = os.listdir(self._vardb_path + cat)
> > > +   maxval = len(pkgs)
> > > +   for i, pkg in enumerate(pkgs):
> > > +   if onProgress:
> > > +   onProgress(maxval, i+1)

Re: [gentoo-portage-dev] [PATCH] Add an emaint module that can scan for failed merges and that can fix failed merges.

[Brian Dolbec]

On Wed, 19 Feb 2014 14:32:02 -0800
Alec Warner  wrote:

> On Wed, Feb 19, 2014 at 12:31 PM, Pavel Kazakov wrote:
> 
> > ---
> >  pym/portage/emaint/modules/merges/__init__.py | 20 
> >  pym/portage/emaint/modules/merges/merges.py   | 70
> > +++
> >  2 files changed, 90 insertions(+)
> >  create mode 100644 pym/portage/emaint/modules/merges/__init__.py
> >  create mode 100644 pym/portage/emaint/modules/merges/merges.py
> >
> > diff --git a/pym/portage/emaint/modules/merges/__init__.py
> > b/pym/portage/emaint/modules/merges/__init__.py
> > new file mode 100644
> > index 000..2cd79af
> > --- /dev/null
> > +++ b/pym/portage/emaint/modules/merges/__init__.py
> > @@ -0,0 +1,20 @@
> > +# Copyright 2005-2014 Gentoo Foundation
> > +# Distributed under the terms of the GNU General Public License v2
> > +
> > +"""Scan for failed merges and fix them.
> > +"""
> >
> 
Correct ^^

> """Scan for failed merges fix them."""

No, your grammar is wrong

> 
> 
> > +
> > +
> > +module_spec = {
> > +   'name': 'merges',
> > +   'description': __doc__,
> > +   'provides':{
> >
> 
> 'module1' ?
> 

It is just an identifier, not used other than to group together
everything for that module.  The plugin system can handle multiple
sub-modules within the same general module directory.  See the
 emaint/modules/move/__init__.py. Move supplies 2 submodules.

The new websync sync module (emerge-webrsync replacement) we started
roughing in also has 2 sub-modules, the 1st will be the module that
runs the original bash script.
The 2nd will be the new python converted code to replace it.

> 
> > +   'module1': {
> > +   'name': "merges",
> > +   'class': "MergesHandler",
> > +   'description': __doc__,
> > +   'functions': ['check', 'fix',],
> > +   'func_desc': {}
> > +   }
> > +   }
> > +   }
> > diff --git a/pym/portage/emaint/modules/merges/merges.py
> > b/pym/portage/emaint/modules/merges/merges.py
> > new file mode 100644
> > index 000..b243082
> > --- /dev/null
> > +++ b/pym/portage/emaint/modules/merges/merges.py
> > @@ -0,0 +1,70 @@
> > +# Copyright 2005-2014 Gentoo Foundation
> > +# Distributed under the terms of the GNU General Public License v2
> > +
> > +import portage
> > +from portage import os
> > +from portage.const import PRIVATE_PATH, VDB_PATH
> > +from portage.util import writemsg
> > +
> > +import shutil
> > +import sys
> > +
> > +if sys.hexversion >= 0x300:
> > +   # pylint: disable=W0622
> > +   long = int
> >
> 
> What is this little guy? Can we just do this in a library someplace?
> 
> 
> > +
> > +class MergesHandler(object):
> > +
> > +   short_desc = "Remove failed merges"
> > +
> >
> 
>  @staticmethod decorator?

Yeah, that is copying legacy emaint code from the original module
classes.


> 
> > +   def name():
> > +   return "merges"
> > +   name = staticmethod(name)
> > +
> > +   def __init__(self):
> >
> 
> Generally you want to be able to change these later, so you might do
> something like:
> 
> def __init__(self, eroot=None, vardb=None, vardb_path=None):
>   self._eroot = error or portage.settings['EROOT']
>   ... and so forth.
> 

The emaint code was not setup to handle init variable assignments.
None of the original module classes used any.  The plugin system
doesn't care.  But the TaskHandler class in main.py would need to be
modified.  Also, all modules must use the same method of initializing,
regardless whether they need it or not.  In the new sync modules all
relevant data is passed in using kwargs, then it saves any info it
needs.

>   Also..why can't self._vardb_path be queried from the vardb?
> 
> 
> > +   self._eroot = portage.settings['EROOT']
> > +   self._vardb = portage.db[self._eroot]["vartree"].dbapi
> > +   self._vardb_path = os.path.join(self._eroot, VDB_PATH) +
> > os.path.sep
> > +
> > +   def can_progressbar(self, func):
> > +   return True
> > +
> > +   def _failed_packages(self, onProgress):
> > +   for cat in os.listdir(self._vardb_path):
> >
>   os.listdir(os.path.join(...)) ?
> 
> > +   pkgs = os.listdir(self._vardb_path + cat)
> > +   maxval = len(pkgs)
> > +   for i, pkg in enumerate(pkgs):
> > +   if onProgress:
> > +   onProgress(maxval, i+1)
> > +
> > +   if '-MERGING-' in pkg:
> > +   yield cat + os.path.sep + pkg
> > +
> > +   def check(self, **kwargs):
> > +   onProgress = kwargs.get('onProgress', None)
> > +   failed_pkgs = []
> > +   for pkg in self._failed_packages(onProgress):
> > +   failed_pkgs.append(pkg)
> > +
> > +  

Re: [gentoo-portage-dev] [PATCH] Add an emaint module that can scan for failed merges and that can fix failed merges.

[Brian Dolbec]

On Wed, 19 Feb 2014 12:31:44 -0800
Pavel Kazakov  wrote:

> ---
>  pym/portage/emaint/modules/merges/__init__.py | 20 
>  pym/portage/emaint/modules/merges/merges.py   | 70 
> +++
>  2 files changed, 90 insertions(+)
>  create mode 100644 pym/portage/emaint/modules/merges/__init__.py
>  create mode 100644 pym/portage/emaint/modules/merges/merges.py
> 
> diff --git a/pym/portage/emaint/modules/merges/__init__.py 
> b/pym/portage/emaint/modules/merges/__init__.py
> new file mode 100644
> index 000..2cd79af
> --- /dev/null
> +++ b/pym/portage/emaint/modules/merges/__init__.py
> @@ -0,0 +1,20 @@
> +# Copyright 2005-2014 Gentoo Foundation
> +# Distributed under the terms of the GNU General Public License v2
> +
> +"""Scan for failed merges and fix them.
> +"""
> +
> +
> +module_spec = {
> + 'name': 'merges',
> + 'description': __doc__,
> + 'provides':{
> + 'module1': {
> + 'name': "merges",
> + 'class': "MergesHandler",
> + 'description': __doc__,
> + 'functions': ['check', 'fix',],
> + 'func_desc': {}
> + }
> + }
> + }
> diff --git a/pym/portage/emaint/modules/merges/merges.py 
> b/pym/portage/emaint/modules/merges/merges.py
> new file mode 100644
> index 000..b243082
> --- /dev/null
> +++ b/pym/portage/emaint/modules/merges/merges.py
> @@ -0,0 +1,70 @@
> +# Copyright 2005-2014 Gentoo Foundation
> +# Distributed under the terms of the GNU General Public License v2
> +
> +import portage
> +from portage import os
> +from portage.const import PRIVATE_PATH, VDB_PATH
> +from portage.util import writemsg
> +
> +import shutil
> +import sys
> +
> +if sys.hexversion >= 0x300:
> + # pylint: disable=W0622
> + long = int
> +
> +class MergesHandler(object):
> +
> + short_desc = "Remove failed merges"
> +
> + def name():
> + return "merges"
> + name = staticmethod(name)
> +
> + def __init__(self):
> + self._eroot = portage.settings['EROOT']
> + self._vardb = portage.db[self._eroot]["vartree"].dbapi
> + self._vardb_path = os.path.join(self._eroot, VDB_PATH) + 
> os.path.sep
> +
> + def can_progressbar(self, func):
> + return True
> +
> + def _failed_packages(self, onProgress):
> + for cat in os.listdir(self._vardb_path):
> + pkgs = os.listdir(self._vardb_path + cat)
> + maxval = len(pkgs)
> + for i, pkg in enumerate(pkgs):
> + if onProgress:
> + onProgress(maxval, i+1)
> +
> + if '-MERGING-' in pkg:
> + yield cat + os.path.sep + pkg
> +
> + def check(self, **kwargs):
> + onProgress = kwargs.get('onProgress', None)
> + failed_pkgs = []
> + for pkg in self._failed_packages(onProgress):
> + failed_pkgs.append(pkg)
> +
> + errors = ["'%s' failed to merge." % x for x in failed_pkgs]
> + return errors
> +
> + def fix(self, **kwargs):
> + onProgress = kwargs.get('onProgress', None)
> + tracking_path = os.path.join(self._eroot, PRIVATE_PATH, 
> 'failed-merges');
> + try:
> + with open(tracking_path, 'w') as tracking_file:
> + for failed_pkg in 
> self._failed_packages(onProgress):
> + tracking_file.write(failed_pkg 
> + '\n')
> + pkg_path = self._vardb_path + 
> failed_pkg
> + # Delete failed merge directory
> + # XXX: Would be a good idea to 
> attempt try removing
> + # package contents to prevent 
> orphaned files
> + shutil.rmtree(pkg_path)
> + # Re-emerge package
> + pkg_name = '=' + 
> failed_pkg.replace('-MERGING-', '')
> + 
> features='FEATURES="-collision-detect -protect-owned"'
> + emerge_cmd="emerge --verbose 
> --oneshot --complete-graph=y"
> + os.system('%s %s %s' % 
> (features, emerge_cmd, pkg_name))
> + except Exception as ex:
> + writemsg('Unable to fix failed package: %s' % str(ex))


Really
?  don't use os.system()  It is nearly obsolete.  use subprocess.call()
or Popen().  call() is a direct sub.  Use Popen for piping output...

Also, it would be better to call emerge with all pkgs in one command.
I know it will rarely be more than 1, maybe 2 but, emer

Re: [gentoo-portage-dev] [PATCH] Add an emaint module that can scan for failed merges and that can fix failed merges.

[Alec Warner]

On Wed, Feb 19, 2014 at 12:31 PM, Pavel Kazakov wrote:

> ---
>  pym/portage/emaint/modules/merges/__init__.py | 20 
>  pym/portage/emaint/modules/merges/merges.py   | 70
> +++
>  2 files changed, 90 insertions(+)
>  create mode 100644 pym/portage/emaint/modules/merges/__init__.py
>  create mode 100644 pym/portage/emaint/modules/merges/merges.py
>
> diff --git a/pym/portage/emaint/modules/merges/__init__.py
> b/pym/portage/emaint/modules/merges/__init__.py
> new file mode 100644
> index 000..2cd79af
> --- /dev/null
> +++ b/pym/portage/emaint/modules/merges/__init__.py
> @@ -0,0 +1,20 @@
> +# Copyright 2005-2014 Gentoo Foundation
> +# Distributed under the terms of the GNU General Public License v2
> +
> +"""Scan for failed merges and fix them.
> +"""
>

"""Scan for failed merges fix them."""


> +
> +
> +module_spec = {
> +   'name': 'merges',
> +   'description': __doc__,
> +   'provides':{
>

'module1' ?


> +   'module1': {
> +   'name': "merges",
> +   'class': "MergesHandler",
> +   'description': __doc__,
> +   'functions': ['check', 'fix',],
> +   'func_desc': {}
> +   }
> +   }
> +   }
> diff --git a/pym/portage/emaint/modules/merges/merges.py
> b/pym/portage/emaint/modules/merges/merges.py
> new file mode 100644
> index 000..b243082
> --- /dev/null
> +++ b/pym/portage/emaint/modules/merges/merges.py
> @@ -0,0 +1,70 @@
> +# Copyright 2005-2014 Gentoo Foundation
> +# Distributed under the terms of the GNU General Public License v2
> +
> +import portage
> +from portage import os
> +from portage.const import PRIVATE_PATH, VDB_PATH
> +from portage.util import writemsg
> +
> +import shutil
> +import sys
> +
> +if sys.hexversion >= 0x300:
> +   # pylint: disable=W0622
> +   long = int
>

What is this little guy? Can we just do this in a library someplace?


> +
> +class MergesHandler(object):
> +
> +   short_desc = "Remove failed merges"
> +
>

 @staticmethod decorator?

> +   def name():
> +   return "merges"
> +   name = staticmethod(name)
> +
> +   def __init__(self):
>

Generally you want to be able to change these later, so you might do
something like:

def __init__(self, eroot=None, vardb=None, vardb_path=None):
  self._eroot = error or portage.settings['EROOT']
  ... and so forth.

  Also..why can't self._vardb_path be queried from the vardb?


> +   self._eroot = portage.settings['EROOT']
> +   self._vardb = portage.db[self._eroot]["vartree"].dbapi
> +   self._vardb_path = os.path.join(self._eroot, VDB_PATH) +
> os.path.sep
> +
> +   def can_progressbar(self, func):
> +   return True
> +
> +   def _failed_packages(self, onProgress):
> +   for cat in os.listdir(self._vardb_path):
>
  os.listdir(os.path.join(...)) ?

> +   pkgs = os.listdir(self._vardb_path + cat)
> +   maxval = len(pkgs)
> +   for i, pkg in enumerate(pkgs):
> +   if onProgress:
> +   onProgress(maxval, i+1)
> +
> +   if '-MERGING-' in pkg:
> +   yield cat + os.path.sep + pkg
> +
> +   def check(self, **kwargs):
> +   onProgress = kwargs.get('onProgress', None)
> +   failed_pkgs = []
> +   for pkg in self._failed_packages(onProgress):
> +   failed_pkgs.append(pkg)
> +
> +   errors = ["'%s' failed to merge." % x for x in failed_pkgs]
> +   return errors
> +
> +   def fix(self, **kwargs):
> +   onProgress = kwargs.get('onProgress', None)
> +   tracking_path = os.path.join(self._eroot, PRIVATE_PATH,
> 'failed-merges');
> +   try:
> +   with open(tracking_path, 'w') as tracking_file:
>

is this unicode safe?


> +   for failed_pkg in
> self._failed_packages(onProgress):
> +
> tracking_file.write(failed_pkg + '\n')
> +   pkg_path =
> self._vardb_path + failed_pkg
>

os.path.join(...)


> +   # Delete failed merge
> directory
> +   # XXX: Would be a good
> idea to attempt try removing
> +   # package contents to
> prevent orphaned files
>

# XXX is terrible style. I realize a bunch of code does that, and it is
stupid.
# use
# TODO: foo




> +   shutil.rmtree(pkg_path)
> +   # Re-emerge package
>
+   pkg_name = '=' +
> failed_pkg.replace('-MERGING-', '')
> +
> features='FEATURES="

[gentoo-portage-dev] [PATCH] Add an emaint module that can scan for failed merges and that can fix failed merges.

[Pavel Kazakov]

---
 pym/portage/emaint/modules/merges/__init__.py | 20 
 pym/portage/emaint/modules/merges/merges.py   | 70 +++
 2 files changed, 90 insertions(+)
 create mode 100644 pym/portage/emaint/modules/merges/__init__.py
 create mode 100644 pym/portage/emaint/modules/merges/merges.py

diff --git a/pym/portage/emaint/modules/merges/__init__.py 
b/pym/portage/emaint/modules/merges/__init__.py
new file mode 100644
index 000..2cd79af
--- /dev/null
+++ b/pym/portage/emaint/modules/merges/__init__.py
@@ -0,0 +1,20 @@
+# Copyright 2005-2014 Gentoo Foundation
+# Distributed under the terms of the GNU General Public License v2
+
+"""Scan for failed merges and fix them.
+"""
+
+
+module_spec = {
+   'name': 'merges',
+   'description': __doc__,
+   'provides':{
+   'module1': {
+   'name': "merges",
+   'class': "MergesHandler",
+   'description': __doc__,
+   'functions': ['check', 'fix',],
+   'func_desc': {}
+   }
+   }
+   }
diff --git a/pym/portage/emaint/modules/merges/merges.py 
b/pym/portage/emaint/modules/merges/merges.py
new file mode 100644
index 000..b243082
--- /dev/null
+++ b/pym/portage/emaint/modules/merges/merges.py
@@ -0,0 +1,70 @@
+# Copyright 2005-2014 Gentoo Foundation
+# Distributed under the terms of the GNU General Public License v2
+
+import portage
+from portage import os
+from portage.const import PRIVATE_PATH, VDB_PATH
+from portage.util import writemsg
+
+import shutil
+import sys
+
+if sys.hexversion >= 0x300:
+   # pylint: disable=W0622
+   long = int
+
+class MergesHandler(object):
+
+   short_desc = "Remove failed merges"
+
+   def name():
+   return "merges"
+   name = staticmethod(name)
+
+   def __init__(self):
+   self._eroot = portage.settings['EROOT']
+   self._vardb = portage.db[self._eroot]["vartree"].dbapi
+   self._vardb_path = os.path.join(self._eroot, VDB_PATH) + 
os.path.sep
+
+   def can_progressbar(self, func):
+   return True
+
+   def _failed_packages(self, onProgress):
+   for cat in os.listdir(self._vardb_path):
+   pkgs = os.listdir(self._vardb_path + cat)
+   maxval = len(pkgs)
+   for i, pkg in enumerate(pkgs):
+   if onProgress:
+   onProgress(maxval, i+1)
+
+   if '-MERGING-' in pkg:
+   yield cat + os.path.sep + pkg
+
+   def check(self, **kwargs):
+   onProgress = kwargs.get('onProgress', None)
+   failed_pkgs = []
+   for pkg in self._failed_packages(onProgress):
+   failed_pkgs.append(pkg)
+
+   errors = ["'%s' failed to merge." % x for x in failed_pkgs]
+   return errors
+
+   def fix(self, **kwargs):
+   onProgress = kwargs.get('onProgress', None)
+   tracking_path = os.path.join(self._eroot, PRIVATE_PATH, 
'failed-merges');
+   try:
+   with open(tracking_path, 'w') as tracking_file:
+   for failed_pkg in 
self._failed_packages(onProgress):
+   tracking_file.write(failed_pkg 
+ '\n')
+   pkg_path = self._vardb_path + 
failed_pkg
+   # Delete failed merge directory
+   # XXX: Would be a good idea to 
attempt try removing
+   # package contents to prevent 
orphaned files
+   shutil.rmtree(pkg_path)
+   # Re-emerge package
+   pkg_name = '=' + 
failed_pkg.replace('-MERGING-', '')
+   
features='FEATURES="-collision-detect -protect-owned"'
+   emerge_cmd="emerge --verbose 
--oneshot --complete-graph=y"
+   os.system('%s %s %s' % 
(features, emerge_cmd, pkg_name))
+   except Exception as ex:
+   writemsg('Unable to fix failed package: %s' % str(ex))
-- 
1.8.3.2

Re: [gentoo-portage-dev] [PATCH] portdbapi: Add cache to avoid repeated failing os.access calls

[Sebastian Luther]

Am 18.02.2014 22:35, schrieb David James:
> """
> +   cache_key = (mycpv, mytree, myrepo)
> +   try:
> +   return self._findname2_cache[cache_key]
> +   except KeyError:
> +   self._findname2_cache[cache_key] = (None, 0)
> 
> 
> To me, it seems potentially error-prone to cache a (potentially)
> incorrect value and then correct it later. 

It is. The problem are all these returns. The whole thing would need to
be reorganized to fix this. I'd rather go for a memoize decorator and
leave that thing alone. If I just could find a usable one.

Would it be possible to
> refactor your patch so that we only cache the value when we know the
> final answer?
>  
> 
> +   except TypeError:
> 
> 
> In what circumstances does it happen that mytree / myrepo are unhashable
> types? Can you add a comment to explain this?

That's my mistake. I was under the impression that mytree is actually
mytreeS and would accept a list. I'll remove the "except TypeError:" in
a future version of the patch.

Re: [gentoo-portage-dev] [PATCH 1/2] Split output for repoman checks into file and message

[Brian Dolbec]

On Wed, 19 Feb 2014 10:33:15 -0800
Brian Dolbec  wrote:

> On Wed, 19 Feb 2014 13:10:04 -0500
> Chris Reffett  wrote:
> 
> > This wraps the output of emerge checks so that a list of length 1-2 is
> > generated. The first element is the file, the second element (optional)
> > is a more descriptive error message. This change will help us eventually
> > introduce more machine-readable output formats.
> > ---
> >  bin/repoman  | 232 
> > +++
> >  pym/repoman/utilities.py |  18 +++-
> >  2 files changed, 128 insertions(+), 122 deletions(-)
> > 
> > diff --git a/bin/repoman b/bin/repoman
> > index 92b..3d5dde4 100755
> > --- a/bin/repoman
> > +++ b/bin/repoman
> > @@ -1402,7 +1402,7 @@ for x in effective_scanlist:
> > repoman_settings['PORTAGE_QUIET'] = '1'
> > if not portage.digestcheck([], repoman_settings, strict=1):
> > stats["manifest.bad"] += 1
> > -   fails["manifest.bad"].append(os.path.join(x, 
> > 'Manifest'))
> > +   fails["manifest.bad"].append([os.path.join(x, 
> > 'Manifest')])
> > repoman_settings.pop('PORTAGE_QUIET', None)
> >
> 
> This looks so,so to me.  I think you are much better off using a
> namedtuple class for these errors instead.  They have built-in
> formatted printing, etc.
> 
> from collections import namedtuple
> 
> and you can have pre-defined named tuple classes that have the error
> message already embedded.
> 
> for some examples see the pyGPG ones I created for gpg data mining:
> 
> https://github.com/dol-sen/pyGPG/blob/master/pyGPG/legend.py
> 
> NOTE: CLASSES is a list of tuples that have the info to define the
> classes that subclass the namedtuple class.  They are initialized by
> the code at the bottom of the page when the page is first loaded into
> memory.  This format saves writing out each individual class by hand
> and makes changes easy.  It also reduced the size of the file to about
> 1/3.  I have done similar to tis in 3 modules in the catalys rewrite
> as well.
> 
> The data is then available in many ways and will have a consistent
> output. 

for smaller simpler examples:

http://git.overlays.gentoo.org/gitweb/?p=proj/catalyst.git;a=blob;f=catalyst/hash_utils.py;h=cd31ad3edbdd412c0da4d968596777a71fbd4beb;hb=refs/heads/3.0

http://git.overlays.gentoo.org/gitweb/?p=proj/catalyst.git;a=blob;f=catalyst/contents.py;h=1a9ed28a58cc63ed954ca8bdbcd1b594e8a7f2e5;hb=refs/heads/3.0


-- 
Brian Dolbec 

Re: [gentoo-portage-dev] [PATCH 1/2] Split output for repoman checks into file and message

[Brian Dolbec]

On Wed, 19 Feb 2014 13:10:04 -0500
Chris Reffett  wrote:

> This wraps the output of emerge checks so that a list of length 1-2 is
> generated. The first element is the file, the second element (optional)
> is a more descriptive error message. This change will help us eventually
> introduce more machine-readable output formats.
> ---
>  bin/repoman  | 232 
> +++
>  pym/repoman/utilities.py |  18 +++-
>  2 files changed, 128 insertions(+), 122 deletions(-)
> 
> diff --git a/bin/repoman b/bin/repoman
> index 92b..3d5dde4 100755
> --- a/bin/repoman
> +++ b/bin/repoman
> @@ -1402,7 +1402,7 @@ for x in effective_scanlist:
>   repoman_settings['PORTAGE_QUIET'] = '1'
>   if not portage.digestcheck([], repoman_settings, strict=1):
>   stats["manifest.bad"] += 1
> - fails["manifest.bad"].append(os.path.join(x, 
> 'Manifest'))
> + fails["manifest.bad"].append([os.path.join(x, 
> 'Manifest')])
>   repoman_settings.pop('PORTAGE_QUIET', None)
>

This looks so,so to me.  I think you are much better off using a
namedtuple class for these errors instead.  They have built-in
formatted printing, etc.

from collections import namedtuple

and you can have pre-defined named tuple classes that have the error
message already embedded.

for some examples see the pyGPG ones I created for gpg data mining:

https://github.com/dol-sen/pyGPG/blob/master/pyGPG/legend.py

NOTE: CLASSES is a list of tuples that have the info to define the
classes that subclass the namedtuple class.  They are initialized by
the code at the bottom of the page when the page is first loaded into
memory.  This format saves writing out each individual class by hand
and makes changes easy.  It also reduced the size of the file to about
1/3.  I have done similar to tis in 3 modules in the catalys rewrite
as well.

The data is then available in many ways and will have a consistent
output. 
-- 
Brian Dolbec 

[gentoo-portage-dev] [PATCH 2/2] Repoman check code cleanup

[Chris Reffett]

Make the repoman check code significantly more consistent in generating
messages (os.path.join() for paths, don't generate a new path when
there's an existing variable, etc.)
---
 bin/repoman | 74 ++---
 1 file changed, 37 insertions(+), 37 deletions(-)

diff --git a/bin/repoman b/bin/repoman
index 3d5dde4..d6d495c 100755
--- a/bin/repoman
+++ b/bin/repoman
@@ -1416,7 +1416,7 @@ for x in effective_scanlist:
if (y in no_exec or y.endswith(".ebuild")) and \
stat.S_IMODE(os.stat(os.path.join(checkdir, 
y)).st_mode) & 0o111:
stats["file.executable"] += 1
-   fails["file.executable"].append([os.path.join(checkdir, 
y)])
+   fails["file.executable"].append([os.path.join(x, y)])
if y.endswith(".ebuild"):
pf = y[:-7]
ebuildlist.append(pf)
@@ -1468,7 +1468,7 @@ for x in effective_scanlist:
index = -1
if index != -1:
stats["file.name"] += 1
-   fails["file.name"].append(["%s/%s" % (checkdir, y), 
+   fails["file.name"].append([os.path.join(x, y), 
"char '%s'" %  y[index]])
 
if not (y in ("ChangeLog", "metadata.xml") or 
y.endswith(".ebuild")):
@@ -1488,7 +1488,7 @@ for x in effective_scanlist:
line += l2
if l2 != 0:
s = s[s.rfind("\n") + 1:]
-   fails["file.UTF8"].append(["%s/%s" % (checkdir, y), 
+   fails["file.UTF8"].append([os.path.join(x, y), 
"line %i, just after: '%s'" % (line, s)])
finally:
if f is not None:
@@ -1557,7 +1557,7 @@ for x in effective_scanlist:
except IOError:
if vcs == "cvs":
stats["CVS/Entries.IO_error"] += 1
-   fails["CVS/Entries.IO_error"].append([checkdir 
+ "/CVS/Entries"])
+   
fails["CVS/Entries.IO_error"].append([os.path.join(x, "/CVS/Entries")])
else:
raise
continue
@@ -1595,11 +1595,11 @@ for x in effective_scanlist:
for entry in mydigests:
if entry not in myfiles_all:
stats["digest.unused"] += 1
-   fails["digest.unused"].append([checkdir + "::" 
+ entry])
+   fails["digest.unused"].append([os.path.join(x, 
"Manifest"), entry])
for entry in myfiles_all:
if entry not in mydigests:
stats["digest.missing"] += 1
-   fails["digest.missing"].append([checkdir + "::" 
+ entry])
+   fails["digest.missing"].append([os.path.join(x, 
"Manifest"), entry])
del myfiles_all
 
if os.path.exists(checkdir + "/files"):
@@ -1631,12 +1631,12 @@ for x in effective_scanlist:
# 20 KiB and 60 KiB causes a warning, while file size 
over 60 KiB causes an error.
elif mystat.st_size > 61440:
stats["file.size.fatal"] += 1
-   fails["file.size.fatal"].append([x + "/files/" 
+ y,
-   "(" + str(mystat.st_size//1024) + " 
KiB)"])
+   
fails["file.size.fatal"].append([os.path.join(x, "files", y),
+   str(mystat.st_size//1024) + " KiB"])
elif mystat.st_size > 20480:
stats["file.size"] += 1
-   fails["file.size"].append([x + "/files/" + y,
-   "(" + str(mystat.st_size//1024) + " 
KiB)"])
+   fails["file.size"].append([os.path.join(x, 
"files", y),
+   str(mystat.st_size//1024) + " KiB"])
 
index = repo_config.find_invalid_path_char(y)
if index != -1:
@@ -1649,19 +1649,19 @@ for x in effective_scanlist:
index = -1
if index != -1:
stats["file.name"] += 1
-   fails["file.name"].append(["%s/files/%s" % 
(checkdir, y),
+   fails["file.name"].append([os.path.join(x, 
"files", y),
"char '%s'" % y[index]])
del mydigests
 
if check_changelog and "ChangeLog" not in checkdirlist:
stats["changelog.missing"] += 1
-   fails["

[gentoo-portage-dev] [PATCH 0/2] Refactor repoman QA handling

[Chris Reffett]

This series of patches alters repoman's QA output to be much more usable. The
first patch changes all checks to output a list of either length 1 or 2,
splitting the file with the error from the error itself. This will be helpful
for making machine-parseable output in the future. The second both makes the
variables used to build the output much more consistent and makes the output
itself more consistent by removing a few instances where the full path was
printed rather than the relative path. This will change the existing repoman
output format and potentially break scripts which rely on the old and
inconsistent behavior.

Chris Reffett (2):
  Split output for repoman checks into file and message
  Repoman check code cleanup

 bin/repoman  | 232 +++
 pym/repoman/utilities.py |  18 +++-
 2 files changed, 128 insertions(+), 122 deletions(-)

-- 
1.8.5.3

[gentoo-portage-dev] [PATCH 1/2] Split output for repoman checks into file and message

[Chris Reffett]

This wraps the output of emerge checks so that a list of length 1-2 is
generated. The first element is the file, the second element (optional)
is a more descriptive error message. This change will help us eventually
introduce more machine-readable output formats.
---
 bin/repoman  | 232 +++
 pym/repoman/utilities.py |  18 +++-
 2 files changed, 128 insertions(+), 122 deletions(-)

diff --git a/bin/repoman b/bin/repoman
index 92b..3d5dde4 100755
--- a/bin/repoman
+++ b/bin/repoman
@@ -1402,7 +1402,7 @@ for x in effective_scanlist:
repoman_settings['PORTAGE_QUIET'] = '1'
if not portage.digestcheck([], repoman_settings, strict=1):
stats["manifest.bad"] += 1
-   fails["manifest.bad"].append(os.path.join(x, 
'Manifest'))
+   fails["manifest.bad"].append([os.path.join(x, 
'Manifest')])
repoman_settings.pop('PORTAGE_QUIET', None)
 
if options.mode == 'manifest-check':
@@ -1416,7 +1416,7 @@ for x in effective_scanlist:
if (y in no_exec or y.endswith(".ebuild")) and \
stat.S_IMODE(os.stat(os.path.join(checkdir, 
y)).st_mode) & 0o111:
stats["file.executable"] += 1
-   fails["file.executable"].append(os.path.join(checkdir, 
y))
+   fails["file.executable"].append([os.path.join(checkdir, 
y)])
if y.endswith(".ebuild"):
pf = y[:-7]
ebuildlist.append(pf)
@@ -1426,17 +1426,17 @@ for x in effective_scanlist:
except KeyError:
allvalid = False
stats["ebuild.syntax"] += 1
-   fails["ebuild.syntax"].append(os.path.join(x, 
y))
+   fails["ebuild.syntax"].append([os.path.join(x, 
y)])
continue
except IOError:
allvalid = False
stats["ebuild.output"] += 1
-   fails["ebuild.output"].append(os.path.join(x, 
y))
+   fails["ebuild.output"].append([os.path.join(x, 
y)])
continue
if not portage.eapi_is_supported(myaux["EAPI"]):
allvalid = False
stats["EAPI.unsupported"] += 1
-   
fails["EAPI.unsupported"].append(os.path.join(x, y))
+   
fails["EAPI.unsupported"].append([os.path.join(x, y)])
continue
pkgs[pf] = Package(cpv=cpv, metadata=myaux,
root_config=root_config, type_name="ebuild")
@@ -1468,8 +1468,8 @@ for x in effective_scanlist:
index = -1
if index != -1:
stats["file.name"] += 1
-   fails["file.name"].append("%s/%s: char '%s'" % \
-   (checkdir, y, y[index]))
+   fails["file.name"].append(["%s/%s" % (checkdir, y), 
+   "char '%s'" %  y[index]])
 
if not (y in ("ChangeLog", "metadata.xml") or 
y.endswith(".ebuild")):
continue
@@ -1488,7 +1488,8 @@ for x in effective_scanlist:
line += l2
if l2 != 0:
s = s[s.rfind("\n") + 1:]
-   fails["file.UTF8"].append("%s/%s: line %i, just after: 
'%s'" % (checkdir, y, line, s))
+   fails["file.UTF8"].append(["%s/%s" % (checkdir, y), 
+   "line %i, just after: '%s'" % (line, s)])
finally:
if f is not None:
f.close()
@@ -1503,8 +1504,8 @@ for x in effective_scanlist:
for l in myf:
if l[:-1][-7:] == ".ebuild":
stats["ebuild.notadded"] += 1
-   fails["ebuild.notadded"].append(
-   os.path.join(x, 
os.path.basename(l[:-1])))
+   fails["ebuild.notadded"].append([
+   os.path.join(x, 
os.path.basename(l[:-1]))])
myf.close()
 
if vcs in ("cvs", "svn", "bzr") and check_ebuild_notadded:
@@ -1556,7 +1557,7 @@ for x in effective_scanlist:
except IOError:
if vcs == "cvs":
stats["CVS/Entries.IO_error"] += 1
-   fails["CVS/Entries.IO_error"].append(checkdir + 
"/CVS/Entries")
+   fails["CVS/Entries.IO_error"].append([che

Re: [gentoo-portage-dev] [PATCH v2] Add --output-style option to repoman

[Chris Reffett]

On 02/13/2014 10:42 AM, Brian Dolbec wrote:
> On Thu, 13 Feb 2014 03:19:35 -0500
> Mike Frysinger  wrote:
> 
>> On Monday, February 10, 2014 20:22:36 Chris Reffett wrote:
>>> This patch adds a --output-style option to repoman, which gives the
>>> user a choice of output formats for the repoman checks. Choices are
>>> "default" (current style) and "column" (a greppable format), but it
>>> should be easy to add more. Fixes bug 481584.
>>
>> i'd expect a proper structured output would make sense to include in
>> the default set.  like JSON.  just create a dict and send it to
>> json.dump().
> 
> He is working on more changes to repoman and the output. So, if you
> can, Chris, then do it, add a json option.
> 
Will do that after my next set of changes to repoman (to be emailed shortly)
> 
>>
>>> v2: Fix docstring to be complete and in the standard format, make
>>> use of default choices in --output-style wrt comments by antarus
>>> and dol-sen
>>
>> erm, i thought the previous docstring was correct.  it followed
>> PEP257 while this new one is like javadoc or something.
>>
> 
> It is the existing format that has been around in portage for years.
> There is even a page for it:
> 
> http://www.gentoo.org/proj/en/portage/doc/policies/docstring-spec.xml
> 
> It is also the style that epydoc recognizes. 
> 
>>> -utilities.format_qa_output(f, stats, fails, dofull, dofail,
>>> options, qawarnings)
>>> +if options.output_style == 'column':
>>> +   utilities.format_qa_output_column(f, stats, fails, dofull,
>>> dofail, options, qawarnings)
>>> +else:
>>> +   utilities.format_qa_output(f, stats, fails, dofull,
>>> dofail, options, qawarnings)
>>
>> use a func pointer instead.
>> format_outputs = {
>>  'column': utilities.format_qa_output_column,
>>  'default': utilities.format_qa_output,
>> }
>> format_output = format_outputs.get(options.output_style,
>>  format_outputs['default'])
>> format_output(f, stats, fails, dofull, dofail, options, qawarnings)
>>
> 
> yeah, make it so.  Good spot, Mike
> 
Committed, thanks for the spot.
> 
> Since Mike was too slow in replying, make another commit to change
> it.
> 
>>> +   formatter.add_literal_data("NumberOf " + category
>>> + " ")
>>
>> prefer to use % rather than + like so:
>>  'NumberOf %s ' % category
>>
>>> +   formatter.add_literal_data("%s" % number)
>>
> 
> well actually, for simple additions like that, string1 + string2, it is
> actually faster.
> But for multiple additions,  %s is much better, faster.  Also if the
> string is translated, then use %s regardless.  That way the %s can be
> moved around for the translation.
> 
>> str(number)
>> -mike
> 
> 
>