From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 19 Nov 2025 21:27:51 GMT
Subject: Re: [PATCH 07/13] builtin/index-pack: fix deferred fsck outside repos
Message-ID: <xmqq1pltbtm0.fsf@gitster.g>
In-Reply-To: <20251119-b4-pks-odb-creation-v1-7-2b2ed2612cb6@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> There's another option though: instead of skipping the final object
> checks, we can die if there are any queued object checks. With this
> change we now die exactly if and only if we would have previously
> segfaulted. Like this we ensure that objects that _may_ fail the
> consistency checks won't be silently skipped, and at the same time we
> give users a much better error message.

A packfile stream may not have the blob objects these tree entries
refer to, in which case index-pack cannot work outside a repository,
but I think that is fine.

> @@ -2110,8 +2110,23 @@ int cmd_index_pack(int argc,
>  	else
>  		close(input_fd);
>  
> -	if (do_fsck_object && fsck_finish(&fsck_options))
> -		die(_("fsck error in pack objects"));
> +	if (do_fsck_object) {
> +		/*
> +		 * We cannot perform queued consistency checks when running
> +		 * outside of a repository because those require us to read
> +		 * from the object database, which is uninitialized.
> +		 *
> +		 * TODO: we may eventually set up an in-memory object database,
> +		 * which would allow us to perform these queued checks.
> +		 */
> +		if (!startup_info->have_repository &&
> +		    fsck_has_queued_checks(&fsck_options))
> +			die(_("cannot perform queued object checks outside "
> +			      "of a repository"));
> +
> +		if (fsck_finish(&fsck_options))
> +			die(_("fsck error in pack objects"));
> +	}

OK.

> +bool fsck_has_queued_checks(struct fsck_options *options)
> +{
> +	return !oidset_equal(&options->gitmodules_found, &options->gitmodules_done) ||
> +	       !oidset_equal(&options->gitattributes_found, &options->gitattributes_done);
> +}

So, if we see a tree entry for these special blobs (and remember
them in the _found oid set) before we see the blobs, fsck_blob()
would notice that it is looking at the blob that is in these _found
set, and throw it in _done set while checking the blob in-core.

A packfile we generate has trees before blobs, so a self contained
pack stream should still be validatable outside a repository with
this code, but other people's reimplementations of Git may produce
a packfile that has a blob before a tree that refers to the blob.
In other words, we can validate a self contained pack stream outside
repository on a best-effort basis.  And that is perfectly fine.