git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Johannes Sixt <j6t@kdbg.org>
Cc: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Git Mailing List" <git@vger.kernel.org>
Subject: Re: [PATCH] t9001-send-email.sh: fix expected absolute paths on Windows
Date: Wed, 2 Jun 2021 13:40:44 +0200 (CEST)	[thread overview]
Message-ID: <nycvar.QRO.7.76.6.2106021330470.55@tvgsbejvaqbjf.bet> (raw)
In-Reply-To: <bb30fe2b-cd75-4782-24a6-08bb002a0367@kdbg.org>

[-- Attachment #1: Type: text/plain, Size: 5163 bytes --]

Hi Hannes,

On Mon, 24 May 2021, Johannes Sixt wrote:

> Git for Windows is a native Windows program that works with native
> absolute paths in the drive letter style C:\dir. The auxiliary
> infrastructure is based on MSYS2, which uses POSIX style /C/dir.

As far as I remember, VMS is also POSIX, and it has a different path
style. Therefore I would probably use the term "Unix style" here instead
of "POSIX style".

But that has nothing to do with the validity of your point: it is still
a correct and important observation.

> When we test for output of absolute paths produced by git.exe, we
> usally have to expect C:\dir style paths. To produce such expected
> paths, we have to use $(pwd) in the test scripts; the alternative,
> $PWD, produces a POSIX style path. ($PWD is a shell variable, and the
> shell is bash, an MSYS2 program, and operates in the POSIX realm.)
>
> There are two recently added tests that were written to expect C:\dir
> paths. The output that is tested is produced by `git send-email`, but
> behind the scenes, this is a Perl script, which also works in the
> POSIX realm and produces /C/dir style output.
>
> In the first test case that is changed here, replace $(pwd) by $PWD
> so that the expected path is constructed using /C/dir style.
>
> The second test case sets core.hooksPath to an absolute path. Since
> the test script talks to native git.exe, it is supposed to place a
> C:/dir style path into the configuration; therefore, keep $(pwd).
> When this configuration value is consumed by the Perl script, it is
> transformed to /C/dir style by the MSYS2 layer and echoed back in
> this form in the error message. Hence, do use $PWD for the expected
> value.
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
>  When I say "the configuration is transformed to /C/dir style", I am
>  actually hand-waving: I can observe that a transformation must
>  happen somewhere, but I actually do not know where the conversion
>  really happens. "The MSYS2 layer" is my best qualified guess.

Indeed, it is the MSYS2 runtime that performs this conversion. Concretely,
whenever you call a non-MSYS2 program from within MSYS2, command-line
arguments that look like Unix paths are converted by replacing forward
slashes with backslashes and by prefixing absolute paths with MSYS2' root
directory (as a Windows style path, of course).

However, in this instance, it is a different problem, I think.

Perl cannot handle Windows style paths. At least _Git's_ Perl scripts
cannot.

For example, the `PATH` variable is assumed to contain colon-separated
directory paths in our scripts. But that is not true on Windows: the colon
already separates the drive letter from the rest of the path, and
therefore the separator used in `PATH` is a _semicolon_.

To help with this, the MSYS2 runtime converts the command-line arguments
and environment variables that look like path lists (such as `PATH`) and
paths (such as `SYSTEMROOT`) from Windows style to Unix style when it
detects that, say, MSYS2's Perl is started from a non-MSYS2 program such
as `git.exe`.

Which means that the Perl code executed in Ævar's tests spits out Unix
style paths.

Happily for us, the MSYS2 Bash with which Git's test suite is expected to
be executed on Windows understands those Unix style paths very well! All
we need to do is to use them here, and that is what your patch does,
therefore:

Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Thank you,
Dscho

>
>  t/t9001-send-email.sh | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> index 65b3035371..68bebc505b 100755
> --- a/t/t9001-send-email.sh
> +++ b/t/t9001-send-email.sh
> @@ -539,15 +539,14 @@ test_expect_success $PREREQ "--validate respects relative core.hooksPath path" '
>  	test_path_is_file my-hooks.ran &&
>  	cat >expect <<-EOF &&
>  	fatal: longline.patch: rejected by sendemail-validate hook
> -	fatal: command '"'"'$(pwd)/my-hooks/sendemail-validate'"'"' died with exit code 1
> +	fatal: command '"'"'$PWD/my-hooks/sendemail-validate'"'"' died with exit code 1
>  	warning: no patches were sent
>  	EOF
>  	test_cmp expect actual
>  '
>
>  test_expect_success $PREREQ "--validate respects absolute core.hooksPath path" '
> -	hooks_path="$(pwd)/my-hooks" &&
> -	test_config core.hooksPath "$hooks_path" &&
> +	test_config core.hooksPath "$(pwd)/my-hooks" &&
>  	test_when_finished "rm my-hooks.ran" &&
>  	test_must_fail git send-email \
>  		--from="Example <nobody@example.com>" \
> @@ -558,7 +557,7 @@ test_expect_success $PREREQ "--validate respects absolute core.hooksPath path" '
>  	test_path_is_file my-hooks.ran &&
>  	cat >expect <<-EOF &&
>  	fatal: longline.patch: rejected by sendemail-validate hook
> -	fatal: command '"'"'$hooks_path/sendemail-validate'"'"' died with exit code 1
> +	fatal: command '"'"'$PWD/my-hooks/sendemail-validate'"'"' died with exit code 1
>  	warning: no patches were sent
>  	EOF
>  	test_cmp expect actual
> --
> 2.31.0.152.g120726e270
>
>

      parent reply	other threads:[~2021-06-02 11:40 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-24 19:38 [PATCH] t9001-send-email.sh: fix expected absolute paths on Windows Johannes Sixt
2021-05-24 20:26 ` Jonathan Nieder
2021-05-24 22:15 ` Ævar Arnfjörð Bjarmason
2021-05-24 23:15   ` Ævar Arnfjörð Bjarmason
2021-05-24 23:14 ` [PATCH 0/2] send-email: pre-release fixes for v2.32.0 Ævar Arnfjörð Bjarmason
2021-05-24 23:14   ` [PATCH 1/2] send-email: fix missing error message regression Ævar Arnfjörð Bjarmason
2021-05-24 23:14   ` [PATCH 2/2] send-email: don't needlessly abs_path() the core.hooksPath Ævar Arnfjörð Bjarmason
2021-05-25  1:03     ` Junio C Hamano
2021-05-25  5:57       ` Ævar Arnfjörð Bjarmason
2021-05-25  6:13         ` Ævar Arnfjörð Bjarmason
2021-05-25  6:21           ` Robert Foss
2021-05-25  6:21         ` Junio C Hamano
2021-05-25 12:09           ` Ævar Arnfjörð Bjarmason
2021-05-25 19:28             ` Junio C Hamano
2021-05-26 11:21               ` [PATCH v2 0/2] " Ævar Arnfjörð Bjarmason
2021-05-26 11:21                 ` [PATCH v2 1/2] " Ævar Arnfjörð Bjarmason
2021-05-26 11:21                 ` [PATCH v2 2/2] send-email: move "hooks_path" invocation to git-send-email.perl Ævar Arnfjörð Bjarmason
2021-05-26  1:22         ` [PATCH 2/2] send-email: don't needlessly abs_path() the core.hooksPath Felipe Contreras
2021-05-25  6:10       ` Robert Foss
2021-06-02 11:40 ` Johannes Schindelin [this message]

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=nycvar.QRO.7.76.6.2106021330470.55@tvgsbejvaqbjf.bet \
    --to=johannes.schindelin@gmx.de \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=j6t@kdbg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).