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

[Alec Warner]

On Thu, Mar 13, 2014 at 3:09 PM, Alec Warner <antarus@g.o> wrote:

>
>
>
> On Wed, Mar 12, 2014 at 11:10 PM, Pavel Kazakov <nullishzero@g.o>wrote:
>
>> ---
>>  pym/portage/emaint/main.py                    |   6 +-
>>  pym/portage/emaint/modules/merges/__init__.py |  30 +++
>>  pym/portage/emaint/modules/merges/merges.py   | 281
>> ++++++++++++++++++++++++++
>>  3 files changed, 315 insertions(+), 2 deletions(-)
>>  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/main.py b/pym/portage/emaint/main.py
>> index 6a17027..646883d 100644
>> --- a/pym/portage/emaint/main.py
>> +++ b/pym/portage/emaint/main.py
>> @@ -98,10 +98,11 @@ def module_opts(module_controller, module):
>>  class TaskHandler(object):
>>         """Handles the running of the tasks it is given"""
>>
>> -       def __init__(self, show_progress_bar=True, verbose=True,
>> callback=None):
>> +       def __init__(self, show_progress_bar=True, verbose=True,
>> callback=None, module_output=None):
>>                 self.show_progress_bar = show_progress_bar
>>                 self.verbose = verbose
>>                 self.callback = callback
>> +               self.module_output = module_output
>>                 self.isatty = os.environ.get('TERM') != 'dumb' and
>> sys.stdout.isatty()
>>                 self.progress_bar = ProgressBar(self.isatty,
>> title="Emaint", max_desc_length=27)
>>
>> @@ -124,6 +125,7 @@ class TaskHandler(object):
>>                                 onProgress = None
>>                         kwargs = {
>>                                 'onProgress': onProgress,
>> +                               'module_output': self.module_output,
>>                                 # pass in a copy of the options so a
>> module can not pollute or change
>>                                 # them for other tasks if there is more
>> to do.
>>                                 'options': options.copy()
>> @@ -219,5 +221,5 @@ def emaint_main(myargv):
>>         # need to pass the parser options dict to the modules
>>         # so they are available if needed.
>>         task_opts = options.__dict__
>> -       taskmaster = TaskHandler(callback=print_results)
>> +       taskmaster = TaskHandler(callback=print_results,
>> module_output=sys.stdout)
>>         taskmaster.run_tasks(tasks, func, status, options=task_opts)
>> diff --git a/pym/portage/emaint/modules/merges/__init__.py
>> b/pym/portage/emaint/modules/merges/__init__.py
>> new file mode 100644
>> index 0000000..96ee71b
>> --- /dev/null
>> +++ b/pym/portage/emaint/modules/merges/__init__.py
>> @@ -0,0 +1,30 @@
>> +# 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': {
>> +               'merges': {
>> +                       'name': "merges",
>> +                       'class': "MergesHandler",
>> +                       'description': __doc__,
>> +                       'functions': ['check', 'fix', 'purge'],
>> +                       'func_desc': {
>> +                               'purge': {
>> +                                       'short': '-P', 'long':
>> '--purge-tracker',
>> +                                       'help': 'Removes the list of
>> previously failed merges.' +
>> +                                                       ' WARNING: Only
>> use this option if you plan on' +
>> +                                                       ' manually fixing
>> them or do not want them'
>> +                                                       ' re-installed.',
>> +                                       'status': "Removing %s",
>> +                                       'action': 'store_true',
>> +                                       'func': 'purge'
>> +                               }
>> +                       }
>> +               }
>> +       }
>> +}
>> diff --git a/pym/portage/emaint/modules/merges/merges.py
>> b/pym/portage/emaint/modules/merges/merges.py
>> new file mode 100644
>> index 0000000..a99dad4
>> --- /dev/null
>> +++ b/pym/portage/emaint/modules/merges/merges.py
>> @@ -0,0 +1,281 @@
>> +# Copyright 2005-2014 Gentoo Foundation
>> +# Distributed under the terms of the GNU General Public License v2
>> +
>> +from _emerge.actions import load_emerge_config
>> +
>> +import portage
>> +from portage import os, _unicode_encode
>> +from portage.const import MERGING_IDENTIFIER, PORTAGE_BIN_PATH,
>> PRIVATE_PATH, \
>> +       VDB_PATH
>> +from portage.dep import isvalidatom
>> +
>>
>
> import shutil
> import subprocess
> import sys
> import time
>
> Alphabetical order.
>
>
>>  +import time
>> +import shutil
>> +import sys
>> +import subprocess
>> +
>> +class TrackingFile(object):
>> +       """File for keeping track of failed merges."""
>> +
>> +
>> +       def __init__(self, tracking_path):
>> +               """
>> +               Create a TrackingFile object.
>> +
>> +               @param tracking_path: file path used to keep track of
>> failed merges
>> +               @type tracking_path: String
>> +               """
>> +               self._tracking_path = _unicode_encode(tracking_path)
>> +
>> +
>> +       def save(self, failed_pkgs):
>> +               """
>> +               Save the specified packages that failed to merge.
>> +
>> +               @param failed_pkgs: dictionary of failed packages
>> +               @type failed_pkgs: dict
>> +               """
>> +               tracking_path = self._tracking_path
>>
>
> You don't appear to do any atomic operations or file locking here, what if
> 2 callers are trying to write to the same filename at once?
> Normally we write to a temporary file and perform an atomic rename (which
> prevents this sort of thing.)
>

portage.util.write_atomic can do this for you.

-A


>
>
>> +               with open(tracking_path, 'w') as tracking_file:
>> +                       for pkg, mtime in failed_pkgs.items():
>> +                               tracking_file.write('%s %s\n' % (pkg,
>> mtime))
>> +
>> +
>> +       def load(self):
>> +               """
>> +               Load previously failed merges.
>> +
>> +               @rtype: dict
>> +               @return: dictionary of packages that failed to merge
>> +               """
>>
> +               tracking_path = self._tracking_path
>> +               if not os.path.exists(tracking_path):
>> +                       return {}
>>
>
> if not self.exists():
>   return {}
>
>
>> +               failed_pkgs = {}
>> +               with open(tracking_path, 'r') as tracking_file:
>> +                       for failed_merge in tracking_file:
>> +                               pkg, mtime = failed_merge.strip().split()
>> +                               failed_pkgs[pkg] = mtime
>> +               return failed_pkgs
>> +
>> +
>> +       def exists(self):
>> +               """
>> +               Check if tracking file exists.
>> +
>> +               @rtype: bool
>> +               @return: true if tracking file exists, false otherwise
>> +               """
>> +               return os.path.exists(self._tracking_path)
>> +
>> +
>> +       def purge(self):
>> +               """Delete previously saved tracking file if one exists."""
>> +               if self.exists():
>> +                       os.remove(self._tracking_path)
>> +
>> +
>> +class MergesHandler(object):
>> +       """Handle failed package merges."""
>> +
>> +       short_desc = "Remove failed merges"
>> +
>> +       @staticmethod
>> +       def name():
>> +               return "merges"
>> +
>> +
>> +       def __init__(self):
>> +               """Create MergesHandler object."""
>> +               eroot = portage.settings['EROOT']
>> +               tracking_path = os.path.join(eroot, PRIVATE_PATH,
>> 'failed-merges');
>> +               self._tracking_file = TrackingFile(tracking_path)
>> +               self._vardb_path = os.path.join(eroot, VDB_PATH)
>> +
>> +
>> +       def can_progressbar(self, func):
>> +               return func == 'check'
>> +
>> +
>> +       def _scan(self, onProgress=None):
>> +               """
>> +               Scan the file system for failed merges and return any
>> found.
>> +
>> +               @param onProgress: function to call for updating progress
>> +               @type onProgress: Function
>> +               @rtype: dict
>> +               @return: dictionary of packages that failed to merges
>> +               """
>> +               failed_pkgs = {}
>> +               for cat in os.listdir(self._vardb_path):
>> +                       pkgs_path = os.path.join(self._vardb_path, cat)
>> +                       if not os.path.isdir(pkgs_path):
>> +                               continue
>> +                       pkgs = os.listdir(pkgs_path)
>> +                       maxval = len(pkgs)
>> +                       for i, pkg in enumerate(pkgs):
>> +                               if onProgress:
>> +                                       onProgress(maxval, i+1)
>> +                               if MERGING_IDENTIFIER in pkg:
>> +                                       mtime =
>> int(os.stat(os.path.join(pkgs_path, pkg)).st_mtime)
>> +                                       pkg = os.path.join(cat, pkg)
>> +                                       failed_pkgs[pkg] = mtime
>> +               return failed_pkgs
>> +
>> +
>> +       def _failed_pkgs(self, onProgress=None):
>> +               """
>> +               Return failed packages from both the file system and
>> tracking file.
>> +
>> +               @rtype: dict
>> +               @return: dictionary of packages that failed to merges
>> +               """
>> +               failed_pkgs = self._scan(onProgress)
>>
>
> Perhaps add an __iter__ function to TrackingFile, that implicitly calls
> load?
>
> Then this can be:
> for pkg, mtime in self._tracking_file:
>
> ?
>
> Just a thought.
>
>
>> +               for pkg, mtime in self._tracking_file.load().items():
>> +                       if pkg not in failed_pkgs:
>> +                               failed_pkgs[pkg] = mtime
>> +               return failed_pkgs
>> +
>> +
>> +       def _remove_failed_dirs(self, failed_pkgs):
>> +               """
>> +               Remove the directories of packages that failed to merge.
>> +
>> +               @param failed_pkgs: failed packages whose directories to
>> remove
>> +               @type failed_pkg: dict
>> +               """
>> +               for failed_pkg in failed_pkgs:
>> +                       pkg_path = os.path.join(self._vardb_path,
>> failed_pkg)
>> +                       # delete failed merge directory if it exists (it
>> might not exist
>> +                       # if loaded from tracking file)
>> +                       if os.path.exists(pkg_path):
>> +                               shutil.rmtree(pkg_path)
>> +                       # TODO: try removing package contents to prevent
>> orphaned
>> +                       # files
>>
>
> I don't get this TODO, can you elaborate?
>
>
>>  +
>> +
>> +       def _get_pkg_atoms(self, failed_pkgs, pkg_atoms, pkg_dne):
>> +               """
>> +               Get the package atoms for the specified failed packages.
>> +
>> +               @param failed_pkgs: failed packages to iterate
>> +               @type failed_pkgs: dict
>> +               @param pkg_atoms: append package atoms to this set
>> +               @type pkg_atoms: set
>> +               @param pkg_dne: append any packages atoms that are
>> invalid to this set
>> +               @type pkg_dne: set
>>
>
> Not following what dne means.
>
>
>> +               """
>> +
>> +               emerge_config = load_emerge_config()
>> +               portdb =
>> emerge_config.target_config.trees['porttree'].dbapi
>> +               for failed_pkg in failed_pkgs:
>> +                       # validate pkg name
>> +                       pkg_name = '%s' %
>> failed_pkg.replace(MERGING_IDENTIFIER, '')
>> +                       pkg_atom = '=%s' % pkg_name
>> +
>> +                       if not isvalidatom(pkg_atom):
>> +                               pkg_dne.append( "'%s' is an invalid
>> package atom." % pkg_atom)
>> +                       if not portdb.cpv_exists(pkg_name):
>> +                               pkg_dne.append( "'%s' does not exist in
>> the portage tree."
>> +                                       % pkg_name)
>> +                       pkg_atoms.add(pkg_atom)
>> +
>> +
>> +       def _emerge_pkg_atoms(self, module_output, pkg_atoms):
>> +               """
>> +               Emerge the specified packages atoms.
>> +
>> +               @param module_output: output will be written to
>> +               @type module_output: Class
>> +               @param pkg_atoms: packages atoms to emerge
>> +               @type pkg_atoms: set
>> +               @rtype: list
>> +               @return: List of results
>> +               """
>> +               # TODO: rewrite code to use portage's APIs instead of a
>> subprocess
>> +               env = {
>> +                       "FEATURES" : "-collision-detect -protect-owned",
>> +                       "PATH" : os.environ["PATH"]
>> +               }
>> +               emerge_cmd = (
>> +                       portage._python_interpreter,
>> +                       '-b',
>> +                       os.path.join(PORTAGE_BIN_PATH, 'emerge'),
>> +                       '--quiet',
>> +                       '--oneshot',
>> +                       '--complete-graph=y'
>> +               )
>> +               results = []
>> +               msg = 'Re-Emerging packages that failed to merge...\n'
>> +               if module_output:
>> +                       module_output.write(msg)
>> +               else:
>> +                       module_output = subprocess.PIPE
>> +                       results.append(msg)
>> +               proc = subprocess.Popen(emerge_cmd + tuple(pkg_atoms),
>> env=env,
>> +                       stdout=module_output, stderr=sys.stderr)
>> +               output = proc.communicate()[0]
>>
>
> This seems sort of weird, perhaps:
>
> output, _ = proc.communicate()
>
>
>>  +               if output:
>> +                       results.append(output)
>> +               if proc.returncode != os.EX_OK:
>> +                        emerge_status = "Failed to emerge '%s'" % ('
>> '.join(pkg_atoms))
>> +               else:
>> +                        emerge_status = "Successfully emerged '%s'" % ('
>> '.join(pkg_atoms))
>> +               results.append(emerge_status)
>> +               return results
>> +
>> +
>> +       def check(self, **kwargs):
>> +               """Check for failed merges."""
>> +               onProgress = kwargs.get('onProgress', None)
>> +               failed_pkgs = self._failed_pkgs(onProgress)
>> +               errors = []
>> +               for pkg, mtime in failed_pkgs.items():
>> +                       mtime_str = time.ctime(int(mtime))
>> +                       errors.append("'%s' failed to merge on '%s'" %
>> (pkg, mtime_str))
>> +               return errors
>> +
>> +
>> +       def fix(self, **kwargs):
>> +               """Attempt to fix any failed merges."""
>> +               module_output = kwargs.get('module_output', None)
>> +               failed_pkgs = self._failed_pkgs()
>> +               if not failed_pkgs:
>> +                       return [ 'No failed merges found. ' ]
>>
>
> Less spacing here:
> return ['No failed merges found.']
>
>
>>  +
>> +               pkg_dne = set()
>> +               pkg_atoms = set()
>> +               self._get_pkg_atoms(failed_pkgs, pkg_atoms, pkg_dne)
>> +               if pkg_dne:
>> +                       return pkg_dne
>> +
>> +               try:
>> +                       self._tracking_file.save(failed_pkgs)
>> +               except IOError as ex:
>> +                       errors = [ 'Unable to save failed merges to
>> tracking file: %s\n'
>> +                               % str(ex) ]
>>
>
> Same here, no spaces before or after [ and ].
>
>
>>  +                       errors.append(', '.join(sorted(failed_pkgs)))
>> +                       return errors
>> +               self._remove_failed_dirs(failed_pkgs)
>> +               results = self._emerge_pkg_atoms(module_output, pkg_atoms)
>> +               # list any new failed merges
>> +               for pkg in sorted(self._scan()):
>> +                       results.append("'%s' still found as a failed
>> merge." % pkg)
>> +               # reload config and remove successful packages from
>> tracking file
>> +               emerge_config = load_emerge_config()
>> +               vardb = emerge_config.target_config.trees['vartree'].dbapi
>> +               still_failed_pkgs = {}
>> +               for pkg, mtime in failed_pkgs.items():
>> +                       pkg_name = '%s' % pkg.replace(MERGING_IDENTIFIER,
>> '')
>> +                       if not vardb.cpv_exists(pkg_name):
>> +                               still_failed_pkgs[pkg] = mtime
>> +               self._tracking_file.save(still_failed_pkgs)
>> +               return results
>> +
>> +
>> +       def purge(self, **kwargs):
>> +               """Attempt to remove previously saved tracking file."""
>> +               if not self._tracking_file.exists():
>> +                       return ['Tracking file not found.']
>> +               self._tracking_file.purge()
>> +               return ['Removed tracking file.']
>> --
>> 1.8.3.2
>>
>>
>>
>