Re: [gentoo-portage-dev] [PATCH v2 gentoolkit 2/2] eclean: Add option to delete binpkgs with changed deps

[Matt Turner <mattst88@gentoo.org>]

On Wed, Mar 11, 2020 at 9:36 PM Matt Turner <mattst88@gentoo.org> wrote:
>
> On Wed, Mar 11, 2020 at 9:31 PM Zac Medico <zmedico@gentoo.org> wrote:
> >
> > On 3/6/20 10:11 PM, Matt Turner wrote:
> > > Signed-off-by: Matt Turner <mattst88@gentoo.org>
> > > ---
> > >  pym/gentoolkit/eclean/cli.py    |  7 ++++++-
> > >  pym/gentoolkit/eclean/search.py | 24 +++++++++++++++++++++++-
> > >  2 files changed, 29 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/pym/gentoolkit/eclean/cli.py b/pym/gentoolkit/eclean/cli.py
> > > index 1a99b3e..39aafd3 100644
> > > --- a/pym/gentoolkit/eclean/cli.py
> > > +++ b/pym/gentoolkit/eclean/cli.py
> > > @@ -147,6 +147,8 @@ def printUsage(_error=None, help=None):
> > >       or help in ('all','packages'):
> > >               print( "Available", yellow("options"),"for the",
> > >                               green("packages"),"action:", file=out)
> > > +             print( yellow("     --changed-deps")+
> > > +                     "               - delete packages for which ebuild dependencies have changed", file=out)
> > >               print( yellow(" -i, --ignore-failure")+
> > >                       "             - ignore failure to locate PKGDIR", file=out)
> > >               print( file=out)
> > > @@ -263,6 +265,8 @@ def parseArgs(options={}):
> > >                               options['size-limit'] = parseSize(a)
> > >                       elif o in ("-v", "--verbose") and not options['quiet']:
> > >                                       options['verbose'] = True
> > > +                     elif o in ("--changed-deps"):
> > > +                             options['changed-deps'] = True
> > >                       elif o in ("-i", "--ignore-failure"):
> > >                               options['ignore-failure'] = True
> > >                       else:
> > > @@ -290,7 +294,7 @@ def parseArgs(options={}):
> > >       getopt_options['short']['distfiles'] = "fs:"
> > >       getopt_options['long']['distfiles'] = ["fetch-restricted", "size-limit="]
> > >       getopt_options['short']['packages'] = "i"
> > > -     getopt_options['long']['packages'] = ["ignore-failure"]
> > > +     getopt_options['long']['packages'] = ["ignore-failure", "changed-deps"]
> > >       # set default options, except 'nocolor', which is set in main()
> > >       options['interactive'] = False
> > >       options['pretend'] = False
> > > @@ -303,6 +307,7 @@ def parseArgs(options={}):
> > >       options['fetch-restricted'] = False
> > >       options['size-limit'] = 0
> > >       options['verbose'] = False
> > > +     options['changed-deps'] = False
> > >       options['ignore-failure'] = False
> > >       # if called by a well-named symlink, set the action accordingly:
> > >       action = None
> > > diff --git a/pym/gentoolkit/eclean/search.py b/pym/gentoolkit/eclean/search.py
> > > index 0efefdb..17655cb 100644
> > > --- a/pym/gentoolkit/eclean/search.py
> > > +++ b/pym/gentoolkit/eclean/search.py
> > > @@ -13,6 +13,8 @@ import sys
> > >  from functools import partial
> > >
> > >  import portage
> > > +from portage.dep import Atom, use_reduce
> > > +from portage.dep._slot_operator import strip_slots
> > >
> > >  import gentoolkit.pprinter as pp
> > >  from gentoolkit.eclean.exclude import (exclDictMatchCP, exclDictExpand,
> > > @@ -488,6 +490,17 @@ class DistfilesSearch(object):
> > >               return clean_me, saved
> > >
> > >
> > > +def _deps_equal(deps_a, deps_b, eapi, uselist=None):
> > > +     """Compare two dependency lists given a set of USE flags"""
> > > +     if deps_a == deps_b: return True
> > > +
> > > +     deps_a = use_reduce(deps_a, uselist=uselist, eapi=eapi, token_class=Atom)
> > > +     deps_b = use_reduce(deps_b, uselist=uselist, eapi=eapi, token_class=Atom)
> > > +     strip_slots(deps_a)
> > > +     strip_slots(deps_b)
> > > +     return deps_a == deps_b
> > > +
> > > +
> > >  def findPackages(
> > >               options,
> > >               exclude=None,
> > > @@ -562,7 +575,16 @@ def findPackages(
> > >
> > >               # Exclude if binpkg exists in the porttree and not --deep
> > >               if not destructive and port_dbapi.cpv_exists(cpv):
> > > -                     continue
> > > +                     if not options['changed-deps']:
> > > +                             continue
> >
> > We can't can't continue above, since that will skip all of the filters
> > that occur later in the loop. So, we have to nest the below changed-deps
> > code under if options['changed-deps']:
>
> I'm happy to make that change, but I don't think it's necessary,
> strictly speaking, since this is inside an 'if not destructive'
> conditional and the only filter afterwards is 'if destructive'.

Wait... the logic was if not destructive and
package-exists-in-porttree -> continue and do not add it to the dead
package list.

I've just changed it so it does that if changed-deps is not set... so
keep the current behavior without --changed-deps.

And if --changed-deps, check the porttree vs binpkg dependencies, and
if they still match, skip.

What is wrong with that logic?