public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: Johannes Schindelin <Johannes.Schindelin@gmx•de>
Cc: git@vger•kernel.org
Subject: Re: [PATCH 1/1] mingw: work around t2300's assuming non-Windows paths
Date: Wed, 22 Jun 2016 13:12:31 -0700	[thread overview]
Message-ID: <xmqqvb11kmog.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <xmqqa8idnpj9.fsf@gitster.mtv.corp.google.com> (Junio C. Hamano's message of "Wed, 22 Jun 2016 09:42:34 -0700")

Junio C Hamano <gitster@pobox•com> writes:

> I think I know that well enough; please sanity check.  My
> understanding is:
> ...
> 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.

Assuming that the sanity check passes, here is a suggested rewrite
to explain the real problem better.

-- >8 --
From: Johannes Schindelin <johannes.schindelin@gmx•de>
Date: Sat, 18 Jun 2016 12:49:11 +0200
Subject: [PATCH] t2300: "git --exec-path" is not usable in $PATH on Windows as-is

The "git" command itself has an internal logic to prepend the
exec-path to the PATH environment variable for processes it spawns.
That is how ". git-sh-setup" in our scripted Porcelains can find the
dot-sourced file in $GIT_EXEC_PATH that is not usually on user's
PATH.

When t2300 runs, because it is not spawned by the "git" command, the
scriptlet being tested did not run with a realistic setting of PATH
environment.  It lacked the exec-path on the PATH, and failed to
find the dot-sourced file.  A recent update to t2300 attempted to do
so, with "PATH=$(git --exec-path):$PATH", which was the recommended
way around v1.6.0 days (a script whose original was written before
that release that survives to this day is likely to have such a line).

However, the "git --exec-path" command outputs C:\path\to\exec\dir
(not /c/path/to/exec/dir) on Windows; the recent update failed to
consider the problem that comes from it.

Even though Git itself, when doing the equivalent internally, does
so in a platform native way (i.e. on Windows, C:\path\to\exec\dir is
prepended to the existing value of %PATH% using ';' as a component
separator), the result is further massaged by bash and gets turned
into $PATH that uses /c/path/to/exec/dir with ':' separating the
components, which is the form understood by bash, so scripted
Porcelains finds commands from PATH correctly even on Windows.

An end user script written in shell, however, cannot prepend
"C:\path\to\exec\dir:" to the existing value of $PATH and expect
bash to magically turn it into the form it understands.  In other
words, "PATH=$(git --exec-path):$PATH" does not work as an emulation
of what "Git" internally does to the PATH on Windows.

To correctly emulate how exec-path is prepended to the PATH
environment internally on Windows, we'd need to convert
C:\git-sdk-64\usr\src\git to at least /c\git-sdk-64\usr\src\git
ourselves before prepending it to PATH.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx•de>
Signed-off-by: Junio C Hamano <gitster@pobox•com>
---
 t/t2300-cd-to-toplevel.sh | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/t/t2300-cd-to-toplevel.sh b/t/t2300-cd-to-toplevel.sh
index cccd7d9..c8de6d8 100755
--- a/t/t2300-cd-to-toplevel.sh
+++ b/t/t2300-cd-to-toplevel.sh
@@ -4,11 +4,19 @@ test_description='cd_to_toplevel'
 
 . ./test-lib.sh
 
+EXEC_PATH="$(git --exec-path)"
+test_have_prereq !MINGW ||
+case "$EXEC_PATH" in
+[A-Za-z]:/*)
+	EXEC_PATH="/${EXEC_PATH%%:*}${EXEC_PATH#?:}"
+	;;
+esac
+
 test_cd_to_toplevel () {
 	test_expect_success $3 "$2" '
 		(
 			cd '"'$1'"' &&
-			PATH="$(git --exec-path):$PATH" &&
+			PATH="$EXEC_PATH:$PATH" &&
 			. git-sh-setup &&
 			cd_to_toplevel &&
 			[ "$(pwd -P)" = "$TOPLEVEL" ]
-- 
2.9.0-346-g30ec1fd


  parent reply	other threads:[~2016-06-22 20:12 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-18 10:49 [PATCH 0/1] Fix testing of `master` on Windows Johannes Schindelin
2016-06-18 10:49 ` [PATCH 1/1] mingw: work around t2300's assuming non-Windows paths Johannes Schindelin
2016-06-20 19:09   ` Junio C Hamano
2016-06-21 12:01     ` Johannes Schindelin
2016-06-21 17:10       ` Junio C Hamano
2016-06-21 17:27         ` Junio C Hamano
2016-06-22  8:25           ` Johannes Schindelin
2016-06-22 16:42             ` Junio C Hamano
2016-06-22 20:06               ` Johannes Schindelin
2016-06-22 20:12               ` Junio C Hamano [this message]
2016-06-22 21:26                 ` Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqvb11kmog.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=Johannes.Schindelin@gmx$(echo .)de \
    --cc=git@vger$(echo .)kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox