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 <dolsen@g.o> wrote:

> On Wed, 19 Feb 2014 12:31:44 -0800
> Pavel Kazakov <nullishzero@g.o> 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 0000000..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 0000000..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 >= 0x3000000:
> > +     # 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...
>

os.system() invokes /bin/sh, so the first problem is you have a quoting
problem.
The second problem is that you are root, and you just told it to execute
'emerge' and not '/usr/bin/emerge' and so it might execute some other
emerge.

The third problem is that the input is not sanitized, and badly named files
can cause code execution.

/var/db/pkg/app-shells/hahaha;rm -rf;-MERGING-

might call
os.system('FEATURES="-collision-detect -protect-owned" emerge --verbose
--oneshot --complete-graph=y =app-shells/hahaha-1.0-r1;rm -rf')

And so forth.


>
> 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.
>
> 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.
>
>
> --
> Brian Dolbec <dolsen>
>
>
>