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

For git.exe alone, use the same HOME directory fallback mechanism as /etc/profile#322

Merged
dscho merged 1 commit intomsysgit:maint-1.9from
linquize:win-home
Feb 25, 2015
Merged

For git.exe alone, use the same HOME directory fallback mechanism as /etc/profile#322
dscho merged 1 commit intomsysgit:maint-1.9from
linquize:win-home

Conversation

@linquize
Copy link

For git.exe alone, use the same HOME directory fallback mechanism as /etc/profile

This allows the fallback mechanism to work outside Git Bash.

@buildhive
Copy link

MSysGit - the development behind Git for Windows » git #264 SUCCESS
This pull request looks good
(what's this?)

@linquize
Copy link
Author

Changed to use is_directory()

@dscho
Copy link
Member

dscho commented Feb 25, 2015

Thank you so much!

My only remaining request is to adjust the commit message a bit. The idea is that we contribute all of the modifications of the Git source code to upstream, eventually, therefore it would be good to abide by their rules. Would you please prefix the patch by mingw:, make sure that the first line is shorter than 73 characters, and maybe mention in the commit message's body why we go through such a dance with the HOME variable? Oh, and please sign off on the patch, as described here.

@buildhive
Copy link

MSysGit - the development behind Git for Windows » git #266 SUCCESS
This pull request looks good
(what's this?)

This allows git executable using the fallback mechanism to work outside Git Bash.
On Windows, $HOME is usually not set.
If $HOME not exist, try $HOMEDRIVE/$HOMEPATH.
If still not exist, try $USERPROFILE.

Signed-off-by: Lin Qiu Ze <linquize@yahoo.com.hk>
@linquize
Copy link
Author

Commit message updated.

@buildhive
Copy link

MSysGit - the development behind Git for Windows » git #267 FAILURE
Looks like there's a problem with this pull request
(what's this?)

dscho added a commit that referenced this pull request Feb 25, 2015
For git.exe alone, use the same HOME directory fallback mechanism as /etc/profile
@dscho dscho merged commit 66b892a into msysgit:maint-1.9 Feb 25, 2015
@dscho
Copy link
Member

dscho commented Feb 25, 2015

Thank you so much!

@kblees
Copy link

kblees commented Feb 25, 2015

Good intentions - unfortunately, you've based your change on code that is not in the master branch.

Rather than fixing any of the pre-existing issues with get_windows_home_directory() discussed in [1], you've added quite a few more:

  • The behaviour of is_directory(NULL) (or rather: stat(NULL)) is undefined. Why remove the NULL-checks?
  • get_windows_home_directory() is a replacement for getenv("HOME"), so it should not set errno (but is_directory() does)
  • Why check is_directory() if HOME has already been properly set by the parent process? Hitting the file system (three times!) will slow down startup of every git process, especially if $HOMEDRIVE is disconnected. And scripted commands launch a lot of processes...
  • get_windows_home_directory() is no longer thread-safe, i.e. a thread may see an incompletely initialized value of the home_directory variable

To set "HOME" in git, I would have expected something like this on master:

@@ -2239,6 +2239,27 @@ void mingw_startup()
    if (!getenv("TERM"))
        setenv("TERM", "cygwin", 1);

+   /* calculate HOME if not set */
+   if (!getenv("HOME")) {
+       /*
+        * try $HOMEDRIVE$HOMEPATH - the home share may be a network
+        * location, thus also check if the path exists (i.e. is not
+        * disconnected)
+        */
+       if (getenv("HOMEDRIVE") && getenv("HOMEPATH")) {
+           struct strbuf buf = STRBUF_INIT;
+           strbuf_addf(&buf, "%s%s", getenv("HOMEDRIVE"),
+                   getenv("HOMEPATH"));
+           if (is_directory(buf.buf))
+               setenv("HOME", buf.buf, 1);
+           strbuf_release(&buf);
+       }
+
+       /* use $USERPROFILE if the home share is not available */
+       if (!getenv("HOME") && getenv("USERPROFILE"))
+           setenv("HOME", getenv("USERPROFILE"), 1);
+   }
+
    /* initialize critical section for waitpid pinfo_t list */
    InitializeCriticalSection(&pinfo_cs);

[1] https://groups.google.com/forum/#!topic/msysgit/wmyWC6M2hl4

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.

4 participants