bug: `git log --grep ... --invert-grep --author=...` negates / ignores --author

Given the following commits:

```
asottile@asottile-VirtualBox:/tmp$ git init test
Initialized empty Git repository in /tmp/test/.git/
asottile@asottile-VirtualBox:/tmp$ cd test/
asottile@asottile-VirtualBox:/tmp/test$
GIT_COMMITTER_EMAIL=foo@example.com GIT_AUTHOR_EMAIL=foo@example.com
git commit --allow-empty -m "foo"
[master (root-commit) c9df62b] foo
asottile@asottile-VirtualBox:/tmp/test$ git commit -m "blah" --allow-empty
[master 9e3ee9b] blah
asottile@asottile-VirtualBox:/tmp/test$ git log
commit 9e3ee9bc1adab2ae8eb1884a8f6237da18dfd27b
Author: Anthony Sottile <asottile@umich.edu>
Date:   Wed May 31 08:40:59 2017 -0700

    blah

commit c9df62b93298a247fcfbe24ed4282ccf95448f47
Author: Anthony Sottile <foo@example.com>
Date:   Wed May 31 08:40:49 2017 -0700

    foo
asottile@asottile-VirtualBox:/tmp/test$ git log --grep bar
--invert-grep --author=foo
commit 9e3ee9bc1adab2ae8eb1884a8f6237da18dfd27b
Author: Anthony Sottile <asottile@umich.edu>
Date:   Wed May 31 08:40:59 2017 -0700

    blah

commit c9df62b93298a247fcfbe24ed4282ccf95448f47
Author: Anthony Sottile <foo@example.com>
Date:   Wed May 31 08:40:49 2017 -0700

    foo
asottile@asottile-VirtualBox:/tmp/test$ git log --author=foocommit
c9df62b93298a247fcfbe24ed4282ccf95448f47
Author: Anthony Sottile <foo@example.com>
Date:   Wed May 31 08:40:49 2017 -0700

    foo
```

I expect the same output from the last two commands, but the `--invert-grep` one seems to match _all_ the commits.

I can try and dig into this if I have time, just trying to get a count using this as a workaround

git log --grep ... --invert-grep --format=%ce | grep ... | wc -l

Anthony

On Wed, May 31, 2017 at 5:45 PM, Anthony Sottile <asottile@umich.edu> wrote:

I had to squint a bit to see what you were getting at here, which is pretty simple: When you provide --invert-grep the --author filter is completely discarded. This is a bug.

I.e. on git.git:

OK:

$ git log --grep=bar --author=Ævar --pretty=format:%an -100 origin/pu
|sort|uniq -c|sort -nr
5 Ævar Arnfjörð Bjarmason

$ git log --author=Ævar --pretty=format:%an -100 origin/pu |sort|uniq -c|sort -nr 100 Ævar Arnfjörð Bjarmason

$ git log --grep=bar --invert-grep --author=Ævar --pretty=format:%an
-100 origin/pu |sort|uniq -c|sort -nr
     78 Junio C Hamano
     14 Jeff King
      2 Andreas Heiduk
      1 Sahil Dua
      1 Rikard Falkeborn
      1 Johannes Sixt
      1 Johannes Schindelin
      1 Ben Peart
      1 Ævar Arnfjörð Bjarmason

That last command should only find my commits, but instead --author is discarded.

On Wed, May 31, 2017 at 08:08:54PM +0200, Ævar Arnfjörð Bjarmason wrote:

I would have thought that the last command wouldn't find _any_ of your commits. Don't we consider --author a greppable pattern and invert it?

By itself:

  $ git log --author=Ævar --invert-grep --format=%an -100 origin/pu |
    sort | uniq -c | sort -rn
       79 Junio C Hamano
        8 Stefan Beller
        8 Jeff King
        1 Sahil Dua
        1 Rikard Falkeborn
        1 Johannes Sixt
        1 Johannes Schindelin
        1 Andreas Heiduk

So that makes sense from the "--author is invertable" world-view.

But I'm puzzled that you show up at all when --grep=bar is added (and moreover, that we get _one_ commit from you, not the 5 found in your initial command). That seems like a bug.

-Peff

On Wed, May 31, 2017 at 11:40 PM, Jeff King <peff@peff.net> wrote:

There's two unrelated things going on here AFAICT:

* Anthony expected --author to be inverted by --invert-grep, but this
is not how it's documented, it's *only* for inverting the --grep
pattern: "Limit the commits output to ones with log message that do
not match the pattern specified with --grep=<pattern>."

* The --author filter should be applied un-inverted, but isn't, it's
seemingly just ignored in the presence of --grep, actually mostly
ignored, this is bizarre:

$ diff -ru <(git log --grep=bar --invert-grep --pretty=format:%an -100 origin/pu |sort|uniq -c|sort -nr) <(git log --grep=bar --invert-grep --pretty=format:%an -100 --author=WeMostlyIgnoreThisButNotReally origin/pu |sort|uniq -c|sort -nr) --- /dev/fd/63 2017-06-01 21:44:08.952583804 +0200 +++ /dev/fd/62 2017-06-01 21:44:08.952583804 +0200

I actually only expected the --grep to be inverted -- I think I'm on the same page with what's documented.

I'd be happy to dig into the code and investigate this some more but I am not familiar with the git codebase, any code hints on where to get bootstrapped?

Anthony

On Thu, Jun 1, 2017 at 12:45 PM, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

One thing that is very correct in what you said is that the documentation of "--invert-grep" is wrong to mention only "--grep". It probably is because the author of "--invert-grep" did not realize that the "--author" thing internally is considered a part of "grep" processing done in revision.c::commit_match() that chooses which commit to show.

As you may have noticed, the integration to the grep infrastructure with "git log" and friends does not expose the power of combination (e.g. "--and", "--not", etc.), unlike "git grep", to the command line. This is purely a technical limitation (e.g. "--not" is taken to mean a completely different thing in "git log" family) and there is no fundamental reason why we shouldn't, but the fact that the end user cannot combine the "--grep" like terms in a flexible way remains from the beginning to this day.

Within that constraint, --grep and things like --author are defined to interact in a certain hardcoded way (the former searches substring in the body part of commit objects while the latter searches substrings in the "header" part of them), simply because defining them to be all ORed together does not give us a very useful semantics.

In general, two or more of these searches always "OR" together, e.g. "git log --grep=foo --grep=bar" finds commits that mention foo or commits that mention bar. However, searches in body and searches in header "AND" together, e.g. "git log --author=Ævar --grep=foo" finds commits that mention foo and written by you. "git log --author=Ævar --author=gitster@ --grep=foo --grep=bar" finds commits that mention either foo or bar and written by either you or me. IIRC, its parse tree would look like

    (AND (OR (author Ævar) (author gitster@))
         (OR (grep foo) (grep bar))

The "--invert-grep" merely adds (NOT ...) around it.

I vaguely recall there was impliations to "all match" when the header search and the body search is used together. Without header search, the usual "git log --grep=foo --grep=bar" becomes

         (OR (grep foo) (grep bar))

and "--all-match" is applied to this "OR" (i.e. by the time the matcher finishes a single commit's body, all of the terms ORed together, i.e. (grep foo) and (grep bar), must have matched at least once for the commit to be considered a match. Obviously, a commit can be authored and commited by only a single person, and applying all-match naively to "git log --author=Ævar --author=gitster@" would not make any sense (by definition the result would be an empty set), and IIRC there is a code that copes with this to avoid requiring a commit that is written by two people (or committed by two people) when "--all-match" is in effect.

So taken all together, your example and its output makes all sense to me. As you mentioned, the examples by the OP was written a bit too dense for me to grok quickly and I didn't look too carefully at them to comment.

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

Hmph, the way I read revision.c::commit_match() is that we check with "--grep=bar --author=Ævar" and then emit the commit if that check fails when --invert-grep is in effect (instead of emitting the commit when the check succeeds, which is the normal case).

So there is one commit that didn't pass "has 'bar' and written by Ævar" check that was written by you in the recent past (iow, recently you were writing bar all over the place). Changes by other people by definition does not pass "has 'bar' and written by Ævar" check, on the other hand.