[BUG] refs: verify does not work if there are v2.43.0 or older worktrees w/o wt. refs

> git-refs-verify(1) checks worktree refs since v2.47.0-111-g7c78d819e6a
> (ref: support multiple worktrees check for refs, 2024-11-20).  This
> causes the command to always exit with code `255` and stderr output
> lines for each worktree created on v2.43.0 or older that does not have
> worktree refs:
>
>     error: cannot open directory .git/worktrees/<worktree name>/refs: No such file or directory

> This is apparently caused by worktrees created on Git v2.43.0 or older.
> Apparently these worktrees don’t have this directory unless there exist
> worktree refs:
>
>     .git/worktrees/<worktree name>/refs

> -- 8< --
> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
> Subject: [PATCH] t0602: demo v2.43.0 worktree problem
>
> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>

>  t/t0602-reffiles-fsck.sh | 43 ++++++++++++++++++++++++++++++++++++++++
> @@ -886,4 +886,47 @@ test_expect_success '--[no-]references option should apply to fsck' '
> +# These worktrees will not have a refs/ directory unless there
> +# actually exist worktree refs
> +test_expect_failure 'works with worktrees from v2.43.0 or older without worktree refs' '
> +       test_when_finished "rm -rf repo" &&
> +       git init repo &&
> +       (
> +               cd repo &&
> +               test_commit initial &&
> +               git checkout -b default-branch &&

> +               git worktree add --detach ./worktree &&
> +               # Simulate old directory layout
> +               rmdir .git/worktrees/worktree/refs &&
> +               git refs verify 2>err &&
> +               test_must_be_empty err
> +       )
> +'
> +
> +test_expect_success 'works with worktrees from v2.43.0 or older with worktree refs' '
> +       test_when_finished "rm -rf repo" &&
> +       git init repo &&
> +       (
> +               cd repo &&
> +               test_commit initial &&
> +               test_commit second &&
> +               git checkout -b default-branch &&

> +               git worktree add --detach ./worktree &&
> +               (
> +                       cd worktree &&
> +                       git bisect start &&
> +                       git bisect bad HEAD &&
> +                       git bisect good initial &&
> +                       # Simulate old directory layout: delete if empty
> +                       # But there should exist a refs/bisect/ directory now
> +                       if [ ! -e ../.git/worktrees/worktree/refs/bisect ]
> +                       then
> +                               rmdir ../.git/worktrees/worktree/refs
> +                       fi &&

> +                       git refs verify 2>err &&
> +                       test_must_be_empty err
> +               )
> +       )
> +'

> On Fri, May 30, 2025 at 3:00 PM <kristofferhaugsbakk@fastmail.com> wrote:
> > git-refs-verify(1) checks worktree refs since v2.47.0-111-g7c78d819e6a
> > (ref: support multiple worktrees check for refs, 2024-11-20).  This
> > causes the command to always exit with code `255` and stderr output
> > lines for each worktree created on v2.43.0 or older that does not have
> > worktree refs:
> >
> >     error: cannot open directory .git/worktrees/<worktree name>/refs: No such file or directory
> 
> Interesting. I didn't follow the topic which introduced 7c78d819e6
> (ref: support multiple worktrees check for refs, 2024-11-20), but I
> can confirm that this is a problem.
> 

> > This is apparently caused by worktrees created on Git v2.43.0 or older.
> > Apparently these worktrees don’t have this directory unless there exist
> > worktree refs:
> >
> >     .git/worktrees/<worktree name>/refs
> 
> Indeed, the "refs" subdirectory was not present by default in older
> Git versions. Were you able to track down which commit is responsible
> for that directory getting created automatically when the worktree
> gets created?
> 

>> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>> Subject: [PATCH] t0602: demo v2.43.0 worktree problem
>>
>> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
>
> Even though this is a bug report and the patch you included doesn't
> provide a fix, you did craft a couple tests, presumably with the
> intention that they should be used by whomever fixes the problem. As
> such, I'll give them a bit of a critique...

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

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

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

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

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

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

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

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

> 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 &&
> +
> +		git worktree add --detach ./worktree &&
> +		# Simulate old directory layout
> +		rm -rf ./git/worktrees/worktree/refs &&
> +		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 &&

> Hi Shejialuo
> 
> On 31/05/2025 04:39, shejialuo wrote:
> > 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;
> >   	}
> 
> I think it would be clearer to write this as
> 
> 	if (is_main_worktree(wt) || errno != ENOENT)
> 		ret = error_errno(_("cannot open directory %s"), sb.buf);
> 	goto out;
> 
> so that the condition that triggers the error message is explicit rather
> than having to mentally invert the condition to figure out when we return an
> error

> On Mon, Jun 02, 2025 at 10:53:50AM +0100, Phillip Wood wrote:
>> Hi Shejialuo
>>
>> On 31/05/2025 04:39, shejialuo wrote:
>>> 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;
>>>    	}
>>
>> I think it would be clearer to write this as
>>
>> 	if (is_main_worktree(wt) || errno != ENOENT)
>> 		ret = error_errno(_("cannot open directory %s"), sb.buf);
>> 	goto out;
>>
>> so that the condition that triggers the error message is explicit rather
>> than having to mentally invert the condition to figure out when we return an
>> error
> 
> The downside though is that this mandates that `is_main_worktree()` must
> never set `errno` itself. So while it may be clearer, the original
> version feels safer to me.

> On Mon, Jun 02, 2025 at 10:53:50AM +0100, Phillip Wood wrote:
>> Hi Shejialuo
>> 
>> On 31/05/2025 04:39, shejialuo wrote:
>> > 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;
>> >   	}
>> 
>> I think it would be clearer to write this as
>> 
>> 	if (is_main_worktree(wt) || errno != ENOENT)
>> 		ret = error_errno(_("cannot open directory %s"), sb.buf);
>> 	goto out;
>> 
>> so that the condition that triggers the error message is explicit rather
>> than having to mentally invert the condition to figure out when we return an
>> error
>
> The downside though is that this mandates that `is_main_worktree()` must
> never set `errno` itself. So while it may be clearer, the original
> version feels safer to me.

> Hi Shejialuo
> 
> On 31/05/2025 04:39, shejialuo wrote:
> > 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;
> >   	}
> 
> I think it would be clearer to write this as
> 
> 	if (is_main_worktree(wt) || errno != ENOENT)
> 		ret = error_errno(_("cannot open directory %s"), sb.buf);
> 	goto out;
> 
> so that the condition that triggers the error message is explicit rather
> than having to mentally invert the condition to figure out when we return an
> error
> 

> +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 &&
> +
> +		git worktree add --detach ./worktree &&
> +		# Simulate old directory layout
> +		rm -rf ./git/worktrees/worktree/refs &&

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

> "git refs verify" doesn't work if there are worktrees created on Git
> v2.43.0 or older versions. These versions 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. And in 7c78d819e6 (ref: support multiple worktrees check for
> refs, 2024-11-20), we assume that all linked worktrees have this
> directory and would wrongly report an error to the user, thus
> introducing compatibility issue.

> 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>
> [snip]
> +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 &&
> +		git worktree add --detach ./worktree &&
> +
> +		cd worktree &&

> +		worktree_refdir="$(git rev-parse --git-dir)/refs" &&
> +		# Simulate old directory layout
> +		rmdir "$worktree_refdir" &&
> +		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

> > +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 &&
> > +		git worktree add --detach ./worktree &&
> > +
> > +		cd worktree &&
> 
> Now in this version you change-directory into `worktree`.  You would
> need a new subshell for that (test style).  But I don’t see the need to
> chdir in the first place?
> 

> > +		worktree_refdir="$(git rev-parse --git-dir)/refs" &&
> > +		# Simulate old directory layout
> > +		rmdir "$worktree_refdir" &&
> > +		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
> 
> The test passes for me.  Also when applying only the test (and not the
> fix) the test fails as expected.  Good.

> [snip]
>
> Range-diff against v2:
> 1:  d949a8a646 ! 1:  e4c32971da fsck: ignore missing "refs" directory
> for linked worktrees
>     @@ t/t0602-reffiles-fsck.sh: test_expect_success 'ref name check
> should be adapted
>      +		test_commit initial &&
>      +		git worktree add --detach ./worktree &&
>      +
>     -+		cd worktree &&
>     -+		worktree_refdir="$(git rev-parse --git-dir)/refs" &&
>     -+		# Simulate old directory layout
>     -+		rmdir "$worktree_refdir" &&
>     -+		git refs verify 2>err &&
>     -+		test_must_be_empty err
>     ++		(
>     ++			cd worktree &&
>     ++			worktree_refdir="$(git rev-parse --git-dir)/refs" &&
>     ++			# Simulate old directory layout
>     ++			rmdir "$worktree_refdir" &&
>     ++			git refs verify 2>err &&
>     ++			test_must_be_empty err
>     ++		)
>      +	)
>      +'
>      +
> --
> 2.49.0