Re: git rebase exec make -C in worktree confuses repo root dir

[phillip.wood123@gmail.com]

On 16/10/2024 22:12, Eric Sunshine wrote:
> On Wed, Oct 16, 2024 at 5:15 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>> On 15/10/2024 21:01, Eric Sunshine wrote:
>>>> Den tis 15 okt. 2024 kl 09:11 skrev Eric Sunshine <sunshine@sunshineco.com>:
>>>>> This looks like unintentional behavior; probably a bug. It seems to be
>>>>> triggered by `git rebase -i` setting GIT_DIR. [...]
>>>>>       % git -C dir rev-parse --show-toplevel
>>>>>       /.../bar
>>>>>       % GIT_DIR=../../foo/.git/worktrees/bar \
>>>>>           git -C dir rev-parse --show-toplevel
>>>>>       /.../bar/dir
>>>>>
>>>>> The `git rev-parse --show-toplevel` invocation with GIT_DIR set is
>>>>> incorrectly returning `/.../bar/dir` rather than `/.../bar`.
>>
>> I'm about to go off the list until the 29th so I wont be working on it
>> soon either but I think the problem is that git sets $GIT_DIR when it is
>> run from a linked worktree. I've reproduced the commit message from
>> ff5b7913f0a (sequencer, stash: fix running from worktree subdir,
>> 2022-01-26) below which I think explains the problem we're seeing here.
>> Unfortunately the approach of setting $GIT_WORK_TREE used in that commit
>> won't work for exec commands as they may be run in a different worktree.
> 
> Maybe. Maybe not. exec'ing a command in a worktree other than the
> current worktree may be a "don't do it if it hurts" situation. The
> same shortcoming you describe would crop up when exec'ing a command in
> a foreign repository from the one in which `git rebase -i` is being
> run. If we look at it that way ("don't do it if it hurts"), then
> perhaps a documentation update is warranted; something along the lines
> [1] which gives explains that GIT_* environment variables should be
> cleared by a Git hook if it needs to peek into a foreign repository or
> other worktree. It's not a perfectly satisfactory answer, but would at
> least (somewhat?) allay your concern about `git rebase -i` setting
> GIT_WORK_TREE automatically.

That's certainly the simplest way forward. Until 434e0636db1 (sequencer: 
do not export GIT_DIR and GIT_WORK_TREE for 'exec', 2021-12-04)
we set both GIT_DIR and GIT_WORK_TREE when running an exec command. As 
the message for that commit explains that behavior did not match the 
scripted rebase which was the motivation to stop doing it. Unfortunately 
that left us in the situation where git will sometimes set GIT_DIR when 
the user has not. Ideally I'd like to understand why we're setting 
GIT_DIR in setup_git_dir() when, at least at fist sight, it seems to be 
unnecessary. I think that would take some time so maybe we should just 
go back to setting GIT_DIR and GIT_WORK_TREE in the sequencer when 
running "exec" commands.

Best Wishes

Phillip

> [1]: https://lore.kernel.org/git/pull.1457.git.1673171924727.gitgitgadget@gmail.com/