Re: [PATCH] fsck: ignore missing "refs" directory for linked worktrees

From: Kristoffer Haugsbakk <code@khaugsbakk.name>
Date: May 31, 2025, 12:17 UTC
Message-ID: <3f731776-9a9e-4c8f-8de9-99d470503345@app.fastmail.com>
In-Reply-To: <aDp55upE6AhYunz7@ArchLinux>

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.

Show 9 quoted lines

> 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.

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@app.fastmail.com/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]

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

Show 42 quoted lines

>
> Check for ENOENT errno before reporting directory access errors for
> linked worktrees to maintain backward compatibility.
>
> Reported-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
> Signed-off-by: shejialuo <shejialuo@gmail.com>
> ---
>  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?

> +		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@app.fastmail.com/T/#mb42bdb046c391f2583c2200668945408a2d0396f

Show 10 quoted lines

> +		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