Re: [PATCH 04/10] builtin/fsck: don't check alternates with "--no-full"
From: Karthik Nayak <hidden>
Date: 2026-08-27 10:12:16
Patrick Steinhardt [off-list ref] writes:
According to git-fsck(1), the "--full" option behaves in the following way: Check not just objects in GIT_OBJECT_DIRECTORY ($GIT_DIR/objects), but also the ones found in alternate object pools listed in GIT_ALTERNATE_OBJECT_DIRECTORIES or $GIT_DIR/objects/info/alternates, and in packed Git archives found in $GIT_DIR/objects/pack and corresponding pack subdirectories in alternate object pools. So ultimately, it is supposed to control two things: (1) whether we only check the main object directory, and (2) whether we check packfiles. In its current state though, the flag only controls whether we check packfiles or not, and if so we verify packfiles of all attached sources. But we also have checks for loose objects in git-fsck(1), and here we unconditionally check them in all sources.
To reiterate, without '--full': Check local loose + alternate loose. No packed objects with '--full': Check local loose + alternate loose. local packed + alternates packed. And we want to only do local loose in the latter. Makes sense.
quoted hunk ↗ jump to hunk
The flag is arguably conflating two unrelated concerns with one another, and it really should be split up into two flags: one that controls how thorough we want to check individual sources, and one that controls which sources we want to check in the first place. So ideally, we would have: - "--include-alternates": check all sources, not only the local one. - "--include-optimized-objects": check not only loose objects, but also those that have been packed. Note that we explicitly don't say "--include-packed-objects" here to be more backend-agnostic. - "--full": implies both of the above flags. This feels out of scope for this series though. So for now, simply fix the code by honoring locality of the sources for loose objects. Signed-off-by: Patrick Steinhardt <redacted> --- builtin/fsck.c | 3 ++- t/t1450-fsck.sh | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-)diff --git a/builtin/fsck.c b/builtin/fsck.c index 5132ff0f15..3f6056535f 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c@@ -1047,7 +1047,8 @@ int cmd_fsck(int argc, mark_object_for_connectivity, repo, 0); } else { for (source = repo->objects->sources; source; source = source->next) - fsck_source(repo, source); + if (check_full || source->local) + fsck_source(repo, source);
So we check the local bit and only fsck that source. Looks good.
quoted hunk ↗ jump to hunk
if (check_full) { struct packed_git *p;diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh index 77cd96de78..1b4074304c 100755 --- a/t/t1450-fsck.sh +++ b/t/t1450-fsck.sh@@ -844,6 +844,11 @@ test_expect_success 'alternate objects are correctly blamed' ' echo "../../alt.git/objects" >.git/objects/info/alternates && mkdir alt.git/objects/$(dirname $path) && >alt.git/objects/$(dirname $path)/$(basename $path) && + + # Without "--full", only the local object source is checked. + git fsck --no-full >out 2>&1 && + test_must_be_empty out && + test_must_fail git fsck >out 2>&1 && test_grep alt.git out ' --2.55.0.822.g20453c30eb.dirty
Attachments
- signature.asc [application/pgp-signature] 690 bytes