Re: [PATCH 2/3] t5309: create failing test for 'git index-pack'

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 23, 2025, 19:37 UTC
Message-ID: <xmqqfrhyy8ia.fsf@gitster.g>
In-Reply-To: <a9430447641ff3b3f519abc0960b6741fd7df700.1745430004.git.gitgitgadget@gmail.com>

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 9 quoted lines

> From: Derrick Stolee <stolee@gmail.com>
>
> This new test demonstrates some behavior where a valid packfile is being
> rejected by the Git client due to the order in which it is resolving
> REF_DELTAs.
>
> The thin packfile has a REF_DELTA chain A->B->C where C is not included
> in the packfile. However, the client repository contains both C and B
> already. Thus, 'git index-pack' is able to resolve A before resolving B.

In order to reconstitute A, B is needed (which recipient has), and in order to reconstitute B, C is needed (which recipient also has).

The index-pack sees delta based on B to recreate A; it should be able to reconstitute A using B that already exists.

OK.

> When resolving B, it then attempts to resolve any other REF_DELTAs that
> are pointing to B as a base. This "revisits" A and complains as if there
> is a cycle, but it did not actually detect a cycle.

That's interesting.

Show 18 quoted lines

> +test_expect_failure 'index-pack works with thin pack A->B->C with B on disk' '
> +	git init server &&
> +	(
> +		cd server &&
> +		test_commit_bulk 4
> +	) &&
> +
> +	A=$(git -C server rev-parse HEAD^{tree}) &&
> +	B=$(git -C server rev-parse HEAD~1^{tree}) &&
> +	C=$(git -C server rev-parse HEAD~2^{tree}) &&
> +	git -C server reset --hard HEAD~1 &&
> +
> +	cat >in <<-EOF &&
> +	REF_DELTA $A $B
> +	REF_DELTA $B $C
> +	EOF
> +
> +	test-tool -C server pack-deltas 2 <in >thin.pack &&

This is minor, but I somehow find it easier to follow without the temporary file, i.e.

	test-tool -C server pack-deltas 2 >thin.pack <<-EOF &&
	REF_DELTA $A $B
	REF_DELTA $B $C
	EOF

> +	git clone "file://$(pwd)/server" client &&

This truly loses A from the resulting "client" repository, but the history still can reach B and C.

Show 5 quoted lines

> +	(
> +		cd client &&
> +		git index-pack --fix-thin --stdin <../thin.pack
> +	)
> +'

Makes sense.