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

In case we add more filters in the future, I'll make the change you suggested.

Thanks a bunch for your reviews!