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

[Alec Warner]

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

> On 03/13/14 15:09, Alec Warner wrote:
> >
> >
> >
> > On Wed, Mar 12, 2014 at 11:10 PM, Pavel Kazakov
> > <nullishzero@g.o <mailto:nullishzero@g.o>> 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
>
>