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

> On Wed, Feb 19, 2014 at 12:31 PM, 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.
> > +"""
> >
>
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 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
> >
>
> 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)
> > +
> > +               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?

you can turn on the unicode option.

>
>
> > +                               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="-collision-detect -protect-owned"'
> > +                                               emerge_cmd="emerge
> > --verbose --oneshot --complete-graph=y"
> > +                                               os.system('%s %s %s' %
> > (features, emerge_cmd, pkg_name))
> >
>
> This is a security vulnerability :)
>
> -A
>

Yeah, but it is the only way to actually fix a failed merge.  You must
turn off collision protect so it can overwrite the stray files from
the previously failed merge.  There often is not a valid CONTENTS file.
Plus it may not be accurate as to the files actually merged.   This
module is for fixing pkgs that fail during the merge phase to the live
filesystem. 




-- 
Brian Dolbec <dolsen>