Re: Re: [PATCH] Add a Windows-specific fallback to getenv("HOME");

Am 04.06.2014 17:46, schrieb Johannes Schindelin:

After a bit of digging in the history and the old googlegroups issue tracker, I think this patch is completely unrelated to the non-ASCII problems.

In summary, this patch fixes 'git config' for the portable version only, and it only does so partially. Thus I don't think its ready for upstream, at least not in its current form. See below for the nasty details.

By that argument we'd have to introduce API abstractions for every environment variable that could possibly resemble a path (PATH, TMPDIR, GIT_DIR, GIT_WORK_DIR, GIT_TRACE* etc.).

We already have similar fallback logic for TMPDIR that is completely non-intrusive to core git code (fully encapsulated in mingw.c, see mingw_getenv (upstream) or mingw_startup (msysgit)). IMO such a solution would be hugely preferable over adding an additional get_home_directory() API (and continuously checking that no new upstream code accidentally introduces another 'getenv("HOME")').

Cheers, Karsten

====

Analysis of $HOME-realted issues:

1. mangled non-ASCII characters in environment variables

E.g. issue 491 [1], reportedly fixed in v1.7.10 ([1] comment #10).

This is actually a bug in msys.dll, and there's nothing that can be done about it from within git.exe. It is also not a problem if git is launched from cmd.exe.

The root cause is that the msys environment is initialized using GetEnvironmentStringsA(), which returns GetOEMCP()-encoded strings (e.g. cp850), rather than GetACP() (e.g. cp1252) as all other *A API functions do [2]. This adds one level of mangling whenever a native Windows program starts an msys program (so e.g. the call chain bash->git->bash->wish would mangle twice, see [1] comment #3).

For the fixed GetEnvironmentStringsA(), see [3] lines 459ff.

(As a side note, $HOMEDRIVE and $HOMEPATH originally did not have this problem, as they were separately initialized from NetUserGetInfoA(). This was changed in v1.6.3, however, at that time etc/profile was still using the broken $USERPROFILE. See [4], [5].)

2. 'git config' doesn't work with disconnected network drives

Issues 259 [6], 497 [7] and 512 [8], fixed in v1.7.0.2 for bash and v1.7.2.3 for cmd.

Apparently, $HOMEDRIVE$HOMEPATH is the home directory on the network, and $USERPROFILE is local. To be able to work offline, we need to check if $HOMEDRIVE$HOMEPATH exists and fall back to $USERPROFILE otherwise.

Note that git-wrapper does _not_ check if $HOMEDRIVE$HOMEPATH actually exists, as the original git.cmd did. This is probably a regression wrt issue 259.

3. HOME is not set when using the portable version

Issue 482 [9], partially fixed in v1.7.2.3 by this patch.

'Partially' because:
- there's no fallback to $USERPROFILE, so it doesn't work with disconnected network drives (see problem 2.)
- it doesn't setenv(HOME) for child processes (at least git-gui accesses $env(HOME) directly, but I haven't checked what happens if HOME is not set)

Incidentally, this patch was first released with v1.7.2.3, which also sets $HOME correctly in both etc/profile and git.cmd. So I suspect that this patch has always been essentially dead code (except perhaps for the portable version, I've never used that).

[1] https://code.google.com/p/msysgit/issues/detail?id=491 [2] http://msdn.microsoft.com/en-us/library/windows/desktop/ms683187%28v=vs.85%29.aspx [3] https://github.com/msysgit/msysgit/blob/msys/src/rt/patches/0013-msys.dll-basic-Unicode-support.patch [4] https://github.com/msysgit/msysgit/blob/msys/src/rt/patches/0007-only-override-the-variables-HOMEPATH-and-HOMEDRIVE-i.patch [5] https://github.com/msysgit/msysgit/commit/6b096c9 [6] https://code.google.com/p/msysgit/issues/detail?id=259 [7] https://code.google.com/p/msysgit/issues/detail?id=497 [8] https://code.google.com/p/msysgit/issues/detail?id=512 [9] https://code.google.com/p/msysgit/issues/detail?id=482