Re: [PATCH v2 3/4] log --count: added test

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 4, 2015, 00:54 UTC
Message-ID: <xmqqd2084u8y.fsf@gitster.dls.corp.google.com>
In-Reply-To: <vpqwpyh4rup.fsf@anie.imag.fr>

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

Show 8 quoted lines

> Also, some revision-limiting options can reduce the count like
>
> git log --grep whatever
>
> and you should check that you actually count the right number here.
>
> (I don't know this part of the code enough, but I'm not sure you
> actually deal with this properly)

Yes, "rev-list", when the revision range is "limited" (with or without pathspec), can give "--count" once limit_list() finishes, but "log" filters the result of limit_list() further with at least three separate phases.

 - options in the "grep" family (--grep/--author/etc.) lets you skip
   commits based on the contents of the commit object;

 - options in the "diff" family (-w/-b/etc.) may let "git log"
   consider a commit because the pathspec limit thought two blobs
   were different at byte-by-byte level, but after running "diff"
   with these "looser" comparison, "git log" may realize that there
   weren't any interesting change introduced by the commit [*1*];

 - and finally, of course "log --max-count=20" may further limit the
   maximum number of commits that are shown.  This of course is not
   interesting in the context of "--count" in the sense that "git
   log --count -20 --grep=foo maint..master" may not be immediately
   a sensible thing to do (but we never know.  Perhaps your user may
   be asking "do we have 20 or more commits that say 'foo' in the
   range?")

An implementation of "--count" to take the first and the third ones in account may not be too hard, but I am fairly familiar with the codepath for the second one and I think it would be very tricky.

Note that these additional things "log" does over "rev-list" *DO* justify addition of "--count" to "log" (because "rev-list --count" cannot emulate these); I am however not sure if it is worth the additional complexity we need to add to the codepath (especially for the second phase). I'd need to take another look at the codepaths involved myself to be sure, but I suspect the damage to the codepath for the second may end up to be extensive when we do decide to fix the possible bug in it.

[Footnote]

*1* They may still show the log message in such a case where "-b/-w" was asked and commit had only whitespace changes, but I think we should consider that a bug.