public inbox for gentoo-portage-dev@lists.gentoo.org
 help / color / mirror / Atom feed
* [gentoo-portage-dev] [PATCH] Add an emaint module that can scan for failed merges and that can fix failed merges.
@ 2014-02-19 20:31 Pavel Kazakov
  2014-02-19 22:32 ` Alec Warner
                   ` (2 more replies)
  0 siblings, 3 replies; 31+ messages in thread
From: Pavel Kazakov @ 2014-02-19 20:31 UTC (permalink / raw
  To: gentoo-portage-dev; +Cc: 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 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))
-- 
1.8.3.2



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [gentoo-portage-dev] [PATCH] Add an emaint module that can scan for failed merges and that can fix failed merges.
  2014-02-19 20:31 [gentoo-portage-dev] [PATCH] Add an emaint module that can scan for failed merges and that can fix failed merges Pavel Kazakov
@ 2014-02-19 22:32 ` Alec Warner
  2014-02-19 23:20   ` Brian Dolbec
  2014-02-20  0:08   ` Pavel Kazakov
  2014-02-19 22:50 ` Brian Dolbec
  2014-02-22  0:34 ` [gentoo-portage-dev] [PATCH v2] " Pavel Kazakov
  2 siblings, 2 replies; 31+ messages in thread
From: Alec Warner @ 2014-02-19 22:32 UTC (permalink / raw
  To: gentoo-portage-dev; +Cc: Pavel Kazakov

[-- Attachment #1: Type: text/plain, Size: 5469 bytes --]

On Wed, Feb 19, 2014 at 12:31 PM, Pavel Kazakov <nullishzero@gentoo.org>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.
> +"""
>

"""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 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?

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


> +               except Exception as ex:
> +                       writemsg('Unable to fix failed package: %s' %
> str(ex))
> --
> 1.8.3.2
>
>
>

[-- Attachment #2: Type: text/html, Size: 8954 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [gentoo-portage-dev] [PATCH] Add an emaint module that can scan for failed merges and that can fix failed merges.
  2014-02-19 20:31 [gentoo-portage-dev] [PATCH] Add an emaint module that can scan for failed merges and that can fix failed merges Pavel Kazakov
  2014-02-19 22:32 ` Alec Warner
@ 2014-02-19 22:50 ` Brian Dolbec
  2014-02-19 23:37   ` Alec Warner
  2014-02-20  0:14   ` Pavel Kazakov
  2014-02-22  0:34 ` [gentoo-portage-dev] [PATCH v2] " Pavel Kazakov
  2 siblings, 2 replies; 31+ messages in thread
From: Brian Dolbec @ 2014-02-19 22:50 UTC (permalink / raw
  To: gentoo-portage-dev

On Wed, 19 Feb 2014 12:31:44 -0800
Pavel Kazakov <nullishzero@gentoo.org> 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...

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>



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [gentoo-portage-dev] [PATCH] Add an emaint module that can scan for failed merges and that can fix failed merges.
  2014-02-19 22:32 ` Alec Warner
@ 2014-02-19 23:20   ` Brian Dolbec
  2014-02-19 23:34     ` Alec Warner
  2014-02-20  0:08   ` Pavel Kazakov
  1 sibling, 1 reply; 31+ messages in thread
From: Brian Dolbec @ 2014-02-19 23:20 UTC (permalink / raw
  To: gentoo-portage-dev

On Wed, 19 Feb 2014 14:32:02 -0800
Alec Warner <antarus@gentoo.org> wrote:

> On Wed, Feb 19, 2014 at 12:31 PM, Pavel Kazakov <nullishzero@gentoo.org>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>



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [gentoo-portage-dev] [PATCH] Add an emaint module that can scan for failed merges and that can fix failed merges.
  2014-02-19 23:20   ` Brian Dolbec
@ 2014-02-19 23:34     ` Alec Warner
  0 siblings, 0 replies; 31+ messages in thread
From: Alec Warner @ 2014-02-19 23:34 UTC (permalink / raw
  To: gentoo-portage-dev

[-- Attachment #1: Type: text/plain, Size: 7927 bytes --]

On Wed, Feb 19, 2014 at 3:20 PM, Brian Dolbec <dolsen@gentoo.org> wrote:

> On Wed, 19 Feb 2014 14:32:02 -0800
> Alec Warner <antarus@gentoo.org> wrote:
>
> > On Wed, Feb 19, 2014 at 12:31 PM, Pavel Kazakov <nullishzero@gentoo.org
> >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
>

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

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)
> > > +
> > > +                               if '-MERGING-' in pkg:
> > > +                                       yield cat + os.path.sep + pkg
>

os.path.join here as well.


> > > +
> > > +       def check(self, **kwargs):
> > > +               onProgress = kwargs.get('onProgress', None)
> > > +               failed_pkgs = []
> > > +               for pkg in self._failed_packages(onProgress):
> > > +                       failed_pkgs.append(pkg)
>

I think we want a set() here, not a list.


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

Thats not what I meant ;)


>
>
>
>
> --
> Brian Dolbec <dolsen>
>
>
>

[-- Attachment #2: Type: text/html, Size: 12139 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [gentoo-portage-dev] [PATCH] Add an emaint module that can scan for failed merges and that can fix failed merges.
  2014-02-19 22:50 ` Brian Dolbec
@ 2014-02-19 23:37   ` Alec Warner
  2014-02-20  0:14   ` Pavel Kazakov
  1 sibling, 0 replies; 31+ messages in thread
From: Alec Warner @ 2014-02-19 23:37 UTC (permalink / raw
  To: gentoo-portage-dev

[-- Attachment #1: Type: text/plain, Size: 6406 bytes --]

On Wed, Feb 19, 2014 at 2:50 PM, Brian Dolbec <dolsen@gentoo.org> wrote:

> On Wed, 19 Feb 2014 12:31:44 -0800
> Pavel Kazakov <nullishzero@gentoo.org> 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>
>
>
>

[-- Attachment #2: Type: text/html, Size: 8880 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [gentoo-portage-dev] [PATCH] Add an emaint module that can scan for failed merges and that can fix failed merges.
  2014-02-19 22:32 ` Alec Warner
  2014-02-19 23:20   ` Brian Dolbec
@ 2014-02-20  0:08   ` Pavel Kazakov
  2014-02-20 10:58     ` Alexander Berntsen
  1 sibling, 1 reply; 31+ messages in thread
From: Pavel Kazakov @ 2014-02-20  0:08 UTC (permalink / raw
  To: gentoo-portage-dev

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 <nullishzero@gentoo.org
> <mailto:nullishzero@gentoo.org>> 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.
>     +"""
> 
> 
> """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 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?
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
>     +
>     +       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?
I can do a unicode encode to make sure it is.
>  
> 
>     +                               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
Fair enough, I'll switch over to TODO.
> 
> 
>  
> 
>     +                                               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 :)
Yikes ;/
> 
> -A
>  
> 
>     +               except Exception as ex:
>     +                       writemsg('Unable to fix failed package: %s'
>     % str(ex))
>     --
>     1.8.3.2
> 
>
>

Regards,
Pavel




^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [gentoo-portage-dev] [PATCH] Add an emaint module that can scan for failed merges and that can fix failed merges.
  2014-02-19 22:50 ` Brian Dolbec
  2014-02-19 23:37   ` Alec Warner
@ 2014-02-20  0:14   ` Pavel Kazakov
  1 sibling, 0 replies; 31+ messages in thread
From: Pavel Kazakov @ 2014-02-20  0:14 UTC (permalink / raw
  To: gentoo-portage-dev

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



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [gentoo-portage-dev] [PATCH] Add an emaint module that can scan for failed merges and that can fix failed merges.
  2014-02-20  0:08   ` Pavel Kazakov
@ 2014-02-20 10:58     ` Alexander Berntsen
  2014-02-20 13:58       ` Brian Dolbec
  2014-02-20 20:10       ` Pavel Kazakov
  0 siblings, 2 replies; 31+ messages in thread
From: Alexander Berntsen @ 2014-02-20 10:58 UTC (permalink / raw
  To: gentoo-portage-dev

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 20/02/14 01:08, Pavel Kazakov wrote:
>> """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?
What the other modules are doing is not necessarily an indication of
what we should do. More often than not, they are the opposite of what
we should do, and following them because "hey it might be terrible,
but at least it's consistently terrible" isn't fruitful. So I agree
with Alec.

If you want consistency, feel free to fix the other docstrings. ;-)

>> 'module1' ?
> All the other modules did it this way, so I wanted consistency.
Same.

>> 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.
Arfrever has apparently inserted this kind of stuff a bunch of places.
We should figure out a more systematic approach... Anyway, if you
don't even need it, get rid of it.
- -- 
Alexander
bernalex@gentoo.org
https://secure.plaimi.net/~alexander
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iF4EAREIAAYFAlMF39gACgkQRtClrXBQc7VSUgD/UpF2Yrb/R72yAv8e9jIV9gj4
GYS+XKL8vTqb3EZr6+AA+wV/D6VQHOA7fHpW/ijaxfY+LYfMpcDxXEgZZXvqoB7C
=xrzm
-----END PGP SIGNATURE-----


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [gentoo-portage-dev] [PATCH] Add an emaint module that can scan for failed merges and that can fix failed merges.
  2014-02-20 10:58     ` Alexander Berntsen
@ 2014-02-20 13:58       ` Brian Dolbec
  2014-02-20 14:09         ` Alexander Berntsen
  2014-02-20 20:10       ` Pavel Kazakov
  1 sibling, 1 reply; 31+ messages in thread
From: Brian Dolbec @ 2014-02-20 13:58 UTC (permalink / raw
  To: gentoo-portage-dev

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On Thu, 20 Feb 2014 11:58:32 +0100
Alexander Berntsen <bernalex@gentoo.org> wrote:

> > 
> If you want consistency, feel free to fix the other docstrings. ;-)
> 
> >> 'module1' ?
> > All the other modules did it this way, so I wanted consistency.
> Same.
> 
>> - -- 
> Alexander
> 

Alexander, did you NOT read my other reply about 'module1'?

While the name is not important, it is a requirement to have a
placeholder identifier of some sort.  The plug-in system requires it.
There must be a way for the module_spec to group the individual modules
that a plug-in module can provide.  While most modules only supply 1
command.  A plug-in module can in fact supply many module
commands, capabilities.

What if a module supplied 10 different command capabilities?
What do you propose as a grouping name method? It must be unique to
the plug-in module. It could be a duplicate of the name field it
contains, but then someone is liable to say that it then doesn't need
the name field.  But it really does, it must pass along all the data
that the sub-module currently has within it.

Actually, the plug-in manager, nor the emaint controller never actually
displays it.  So it can be an sha hash just like git uses for
commits.  So, please suggest something.

Here is an real module spec example:

#!/usr/bin/python -O
# vim: noet :
#
# Copyright 2010 Gentoo Technologies, Inc.
# Distributed under the terms of the GNU General Public License v2 or later
#
# $Header$

"""'This emaint module provides checks and maintenance for:
  1) "Performing package move updates for installed packages",
  2)"Perform package move updates for binary packages"
"""


module_spec = {
	'name': 'move',
	'description': "Provides functions to check for and move packages " +\
		"either installed or binary packages stored on this system",
	'provides':{
		'module1': {
			'name': "moveinst",
			'class': "MoveInstalled",
			'description': "Perform package move updates for installed packages",
			'options': ['check', 'fix'],
			'functions': ['check', 'fix'],
			'func_desc': {
				}
			},
		'module2':{
			'name': "movebin",
			'class': "MoveBinary",
			'description': "Perform package move updates for binary packages",
			'functions': ['check', 'fix'],
			'func_desc': {
				}
			}
		}
	}


- ------------------------
/* Sorry for the rant */

Pavel is trying to learn the plug-in system, so that he can better help
with the new plug-in sync system.  So doing what other modules did is
not a crime.
- -- 
Brian Dolbec <dolsen>

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQF8BAEBCgBmBQJTBgoIXxSAAAAAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25zLm9w
ZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXQ4Njg4RkQxQ0M3MUMxQzA0RUFFQTQyMzcy
MjE0RDkwQTAxNEYxN0NCAAoJECIU2QoBTxfLsnsH/RpPMwSY9t6kpJwv37v/JSBX
thk0jBikUytW2OmbyuASilEb5VQwL9sYpCcGK0EHgMCaUDwm/GhK3L1k6Qlzwf4i
zSPX3cnKB5EU4OuXY5B+8FuOn1U2MJz6TAlArCvMRTXe/pwbnaVC8kXJctvZCCy4
ISblnLAYyxH0Uun1BdPy9zFjBuwHc1G9ZKZtNBIvaoIKHlQArtUwwSxr6DIt1cG4
X6WEEs+ZJrsAD+SbjxpqvGmmSP8NDXfeiJOUVKJ6PIJybnCZHZVZnwHdd5tHKBjY
Fsx3yKzjOGCM1fN950XASHG6TNAW9nvGjRXBjMdVOZ0LImBR3Y6Q6ewqnlEd2S4=
=gHPH
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [gentoo-portage-dev] [PATCH] Add an emaint module that can scan for failed merges and that can fix failed merges.
  2014-02-20 13:58       ` Brian Dolbec
@ 2014-02-20 14:09         ` Alexander Berntsen
  0 siblings, 0 replies; 31+ messages in thread
From: Alexander Berntsen @ 2014-02-20 14:09 UTC (permalink / raw
  To: gentoo-portage-dev

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 20/02/14 14:58, Brian Dolbec wrote:
> Alexander, did you NOT read my other reply about 'module1'?
Obviously not closely enough. A bit tired today. :-)

> Pavel is trying to learn the plug-in system, so that he can better 
> help with the new plug-in sync system.  So doing what other
> modules did is not a crime.
Not at all. I'm just saying he should not assume that what they are
doing is necessarily correct. He has the right attitude in asking the
list for feedback on such matters rather than blindly accepting the
old src as Testament. :-)
- -- 
Alexander
bernalex@gentoo.org
https://secure.plaimi.net/~alexander
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iF4EAREIAAYFAlMGDKwACgkQRtClrXBQc7UzLQD/cRDGJ7p5TZa7eDmMiGJu/qA9
NDYZXFikQQbyX2ZZ3hgA/iuutx/BEXx1SMJj4T1Ym2hCgrdX1KD74hY1faHt/AEy
=3iIe
-----END PGP SIGNATURE-----


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [gentoo-portage-dev] [PATCH] Add an emaint module that can scan for failed merges and that can fix failed merges.
  2014-02-20 10:58     ` Alexander Berntsen
  2014-02-20 13:58       ` Brian Dolbec
@ 2014-02-20 20:10       ` Pavel Kazakov
  1 sibling, 0 replies; 31+ messages in thread
From: Pavel Kazakov @ 2014-02-20 20:10 UTC (permalink / raw
  To: gentoo-portage-dev

On 02/20/2014 02:58 AM, Alexander Berntsen wrote:
> On 20/02/14 01:08, Pavel Kazakov wrote:
>>> """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?
> What the other modules are doing is not necessarily an indication 
> of what we should do. More often than not, they are the opposite
> of what we should do, and following them because "hey it might be 
> terrible, but at least it's consistently terrible" isn't fruitful. 
> So I agree with Alec.
> 
> If you want consistency, feel free to fix the other docstrings. 
> ;-)

I agree that consistency for its own sake, if dealing with bad code,
isn't very fruitful, but I do think consistency matters when dealing
with style conventions. As far as I can tell, having """ on a newline
is purely a style choice and doesn't have any impact on how the code
functions. So for cases where style might be ambiguous, I choose
what's already there.

You could argue that PEP 257 recommends having a one-liner include the
""" on the same line, but the portage project doesn't exactly follow
all PEPs anyway ;)

Regards,
Pavel


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [gentoo-portage-dev] [PATCH v2] Add an emaint module that can scan for failed merges and that can fix failed merges.
  2014-02-19 20:31 [gentoo-portage-dev] [PATCH] Add an emaint module that can scan for failed merges and that can fix failed merges Pavel Kazakov
  2014-02-19 22:32 ` Alec Warner
  2014-02-19 22:50 ` Brian Dolbec
@ 2014-02-22  0:34 ` Pavel Kazakov
  2014-02-22 18:59   ` Brian Dolbec
  2014-03-13  6:10   ` [gentoo-portage-dev] [PATCH 1/3] Move -MERGING- string to a constant variable Pavel Kazakov
  2 siblings, 2 replies; 31+ messages in thread
From: Pavel Kazakov @ 2014-02-22  0:34 UTC (permalink / raw
  To: gentoo-portage-dev; +Cc: Pavel Kazakov

---
 pym/portage/emaint/main.py                    |   6 +-
 pym/portage/emaint/modules/merges/__init__.py |  19 ++++
 pym/portage/emaint/modules/merges/merges.py   | 121 ++++++++++++++++++++++++++
 3 files changed, 144 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 a03ffd7..c1e0c0b 100644
--- a/pym/portage/emaint/main.py
+++ b/pym/portage/emaint/main.py
@@ -95,10 +95,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)
 
@@ -121,6 +122,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()
@@ -215,5 +217,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..638d3da
--- /dev/null
+++ b/pym/portage/emaint/modules/merges/__init__.py
@@ -0,0 +1,19 @@
+# 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',],
+			'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..f03d9c4
--- /dev/null
+++ b/pym/portage/emaint/modules/merges/merges.py
@@ -0,0 +1,121 @@
+# Copyright 2005-2014 Gentoo Foundation
+# Distributed under the terms of the GNU General Public License v2
+
+import portage
+from portage import os, _unicode_encode
+from portage.const import PORTAGE_BIN_PATH, PRIVATE_PATH, VDB_PATH
+from portage.dep import isvalidatom
+
+import shutil
+import sys
+import subprocess
+
+class MergesHandler(object):
+
+	short_desc = "Remove failed merges"
+
+	@staticmethod
+	def name():
+		return "merges"
+
+
+	def __init__(self):
+		self._eroot = portage.settings['EROOT']
+		self._vardb_path = os.path.join(self._eroot, VDB_PATH)
+
+
+	def can_progressbar(self, func):
+		if func == 'fix':
+			return False
+		return True
+
+
+	def _failed_packages(self, onProgress=None):
+		for cat in os.listdir(self._vardb_path):
+			pkgs = os.listdir(os.path.join(self._vardb_path, cat))
+			maxval = len(pkgs)
+			for i, pkg in enumerate(pkgs):
+				if onProgress:
+					onProgress(maxval, i+1)
+
+				if '-MERGING-' in pkg:
+					yield os.path.join(cat, pkg)
+
+
+	def check(self, **kwargs):
+		onProgress = kwargs.get('onProgress', None)
+		failed_pkgs = set(self._failed_packages(onProgress))
+		errors = ["'%s' failed to merge." % x for x in failed_pkgs]
+		return errors
+
+
+	def fix(self, **kwargs):
+		module_output = kwargs.get('module_output', None)
+		failed_pkgs = set(self._failed_packages())
+		if not failed_pkgs:
+			return [ 'No failed merges found. ' ]
+
+		tracking_path = os.path.join(self._eroot, PRIVATE_PATH, 'failed-merges');
+		try:
+			with open(_unicode_encode(tracking_path), 'w') as tracking_file:
+				tracking_file.write('\n'.join(failed_pkgs))
+		except IOError as ex:
+			errors = [ 'Unable to save failed merges to tracking file: %s\n'
+				% str(ex) ]
+			errors.append(', '.join(failed_pkgs))
+
+			return errors
+
+		pkg_dne = []
+		pkg_atoms = set()
+		for failed_pkg in failed_pkgs:
+			# validate pkg name
+			pkg_name = '%s' % failed_pkg.replace('-MERGING-', '')
+			pkg_atom = '=%s' % pkg_name
+			if not isvalidatom(pkg_atom):
+				pkg_dne.append( "'%s' is an invalid package atom." % pkg_atom)
+			if not portage.portdb.cpv_exists(pkg_name):
+				pkg_dne.append( "'%s' does not exist in the portage tree."
+					% pkg_name)
+			pkg_atoms.add(pkg_atom)
+		if pkg_dne:
+			return pkg_dne
+
+		for failed_pkg in failed_pkgs:
+			pkg_path = os.path.join(self._vardb_path, failed_pkg)
+			# delete failed merge directory
+			shutil.rmtree(pkg_path)
+			# TODO: try removing package contents to prevent orphaned
+			# files
+
+		# 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]
+		if output:
+			results.append(output)
+		if proc.returncode != os.EX_OK:
+			 finish = "Failed to emerge '%s'" % (' '.join(pkg_atoms))
+		else:
+			 finish = "Successfully emerged '%s'" % (' '.join(pkg_atoms))
+		results.append(finish)
+		return results
-- 
1.8.3.2



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [gentoo-portage-dev] [PATCH v2] Add an emaint module that can scan for failed merges and that can fix failed merges.
  2014-02-22  0:34 ` [gentoo-portage-dev] [PATCH v2] " Pavel Kazakov
@ 2014-02-22 18:59   ` Brian Dolbec
  2014-03-13  6:10   ` [gentoo-portage-dev] [PATCH 1/3] Move -MERGING- string to a constant variable Pavel Kazakov
  1 sibling, 0 replies; 31+ messages in thread
From: Brian Dolbec @ 2014-02-22 18:59 UTC (permalink / raw
  To: gentoo-portage-dev

On Fri, 21 Feb 2014 16:34:47 -0800
Pavel Kazakov <nullishzero@gentoo.org> wrote:

> ---
>  pym/portage/emaint/main.py                    |   6 +-
>  pym/portage/emaint/modules/merges/__init__.py |  19 ++++
>  pym/portage/emaint/modules/merges/merges.py   | 121 ++++++++++++++++++++++++++
>  3 files changed, 144 insertions(+), 2 deletions(-)
>  create mode 100644 pym/portage/emaint/modules/merges/__init__.py
>  create mode 100644 pym/portage/emaint/modules/merges/merges.py
> 

...

> +		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]
> +		if output:
> +			results.append(output)
> +		if proc.returncode != os.EX_OK:
> +			 finish = "Failed to emerge '%s'" % (' '.join(pkg_atoms))
> +		else:
> +			 finish = "Successfully emerged '%s'" % (' '.join(pkg_atoms))
> +		results.append(finish)
> +		return results

With IRC being crapped out with netsplits...

Sorry, but I didn't notice this until reviewing this v2 submission.
Even though I gave you the thumbs up from the last paste.  This code is
not cleaning up the tracking file used for safety backup. What' more, a
second run will trounce the first file saved, even if emerge totally
failed in the first.

So, it needs this additional work, to be complete (sorry):

1) check for the existence of the tracking file in both 
   check() and fix()
2) add those pkgs to the list reported in check()
3) add them to a fix() run.
4) at the end of the emerge run in fix(), re-scan for -MERGING- pkgs
   again. list any new ones.
5) reload portage for it to see the new vardb
6) check for the pkgs re-emerged to ensure they were successful 
   ie: vardb = ...  
       vardb.cpv_exists(pkg)
7) remove any sucessful pkgs from the tracking file.

8) I also think we should add the date/time info when they ere
added to the tracking file and cleaned from the filesystem

P.S. I can help with this if you like.

-- 
Brian Dolbec <dolsen>



^ permalink raw reply	[flat|nested] 31+ messages in thread

* [gentoo-portage-dev] [PATCH 1/3] Move -MERGING- string to a constant variable.
  2014-02-22  0:34 ` [gentoo-portage-dev] [PATCH v2] " Pavel Kazakov
  2014-02-22 18:59   ` Brian Dolbec
@ 2014-03-13  6:10   ` Pavel Kazakov
  2014-03-13  6:10     ` [gentoo-portage-dev] [PATCH 2/3] Remove extra whitespace Pavel Kazakov
                       ` (3 more replies)
  1 sibling, 4 replies; 31+ messages in thread
From: Pavel Kazakov @ 2014-03-13  6:10 UTC (permalink / raw
  To: gentoo-portage-dev; +Cc: Pavel Kazakov

---
 pym/portage/const.py          | 1 +
 pym/portage/dbapi/__init__.py | 4 +++-
 pym/portage/dbapi/vartree.py  | 8 ++++----
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/pym/portage/const.py b/pym/portage/const.py
index 1785bff..fad69ef 100644
--- a/pym/portage/const.py
+++ b/pym/portage/const.py
@@ -74,6 +74,7 @@ MOVE_BINARY              = "/bin/mv"
 PRELINK_BINARY           = "/usr/sbin/prelink"
 
 INVALID_ENV_FILE         = "/etc/spork/is/not/valid/profile.env"
+MERGING_IDENTIFIER       = "-MERGING-"	
 REPO_NAME_FILE           = "repo_name"
 REPO_NAME_LOC            = "profiles" + "/" + REPO_NAME_FILE
 
diff --git a/pym/portage/dbapi/__init__.py b/pym/portage/dbapi/__init__.py
index a20a1e8..6638352 100644
--- a/pym/portage/dbapi/__init__.py
+++ b/pym/portage/dbapi/__init__.py
@@ -16,6 +16,8 @@ portage.proxy.lazyimport.lazyimport(globals(),
 	'portage.versions:catsplit,catpkgsplit,vercmp,_pkg_str',
 )
 
+from portage.const import MERGING_IDENTIFIER
+
 from portage import os
 from portage import auxdbkeys
 from portage.eapi import _get_eapi_attrs
@@ -278,7 +280,7 @@ class dbapi(object):
 		return True
 
 	def invalidentry(self, mypath):
-		if '/-MERGING-' in mypath:
+		if MERGING_IDENTIFIER in mypath:
 			if os.path.exists(mypath):
 				writemsg(colorize("BAD", _("INCOMPLETE MERGE:"))+" %s\n" % mypath,
 					noiselevel=-1)
diff --git a/pym/portage/dbapi/vartree.py b/pym/portage/dbapi/vartree.py
index 6417a56..5f5f5ce 100644
--- a/pym/portage/dbapi/vartree.py
+++ b/pym/portage/dbapi/vartree.py
@@ -45,7 +45,7 @@ portage.proxy.lazyimport.lazyimport(globals(),
 )
 
 from portage.const import CACHE_PATH, CONFIG_MEMORY_FILE, \
-	PORTAGE_PACKAGE_ATOM, PRIVATE_PATH, VDB_PATH
+	MERGING_IDENTIFIER, PORTAGE_PACKAGE_ATOM, PRIVATE_PATH, VDB_PATH
 from portage.dbapi import dbapi
 from portage.exception import CommandNotFound, \
 	InvalidData, InvalidLocation, InvalidPackageName, \
@@ -104,7 +104,7 @@ class vardbapi(dbapi):
 
 	_excluded_dirs = ["CVS", "lost+found"]
 	_excluded_dirs = [re.escape(x) for x in _excluded_dirs]
-	_excluded_dirs = re.compile(r'^(\..*|-MERGING-.*|' + \
+	_excluded_dirs = re.compile(r'^(\..*|' + MERGING_IDENTIFIER + '.*|' + \
 		"|".join(_excluded_dirs) + r')$')
 
 	_aux_cache_version        = "1"
@@ -446,7 +446,7 @@ class vardbapi(dbapi):
 				if self._excluded_dirs.match(y) is not None:
 					continue
 				subpath = x + "/" + y
-				# -MERGING- should never be a cpv, nor should files.
+				# MERGING_IDENTIFIER should never be a cpv, nor should files.
 				try:
 					if catpkgsplit(subpath) is None:
 						self.invalidentry(self.getpath(subpath))
@@ -1504,7 +1504,7 @@ class dblink(object):
 		self.dbroot = normalize_path(os.path.join(self._eroot, VDB_PATH))
 		self.dbcatdir = self.dbroot+"/"+cat
 		self.dbpkgdir = self.dbcatdir+"/"+pkg
-		self.dbtmpdir = self.dbcatdir+"/-MERGING-"+pkg
+		self.dbtmpdir = self.dbcatdir+MERGING_IDENTIFIER+pkg
 		self.dbdir = self.dbpkgdir
 		self.settings = mysettings
 		self._verbose = self.settings.get("PORTAGE_VERBOSE") == "1"
-- 
1.8.3.2



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [gentoo-portage-dev] [PATCH 2/3] Remove extra whitespace.
  2014-03-13  6:10   ` [gentoo-portage-dev] [PATCH 1/3] Move -MERGING- string to a constant variable Pavel Kazakov
@ 2014-03-13  6:10     ` Pavel Kazakov
  2014-03-13  6:10     ` [gentoo-portage-dev] [PATCH 3/3] Add an emaint module that can scan for failed merges and that can fix failed merges Pavel Kazakov
                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 31+ messages in thread
From: Pavel Kazakov @ 2014-03-13  6:10 UTC (permalink / raw
  To: gentoo-portage-dev; +Cc: Pavel Kazakov

---
 pym/portage/const.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pym/portage/const.py b/pym/portage/const.py
index fad69ef..50f0719 100644
--- a/pym/portage/const.py
+++ b/pym/portage/const.py
@@ -74,7 +74,7 @@ MOVE_BINARY              = "/bin/mv"
 PRELINK_BINARY           = "/usr/sbin/prelink"
 
 INVALID_ENV_FILE         = "/etc/spork/is/not/valid/profile.env"
-MERGING_IDENTIFIER       = "-MERGING-"	
+MERGING_IDENTIFIER       = "-MERGING-"
 REPO_NAME_FILE           = "repo_name"
 REPO_NAME_LOC            = "profiles" + "/" + REPO_NAME_FILE
 
-- 
1.8.3.2



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [gentoo-portage-dev] [PATCH 3/3] Add an emaint module that can scan for failed merges and that can fix failed merges.
  2014-03-13  6:10   ` [gentoo-portage-dev] [PATCH 1/3] Move -MERGING- string to a constant variable Pavel Kazakov
  2014-03-13  6:10     ` [gentoo-portage-dev] [PATCH 2/3] Remove extra whitespace Pavel Kazakov
@ 2014-03-13  6:10     ` Pavel Kazakov
  2014-03-13 22:09       ` Alec Warner
  2014-03-13  7:18     ` [gentoo-portage-dev] [PATCH 1/3] Move -MERGING- string to a constant variable Pavel Kazakov
  2014-03-31  1:24     ` [gentoo-portage-dev] [PATCH v2 1/2] Move -MERGING- string to a constant Pavel Kazakov
  3 siblings, 1 reply; 31+ messages in thread
From: Pavel Kazakov @ 2014-03-13  6:10 UTC (permalink / raw
  To: gentoo-portage-dev; +Cc: Pavel Kazakov

---
 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 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
+		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 {}
+		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)
+		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
+
+
+	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
+		"""
+
+		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]
+		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. ' ]
+
+		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) ]
+			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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [gentoo-portage-dev] [PATCH 1/3] Move -MERGING- string to a constant variable.
  2014-03-13  6:10   ` [gentoo-portage-dev] [PATCH 1/3] Move -MERGING- string to a constant variable Pavel Kazakov
  2014-03-13  6:10     ` [gentoo-portage-dev] [PATCH 2/3] Remove extra whitespace Pavel Kazakov
  2014-03-13  6:10     ` [gentoo-portage-dev] [PATCH 3/3] Add an emaint module that can scan for failed merges and that can fix failed merges Pavel Kazakov
@ 2014-03-13  7:18     ` Pavel Kazakov
  2014-03-31  1:24     ` [gentoo-portage-dev] [PATCH v2 1/2] Move -MERGING- string to a constant Pavel Kazakov
  3 siblings, 0 replies; 31+ messages in thread
From: Pavel Kazakov @ 2014-03-13  7:18 UTC (permalink / raw
  To: gentoo-portage-dev

On 03/12/14 23:10, Pavel Kazakov wrote:
> ---
>  pym/portage/const.py          | 1 +
>  pym/portage/dbapi/__init__.py | 4 +++-
>  pym/portage/dbapi/vartree.py  | 8 ++++----
>  3 files changed, 8 insertions(+), 5 deletions(-)
...
> @@ -1504,7 +1504,7 @@ class dblink(object):
>  		self.dbroot = normalize_path(os.path.join(self._eroot, VDB_PATH))
>  		self.dbcatdir = self.dbroot+"/"+cat
>  		self.dbpkgdir = self.dbcatdir+"/"+pkg
> -		self.dbtmpdir = self.dbcatdir+"/-MERGING-"+pkg
> +		self.dbtmpdir = self.dbcatdir+MERGING_IDENTIFIER+pkg
>  		self.dbdir = self.dbpkgdir
>  		self.settings = mysettings
>  		self._verbose = self.settings.get("PORTAGE_VERBOSE") == "1"

Just a quick update (don't want to re-submit the patches): I've squashed
patches 1 and 2, and changed commit message to 'Move -MERGING- string to
a constant.'

Regards,
Pavel


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [gentoo-portage-dev] [PATCH 3/3] Add an emaint module that can scan for failed merges and that can fix failed merges.
  2014-03-13  6:10     ` [gentoo-portage-dev] [PATCH 3/3] Add an emaint module that can scan for failed merges and that can fix failed merges Pavel Kazakov
@ 2014-03-13 22:09       ` Alec Warner
  2014-03-14  5:52         ` Alec Warner
  2014-03-15  5:35         ` Pavel Kazakov
  0 siblings, 2 replies; 31+ messages in thread
From: Alec Warner @ 2014-03-13 22:09 UTC (permalink / raw
  To: gentoo-portage-dev; +Cc: Pavel Kazakov

[-- Attachment #1: Type: text/plain, Size: 17032 bytes --]

On Wed, Mar 12, 2014 at 11:10 PM, Pavel Kazakov <nullishzero@gentoo.org>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
>
>
>

[-- Attachment #2: Type: text/html, Size: 24312 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [gentoo-portage-dev] [PATCH 3/3] Add an emaint module that can scan for failed merges and that can fix failed merges.
  2014-03-13 22:09       ` Alec Warner
@ 2014-03-14  5:52         ` Alec Warner
  2014-03-15  5:35         ` Pavel Kazakov
  1 sibling, 0 replies; 31+ messages in thread
From: Alec Warner @ 2014-03-14  5:52 UTC (permalink / raw
  To: gentoo-portage-dev; +Cc: Pavel Kazakov

[-- Attachment #1: Type: text/plain, Size: 17676 bytes --]

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

>
>
>
> On Wed, Mar 12, 2014 at 11:10 PM, Pavel Kazakov <nullishzero@gentoo.org>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
>>
>>
>>
>

[-- Attachment #2: Type: text/html, Size: 24987 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [gentoo-portage-dev] [PATCH 3/3] Add an emaint module that can scan for failed merges and that can fix failed merges.
  2014-03-13 22:09       ` Alec Warner
  2014-03-14  5:52         ` Alec Warner
@ 2014-03-15  5:35         ` Pavel Kazakov
  2014-03-15 17:43           ` Alec Warner
  1 sibling, 1 reply; 31+ messages in thread
From: Pavel Kazakov @ 2014-03-15  5:35 UTC (permalink / raw
  To: gentoo-portage-dev

On 03/13/14 15:09, Alec Warner wrote:
>
>
>
> On Wed, Mar 12, 2014 at 11:10 PM, Pavel Kazakov
> <nullishzero@gentoo.org <mailto:nullishzero@gentoo.org>> wrote:
>
>     ---
>      pym/portage/emaint/main.py                    |   6 +-
>

...

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

That's actually a neat idea. Going to do it.

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

Right now the current code deletes only the failed merge directory (e.g.
/var/db/pkg/app-misc/-MERGING-lsx-0.1/).

In /var/db/pkg/app-misc/-MERGING-lsx-0.1/, there should be a file called
CONTENTS that records the package contents (e.g. obj
/usr/bin/lsx-suckless 478333aa1cb7761bc8e3a400cbd976ab 1394688298).
However, during a failed merge, the CONTENTS file may not be there or
may be corrupt, so my code doesn't handle removing the contents right
now, only the failed merge directory.

Any suggestions for better wording?

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

How about pkg_invalid_entries? And I can change the comment to '@param
pkg_invalid_entries: append any packages that are invalid to this set'

After looking at the function more, I think it might make sense to
instead return pkg_atoms and pkg_invalid_entries as a tuple, instead of
having the calling code pass them in.

>  
>
>     +               """
>     +
>     +               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()

I'll change it, but in fairness 'proc.communicate()[0]' is how both the
official python documentation and other areas of portage do it ;)

>  
>
>     +               if output:
>     +                       results.append(output)
>     +               if proc.returncode != os.EX_OK:
>     +                        emerge_status = "Failed to emerge '%s'" %
>     (' '.join(pkg_atoms))
>     +               else:
>
...

I'll incorporate your other changes.

Regards,
Pavel


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [gentoo-portage-dev] [PATCH 3/3] Add an emaint module that can scan for failed merges and that can fix failed merges.
  2014-03-15  5:35         ` Pavel Kazakov
@ 2014-03-15 17:43           ` Alec Warner
  0 siblings, 0 replies; 31+ messages in thread
From: Alec Warner @ 2014-03-15 17:43 UTC (permalink / raw
  To: gentoo-portage-dev

[-- Attachment #1: Type: text/plain, Size: 6903 bytes --]

On Fri, Mar 14, 2014 at 10:35 PM, Pavel Kazakov <nullishzero@gentoo.org>wrote:

> On 03/13/14 15:09, Alec Warner wrote:
> >
> >
> >
> > On Wed, Mar 12, 2014 at 11:10 PM, Pavel Kazakov
> > <nullishzero@gentoo.org <mailto:nullishzero@gentoo.org>> wrote:
> >
> >     ---
> >      pym/portage/emaint/main.py                    |   6 +-
> >
>
> ...
>
> > 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.
>
> That's actually a neat idea. Going to do it.
>
> >
> >
> >     +               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?
>
> Right now the current code deletes only the failed merge directory (e.g.
> /var/db/pkg/app-misc/-MERGING-lsx-0.1/).
>
> In /var/db/pkg/app-misc/-MERGING-lsx-0.1/, there should be a file called
> CONTENTS that records the package contents (e.g. obj
> /usr/bin/lsx-suckless 478333aa1cb7761bc8e3a400cbd976ab 1394688298).
> However, during a failed merge, the CONTENTS file may not be there or
> may be corrupt, so my code doesn't handle removing the contents right
> now, only the failed merge directory.
>
> Any suggestions for better wording?
>
>
an upcase CONTENTS, or just talk about vdb or metadata specifically.



> >
> >
> >     +
> >     +
> >     +       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.
>
> How about pkg_invalid_entries? And I can change the comment to '@param
> pkg_invalid_entries: append any packages that are invalid to this set'
>
> After looking at the function more, I think it might make sense to
> instead return pkg_atoms and pkg_invalid_entries as a tuple, instead of
> having the calling code pass them in.
>

sgtm

>
> >
> >
> >     +               """
> >     +
> >     +               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()
>
> I'll change it, but in fairness 'proc.communicate()[0]' is how both the
> official python documentation and other areas of portage do it ;)
>

Prefer the existing (foo[0]) style then. I've been writing too much go ;)

>
> >
> >
> >     +               if output:
> >     +                       results.append(output)
> >     +               if proc.returncode != os.EX_OK:
> >     +                        emerge_status = "Failed to emerge '%s'" %
> >     (' '.join(pkg_atoms))
> >     +               else:
> >
> ...
>
> I'll incorporate your other changes.
>
> Regards,
> Pavel
>
>

[-- Attachment #2: Type: text/html, Size: 10138 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [gentoo-portage-dev] [PATCH v2 1/2] Move -MERGING- string to a constant.
  2014-03-13  6:10   ` [gentoo-portage-dev] [PATCH 1/3] Move -MERGING- string to a constant variable Pavel Kazakov
                       ` (2 preceding siblings ...)
  2014-03-13  7:18     ` [gentoo-portage-dev] [PATCH 1/3] Move -MERGING- string to a constant variable Pavel Kazakov
@ 2014-03-31  1:24     ` Pavel Kazakov
  2014-03-31  1:24       ` [gentoo-portage-dev] [PATCH v2 2/2] Add an emaint module that can scan for failed merges and that can fix failed merges Pavel Kazakov
  2014-03-31  2:06       ` [gentoo-portage-dev] [PATCH v2 1/2] Move -MERGING- string to a constant Brian Dolbec
  3 siblings, 2 replies; 31+ messages in thread
From: Pavel Kazakov @ 2014-03-31  1:24 UTC (permalink / raw
  To: gentoo-portage-dev; +Cc: Pavel Kazakov

Remove extra whitespace.
---
 pym/portage/const.py          | 1 +
 pym/portage/dbapi/__init__.py | 4 +++-
 pym/portage/dbapi/vartree.py  | 8 ++++----
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/pym/portage/const.py b/pym/portage/const.py
index 1785bff..50f0719 100644
--- a/pym/portage/const.py
+++ b/pym/portage/const.py
@@ -74,6 +74,7 @@ MOVE_BINARY              = "/bin/mv"
 PRELINK_BINARY           = "/usr/sbin/prelink"
 
 INVALID_ENV_FILE         = "/etc/spork/is/not/valid/profile.env"
+MERGING_IDENTIFIER       = "-MERGING-"
 REPO_NAME_FILE           = "repo_name"
 REPO_NAME_LOC            = "profiles" + "/" + REPO_NAME_FILE
 
diff --git a/pym/portage/dbapi/__init__.py b/pym/portage/dbapi/__init__.py
index a20a1e8..6638352 100644
--- a/pym/portage/dbapi/__init__.py
+++ b/pym/portage/dbapi/__init__.py
@@ -16,6 +16,8 @@ portage.proxy.lazyimport.lazyimport(globals(),
 	'portage.versions:catsplit,catpkgsplit,vercmp,_pkg_str',
 )
 
+from portage.const import MERGING_IDENTIFIER
+
 from portage import os
 from portage import auxdbkeys
 from portage.eapi import _get_eapi_attrs
@@ -278,7 +280,7 @@ class dbapi(object):
 		return True
 
 	def invalidentry(self, mypath):
-		if '/-MERGING-' in mypath:
+		if MERGING_IDENTIFIER in mypath:
 			if os.path.exists(mypath):
 				writemsg(colorize("BAD", _("INCOMPLETE MERGE:"))+" %s\n" % mypath,
 					noiselevel=-1)
diff --git a/pym/portage/dbapi/vartree.py b/pym/portage/dbapi/vartree.py
index 6417a56..5f5f5ce 100644
--- a/pym/portage/dbapi/vartree.py
+++ b/pym/portage/dbapi/vartree.py
@@ -45,7 +45,7 @@ portage.proxy.lazyimport.lazyimport(globals(),
 )
 
 from portage.const import CACHE_PATH, CONFIG_MEMORY_FILE, \
-	PORTAGE_PACKAGE_ATOM, PRIVATE_PATH, VDB_PATH
+	MERGING_IDENTIFIER, PORTAGE_PACKAGE_ATOM, PRIVATE_PATH, VDB_PATH
 from portage.dbapi import dbapi
 from portage.exception import CommandNotFound, \
 	InvalidData, InvalidLocation, InvalidPackageName, \
@@ -104,7 +104,7 @@ class vardbapi(dbapi):
 
 	_excluded_dirs = ["CVS", "lost+found"]
 	_excluded_dirs = [re.escape(x) for x in _excluded_dirs]
-	_excluded_dirs = re.compile(r'^(\..*|-MERGING-.*|' + \
+	_excluded_dirs = re.compile(r'^(\..*|' + MERGING_IDENTIFIER + '.*|' + \
 		"|".join(_excluded_dirs) + r')$')
 
 	_aux_cache_version        = "1"
@@ -446,7 +446,7 @@ class vardbapi(dbapi):
 				if self._excluded_dirs.match(y) is not None:
 					continue
 				subpath = x + "/" + y
-				# -MERGING- should never be a cpv, nor should files.
+				# MERGING_IDENTIFIER should never be a cpv, nor should files.
 				try:
 					if catpkgsplit(subpath) is None:
 						self.invalidentry(self.getpath(subpath))
@@ -1504,7 +1504,7 @@ class dblink(object):
 		self.dbroot = normalize_path(os.path.join(self._eroot, VDB_PATH))
 		self.dbcatdir = self.dbroot+"/"+cat
 		self.dbpkgdir = self.dbcatdir+"/"+pkg
-		self.dbtmpdir = self.dbcatdir+"/-MERGING-"+pkg
+		self.dbtmpdir = self.dbcatdir+MERGING_IDENTIFIER+pkg
 		self.dbdir = self.dbpkgdir
 		self.settings = mysettings
 		self._verbose = self.settings.get("PORTAGE_VERBOSE") == "1"
-- 
1.8.3.2



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [gentoo-portage-dev] [PATCH v2 2/2] Add an emaint module that can scan for failed merges and that can fix failed merges.
  2014-03-31  1:24     ` [gentoo-portage-dev] [PATCH v2 1/2] Move -MERGING- string to a constant Pavel Kazakov
@ 2014-03-31  1:24       ` Pavel Kazakov
  2014-03-31  2:06         ` Brian Dolbec
  2014-03-31  2:06       ` [gentoo-portage-dev] [PATCH v2 1/2] Move -MERGING- string to a constant Brian Dolbec
  1 sibling, 1 reply; 31+ messages in thread
From: Pavel Kazakov @ 2014-03-31  1:24 UTC (permalink / raw
  To: gentoo-portage-dev; +Cc: Pavel Kazakov

---
 pym/portage/emaint/main.py                    |   6 +-
 pym/portage/emaint/modules/merges/__init__.py |  30 +++
 pym/portage/emaint/modules/merges/merges.py   | 290 ++++++++++++++++++++++++++
 3 files changed, 324 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..9cd4ea2
--- /dev/null
+++ b/pym/portage/emaint/modules/merges/merges.py
@@ -0,0 +1,290 @@
+# 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
+
+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
+		lines = ['%s %s' % (pkg, mtime) for pkg, mtime in failed_pkgs.items()]
+		portage.util.write_atomic(tracking_path, '\n'.join(lines))
+
+
+	def load(self):
+		"""
+		Load previously failed merges.
+
+		@rtype: dict
+		@return: dictionary of packages that failed to merge
+		"""
+		tracking_path = self._tracking_path
+		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)
+
+
+	def __iter__(self):
+		"""
+		Provide an interator over failed merges.
+
+		@return: iterator of packages that failed to merge
+		"""
+		return self.load().items().__iter__()
+
+
+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)
+		for pkg, mtime in self._tracking_file:
+			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
+
+
+	def _get_pkg_atoms(self, failed_pkgs, pkg_atoms, pkg_invalid_entries):
+		"""
+		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_invalid_entries: append any packages that are invalid to this set
+		@type pkg_invalid_entries: set
+		"""
+
+		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_invalid_entries.append("'%s' is an invalid package atom." 
+					% pkg_atom)
+			if not portdb.cpv_exists(pkg_name):
+				pkg_invalid_entries.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]
+		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.']
+
+		pkg_invalid_entries = set()
+		pkg_atoms = set()
+		self._get_pkg_atoms(failed_pkgs, pkg_atoms, pkg_invalid_entries)
+		if pkg_invalid_entries:
+			return pkg_invalid_entries
+
+		try:
+			self._tracking_file.save(failed_pkgs)
+		except IOError as ex:
+			errors = ['Unable to save failed merges to tracking file: %s\n'
+				% str(ex)]
+			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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [gentoo-portage-dev] [PATCH v2 1/2] Move -MERGING- string to a constant.
  2014-03-31  1:24     ` [gentoo-portage-dev] [PATCH v2 1/2] Move -MERGING- string to a constant Pavel Kazakov
  2014-03-31  1:24       ` [gentoo-portage-dev] [PATCH v2 2/2] Add an emaint module that can scan for failed merges and that can fix failed merges Pavel Kazakov
@ 2014-03-31  2:06       ` Brian Dolbec
  1 sibling, 0 replies; 31+ messages in thread
From: Brian Dolbec @ 2014-03-31  2:06 UTC (permalink / raw
  To: gentoo-portage-dev

On Sun, 30 Mar 2014 18:24:22 -0700
Pavel Kazakov <nullishzero@gentoo.org> wrote:

> Remove extra whitespace.
> ---
>  pym/portage/const.py          | 1 +
>  pym/portage/dbapi/__init__.py | 4 +++-
>  pym/portage/dbapi/vartree.py  | 8 ++++----
>  3 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/pym/portage/const.py b/pym/portage/const.py
> index 1785bff..50f0719 100644
> --- a/pym/portage/const.py
> +++ b/pym/portage/const.py

It all looked really good to me.
 {:-D
-- 
Brian Dolbec <dolsen>



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [gentoo-portage-dev] [PATCH v2 2/2] Add an emaint module that can scan for failed merges and that can fix failed merges.
  2014-03-31  1:24       ` [gentoo-portage-dev] [PATCH v2 2/2] Add an emaint module that can scan for failed merges and that can fix failed merges Pavel Kazakov
@ 2014-03-31  2:06         ` Brian Dolbec
  2014-03-31  7:00           ` Alexander Berntsen
  0 siblings, 1 reply; 31+ messages in thread
From: Brian Dolbec @ 2014-03-31  2:06 UTC (permalink / raw
  To: gentoo-portage-dev

On Sun, 30 Mar 2014 18:24:23 -0700
Pavel Kazakov <nullishzero@gentoo.org> wrote:

> ---
>  pym/portage/emaint/main.py                    |   6 +-
>  pym/portage/emaint/modules/merges/__init__.py |  30 +++
>  pym/portage/emaint/modules/merges/merges.py   | 290
> ++++++++++++++++++++++++++ 3 files changed, 324 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
>

It all looked really good to me.
 {:-D

Now if we can only get the rest of the portage code nice, clean like
this :)

-- 
Brian Dolbec <dolsen>



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [gentoo-portage-dev] [PATCH v2 2/2] Add an emaint module that can scan for failed merges and that can fix failed merges.
  2014-03-31  2:06         ` Brian Dolbec
@ 2014-03-31  7:00           ` Alexander Berntsen
  2014-04-11 22:05             ` Pavel Kazakov
  0 siblings, 1 reply; 31+ messages in thread
From: Alexander Berntsen @ 2014-03-31  7:00 UTC (permalink / raw
  To: gentoo-portage-dev

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 31/03/14 04:06, Brian Dolbec wrote:
> It all looked really good to me.
>  {:-D
Yep.

Please update the commit messages to follow DEVELOPING though.
- -- 
Alexander
bernalex@gentoo.org
https://secure.plaimi.net/~alexander
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iF4EAREIAAYFAlM5EoUACgkQRtClrXBQc7WgWQEAqsAI1VlSVGx1sHLFAezXGZwH
mcG9QwVXtmNM7zSIpnsA/0Rb8sAWiLFD4Z/6NwlD6w9HOyPE/7j7+4r+wCwzeBEK
=NtBm
-----END PGP SIGNATURE-----


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [gentoo-portage-dev] [PATCH v2 2/2] Add an emaint module that can scan for failed merges and that can fix failed merges.
  2014-03-31  7:00           ` Alexander Berntsen
@ 2014-04-11 22:05             ` Pavel Kazakov
  2014-04-11 23:03               ` Brian Dolbec
  0 siblings, 1 reply; 31+ messages in thread
From: Pavel Kazakov @ 2014-04-11 22:05 UTC (permalink / raw
  To: gentoo-portage-dev

On 03/31/2014 12:00 AM, Alexander Berntsen wrote:
> On 31/03/14 04:06, Brian Dolbec wrote:
>> It all looked really good to me. {:-D
> Yep.
> 
> Please update the commit messages to follow DEVELOPING though.
> 

Sure. How are the following updates?

For the first patch, I'll remove the second message line so the
message is only:
Move -MERGING- string to a constant

For the second patch, I'll update the message to:
Add emaint module that handles failed merges

The emaint module can scan for failed merges and fix any failed merges
found. The emaint module also keeps track of failed merges using a
tracking file.

Regards,
Pavel


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [gentoo-portage-dev] [PATCH v2 2/2] Add an emaint module that can scan for failed merges and that can fix failed merges.
  2014-04-11 22:05             ` Pavel Kazakov
@ 2014-04-11 23:03               ` Brian Dolbec
  2014-04-18 19:26                 ` Pavel Kazakov
  0 siblings, 1 reply; 31+ messages in thread
From: Brian Dolbec @ 2014-04-11 23:03 UTC (permalink / raw
  To: gentoo-portage-dev

On Fri, 11 Apr 2014 15:05:09 -0700
Pavel Kazakov <nullishzero@gentoo.org> wrote:

> On 03/31/2014 12:00 AM, Alexander Berntsen wrote:
> > On 31/03/14 04:06, Brian Dolbec wrote:
> >> It all looked really good to me. {:-D
> > Yep.
> > 
> > Please update the commit messages to follow DEVELOPING though.
> > 
> 
> Sure. How are the following updates?
> 
> For the first patch, I'll remove the second message line so the
> message is only:
> Move -MERGING- string to a constant
> 
> 

No, there is never suppose to be more than a blank line for the second
line.  Then for lines 3+  fill your boots :)

the idea is to have one short descriptive line, followed by a blank
line, then as many detail lines as needed.

So, in this case dropping out the whitespace changes is minor, but
still relevant.  so it should be kept on line 3.


> For the second patch, I'll update the message to:
> Add emaint module that handles failed merges
> 
> The emaint module can scan for failed merges and fix any failed merges
> found. The emaint module also keeps track of failed merges using a
> tracking file.
> 
> Regards,
> Pavel
> 

I was thinking more along the lines of:

New emaint module: Merges

This emaint module scans for failed package merges and will display
or fix any failed packages found. 
This module also saves failed merges found using a tracking file.  
Subsequent runs of the module will re-load that info for re-display,
re-emerge of those packages.


-- 
Brian Dolbec <dolsen>



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [gentoo-portage-dev] [PATCH v2 2/2] Add an emaint module that can scan for failed merges and that can fix failed merges.
  2014-04-11 23:03               ` Brian Dolbec
@ 2014-04-18 19:26                 ` Pavel Kazakov
  2014-04-18 19:37                   ` Brian Dolbec
  0 siblings, 1 reply; 31+ messages in thread
From: Pavel Kazakov @ 2014-04-18 19:26 UTC (permalink / raw
  To: gentoo-portage-dev

On 04/11/2014 04:03 PM, Brian Dolbec wrote:
> On Fri, 11 Apr 2014 15:05:09 -0700
> Pavel Kazakov <nullishzero@gentoo.org> wrote:
> 
>> On 03/31/2014 12:00 AM, Alexander Berntsen wrote:
>>> On 31/03/14 04:06, Brian Dolbec wrote:
>>>> It all looked really good to me. {:-D
>>> Yep.
>>>
>>> Please update the commit messages to follow DEVELOPING though.
>>>
>>
>> Sure. How are the following updates?
>>
>> For the first patch, I'll remove the second message line so the
>> message is only:
>> Move -MERGING- string to a constant
>>
>>
> 
> No, there is never suppose to be more than a blank line for the second
> line.  Then for lines 3+  fill your boots :)
> 
> the idea is to have one short descriptive line, followed by a blank
> line, then as many detail lines as needed.
> 
> So, in this case dropping out the whitespace changes is minor, but
> still relevant.  so it should be kept on line 3.
> 
> 
>> For the second patch, I'll update the message to:
>> Add emaint module that handles failed merges
>>
>> The emaint module can scan for failed merges and fix any failed merges
>> found. The emaint module also keeps track of failed merges using a
>> tracking file.
>>
>> Regards,
>> Pavel
>>
> 
> I was thinking more along the lines of:
> 
> New emaint module: Merges
> 
> This emaint module scans for failed package merges and will display
> or fix any failed packages found. 
> This module also saves failed merges found using a tracking file.  
> Subsequent runs of the module will re-load that info for re-display,
> re-emerge of those packages.
> 
> 

Both suggestions look good to me. Should I update the commit messages,
merge to master, and push?

Regards,
Pavel


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [gentoo-portage-dev] [PATCH v2 2/2] Add an emaint module that can scan for failed merges and that can fix failed merges.
  2014-04-18 19:26                 ` Pavel Kazakov
@ 2014-04-18 19:37                   ` Brian Dolbec
  0 siblings, 0 replies; 31+ messages in thread
From: Brian Dolbec @ 2014-04-18 19:37 UTC (permalink / raw
  To: gentoo-portage-dev

On Fri, 18 Apr 2014 12:26:13 -0700
Pavel Kazakov <nullishzero@gentoo.org> wrote:

> On 04/11/2014 04:03 PM, Brian Dolbec wrote:
> > On Fri, 11 Apr 2014 15:05:09 -0700
> > Pavel Kazakov <nullishzero@gentoo.org> wrote:
> > 
> >> On 03/31/2014 12:00 AM, Alexander Berntsen wrote:
> >>> On 31/03/14 04:06, Brian Dolbec wrote:
> >>>> It all looked really good to me. {:-D
> >>> Yep.
> >>>
> >>> Please update the commit messages to follow DEVELOPING though.
> >>>
> >>
> >> Sure. How are the following updates?
> >>
> >> For the first patch, I'll remove the second message line so the
> >> message is only:
> >> Move -MERGING- string to a constant
> >>
> >>
> > 
> > No, there is never suppose to be more than a blank line for the
> > second line.  Then for lines 3+  fill your boots :)
> > 
> > the idea is to have one short descriptive line, followed by a blank
> > line, then as many detail lines as needed.
> > 
> > So, in this case dropping out the whitespace changes is minor, but
> > still relevant.  so it should be kept on line 3.
> > 
> > 
> >> For the second patch, I'll update the message to:
> >> Add emaint module that handles failed merges
> >>
> >> The emaint module can scan for failed merges and fix any failed
> >> merges found. The emaint module also keeps track of failed merges
> >> using a tracking file.
> >>
> >> Regards,
> >> Pavel
> >>
> > 
> > I was thinking more along the lines of:
> > 
> > New emaint module: Merges
> > 
> > This emaint module scans for failed package merges and will display
> > or fix any failed packages found. 
> > This module also saves failed merges found using a tracking file.  
> > Subsequent runs of the module will re-load that info for re-display,
> > re-emerge of those packages.
> > 
> > 
> 
> Both suggestions look good to me. Should I update the commit messages,
> merge to master, and push?
> 
> Regards,
> Pavel
> 

update the messages, but hold off pushing them to master until I get
2.2.11 released.  I had intended ot have done that already, but Zac saw
a problem in one bugfix I committed, so will wait for that so I don't
introduce another bug.

-- 
Brian Dolbec <dolsen>



^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2014-04-18 19:38 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-19 20:31 [gentoo-portage-dev] [PATCH] Add an emaint module that can scan for failed merges and that can fix failed merges Pavel Kazakov
2014-02-19 22:32 ` Alec Warner
2014-02-19 23:20   ` Brian Dolbec
2014-02-19 23:34     ` Alec Warner
2014-02-20  0:08   ` Pavel Kazakov
2014-02-20 10:58     ` Alexander Berntsen
2014-02-20 13:58       ` Brian Dolbec
2014-02-20 14:09         ` Alexander Berntsen
2014-02-20 20:10       ` Pavel Kazakov
2014-02-19 22:50 ` Brian Dolbec
2014-02-19 23:37   ` Alec Warner
2014-02-20  0:14   ` Pavel Kazakov
2014-02-22  0:34 ` [gentoo-portage-dev] [PATCH v2] " Pavel Kazakov
2014-02-22 18:59   ` Brian Dolbec
2014-03-13  6:10   ` [gentoo-portage-dev] [PATCH 1/3] Move -MERGING- string to a constant variable Pavel Kazakov
2014-03-13  6:10     ` [gentoo-portage-dev] [PATCH 2/3] Remove extra whitespace Pavel Kazakov
2014-03-13  6:10     ` [gentoo-portage-dev] [PATCH 3/3] Add an emaint module that can scan for failed merges and that can fix failed merges Pavel Kazakov
2014-03-13 22:09       ` Alec Warner
2014-03-14  5:52         ` Alec Warner
2014-03-15  5:35         ` Pavel Kazakov
2014-03-15 17:43           ` Alec Warner
2014-03-13  7:18     ` [gentoo-portage-dev] [PATCH 1/3] Move -MERGING- string to a constant variable Pavel Kazakov
2014-03-31  1:24     ` [gentoo-portage-dev] [PATCH v2 1/2] Move -MERGING- string to a constant Pavel Kazakov
2014-03-31  1:24       ` [gentoo-portage-dev] [PATCH v2 2/2] Add an emaint module that can scan for failed merges and that can fix failed merges Pavel Kazakov
2014-03-31  2:06         ` Brian Dolbec
2014-03-31  7:00           ` Alexander Berntsen
2014-04-11 22:05             ` Pavel Kazakov
2014-04-11 23:03               ` Brian Dolbec
2014-04-18 19:26                 ` Pavel Kazakov
2014-04-18 19:37                   ` Brian Dolbec
2014-03-31  2:06       ` [gentoo-portage-dev] [PATCH v2 1/2] Move -MERGING- string to a constant Brian Dolbec

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox