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

From: Eric Sunshine <sunshine@sunshineco.com>
Date: May 30, 2025, 22:23 UTC
Message-ID: <CAPig+cQiw03qfwwE9Md+LdKeS-6BGx0M1+0YYDUDXO9UPVo+wg@mail.gmail.com>
In-Reply-To: <1d8f471b6dcb7e952afea834490be195189492a7.1748629208.git.code@khaugsbakk.name>

On Fri, May 30, 2025 at 3:00 PM <kristofferhaugsbakk@fastmail.com> wrote:

Show 7 quoted lines

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

Show 5 quoted lines

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

Show 5 quoted lines

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

Show 11 quoted lines

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

This `git checkout -b` seems unnecessary. The expected test failure occurs without this step. As such, it's probably just noise which will confuse readers rather than help them. I suggest omitting it.

Show 16 quoted lines

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

Unnecessary branch creation?

Show 12 quoted lines

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

A few comments...

First, I'm having trouble understanding what the intention is here; the comment does not illuminate. Even with v2.43.0, .git/worktrees/worktree/refs/bisect exists after "git bisect bad HEAD", so it seems that the `if` condition can never fail, and the `rmdir` is dead code.

Second, this project uses `test` rather than `[` in shell scripts.

Finally, I see that other parts of the script are already (perhaps) too intimate with the structure of the .git/ directory, and you may have simply been following suit, but these days we often want to abstract away such familiarity. Hence, rather than hardcoding the path "../.git/worktrees/<worktree>/refs", you could do this:

    refs="$(git rev-parse --git-dir)/refs" &&
    if test ! -e "$refs/bisect"
    then
        rmdir "$refs"
    fi &&

Show 5 quoted lines

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

Overall, although the first new test makes sense, it is not at all clear to me what the second test is checking or what its purpose is.