Re: [PATCH 1/3] builtin/repack: fix geometric repacks with promisor remotes

> When repacking a repository with promisor remotes git-repack(1) knows to
> pass "--exclude-promisor-objects" to git-pack-objects(1). This option
> ensures that the new pack will not contain any promised object that do
> not yet exist locally.
> 
> This command line option is incompatible with "--stdin-packs": the
> latter option enables the rev-walk-based machinery to figure out which
> objects to add to the pack, whereas the former tells git-pack-objects(1)
> to merge all packs passed via stdin into one large pack. As we do not
> know to filter those packs via the passed-in revisions it is clear that
> at the current point in time nothing sensible comes out of combining
> these two options.

> But there is one case where git-repack(1) decides to pass both options:
> when performing a geometric repack we always pass "--stdin-packs" to
> identify the packs that should be merged. So if one performs a geometric
> repack in a partial clone we'll end up with both options, and that
> causes the repack to fail.
> 
> Fix this issue by never passing "--exclude-promisor-objects" when we
> have a geometric split factor. We don't need the option anyway when
> doing a geometric repack as we will only ever pack loose objects or
> merge multiple packs. And neither of those cases can yield a promisor
> object.

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/repack.c            |  5 +++--
>  t/t7703-repack-geometric.sh | 26 ++++++++++++++++++++++++++
>  2 files changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/builtin/repack.c b/builtin/repack.c
> index d9012141f6..4621eed3e6 100644
> --- a/builtin/repack.c
> +++ b/builtin/repack.c
> @@ -294,9 +294,10 @@ int cmd_repack(int argc,
>  		strvec_push(&cmd.args, "--all");
>  		strvec_push(&cmd.args, "--reflog");
>  		strvec_push(&cmd.args, "--indexed-objects");
> +
> +		if (repo_has_promisor_remote(repo))
> +			strvec_push(&cmd.args, "--exclude-promisor-objects");
>  	}
> -	if (repo_has_promisor_remote(repo))
> -		strvec_push(&cmd.args, "--exclude-promisor-objects");

>  	if (!write_midx) {
>  		if (write_bitmaps > 0)
>  			strvec_push(&cmd.args, "--write-bitmap-index");
> diff --git a/t/t7703-repack-geometric.sh b/t/t7703-repack-geometric.sh
> index 9fc1626fbf..6d2c712bff 100755
> --- a/t/t7703-repack-geometric.sh
> +++ b/t/t7703-repack-geometric.sh
> @@ -445,4 +445,30 @@ test_expect_success '--geometric -l disables writing bitmaps with non-local pack
>  	test_path_is_file member/.git/objects/pack/multi-pack-index-*.bitmap
>  '
>  
> +test_expect_success '--geometric works with promisor packs' '
> +	test_when_finished "rm -fr remote local" &&
> +
> +	git init remote &&
> +	test_commit -C remote first file first &&
> +	test_commit -C remote second file second &&
> +	git -C remote config set uploadpack.allowfilter 1 &&
> +	git -C remote config set uploadpack.allowanysha1inwant 1 &&
> +	git -C remote repack -Ad &&
> +
> +	git clone --filter=blob:none file://"$(pwd)"/remote local &&
> +	git -C local rev-list --objects --missing=print HEAD >missing-objects &&
> +	test_grep "^?" missing-objects &&
> +
> +	# Assert that promisor packs are left alone and that we still manage to
> +	# create new geometric packs.
> +	ls local/.git/objects/pack/*.promisor >promisors-before &&
> +	ls local/.git/objects/pack/*.pack >packs-before &&
> +	test_commit -C local change &&
> +	git -C local repack --geometric=2 &&
> +	ls local/.git/objects/pack/*.promisor >promisors-after &&
> +	ls local/.git/objects/pack/*.pack >packs-after &&
> +	! cmp packs-before packs-after &&
> +	test_cmp promisors-before promisors-after