Re: [PATCH] rebase: exec leaks GIT_DIR to environment

From: Jacob Keller <jacob.keller@gmail.com>
Date: Oct 30, 2017, 02:53 UTC
Message-ID: <CA+P7+xrXLcTQpPWgzLwt_yZo=QdfetF36jrc_TtXfqMKR2Hh3w@mail.gmail.com>
In-Reply-To: <xmqq1sll8j6f.fsf@gitster.mtv.corp.google.com>

On Sun, Oct 29, 2017 at 7:26 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 28 quoted lines

> Phillip Wood <phillip.wood@talktalk.net> writes:
>
>> Just clearing GIT_DIR does not match the behavior of the shell version
>> (tested by passing -p to avoid rebase--helper) as that passes GIT_DIR to
>> exec commands if it has been explicitly set. I think that users that set
>> GIT_DIR on the command line would expect it to be propagated to exec
>> commands.
>>
>> $ git rebase -px'echo $GIT_DIR' @
>>
>>                                                             Merge commit
>> '7c2f1abd64' into phil
>> Executing: echo $GIT_DIR
>>
>> Successfully rebased and updated refs/heads/phil.
>>
>> $ env GIT_DIR=.git git rebase -px'echo $GIT_DIR' @
>>
>>                                                             Merge commit
>> '7c2f1abd64' into phil
>> Executing: echo $GIT_DIR
>> /home/phil/Documents/src/git/.git/worktrees/git-next
>> Successfully rebased and updated refs/heads/phil.
>
> Hmmm, I do not mess with GIT_DIR at all in my workflow, so I am
> having a bit of hard time judging if this regression is serious
> enough to be a release blocker.
>

So, I don't directly mess with GIT_DIR in my use case either, I just happened to run a build from a sub directory which relies on git commands continuing to work. However, because GIT_DIR was set to a relative path, it broke this Make in the subdirectory, which resulted in the problem occurring, but *only* during the exec command, if I ran the command manually after reach rebase step, it worked fine (since GIT_DIR was no longer set).

I don't know how big a deal it is, since I didn't notice it for quite some time.

Thanks, Jake

Show 11 quoted lines

> I'd prefer to avoid reverting the whole js/rebase-i-final topic from
> 'master' this late in the game, even though I do not expect we would
> see the remainder of the system gets broken due to hidden dependency
> on the topic, because the changes on the topic are relatively well
> isolated.
>
>
> 570676e011
> d1114d87c7
> 2f0e14e649
> 5f3108b7b6

I am pretty confident we can fix it. I think the easiest solution would be to just make sure GIT_DIR is an absolute path when passing it to the exec command.

Thanks, Jake