Re: [PATCH 2/2] maintenance: trigger --auto for promisor rollups

[Patrick Steinhardt <ps@pks.im>]

On Tue, Aug 04, 2026 at 08:57:46PM -0700, Taylor Blau wrote:
> Commit 9bc151850c (builtin/maintenance: introduce "geometric-repack"
> task, 2025-10-24) added an auto condition for the geometric-repack
> task. It runs the task when ordinary packs need to be combined or when
> the number of loose objects crosses the configured threshold.
> 
> Later on in commit dcc9c7ef47 (builtin/repack: handle promisor packs
> with geometric repacking, 2026-01-05), the geometric repack machinery
> started handling promisor packs separately, but did not correspondingly
> update the auto condition.
> 
> As a result, a repository can have promisor packs ready to combine
> while its non-promisor packs and loose object count require no work. In
> that case, `--auto` skips the task even though a geometric repack
> would combine at least two promisor packs.
> 
> Check `geometry.promisor_split` alongside `geometry.split`.

Yeah, this is a more obviously correct thing to do compared to the
preceding patch.

> diff --git a/builtin/gc.c b/builtin/gc.c
> index ed75c12c43..e9572940dc 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -1649,7 +1649,7 @@ static int geometric_repack_auto_condition(struct gc_config *cfg UNUSED)
>  	 * When we'd merge at least two packs with one another we always
>  	 * perform the repack.
>  	 */
> -	if (geometry.split) {
> +	if (geometry.split || geometry.promisor_split) {
>  		ret = 1;
>  		goto out;
>  	}

This looks obviously correct.

> diff --git a/t/t5331-pack-objects-stdin.sh b/t/t5331-pack-objects-stdin.sh
> index c74b5861af..2a983e28ac 100755
> --- a/t/t5331-pack-objects-stdin.sh
> +++ b/t/t5331-pack-objects-stdin.sh
> @@ -368,7 +368,8 @@ test_expect_success '--stdin-packs does not perform backfill fetch' '
>  	git -C remote config set --local uploadpack.allowfilter 1 &&
>  	git -C remote config set --local uploadpack.allowanysha1inwant 1 &&
>  
> -	git clone --filter=tree:0 "file://$(pwd)/remote" client &&
> +	git -c maintenance.auto=false clone --filter=tree:0 \
> +		"file://$(pwd)/remote" client &&
>  	(
>  		cd client &&
>  		ls .git/objects/pack/*.promisor | sed "s|.*/||; s/\.promisor$/.pack/" >packs &&

Curious that git-clone(1) already spawns maintenance, but with "tree:0"
we may end up fetching multiple promisor packs from the remote as we
discover more trees to backfill. So this makes sense.

> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
> index ba5b359e77..fb5f2d8902 100755
> --- a/t/t7900-maintenance.sh
> +++ b/t/t7900-maintenance.sh
> @@ -759,6 +759,29 @@ test_expect_success 'geometric repacking with --auto' '
>  	)
>  '
>  
> +test_expect_success 'geometric repacking with --auto handles promisor packs' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		git config set maintenance.auto false &&
> +		git remote add promisor garbage &&
> +		git config set remote.promisor.promisor true &&
> +
> +		for n in $(test_seq 6)
> +		do
> +			test_commit $n || return 1
> +		done &&
> +
> +		pack_promisor 1 >/dev/null &&
> +		pack_promisor 1..2 >/dev/null &&
> +		pack_promisor 2..6 >/dev/null &&
> +		git prune-packed &&
> +
> +		test_geometric_repack_needed true auto=9000

The auto-value here doesn't matter at all, as we shouldn't have any
loose objects in the first place and really only want to trigger
maintenance because of the promisors. Makes sense.

Thanks!

Patrick