Re: [PATCH v2 11/19] pack-objects: use bitmaps when packing objects

From: Jeff King <peff@peff.net>
Date: Oct 30, 2013, 20:07 UTC
Message-ID: <20131030200754.GB23011@sigill.intra.peff.net>
In-Reply-To: <CACsJy8B3VS=WSNeF35_JTiF6byZiefvEYnadkC8BAmKG5Z7gQQ@mail.gmail.com>

On Wed, Oct 30, 2013 at 05:28:08PM +0700, Nguyen Thai Ngoc Duy wrote:

Show 11 quoted lines

> > One other criterion I should have mentioned: we must be using the
> > internal rev-list. That prevented us in v1.8.4.1 and earlier from using
> > bitmaps for shallow fetches. But as of v1.8.4.2, we always use
> > pack-objects' rev-walker. We may need to pass --no-use-bitmap-index for
> > shallow fetches.
> 
> I don't think a new option is needed. The code just needs to check if
> there are any commit grafts. If there are, fall back to the old way.
> That covers both shallow fetches and some rare grafted repos. I think
> refs/replace/* does not impact rev walking, so we should be fine if
> it's used.

I think there are two cases that we need to consider:

  1. We have a full repo and somebody requests a shallow clone for us.
     We probably do not want to use bitmaps here.  In the series we have
     been testing, shallow clones turned off bitmaps because we do not
     use the internal rev_list. But as of cdab485 (upload-pack: delegate
     rev walking in shallow fetch to pack-objects), that distinction
     doesn't hold. I think we can check the use of --shallow-file
     instead of explicitly turning off bitmaps there.

  2. We have a shallow clone that wants to repack. We probably want to
     turn off bitmap writing here. I don't think that grafts actually
     matter here, because pack-objects should always be looking at the
     true graph. It would mean that using "git rev-list
     --use-bitmap-index" does not respect the grafts, and we should
     probably disable it in that case (and ditto for replacements).

-Peff