From: Jonathan Tan <jonathantanmy@google.com>
Date: Tue, 22 Sep 2020 23:52:58 GMT
Subject: Re: [PATCH v3 8/8] maintenance: add incremental-repack auto condition
Message-ID: <20200922235258.920928-1-jonathantanmy@google.com>
In-Reply-To: <e9bb32f53ade2067f773bfe6e5c13ed1a5d694a6.1598380599.git.gitgitgadget@gmail.com>

> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
> index 5c08afc19a..6f878b0141 100755
> --- a/t/t7900-maintenance.sh
> +++ b/t/t7900-maintenance.sh
> @@ -220,4 +220,35 @@ test_expect_success EXPENSIVE 'incremental-repack 2g limit' '
>  		 --no-progress --batch-size=2147483647 <run-2g.txt
>  '
>  
> +test_expect_success 'maintenance.incremental-repack.auto' '
> +	git repack -adk &&
> +	git config core.multiPackIndex true &&
> +	git multi-pack-index write &&

After this, there is exactly one pack, and it is in the MIDX.

> +	GIT_TRACE2_EVENT="$(pwd)/midx-init.txt" git \
> +		-c maintenance.incremental-repack.auto=1 \
> +		maintenance run --auto --task=incremental-repack 2>/dev/null &&
> +	test_subcommand ! git multi-pack-index write --no-progress <midx-init.txt &&
> +	for i in 1 2

Same comment as in the earlier patch - why 2 iterations?

> +	do
> +		test_commit A-$i &&
> +		git pack-objects --revs .git/objects/pack/pack <<-\EOF &&
> +		HEAD
> +		^HEAD~1
> +		EOF
> +		GIT_TRACE2_EVENT=$(pwd)/trace-A-$i git \
> +			-c maintenance.incremental-repack.auto=2 \
> +			maintenance run --auto --task=incremental-repack 2>/dev/null &&
> +		test_subcommand ! git multi-pack-index write --no-progress <trace-A-$i &&

OK - one pack not in the MIDX, so this does not get run.

> +		test_commit B-$i &&
> +		git pack-objects --revs .git/objects/pack/pack <<-\EOF &&
> +		HEAD
> +		^HEAD~1
> +		EOF
> +		GIT_TRACE2_EVENT=$(pwd)/trace-B-$i git \
> +			-c maintenance.incremental-repack.auto=2 \
> +			maintenance run --auto --task=incremental-repack 2>/dev/null &&
> +		test_subcommand git multi-pack-index write --no-progress <trace-B-$i || return 1

OK - 2 packs not in the MIDX, so this gets run.

As I said in my review of patch 1, apart from my minor comments in this
and the preceding patches, these patches look good to me.