Subject: Re: [PATCH v2.5 01/11] bundle: test unbundling with incomplete history

From: Junio C Hamano <[email-masked]>
Date: Tue, 24 Jan 2023 09:16:00 -0800

Derrick Stolee <[email-masked]> writes:

> In order to construct a broken history, perform a shallow clone of a
> repository with a linear history, but whose default branch ('base') has
> a single commit, so dropping the shallow markers leaves a complete
> history from that reference. However, the 'tip' reference adds a
> shallow commit whose parent is missing in the cloned repository. Trying
> to unbundle a bundle with the 'tip' as a prerequisite will succeed past
> the object store check and move into the reachability check.

It makes it sound convoluted set-up for tests, but I guess it is the
most direct way to get to the state you want to test, which is good.

In practice, the problem would appear when you create a multi-commit
branch, which then is discarded.  GC then decides to expire the
older part of the commit chain while leaving the commits near the
tip still in the object store.  So the problem can happen without
users doing anything esoteric, and is very much worth testing.

> +test_expect_success 'verify catches unreachable, broken prerequisites' '
> +	test_when_finished rm -rf clone-from clone-to &&

OK, so my understanding of what happens is ...

> +	git init clone-from &&
> +	(
> +		cd clone-from &&
> +		git checkout -b base &&
> +		test_commit A &&
> +		git checkout -b tip &&
> +		git commit --allow-empty -m "will drop by shallow" &&
> +		git commit --allow-empty -m "will keep by shallow" &&
> +		git commit --allow-empty -m "for bundle, not clone" &&
> +		git bundle create tip.bundle tip~1..tip &&

... there is a single strand of pearls

	A---D---K---B tip

where D is with "will drop by shallow" message.  The bundle
is prepared to give a history leading to B while requiring K.

> +		git reset --hard HEAD~1 &&
> +		git checkout base

Then B is thrown away before the history is cloned.

> +	) &&
> +	BAD_OID=$(git -C clone-from rev-parse tip~1) &&
> +	TIP_OID=$(git -C clone-from rev-parse tip) &&
> +	git clone --depth=1 --no-single-branch \
> +		"file://$(pwd)/clone-from" clone-to &&
> +	(
> +		cd clone-to &&

The cloned repository should have

	A---d---K

where D is missing behind the shallow boundary, origin/tip pointing
at K.

> +		# Set up broken history by removing shallow markers
> +		git update-ref -d refs/remotes/origin/tip &&

But we remove origin/tip, so K (and its trees and blobs) is totally
disconnected.

> +		rm .git/shallow &&

And then this removes the shallow info that makes us to pretend that
K does not have D (missing) as its parent.  Now we lack the required
parent D if we start traversing from K.

> +		# Verify should fail
> +		test_must_fail git bundle verify \
> +			../clone-from/tip.bundle 2>err &&

verify_bundle() wants to see traversal from "--all" to hit the
prerequisite objects and K certainly cannot be reached by any ref.

OK.  So we ended up with a repository where we are on 'base' branch,
and origin/HEAD and origin/base remote-tracking refs exist, all of
these refs pointing at A.  Plus K exists but not D, but it is fine
because K is not referenced by any ref.

This is perfectly constructed test case that checks a very
interesting scenario.  It is as if the commit chain D---K was
discarded (via "git branch -D") and then D got expired for being too
old but K is not old enough.

We want to ensure "git bundle verify" and "git fetch ./bundle.file"
in this healthy repository, where its refs do honor the promise, but
its object store has unconnected commits (like "K") that are not
complete, behaves sensibly.  If we loosen "prerequisites must be
reachable from refs" to "prerequisites must exist", it will lead to
repository corruption if we allow the bundle to be unbundled and its
tips made into our refs, because these new refs point at incomplete
objects.

Excellent.

> +		# Unbundling should fail
> +		test_must_fail git bundle unbundle \
> +			../clone-from/tip.bundle 2>err &&
> +		grep "Could not read $BAD_OID" err &&
> +		grep "Failed to traverse parents of commit $TIP_OID" err
> +	)
> +'