From: Jeff King <peff@peff.net>
Date: Mon, 23 Feb 2026 11:11:20 GMT
Subject: Re: [PATCH 4/4] pack-check: fix verification of large objects
Message-ID: <20260223111120.GC215364@coredump.intra.peff.net>
In-Reply-To: <20260223-pks-fsck-fix-v1-4-c29036832b6e@pks.im>

On Mon, Feb 23, 2026 at 10:50:43AM +0100, Patrick Steinhardt wrote:

> diff --git a/pack-check.c b/pack-check.c
> index 46782a29d5..6149567060 100644
> --- a/pack-check.c
> +++ b/pack-check.c
> @@ -155,7 +155,7 @@ static int verify_packfile(struct repository *r,
>  			err = error("packed %s from %s is corrupt",
>  				    oid_to_hex(&oid), p->pack_name);
>  		else if (!data &&
> -			 (!(stream = odb_read_stream_open(r->objects, &oid, NULL)) ||
> +			 (packfile_read_object_stream(&stream, p, entries[i].offset) < 0 ||

And now this change is delightfully simple.

> +test_expect_success 'fsck handles multiple packfiles with big blobs' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		blob_one=$(test-tool genrandom one 200k | git hash-object -t blob -w --stdin) &&
> +		blob_two=$(test-tool genrandom two 200k | git hash-object -t blob -w --stdin) &&
> +		printf "%s\n" "$blob_one" | git pack-objects .git/objects/pack/pack &&
> +		printf "%s\n" "$blob_two" | git pack-objects .git/objects/pack/pack &&
> +		remove_object "$blob_one" &&
> +		remove_object "$blob_two" &&
> +		git -c core.bigFileThreshold=100k fsck
> +	)
> +'

I like seeing this much-more-specific test case. It does sort of become
a noop if we fix the iteration problem, though.

A more concrete test would probably be something like:

   1. Two packs, $X and $Y, both contain the same object.

   2. The object is corrupt in $X but not in $Y.

   3. Running fsck detects that one copy is corrupt but the other is
      not.

Right now it may or may not fail depending on the ordering of the packs
in the MRU list (which we might be able to tweak via mtimes). But
hopefully in the "after" state it should deterministically complain
about $X.

-Peff