git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Subject: [PATCH v2 0/2] corner cases with "rev-list --use-bitmap-index -n"
Date: Fri, 3 Jun 2016 03:06:49 -0400	[thread overview]
Message-ID: <20160603070649.GA24120@sigill.intra.peff.net> (raw)
In-Reply-To: <20160602231031.GA11247@sigill.intra.peff.net>

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

> 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

  reply	other threads:[~2016-06-03  7:07 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-02 23:09 [PATCH 0/2] corner cases with "rev-list --use-bitmap-index -n" Jeff King
2016-06-02 23:10 ` [PATCH 1/2] rev-list: "adjust" results of "--count " Jeff King
2016-06-03  7:06   ` Jeff King [this message]
2016-06-03  7:07     ` [PATCH v2 " Jeff King
2016-06-03  7:08     ` [PATCH v2 2/2] rev-list: disable bitmaps when "-n" is used with listing objects Jeff King
2016-06-02 23:10 ` [PATCH " Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160603070649.GA24120@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).