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

[Matt Turner]

On Wed, Mar 11, 2020 at 10:23 PM Zac Medico  wrote:
> The coupling with --destructive logic complicates matters. It raises the
> question, why isn't --time-limit logic also coupled with --destructive
> logic? I think "in order to preserve the status quo" is a reasonable
> answer to this question.

Yeah :(

It's clear to me that these options were added in a very ad hoc
manner. And... named badly if I do say so. E.g., destructive and
package_names do not correspond, at least in my mind, to the
operations they do.

I'd like to clean those up as a follow on.

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

[Zac Medico]

On 3/11/20 9:43 PM, Matt Turner wrote:
> On Wed, Mar 11, 2020 at 9:36 PM Matt Turner  wrote:
>>
>> On Wed, Mar 11, 2020 at 9:31 PM Zac Medico  wrote:
>>> 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.

Yeah, you're right.

> What is wrong with that logic?

The coupling with --destructive logic complicates matters. It raises the
question, why isn't --time-limit logic also coupled with --destructive
logic? I think "in order to preserve the status quo" is a reasonable
answer to this question.
-- 
Thanks,
Zac



signature.asc
Description: OpenPGP digital signature

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

[Matt Turner]

On Wed, Mar 11, 2020 at 9:36 PM Matt Turner  wrote:
>
> On Wed, Mar 11, 2020 at 9:31 PM Zac Medico  wrote:
> >
> > On 3/6/20 10:11 PM, Matt Turner wrote:
> > > Signed-off-by: Matt Turner 
> > > ---
> > >  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 

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

[Zac Medico]

On 3/11/20 9:36 PM, Matt Turner wrote:
> On Wed, Mar 11, 2020 at 9:31 PM Zac Medico  wrote:
>>> @@ -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.

Yeah, I think it significantly reduces the cognitive burden to the
reader as well. Thanks!

> Thanks a bunch for your reviews!

You're welcome!
-- 
Thanks,
Zac



signature.asc
Description: OpenPGP digital signature

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

[Matt Turner]

On Wed, Mar 11, 2020 at 9:31 PM Zac Medico  wrote:
>
> On 3/6/20 10:11 PM, Matt Turner wrote:
> > Signed-off-by: Matt Turner 
> > ---
> >  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!

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

[Zac Medico]

On 3/11/20 4:32 PM, Matt Turner wrote:
> On Tue, Mar 10, 2020 at 8:30 PM Zac Medico  wrote:
>>
>> On 3/6/20 10:11 PM, Matt Turner wrote:
>>> +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)
>>
>> It's pure luck that passing a list of depstrings to use_reduce works
>> here, so it will be more future-proof to use ' '.join(depstr) instead.
>> The relevant code in use_reduce looks like this:
>>
>> if isinstance(depstr, list):
>> if portage._internal_caller:
>> warnings.warn(_("Passing paren_reduced dep arrays to %s is 
>> deprecated. " + \
>> "Pass the original dep string instead.") % \
>> ('portage.dep.use_reduce',), DeprecationWarning, 
>> stacklevel=2)
>> depstr = paren_enclose(depstr)
> 
> Okay, thank you. I've fixed this with:
> 
> - binpkg_deps = bin_dbapi.aux_get(cpv, keys)
> - ebuild_deps = port_dbapi.aux_get(cpv, keys)
> + binpkg_deps = ' '.join(bin_dbapi.aux_get(cpv, keys))
> + ebuild_deps = ' '.join(port_dbapi.aux_get(cpv, keys))
> 
> With that fixed, do the patches look good to you?

Yeah, looks good (with options['changed-deps'] logic fix from my other
email).
-- 
Thanks,
Zac



signature.asc
Description: OpenPGP digital signature

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

[Zac Medico]

On 3/6/20 10:11 PM, Matt Turner wrote:
> Signed-off-by: Matt Turner 
> ---
>  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']:

> +
> + keys = ('RDEPEND', 'PDEPEND')
> + binpkg_deps = bin_dbapi.aux_get(cpv, keys)
> + ebuild_deps = port_dbapi.aux_get(cpv, keys)
> + uselist = bin_dbapi.aux_get(cpv, ['USE'])[0].split()
> +
> + if _deps_equal(binpkg_deps, ebuild_deps, cpv.eapi, 
> uselist):
> + continue
>  
>   if destructive and var_dbapi.cpv_exists(cpv):
>   # Exclude if an instance of the package is installed 
> due to
> 


-- 
Thanks,
Zac



signature.asc
Description: OpenPGP digital signature

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

[Matt Turner]

On Tue, Mar 10, 2020 at 8:30 PM Zac Medico  wrote:
>
> On 3/6/20 10:11 PM, Matt Turner wrote:
> > +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)
>
> It's pure luck that passing a list of depstrings to use_reduce works
> here, so it will be more future-proof to use ' '.join(depstr) instead.
> The relevant code in use_reduce looks like this:
>
> if isinstance(depstr, list):
> if portage._internal_caller:
> warnings.warn(_("Passing paren_reduced dep arrays to %s is 
> deprecated. " + \
> "Pass the original dep string instead.") % \
> ('portage.dep.use_reduce',), DeprecationWarning, 
> stacklevel=2)
> depstr = paren_enclose(depstr)

Okay, thank you. I've fixed this with:

- binpkg_deps = bin_dbapi.aux_get(cpv, keys)
- ebuild_deps = port_dbapi.aux_get(cpv, keys)
+ binpkg_deps = ' '.join(bin_dbapi.aux_get(cpv, keys))
+ ebuild_deps = ' '.join(port_dbapi.aux_get(cpv, keys))

With that fixed, do the patches look good to you?

Re: [gentoo-portage-dev] [PATCH] repoman.modules.vcs.git.changes: reindex (bug 712106)

[Zac Medico]

On 3/11/20 9:37 AM, Alec Warner wrote:
> On Wed, Mar 11, 2020 at 12:16 AM Zac Medico  > wrote:
> 
> For files returned by git diff-index, call git update-index in order
> to ensure that the index reflects the state on disk. This will prevent
> incorrect assumptions in cases where the index is missing or stale for
> some reason. Since repoman uses this information to decide when to
> update copyright header dates, this can prevent spurious copyright
> header updates.
> 
> Signed-off-by: Zac Medico  >
> Bug: https://bugs.gentoo.org/712106
> ---
>  repoman/lib/repoman/modules/vcs/git/changes.py | 15 ---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/repoman/lib/repoman/modules/vcs/git/changes.py
> b/repoman/lib/repoman/modules/vcs/git/changes.py
> index 7e9ac1eb5..ebf770d53 100644
> --- a/repoman/lib/repoman/modules/vcs/git/changes.py
> +++ b/repoman/lib/repoman/modules/vcs/git/changes.py
> @@ -29,8 +29,14 @@ class Changes(ChangesBase):
>                 '''
>                 super(Changes, self).__init__(options, repo_settings)
> 
> -       def _scan(self):
> -               '''VCS type scan function, looks for all detectable
> changes'''
> +       def _scan(self, _reindex=True):
> 
> 
> Why the underscore prefix?
> 
> -A

I can't think of a reason to use this parameter aside from this
recursive call, and the underscore hints at this.
-- 
Thanks,
Zac



signature.asc
Description: OpenPGP digital signature

Re: [gentoo-portage-dev] [PATCH] repoman.modules.vcs.git.changes: reindex (bug 712106)

[Alec Warner]

On Wed, Mar 11, 2020 at 12:16 AM Zac Medico  wrote:

> For files returned by git diff-index, call git update-index in order
> to ensure that the index reflects the state on disk. This will prevent
> incorrect assumptions in cases where the index is missing or stale for
> some reason. Since repoman uses this information to decide when to
> update copyright header dates, this can prevent spurious copyright
> header updates.
>
> Signed-off-by: Zac Medico 
> Bug: https://bugs.gentoo.org/712106
> ---
>  repoman/lib/repoman/modules/vcs/git/changes.py | 15 ---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/repoman/lib/repoman/modules/vcs/git/changes.py
> b/repoman/lib/repoman/modules/vcs/git/changes.py
> index 7e9ac1eb5..ebf770d53 100644
> --- a/repoman/lib/repoman/modules/vcs/git/changes.py
> +++ b/repoman/lib/repoman/modules/vcs/git/changes.py
> @@ -29,8 +29,14 @@ class Changes(ChangesBase):
> '''
> super(Changes, self).__init__(options, repo_settings)
>
> -   def _scan(self):
> -   '''VCS type scan function, looks for all detectable
> changes'''
> +   def _scan(self, _reindex=True):
>

Why the underscore prefix?

-A


> +   '''
> +   VCS type scan function, looks for all detectable changes
> +
> +   @param _reindex: ensure that the git index reflects the
> state on
> +   disk for files returned by git diff-index
> +   @type _reindex: bool
> +   '''
> with repoman_popen(
> "git diff-index --name-only "
> "--relative --diff-filter=M HEAD") as f:
> @@ -51,6 +57,9 @@ class Changes(ChangesBase):
> removed = f.readlines()
> self.removed = ["./" + elem[:-1] for elem in removed]
> del removed
> +   if _reindex and (self.changed or self.new or self.removed):
> +   self.update_index([], self.changed + self.new +
> self.removed)
> +   self._scan(_reindex=False)
>
> @property
> def unadded(self):
> @@ -91,7 +100,7 @@ class Changes(ChangesBase):
> # of the working tree.
> myfiles = mymanifests + myupdates
> myfiles.sort()
> -   update_index_cmd = ["git", "update-index"]
> +   update_index_cmd = ["git", "update-index", "--add",
> "--remove"]
> update_index_cmd.extend(f.lstrip("./") for f in myfiles)
> if self.options.pretend:
> print("(%s)" % (" ".join(update_index_cmd),))
> --
> 2.24.1
>
>
>

[gentoo-portage-dev] [PATCH] repoman.modules.vcs.git.changes: reindex (bug 712106)

[Zac Medico]

For files returned by git diff-index, call git update-index in order
to ensure that the index reflects the state on disk. This will prevent
incorrect assumptions in cases where the index is missing or stale for
some reason. Since repoman uses this information to decide when to
update copyright header dates, this can prevent spurious copyright
header updates.

Signed-off-by: Zac Medico 
Bug: https://bugs.gentoo.org/712106
---
 repoman/lib/repoman/modules/vcs/git/changes.py | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/repoman/lib/repoman/modules/vcs/git/changes.py 
b/repoman/lib/repoman/modules/vcs/git/changes.py
index 7e9ac1eb5..ebf770d53 100644
--- a/repoman/lib/repoman/modules/vcs/git/changes.py
+++ b/repoman/lib/repoman/modules/vcs/git/changes.py
@@ -29,8 +29,14 @@ class Changes(ChangesBase):
'''
super(Changes, self).__init__(options, repo_settings)
 
-   def _scan(self):
-   '''VCS type scan function, looks for all detectable changes'''
+   def _scan(self, _reindex=True):
+   '''
+   VCS type scan function, looks for all detectable changes
+
+   @param _reindex: ensure that the git index reflects the state on
+   disk for files returned by git diff-index
+   @type _reindex: bool
+   '''
with repoman_popen(
"git diff-index --name-only "
"--relative --diff-filter=M HEAD") as f:
@@ -51,6 +57,9 @@ class Changes(ChangesBase):
removed = f.readlines()
self.removed = ["./" + elem[:-1] for elem in removed]
del removed
+   if _reindex and (self.changed or self.new or self.removed):
+   self.update_index([], self.changed + self.new + 
self.removed)
+   self._scan(_reindex=False)
 
@property
def unadded(self):
@@ -91,7 +100,7 @@ class Changes(ChangesBase):
# of the working tree.
myfiles = mymanifests + myupdates
myfiles.sort()
-   update_index_cmd = ["git", "update-index"]
+   update_index_cmd = ["git", "update-index", "--add", "--remove"]
update_index_cmd.extend(f.lstrip("./") for f in myfiles)
if self.options.pretend:
print("(%s)" % (" ".join(update_index_cmd),))
-- 
2.24.1