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

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

[Phillip Wood]

On 28/11/2024 07:36, Eric Sunshine wrote:
> On Wed, Nov 27, 2024 at 11:04 PM David Moberg <kaddkaka@gmail.com> wrote:
>>> To: Elijah Newren <newren@gmail.com>
>>>> 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...
>>>
>>> So, what is the way forward? Is the option of setting GIT_WORK_TREE more robust?
>>
>> Hi, sorry for posting this on the side but I only have Gmail on android which doesn't allow text only emails (I think).
>>
>> Is there any work ongoing on this item? I think I lost the conversation and/or fell out of it.
> 
> The conversation ended at [1]. As far as I know, nobody is working on it.
> 
> I think Phillip did a good job in [1] of summarizing the two paths
> toward a possible fix. Unfortunately, both paths will probably require
> a good deal of spelunking through the code and the history to
> understand why the logic is the way it is, and (importantly) how to
> craft a solution which won't break existing legitimate use-cases.

I think the solution of always setting GIT_WORK_TREE and GIT_DIR is 
probably the most practical. Even if we find a way to not set GIT_DIR 
when git is run from a linked worktree I think we will still run into 
problems when core.worktree is set as that also appears to result in 
GIT_DIR being set without GIT_WORK_TREE.

> So, it's not necessarily a small task, and whoever digs into it (if
> anyone volunteers) will likely need to devote a good deal of time and
> effort to it. (My Git time is severely limited these days, so that
> person is unlikely to be me.)

In principle I'm interested in fixing this but in practice I'm unlikely 
to have time to work on it until the end of Outreachy in March. If 
someone else wants to take a look at it in the meantime I'll definitely 
try and make time to answer questions and review patches

Best Wishes

Phillip

> [1]: https://lore.kernel.org/git/743043bf-60b7-4ed7-8cf2-4f3f972968a6@gmail.com/

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

[Eric Sunshine]

[reincluding mailing list; David dropped the discussion off the
mailing list only because he couldn't get his email client to send
plain text, not because the conversation was intentionally private]

On Thu, Nov 28, 2024 at 10:07 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
> On 28/11/2024 07:36, Eric Sunshine wrote:
> > On Wed, Nov 27, 2024 at 11:04 PM David Moberg <kaddkaka@gmail.com> wrote:
> >>> To: Elijah Newren <newren@gmail.com>
> >>>> 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...
> >>>
> >>> So, what is the way forward? Is the option of setting GIT_WORK_TREE more robust?
> >>
> >> Hi, sorry for posting this on the side but I only have Gmail on android which doesn't allow text only emails (I think).
> >>
> >> Is there any work ongoing on this item? I think I lost the conversation and/or fell out of it.
> >
> > The conversation ended at [1]. As far as I know, nobody is working on it.
> >
> > I think Phillip did a good job in [1] of summarizing the two paths
> > toward a possible fix. Unfortunately, both paths will probably require
> > a good deal of spelunking through the code and the history to
> > understand why the logic is the way it is, and (importantly) how to
> > craft a solution which won't break existing legitimate use-cases.
>
> I think the solution of always setting GIT_WORK_TREE and GIT_DIR is
> probably the most practical. Even if we find a way to not set GIT_DIR
> when git is run from a linked worktree I think we will still run into
> problems when core.worktree is set as that also appears to result in
> GIT_DIR being set without GIT_WORK_TREE.
>
> > So, it's not necessarily a small task, and whoever digs into it (if
> > anyone volunteers) will likely need to devote a good deal of time and
> > effort to it. (My Git time is severely limited these days, so that
> > person is unlikely to be me.)
>
> In principle I'm interested in fixing this but in practice I'm unlikely
> to have time to work on it until the end of Outreachy in March. If
> someone else wants to take a look at it in the meantime I'll definitely
> try and make time to answer questions and review patches
>
> Best Wishes
>
> Phillip
>
> > [1]: https://lore.kernel.org/git/743043bf-60b7-4ed7-8cf2-4f3f972968a6@gmail.com/