Re: "git worktree repair" modifies the wrong repository

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Sep 19, 2024, 08:58 UTC
Message-ID: <CAPig+cQ8=Y6sVvN_dBo_GQ5nLkQ4GJ7AM6mE2kt_2QV7CR0omg@mail.gmail.com>
In-Reply-To: <E1sr5iF-0007zV-2k@binarylane-bailey.stuart.id.au>

On Wed, Sep 18, 2024 at 9:27 PM Russell Stuart <russell+git.vger.kernel.org@stuart.id.au> wrote:

> What did you do before the bug happened? (Steps to reproduce your issue)
>     /tmp/gb/a$ git init r
>     /tmp/gb/a/r$ git worktree add ../r.foo

Had you run `git worktree list` at this point in /tmp/gb/a/r/, you would have seen:

    /tmp/gb/a/r      3699610 [main]
    /tmp/gb/a/r.foo  3699610 [r.foo]

>     /tmp/gb/a/r$ cp -a ../* ../../b/.
>     /tmp/gb/a/r$ cd ../../b/r

Had you run `git worktree list` at this point in /tmp/gb/b/r/, you would have seen:

    /tmp/gb/b/r      3699610 [main]
    /tmp/gb/a/r.foo  3699610 [r.foo]

from which it can be seen that /tmp/gb/b/r/.git/worktree/r.foo/gitdir is pointing at /tmp/gb/a/r.foo, which makes sense since that gitdir file is an exact copy of /tmp/gb/a/r/.git/worktree/r.foo/gitdir. So /tmp/gb/a/r.foo is being shared by both repositories, /tmp/gb/a/ and /tmp/gb/b/, and neither one knows about /tmp/gb/b/r.foo.

Show 11 quoted lines

>     /tmp/gb/b/r$ git worktree repair ../r.foo
>     repair: gitdir incorrect: /tmp/gb/a/r/.git/worktrees/r.foo/gitdir
>     repair: .git file incorrect: /tmp/gb/a/r.foo
>     /tmp/gb/b/r$ cat .git/worktrees/r.foo/gitdir
>     /tmp/gb/a/r.foo/.git
>     /tmp/gb/b/r$ cat ../r.foo/.git
>     gitdir: /tmp/gb/a/r/.git/worktrees/r.foo
>     /tmp/gb/b/r$ cat ../../a/r/.git/worktrees/r.foo/gitdir
>     /tmp/gb/b/r.foo/.git
>     /tmp/gb/b/r$ cat ../../a/r.foo/.git
>     gitdir: /tmp/gb/b/r/.git/worktrees/r.foo

The implementation of `git worktree repair` started out (relatively) simple, handling the most common cases, and has been extended to handle additional cases when identified and reported. The case of merely copying a repository and its worktrees (and leaving the originals intact) is not yet implemented, so the above all makes sense given the current implementation.

Show 7 quoted lines

> What did you expect to happen? (Expected behavior)
>     I expected "/tmp/gb/b/r$ git worktree repair ../r.foo" to alter the
>     repositories it was run from, ie those under the directory "/tmp/gb/b".
>
> What happened instead? (Actual behavior)
>     "/tmp/gb/b/r$ git worktree repair ../r.foo" modified the repositories
>     under the directory "/tmp/gb/a".

I have a tentative fix implemented which seems to handle this case correctly. Once I've finished polishing it and formalizing the related test, I'll send out a patch.