On Wed, Oct 16, 2024 at 1:36 PM Taylor Blau <me@xxxxxxxxxxxx> wrote: > > On Wed, Oct 16, 2024 at 10:15:49AM +0100, Phillip Wood wrote: > > On 15/10/2024 21:01, Eric Sunshine wrote: > > > On Tue, Oct 15, 2024 at 2:55 PM David Moberg <kaddkaka@xxxxxxxxx> wrote: > > > > Den tis 15 okt. 2024 kl 09:11 skrev Eric Sunshine <sunshine@xxxxxxxxxxxxxx>: > > > > > This looks like unintentional behavior; probably a bug. It seems to be > > > > > triggered by `git rebase -i` setting GIT_DIR. Here's an even simpler > > > > > reproduction recipe: > > > > > > > > > > % git init foo > > > > > % cd foo > > > > > % mkdir dir > > > > > % echo foo >dir/file > > > > > % git add dir/file > > > > > % git commit -m foo > > > > > % git worktree add ../bar > > > > > % cd ../bar > > > > > % 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`. > > > > > > > > Thanks, that is indeed a much smaller example and it seems to exhibit > > > > the same issue. Can we figure out how to fix it? > > > > > > Someone is going to have to dig into the code, but my Git time is very > > > limited right now, so perhaps someone else can do the digging. > > > > 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. Naively I feel that if > > setup_git_directory() has found ".git" then any git subprocesses run in the > > worktree should also be able to find ".git" and so it should not be setting > > $GIT_DIR but there maybe there is some subtlety I'm missing > > Let's see if that commit's author Elijah (CC'd) has any other thoughts. > > Thanks, > Taylor My commit merely pointed out that long before that commit came along, if GIT_DIR is set but GIT_WORK_TREE is not, then the working tree is assumed to be ".". As such, a command like the above where `--show-toplevel` is run with just GIT_DIR set (to anything) will merely expand "." and show you that path. If you are going to be having subprocesses that depend upon the git directory and the git working tree, I think there are two options: * Set GIT_WORK_TREE in addition to GIT_DIR (as my patch does in certain cases) * Stop setting GIT_DIR if you're not going to set GIT_WORK_TREE The second point is a bit harder since setup.c automatically sets GIT_DIR for you in various cases, so if you want to go that route it really means you'd have to actively unset GIT_DIR in those cases. But, you'd have to be careful since you only want to unset it when setup_discovered_git_dir() sets it for you, not when the user who invoked your command had manually set GIT_DIR. So, there is a little bit of a pickle here...
