[BUG/RFC] Git Gui: GIT_DIR leaks to spawned Git Bash

[BUG/RFC] Git Gui: GIT_DIR leaks to spawned Git Bash

[Max Kirillov]

Hello.

Apparently various GIT_* environment variables (most
interesting is GIT_DIR but AFAIR there were more) leak to
shell session launched from Git Gui's "Git Bash" menu item.
Which can cause unexpected results if user navigates in that
shell to some other repository (in can also include starting
another terminal window etc. to make user forget original
context) and tries to do something there.

I think is would be appropriate to clean up the environment
while starting any program from git gui. It may have
downside that the environment could be set intentionally,
and now is's removed, but (1) there seems to be no way to
distinguish intentionally vs automatically set GIT_DIR, and
(2) the spawned program may be intended to be used
independly even if the environment was set intentionally for
git gui.

But I would like to note that I find it wrong for the "git"
dispatcher to set up any enrironment to the spawned
command-specific executable. Because it mixes the concerns
of that variables - are they user-settable variables to
tweak the git's behavior, or internal ones to transport the
discovered paths and other info to scripts which cannot do
the standard discovery? Not clear. There has been bugs
related to that already, for example [1].

[1] https://public-inbox.org/git/1409784120-2228-1-git-send-email-max@max630.net/

-- 
Max

Re: [BUG/RFC] Git Gui: GIT_DIR leaks to spawned Git Bash

[Johannes Schindelin]

Hi Max,

On Tue, 17 Jan 2017, Max Kirillov wrote:

> Apparently various GIT_* environment variables (most
> interesting is GIT_DIR but AFAIR there were more) leak to
> shell session launched from Git Gui's "Git Bash" menu item.

Given that you call it from Git GUI, i.e. for a *specific* worktree, I do
not think that this bug is *all* that critical. After all, you won't even
notice unless you use the very same Git Bash to navigate to a *different*
worktree.

Having said that, if you have the time and energy to come up with a patch,
I will definitely try my best to help you get it integrated.

And having said *that*, I have to admit that I was unable to reproduce:
GIT_DIR was set neither when starting Git GUI from the Start Menu (and
then navigating to a previously-opened worktree because I am too lazy to
navigate manually), nor when starting Git GUI from a Git CMD in a worktree
via `git gui`. In both cases, the environment contained only the following
variables whose name starts with "GIT_": $GIT_ASKPASS, $GIT_ASK_YESNO and
$GIT_EXEC_PATH.

I tested with Git for Windows v2.11.0(3), the latest offical version.

Thanks,
Johannes

Re: [BUG/RFC] Git Gui: GIT_DIR leaks to spawned Git Bash

[Max Kirillov]

On Tue, Jan 17, 2017 at 11:52:29AM +0100, Johannes Schindelin wrote:
> Hi Max,
> 
> On Tue, 17 Jan 2017, Max Kirillov wrote:
> 
>> Apparently various GIT_* environment variables (most
>> interesting is GIT_DIR but AFAIR there were more) leak to
>> shell session launched from Git Gui's "Git Bash" menu item.

...

> After all, you won't even notice unless you use the very
> same Git Bash to navigate to a *different* worktree.

That's right, I haven't been noticing it myself for quite a
time. But once I decided to tweak my habits to having only
one terminal window opened, and immediately noticed.

> And having said *that*, I have to admit that I was unable to reproduce

I have found exact way to reproduce it and actually the
reason. Turns out to be obvious mistake in code. PR is in
github: https://github.com/git-for-windows/git/pull/1032

-- 
Max

Re: [BUG/RFC] Git Gui: GIT_DIR leaks to spawned Git Bash

[Johannes Schindelin]

Hi Max,

On Tue, 17 Jan 2017, Max Kirillov wrote:

> On Tue, Jan 17, 2017 at 11:52:29AM +0100, Johannes Schindelin wrote:
> 
> > And having said *that*, I have to admit that I was unable to reproduce
> 
> I have found exact way to reproduce it and actually the reason. Turns
> out to be obvious mistake in code. PR is in github:
> https://github.com/git-for-windows/git/pull/1032

As I pointed out in my review, I think that GIT_DIR is actually only set
for `gitk` in the non-submodule case. So I guess if I run Git GUI, spawn
gitk, and then Git Bash, it will have GIT_DIR set.

*clicketyclick*

Bingo. That reproduces the problem here.

Thanks,
Johannes