Re: [PATCH] Clear --exclude list after 'git rev-parse --all'

From: Jeff King <peff@peff.net>
Date: Oct 25, 2018, 10:45 UTC
Message-ID: <20181025104542.GA14455@sigill.intra.peff.net>
In-Reply-To: <CAHc6FU5CpBNpzfPs0hkNm9Hw1iYnALvUY4WL1z=sPVT10fq+zw@mail.gmail.com>

On Wed, Oct 24, 2018 at 11:49:06AM +0200, Andreas Gruenbacher wrote:

Show 22 quoted lines

> > That is why I asked what "problem" this patch fixes.  Without
> > answering that question, it is unclear if the patch is completing
> > missing coverage for "--all", or it is cargo culting an useless
> > clearing done for "--glob" and friends to code for "--all" that did
> > not do the same useless clearing.
> 
> This is how the --exclude option is described:
> 
>        --exclude=<glob-pattern>
>            Do not include refs matching <glob-pattern> that the next
> --all, --branches,
>            --tags, --remotes, or --glob would otherwise consider.
> Repetitions of this
>            option accumulate exclusion patterns up to the next --all,
> --branches, --tags,
>            --remotes, or --glob option (other options or arguments do not clear
>            accumulated patterns).
> 
> I'm assuming that some thought has gone into making the options behave
> in this particular way. The implementation in revision.c follows this
> pattern, but the implementation in builtin/rev-parse.c only almost
> does.

Yeah. I think this is just a bug in 9dc01bf063 (rev-parse: introduce --exclude=<glob> to tame wildcards, 2013-11-01), in that it's handling of --all differed from e7b432c521 (revision: introduce --exclude=<glob> to tame wildcards, 2013-08-30). The clearing is very much intentional and documented, and in general rev-parse should behave the same as rev-list.

An easy test is:

  $ git rev-list --no-walk --exclude='*' --all --all
  3289ca716320457af5d2dd550b716282ad22da11
  ...a bunch of other tip commits...

  $ git rev-parse --exclude='*' --all --all
  [no output, but it should print those same tip commits]

-Peff