From: Derrick Stolee <stolee@gmail.com>
Date: Fri, 13 Jul 2018 00:50:36 GMT
Subject: Re: [PATCH v4 16/23] config: create core.multiPackIndex setting
Message-ID: <61de4c01-daeb-2fad-491e-aa06facc3db8@gmail.com>
In-Reply-To: <xmqqin5kupu3.fsf@gitster-ct.c.googlers.com>

On 7/12/2018 5:05 PM, Junio C Hamano wrote:
> Derrick Stolee <stolee@gmail.com> writes:
>
>> 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?
Sorry. I squashed the commits, and intended to drop this second commit 
message.
>
>> 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.
This is the first time we _need_ them in the .git/object directory.
>>   '
>>   
>> +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.

Sorry that I missed these two.

Thanks,

-Stolee