Re: [PATCH 4/4] pack-check: fix verification of large objects
- From
Jeff King <peff@peff.net>
- Date
- Feb 23, 2026, 11:11 UTC
- 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:
Show 10 quoted lines
> 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.
Show 14 quoted lines
> +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