Re: [PATCH v4 16/23] config: create core.multiPackIndex setting

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 12, 2018, 21:05 UTC
Message-ID: <xmqqin5kupu3.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180712193940.21065-17-dstolee@microsoft.com>

Derrick Stolee <stolee@gmail.com> writes:

Show 21 quoted lines

> The core.multiPackIndex config setting controls the multi-pack-
> index (MIDX) feature. If false, the setting will disable all reads
> from the multi-pack-index file.
>
> Read this config setting in the new prepare_multi_pack_index_one()
> which is called during prepare_packed_git(). This check is run once
> per repository.
>
> Add comparison commands in t5319-multi-pack-index.sh to check
> typical Git behavior remains the same as the config setting is turned
> on and off. This currently includes 'git rev-list' and 'git log'
> commands to trigger several object database reads. Currently, these
> would only catch an error in the prepare_multi_pack_index_one(), but
> with later commits will catch errors in object lookups, abbreviations,
> and approximate object counts.
>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
>
> midx: prepare midxed_git struct
>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>

What is going on around here?

Show 26 quoted lines

> diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
> index 4a4fa26f7a..601e28a2f0 100755
> --- a/t/t5319-multi-pack-index.sh
> +++ b/t/t5319-multi-pack-index.sh
> @@ -3,6 +3,8 @@
>  test_description='multi-pack-indexes'
>  . ./test-lib.sh
>  
> +objdir=.git/objects
> +
>  midx_read_expect () {
>  	NUM_PACKS=$1
>  	NUM_OBJECTS=$2
> @@ -76,18 +78,35 @@ test_expect_success 'create objects' '
>  '
>  
>  test_expect_success 'write midx with one v1 pack' '
> -	pack=$(git pack-objects --index-version=1 pack/test <obj-list) &&
> -	test_when_finished rm pack/test-$pack.pack pack/test-$pack.idx pack/multi-pack-index &&
> -	git multi-pack-index --object-dir=. write &&
> -	midx_read_expect 1 18 4 .
> +	pack=$(git pack-objects --index-version=1 $objdir/pack/test <obj-list) &&
> +	test_when_finished rm $objdir/pack/test-$pack.pack \
> +		$objdir/pack/test-$pack.idx $objdir/pack/multi-pack-index &&
> +	git multi-pack-index --object-dir=$objdir write &&
> +	midx_read_expect 1 18 4 $objdir

Hmph, so we used to run tests as if $cwd were GIT_OBJECT_DIRECTORY but now we are running them from the top-level of the working tree, just like all the other tests? Interesting.

Show 9 quoted lines

>  '
>  
> +midx_git_two_modes() {
> +	git -c core.multiPackIndex=false $1 >expect &&
> +	git -c core.multiPackIndex=true $1 >actual &&
> +	test_cmp expect actual
> +}
> +
> +compare_results_with_midx() {

Style: "compare_results_with_midx () {", just like mdx_read_expect
near the top of the file, but unlike midx_git_two_modes we see
nearby.  Please keep "git grep 'funcname () {'" a usable way to
locate where a shell function is defined without forcing people to
type an asterisk.