Am 04.06.2014 17:46, schrieb Johannes Schindelin:
> Hi kusma,
>
> On Wed, 4 Jun 2014, Johannes Schindelin wrote:
>
>> The problem arises whenever git.exe calls subprocesses. You can pollute
>> the environment by setting HOME, I do not recall the details, but I
>> remember that we had to be very careful *not* to do that, hence the patch.
>> Sorry, has been a long time.
>
> Actually, a quick search in my Applegate vaults^W^Wmail archives suggests
> that we had tons of troubles with non-ASCII characters in the path.
>
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.
> I would be strongly
> in favor of fixing the problem by the root: avoiding to have Git rely on
> the HOME environment variable to be set, but instead add a clean API call
> that even says what it is supposed to do: gimme the user's home
> directory's path. And that is exactly what the patch does.
>
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html