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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

> 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@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]

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?

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

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





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux