On Sat, May 31, 2025 at 02:17:34PM +0200, Kristoffer Haugsbakk wrote: > On Sat, May 31, 2025, at 05:39, shejialuo wrote: > > It is reported that "git refs verify" would fail when encountering > > worktrees created on Git v2.43.0 or older versions. These versions > > Nit: maybe > > "git refs verify" doesn't work if there are worktrees created on Git > v2.43.0 ... > > The part about it specifically not working if there are no worktree refs > might be obvious when you take in all of the context here (no refs/ > directory). I don’t know. > Right, I will improve this. > > don't automatically create the "refs" directory, causing the error: > > > > error: cannot open directory .git/worktrees/<worktree name>/refs: > > No such file or directory > > > > Since 8f4c00de95 (builtin/worktree: create refdb via ref backend, > > 2024-01-08), we automatically create the "refs" directory for new > > worktrees. However, the fsck code incorrectly assumes all linked > > worktrees have this directory, thus introducing compatibility issue. > > Thanks for finding that commit. > > At this point in the message it seems like the fsck code never worked > with these old linked worktrees. But `git refs verify` used to work > with them until 7c78d819e6a (ref: support multiple worktrees check for > refs, 2024-11-20) which was part of v2.48.0. So I think it’s worth > mentioning that commit as well. > Good suggestion. > You wrote on the first email which I’ll just respond to here since > it’s relevant: > > https://lore.kernel.org/git/a2a50127-6ab9-4d8a-abcc-b1a741df293e@xxxxxxxxxxxxxxxx/T/#mada29f8b0d02091d21412d8bd57cc666bc657c04 > > > > And hope that this fix would land in this release. > > Like I said in the first email the only minor regression in this release > cycle is that git-fsck(1) reports these errors on stderr because the > default `--reference`. This was how I spotted the issue on rc0. But I > neglected to mention that the commit that introduced `--references` > (default) for git-fsck(1) is v2.48.0-rc1-49-gc1cf918d3ad (builtin/fsck: > add `git refs verify` child process, 2025-02-28).[1] > Because we would call "git refs verify" subprocess in "git-fsck(1)" in this release cycle, I just want to fix this problem before the release. Thus, it won't affect the users. > So based on the last what’s cooking email[2] it depends on if the stderr > output regresssion from git-fsck(1) in this release cycle is severe > enough to need be fixed in this release. Because the `git refs-verify` > problem has been there since v2.48.0. > > † 1: Part of the reason for neglecting that was that building that > commit fails for me. So the bisection skipped it and couldn’t find the > commit. Is that just me? The merge commit does build de35b7b3ff (Merge > branch 'sj/ref-consistency-checks-more', 2025-03-26). I changed > `start_progress(r, _("Checking ref database"), 1);` to > `progress = start_progress(_("Checking ref database"), 1);`. I > don’t know if that is wrong but it made the bisection script run > without having to error out with `125`. > > [2]: https://lore.kernel.org/git/xmqqiklhd3tc.fsf@gitster.g/T/#u > > > > > Check for ENOENT errno before reporting directory access errors for > > linked worktrees to maintain backward compatibility. > > > > Reported-by: Kristoffer Haugsbakk <code@xxxxxxxxxxxxxxx> > > Signed-off-by: shejialuo <shejialuo@xxxxxxxxx> > > --- > > refs/files-backend.c | 3 +++ > > t/t0602-reffiles-fsck.sh | 15 +++++++++++++++ > > 2 files changed, 18 insertions(+) > > > > diff --git a/refs/files-backend.c b/refs/files-backend.c > > index 4d1f65a57a..bf6f89b1d1 100644 > > --- a/refs/files-backend.c > > +++ b/refs/files-backend.c > > @@ -3762,6 +3762,9 @@ static int files_fsck_refs_dir(struct ref_store > > *ref_store, > > > > iter = dir_iterator_begin(sb.buf, 0); > > if (!iter) { > > + if (errno == ENOENT && !is_main_worktree(wt)) > > + goto out; > > + > > ret = error_errno(_("cannot open directory %s"), sb.buf); > > goto out; > > } > > diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh > > index f671ac4d3a..615b7c0683 100755 > > --- a/t/t0602-reffiles-fsck.sh > > +++ b/t/t0602-reffiles-fsck.sh > > @@ -110,6 +110,21 @@ test_expect_success 'ref name check should be > > adapted into fsck messages' ' > > ) > > ' > > > > +test_expect_success 'no refs directory of worktree should not cause problems' ' > > + test_when_finished "rm -rf repo" && > > + git init repo && > > + ( > > + cd repo && > > + test_commit initial && > > + > > Nit: blank line? > I would improve this. Normally in this test file, I would add a blank line to indicate the basic setup ends. So, I should do the following cd repo && test_commit initial && git worktree add --detach ./worktree && rm -rf ... I would improve this in the next version. > > + git worktree add --detach ./worktree && > > + # Simulate old directory layout > > + rm -rf ./git/worktrees/worktree/refs && > > Eric[3] had a `git rev-parse --git-dir` suggestion instead of using `.git`. > > [3]: https://lore.kernel.org/git/a2a50127-6ab9-4d8a-abcc-b1a741df293e@xxxxxxxxxxxxxxxx/T/#mb42bdb046c391f2583c2200668945408a2d0396f > Good suggestion, let me improve this. > > + git refs verify 2>err && > > + test_must_be_empty err > > + ) > > +' > > + > > test_expect_success 'ref name check should work for multiple worktrees' ' > > test_when_finished "rm -rf repo" && > > git init repo && > > -- > > 2.49.0 Thanks, Jialuo
