git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 1/1] mingw: work around t2300's assuming non-Windows paths

From
Junio C Hamano <gitster@pobox.com>
Date
Jun 22, 2016, 16:42 UTC
Message-ID
<xmqqa8idnpj9.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<alpine.DEB.2.20.1606220927350.10382@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 7 quoted lines
> On Tue, 21 Jun 2016, Junio C Hamano wrote:
>
>> I said $PATH because --exec-path does not care what you do with
>> %PATH% but it deeply cares that its output is usable in $PATH.
>
> The really, really, really important part to keep in mind is that there is
> no $PATH on Windows.

I think I know that well enough; please sanity check. My understanding is:

 * Your (emulated) getenv(3) reads %PATH% which may look like
   "c:\a\b;c:\c\d", i.e. Windows style.
 * Your argv_exec_path also looks like "c:\e\f", i.e. Windows
   style.
 * Your setup_path() would yield "c:\e\f;c:\a\b;c:\c\d" because it
   concatenates the above two with PATH_SEP, i.e. Windows style, and
   your setenv(3) will set that to %PATH%.
 * After all that happens, your run_command(), execv_git_cmd(),
   etc. would honor that %PATH%.
 * In all of the above, a back-slash can be (and may indeed be) a
   forward-slash, as library functions and system calls are prepared
   to take both since the good old DOS days.  I.e. "c:\a\b" can be
   "c:/a/b".  It cannot be "/c/a/b", however.
 * When bash gets spawned (perhaps because a hook is written in that
   language, perhaps because child_process.use_shell is set when
   executing an alias "!cmd", running a pager, or running an
   editor), the value of %PATH% derived by the above sequence is not
   exposed as $PATH.  There is the "rewrite leading C: with /C/"
   outside us (i.e. in bash).

And that is why I suggested to keep that "internal paths are in platform native format" and apply the same conversion as what bash does immediately before the returned value from git_exec_path() is fed to puts(). That way, "$PATH" (not %PATH%) can be modified with "$(git --exec-path)" in scripts the same way on all the platforms and you do not have to break people's hooks.

Having said that, I realize that I missed one huge thing to take into consideration. I assume that you have been shipping Git for Windows with this "'git --exec-path' gives c:\a\b, not /c/a/b" feature for a long time, so existing Windows users will be broken if we "fix" this (which would allow them to use shell scripts their friends use on other platforms without modification). So from that point alone, "PATH=$(git --exec-path):$PATH in shell should work on any platform and "git --exec-path" should be fixed" would not fly. This simply is too late to fix.

The patch under discussion is the only door left for that test, and a similar trickery is needed for any end-user scripts used for hooks and aliases that use 'git --exec-path', if they ever want to be cross-platform.

So let's take that "if Windows do this" change to t2300 as-is.
Previous: Johannes SchindelinNext: Johannes Schindelin
Message 8 of 11 in “Fix testing of `master` on Windows”
  1. 0/1 Fix testing of `master` on WindowsJohannes Schindelin, Jun 18, 2016
  2. 1/1 mingw: work around t2300's assuming non-Windows pathsJohannes Schindelin, Jun 18, 2016
  3. Junio C HamanoJun 20, 2016
  4. Johannes SchindelinJun 21, 2016
  5. Junio C HamanoJun 21, 2016
  6. Junio C HamanoJun 21, 2016
  7. Johannes SchindelinJun 22, 2016
  8. Junio C HamanoJun 22, 2016
  9. Johannes SchindelinJun 22, 2016
  10. Junio C HamanoJun 22, 2016
  11. Johannes SchindelinJun 22, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.