[PATCH v2 0/2] corner cases with "rev-list --use-bitmap-index -n"

From: Jeff King <peff@peff.net>
Date: Jun 3, 2016, 07:06 UTC
Message-ID: <20160603070649.GA24120@sigill.intra.peff.net>
In-Reply-To: <20160602231031.GA11247@sigill.intra.peff.net>

On Thu, Jun 02, 2016 at 07:10:31PM -0400, Jeff King wrote:

Show 10 quoted lines

> diff --git a/builtin/rev-list.c b/builtin/rev-list.c
> index 275da0d..aaa79a3 100644
> --- a/builtin/rev-list.c
> +++ b/builtin/rev-list.c
> @@ -360,6 +360,8 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
>  			uint32_t commit_count;
>  			if (!prepare_bitmap_walk(&revs)) {
>  				count_bitmap_commit_list(&commit_count, NULL, NULL, NULL);
> +				if (revs.max_count && revs.max_count < commit_count)
> +					commit_count = revs.max_count;

...aaaaand this patch is totally wrong. For one thing, the "not set" sentinel value for max_count is "-1", not "0", so it didn't kick in for "-n0".

And for another, any fallback traversal will actually decrement revs.max_count, so we have to save the value before traversing.

And as luck would have it, those two bugs cancel each other out in many cases (including the ones in the test suite!). If you pass "-n1", gets decremented to "0". That would give us a bogus adjustment, but instead we decide that "0" means no adjustement is necessary, and return the number of commits we traversed, which is the right answer as long as we didn't hit any bitmaps on the way down (or we hit one right away, in which case we didn't decrement at all).

Updated patches in a moment (the second has the same sentinel problem, too).

-Peff