Re: [PATCH v5] pack-objects: teach it to use reachability bitmap index when generating non-stdout pack too
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 8, 2016, 20:53 UTC
- Message-ID
- <xmqqbn13dltr.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20160808185541.6433-1-kirr@nexedi.com>
Kirill Smelkov <kirr@nexedi.com> writes:
Show 11 quoted lines
> diff --git a/Documentation/config.txt b/Documentation/config.txt > index bc1c433..4ba0c4a 100644 > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -2244,6 +2244,9 @@ pack.useBitmaps:: > to stdout (e.g., during the server side of a fetch). Defaults to > true. You should not generally need to turn this off unless > you are debugging pack bitmaps. > ++ > +*NOTE*: when packing to file (e.g., on repack) the default is always not to use > + pack bitmaps.
This is a bit hard to read and understand.
The patched result starts with "When true, git will use bitmap when packing to stdout", i.e. when packing to file, git will not. So this *NOTE* is repeating the same thing. The reader is made to wonder "Why does it need to repeat the same thing? Does this mean when the variable is set, a pack sent to a disk uses the bitmap?"
I think what you actually do in the code is to make the variable affect _only_ the standard-output case, and users need a command line option if they want to use bitmap when writing to a file (the code to do so looks correctly done).
Show 19 quoted lines
> diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh > index 3893afd..ffecc6a 100755 > --- a/t/t5310-pack-bitmaps.sh > +++ b/t/t5310-pack-bitmaps.sh > @@ -118,6 +118,18 @@ test_expect_success 'incremental repack can disable bitmaps' ' > git repack -d --no-write-bitmap-index > ' > > +test_expect_success 'pack-objects to file can use bitmap' ' > + # make sure we still have 1 bitmap index from previous tests > + ls .git/objects/pack/ | grep bitmap >output && > + test_line_count = 1 output && > + # verify equivalent packs are generated with/without using bitmap index > + packasha1=$(git pack-objects --no-use-bitmap-index --all packa </dev/null) && > + packbsha1=$(git pack-objects --use-bitmap-index --all packb </dev/null) && > + git show-index <packa-$packasha1.idx | cut -d" " -f2 >packa.objects && > + git show-index <packb-$packbsha1.idx | cut -d" " -f2 >packb.objects && > + test_cmp packa.objects packb.objects > +'
Looks good.