Skip to content
This repository was archived by the owner on Nov 9, 2017. It is now read-only.

Conversation

@dscho
Copy link
Member

@dscho dscho commented Feb 16, 2014

We really should not need to set HOME globally anymore because we set it
in Git Bash, in Git Cmd and in compat/mingw.c.

Setting HOME caused problems, in particular when we hardcoded it to
the administrator's HOME if Git was installed using the administrator's
account.

Signed-off-by: Johannes Schindelin [email protected]

We really should not need to set HOME globally anymore because we set it
in Git Bash in /etc/profile, and in the Git wrappers for cmd.exe.

Setting HOME caused problems, in particular when we hardcoded it to
the administrator's HOME if Git was installed using the administrator's
account.

This closes msysgit/git#119.

Signed-off-by: Johannes Schindelin <[email protected]>
@dscho
Copy link
Member Author

dscho commented Feb 16, 2014

@sschuberth this is not tested on my side but should be correct (read: I cannot see what could be wrong with the patch ;-)).

@sschuberth
Copy link
Contributor

About the wording in the commit message: We do not set HOME "in Git Bash", but "for Git Bash in etc/profile". We do not have "Git Cmd" or git.cmd anymore, but the git.exe wrapper and the gitk.cmd wrapper (which both set HOME). Also, if merging this should auto-close issue 119 (which IMHO it should), then you should mention it using this syntax.

@sschuberth
Copy link
Contributor

Regarding the code, you should also remove the logic to "Delete HOME if a previous installation modified it" and to "Reset the current user's HOME if we modified it" (search for these comments in the code).

@dscho
Copy link
Member Author

dscho commented Feb 17, 2014

That's funny... I force-pushed but the pull request was not updated...

I think I addressed all your concerns about the pull request; feel free to merge/adjust!

@sschuberth
Copy link
Contributor

The PR was updated, just not the first comment in this conversation (which defaults to the original PR's first commit's message).

sschuberth added a commit that referenced this pull request Feb 17, 2014
Do not set the HOME variable in the installer
@sschuberth sschuberth merged commit 2286e82 into master Feb 17, 2014
@sschuberth sschuberth deleted the homeless branch February 17, 2014 19:57
@dscho
Copy link
Member Author

dscho commented Feb 17, 2014

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants