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 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.)


> +               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
>
>
>