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

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

quoted

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