git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Derrick Stolee <derrickstolee@github.com>
Cc: Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org, me@ttaylorr.com, vdye@github.com,
	avarab@gmail.com, steadmon@google.com, chooglen@google.com
Subject: Re: [PATCH v2.5 01/11] bundle: test unbundling with incomplete history
Date: Tue, 24 Jan 2023 09:16:00 -0800	[thread overview]
Message-ID: <xmqqv8kvhmzz.fsf@gitster.g> (raw)
In-Reply-To: <01f97aff-58a1-ef2c-e668-d37ea513c64e@github.com> (Derrick Stolee's message of "Tue, 24 Jan 2023 09:14:36 -0500")

Derrick Stolee <derrickstolee@github.com> 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
> +	)
> +'

  reply	other threads:[~2023-01-24 17:16 UTC|newest]

Thread overview: 79+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-06 20:36 [PATCH 0/8] Bundle URIs V: creationToken heuristic for incremental fetches Derrick Stolee via GitGitGadget
2023-01-06 20:36 ` [PATCH 1/8] t5558: add tests for creationToken heuristic Derrick Stolee via GitGitGadget
2023-01-17 18:17   ` Victoria Dye
2023-01-17 21:00     ` Derrick Stolee
2023-01-06 20:36 ` [PATCH 2/8] bundle-uri: parse bundle.heuristic=creationToken Derrick Stolee via GitGitGadget
2023-01-09  2:38   ` Junio C Hamano
2023-01-09 14:20     ` Derrick Stolee
2023-01-17 19:13   ` Victoria Dye
2023-01-06 20:36 ` [PATCH 3/8] bundle-uri: parse bundle.<id>.creationToken values Derrick Stolee via GitGitGadget
2023-01-09  3:08   ` Junio C Hamano
2023-01-09 14:41     ` Derrick Stolee
2023-01-17 19:24   ` Victoria Dye
2023-01-06 20:36 ` [PATCH 4/8] bundle-uri: download in creationToken order Derrick Stolee via GitGitGadget
2023-01-09  3:22   ` Junio C Hamano
2023-01-09 14:58     ` Derrick Stolee
2023-01-19 18:32   ` Victoria Dye
2023-01-20 14:56     ` Derrick Stolee
2023-01-06 20:36 ` [PATCH 5/8] clone: set fetch.bundleURI if appropriate Derrick Stolee via GitGitGadget
2023-01-19 19:42   ` Victoria Dye
2023-01-20 15:42     ` Derrick Stolee
2023-01-06 20:36 ` [PATCH 6/8] bundle-uri: drop bundle.flag from design doc Derrick Stolee via GitGitGadget
2023-01-19 19:44   ` Victoria Dye
2023-01-06 20:36 ` [PATCH 7/8] fetch: fetch from an external bundle URI Derrick Stolee via GitGitGadget
2023-01-19 20:34   ` Victoria Dye
2023-01-20 15:47     ` Derrick Stolee
2023-01-06 20:36 ` [PATCH 8/8] bundle-uri: store fetch.bundleCreationToken Derrick Stolee via GitGitGadget
2023-01-19 22:24   ` Victoria Dye
2023-01-20 15:53     ` Derrick Stolee
2023-01-23 15:21 ` [PATCH v2 00/10] Bundle URIs V: creationToken heuristic for incremental fetches Derrick Stolee via GitGitGadget
2023-01-23 15:21   ` [PATCH v2 01/10] bundle: optionally skip reachability walk Derrick Stolee via GitGitGadget
2023-01-23 18:03     ` Junio C Hamano
2023-01-23 18:24       ` Derrick Stolee
2023-01-23 20:13         ` Junio C Hamano
2023-01-23 22:30           ` Junio C Hamano
2023-01-24 12:27             ` Derrick Stolee
2023-01-24 14:14               ` [PATCH v2.5 01/11] bundle: test unbundling with incomplete history Derrick Stolee
2023-01-24 17:16                 ` Junio C Hamano [this message]
2023-01-24 14:16               ` [PATCH v2.5 02/11] bundle: verify using connected() Derrick Stolee
2023-01-24 17:33                 ` Junio C Hamano
2023-01-24 18:46                   ` Derrick Stolee
2023-01-24 20:41                     ` Junio C Hamano
2023-01-24 15:22               ` [PATCH v2 01/10] bundle: optionally skip reachability walk Junio C Hamano
2023-01-23 21:08         ` Junio C Hamano
2023-01-23 15:21   ` [PATCH v2 02/10] t5558: add tests for creationToken heuristic Derrick Stolee via GitGitGadget
2023-01-27 19:15     ` Victoria Dye
2023-01-23 15:21   ` [PATCH v2 03/10] bundle-uri: parse bundle.heuristic=creationToken Derrick Stolee via GitGitGadget
2023-01-23 15:21   ` [PATCH v2 04/10] bundle-uri: parse bundle.<id>.creationToken values Derrick Stolee via GitGitGadget
2023-01-23 15:21   ` [PATCH v2 05/10] bundle-uri: download in creationToken order Derrick Stolee via GitGitGadget
2023-01-27 19:17     ` Victoria Dye
2023-01-27 19:32       ` Junio C Hamano
2023-01-30 18:43         ` Derrick Stolee
2023-01-30 19:02           ` Junio C Hamano
2023-01-30 19:12             ` Derrick Stolee
2023-01-23 15:21   ` [PATCH v2 06/10] clone: set fetch.bundleURI if appropriate Derrick Stolee via GitGitGadget
2023-01-23 15:21   ` [PATCH v2 07/10] bundle-uri: drop bundle.flag from design doc Derrick Stolee via GitGitGadget
2023-01-23 15:21   ` [PATCH v2 08/10] fetch: fetch from an external bundle URI Derrick Stolee via GitGitGadget
2023-01-27 19:18     ` Victoria Dye
2023-01-23 15:21   ` [PATCH v2 09/10] bundle-uri: store fetch.bundleCreationToken Derrick Stolee via GitGitGadget
2023-01-23 15:21   ` [PATCH v2 10/10] bundle-uri: test missing bundles with heuristic Derrick Stolee via GitGitGadget
2023-01-27 19:21     ` Victoria Dye
2023-01-30 18:47       ` Derrick Stolee
2023-01-27 19:28   ` [PATCH v2 00/10] Bundle URIs V: creationToken heuristic for incremental fetches Victoria Dye
2023-01-31 13:29   ` [PATCH v3 00/11] " Derrick Stolee via GitGitGadget
2023-01-31 13:29     ` [PATCH v3 01/11] bundle: test unbundling with incomplete history Derrick Stolee via GitGitGadget
2023-01-31 13:29     ` [PATCH v3 02/11] bundle: verify using check_connected() Derrick Stolee via GitGitGadget
2023-01-31 17:35       ` Junio C Hamano
2023-01-31 19:31         ` Derrick Stolee
2023-01-31 19:36           ` Junio C Hamano
2023-01-31 13:29     ` [PATCH v3 03/11] t5558: add tests for creationToken heuristic Derrick Stolee via GitGitGadget
2023-01-31 13:29     ` [PATCH v3 04/11] bundle-uri: parse bundle.heuristic=creationToken Derrick Stolee via GitGitGadget
2023-01-31 13:29     ` [PATCH v3 05/11] bundle-uri: parse bundle.<id>.creationToken values Derrick Stolee via GitGitGadget
2023-01-31 21:22       ` Junio C Hamano
2023-01-31 13:29     ` [PATCH v3 06/11] bundle-uri: download in creationToken order Derrick Stolee via GitGitGadget
2023-01-31 13:29     ` [PATCH v3 07/11] clone: set fetch.bundleURI if appropriate Derrick Stolee via GitGitGadget
2023-01-31 13:29     ` [PATCH v3 08/11] bundle-uri: drop bundle.flag from design doc Derrick Stolee via GitGitGadget
2023-01-31 13:29     ` [PATCH v3 09/11] fetch: fetch from an external bundle URI Derrick Stolee via GitGitGadget
2023-01-31 13:29     ` [PATCH v3 10/11] bundle-uri: store fetch.bundleCreationToken Derrick Stolee via GitGitGadget
2023-01-31 13:29     ` [PATCH v3 11/11] bundle-uri: test missing bundles with heuristic Derrick Stolee via GitGitGadget
2023-01-31 22:01     ` [PATCH v3 00/11] Bundle URIs V: creationToken heuristic for incremental fetches Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqv8kvhmzz.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=chooglen@google.com \
    --cc=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=me@ttaylorr.com \
    --cc=steadmon@google.com \
    --cc=vdye@github.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).