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: Thomas Braun <thomas.braun@byte-physics•de>, git@vger•kernel.org
Subject: Re: [PATCH 13/19] mingw: outsmart MSYS2's path substitution in t1508
Date: Sun, 24 Jan 2016 18:03:40 -0800	[thread overview]
Message-ID: <xmqqa8nubekj.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <1a4477f951edc9f58a24163d3935a7b35a3f14b2.1453650173.git.johannes.schindelin@gmx.de> (Johannes Schindelin's message of "Sun, 24 Jan 2016 16:45:09 +0100 (CET)")

Johannes Schindelin <johannes.schindelin@gmx•de> writes:

> From: Thomas Braun <thomas.braun@byte-physics•de>
>
> A string of the form "@/abcd" is considered a file path
> by the msys layer and therefore translated to a Windows path.
>
> Here the trick is to double the slashes.
>
> The MSYS2 patch translation can be studied by calling
>
> 	test-path-utils print_path <path>
>
> Signed-off-by: Thomas Braun <thomas.braun@byte-physics•de>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx•de>
> ---

This feels wrong.

The point of this test is that you can ask to checkout a branch
whose name is a strangely looking "@/at-test", and a ref whose name
is "refs/heads/@/at-test" indeed is created.

The current "checkout" may be lazy and not signal an error for a
branch name with two consecutive slashes, but I wouldn't be
surprised if we tighten that later, and more importantly, I do not
think we ever promised users if you asked a branch "a//b" to be
created, we would create "refs/heads/a/b".

The new test hardcodes and promises such an incompatible behaviour,
i.e. a request to create "@//b" results in "@/b" created, only to
users on MINGW, fracturing the expectations of the Git userbase.

Wouldn't it be better to declare "On other people's Git, @/foo is
just as normal a branch name as a/foo, but on MINGW @/foo cannot be
used" by skipping some tests using prerequisites instead?


>  t/t1508-at-combinations.sh | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/t/t1508-at-combinations.sh b/t/t1508-at-combinations.sh
> index 078e119..1d9fd7b 100755
> --- a/t/t1508-at-combinations.sh
> +++ b/t/t1508-at-combinations.sh
> @@ -29,13 +29,22 @@ fail() {
>  	"$@" failure
>  }
>  
> +if test_have_prereq MINGW
> +then
> +	# MSYS2 interprets `@/abc` to be a file list, and wants to substitute
> +	# the Unix-y path with a Windows one (e.g. @C:\msys64\abc)
> +	AT_SLASH=@//at-test
> +else
> +	AT_SLASH=@/at-test
> +fi
> +
>  test_expect_success 'setup' '
>  	test_commit master-one &&
>  	test_commit master-two &&
>  	git checkout -b upstream-branch &&
>  	test_commit upstream-one &&
>  	test_commit upstream-two &&
> -	git checkout -b @/at-test &&
> +	git checkout -b $AT_SLASH &&
>  	git checkout -b @@/at-test &&
>  	git checkout -b @at-test &&
>  	git checkout -b old-branch &&
> @@ -64,7 +73,7 @@ check "@{-1}@{u}@{1}" commit master-one
>  check "@" commit new-two
>  check "@@{u}" ref refs/heads/upstream-branch
>  check "@@/at-test" ref refs/heads/@@/at-test
> -check "@/at-test" ref refs/heads/@/at-test
> +check "$AT_SLASH" ref refs/heads/@/at-test
>  check "@at-test" ref refs/heads/@at-test
>  nonsense "@{u}@{-1}"
>  nonsense "@{0}@{0}"

  reply	other threads:[~2016-01-25  2:04 UTC|newest]

Thread overview: 113+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-24 15:43 [PATCH 00/19] Let Git's tests pass on Windows Johannes Schindelin
2016-01-24 15:43 ` [PATCH 01/19] mingw: let's use gettext with MSYS2 Johannes Schindelin
2016-01-25  1:42   ` Junio C Hamano
2016-01-25 16:16     ` Johannes Schindelin
2016-01-25 18:53       ` Junio C Hamano
2016-01-24 15:43 ` [PATCH 02/19] mingw: do not trust MSYS2's MinGW gettext.sh Johannes Schindelin
2016-01-24 15:43 ` [PATCH 03/19] Git.pm: stop assuming that absolute paths start with a slash Johannes Schindelin
2016-01-24 15:43 ` [PATCH 04/19] mingw: factor out Windows specific environment setup Johannes Schindelin
2016-01-24 15:43 ` [PATCH 05/19] mingw: prepare the TMPDIR environment variable for shell scripts Johannes Schindelin
2016-01-25  2:11   ` Eric Sunshine
2016-01-26  8:38     ` Johannes Schindelin
2016-01-24 15:44 ` [PATCH 06/19] mingw: try to delete target directory before renaming Johannes Schindelin
2016-01-24 17:42   ` Philip Oakley
2016-01-25  6:59     ` Johannes Schindelin
2016-01-25 18:55       ` Junio C Hamano
2016-01-26  8:14         ` Johannes Schindelin
2016-01-24 15:44 ` [PATCH 07/19] mingw: let lstat() fail with errno == ENOTDIR when appropriate Johannes Schindelin
2016-01-24 15:44 ` [PATCH 08/19] mingw: fix t5601-clone.sh Johannes Schindelin
2016-01-24 15:44 ` [PATCH 09/19] mingw: accomodate t0060-path-utils for MSYS2 Johannes Schindelin
2016-01-24 19:51   ` Johannes Sixt
2016-01-25 15:39     ` Johannes Schindelin
2016-01-24 15:44 ` [PATCH 10/19] mingw: disable mkfifo-based tests Johannes Schindelin
2016-01-24 15:44 ` [PATCH 11/19] tests: turn off git-daemon tests if FIFOs are not available Johannes Schindelin
2016-01-24 15:45 ` [PATCH 12/19] mingw: do not use symlinks with SVN in t9100 Johannes Schindelin
2016-01-25  1:51   ` Junio C Hamano
2016-01-25 16:53     ` Johannes Schindelin
2016-01-24 15:45 ` [PATCH 13/19] mingw: outsmart MSYS2's path substitution in t1508 Johannes Schindelin
2016-01-25  2:03   ` Junio C Hamano [this message]
2016-01-25  6:22     ` Eric Sunshine
2016-01-25 18:50       ` Junio C Hamano
2016-01-26  6:53         ` Johannes Schindelin
2016-01-25 16:30     ` Johannes Schindelin
2016-01-25 17:04       ` Johannes Schindelin
2016-01-24 15:45 ` [PATCH 14/19] mingw: fix t9700's assumption about directory separators Johannes Schindelin
2016-01-24 15:45 ` [PATCH 15/19] mingw: work around pwd issues in the tests Johannes Schindelin
2016-01-24 15:45 ` [PATCH 16/19] mingw: avoid absolute path in t0008 Johannes Schindelin
2016-01-25  2:11   ` Junio C Hamano
2016-01-25 16:48     ` Johannes Schindelin
2016-01-25 19:31       ` Ray Donnelly
2016-01-25 22:20       ` Junio C Hamano
2016-01-26  8:00         ` Johannes Schindelin
2016-01-24 15:45 ` [PATCH 17/19] mingw: fix git-svn tests that expect chmod to work Johannes Schindelin
2016-01-25  2:14   ` Junio C Hamano
2016-01-26  6:31     ` Johannes Schindelin
2016-01-26 17:42       ` Junio C Hamano
2016-01-24 15:45 ` [PATCH 18/19] mingw: skip a couple of git-svn tests that cannot pass on Windows Johannes Schindelin
2016-01-25  2:16   ` Junio C Hamano
2016-01-26  6:45     ` Johannes Schindelin
2016-01-24 15:45 ` [PATCH 19/19] mingw: do not bother to test funny file names Johannes Schindelin
2016-01-25  1:34 ` [PATCH 00/19] Let Git's tests pass on Windows Junio C Hamano
2016-01-25 15:45   ` Johannes Schindelin
2016-01-26 14:34 ` [PATCH v2 " Johannes Schindelin
2016-01-26 14:34   ` [PATCH v2 01/19] mingw: let's use gettext with MSYS2 Johannes Schindelin
2016-01-26 14:34   ` [PATCH v2 02/19] mingw: do not trust MSYS2's MinGW gettext.sh Johannes Schindelin
2016-01-26 14:34   ` [PATCH v2 03/19] Git.pm: stop assuming that absolute paths start with a slash Johannes Schindelin
2016-01-26 14:34   ` [PATCH v2 04/19] mingw: factor out Windows specific environment setup Johannes Schindelin
2016-01-26 14:34   ` [PATCH v2 05/19] mingw: prepare the TMPDIR environment variable for shell scripts Johannes Schindelin
2016-01-26 14:34   ` [PATCH v2 06/19] mingw: try to delete target directory before renaming Johannes Schindelin
2016-01-26 14:34   ` [PATCH v2 07/19] mingw: let lstat() fail with errno == ENOTDIR when appropriate Johannes Schindelin
2016-01-26 14:34   ` [PATCH v2 08/19] mingw: fix t5601-clone.sh Johannes Schindelin
2016-01-26 14:35   ` [PATCH v2 09/19] mingw: accomodate t0060-path-utils for MSYS2 Johannes Schindelin
2016-01-26 14:35   ` [PATCH v2 10/19] mingw: disable mkfifo-based tests Johannes Schindelin
2016-01-26 14:35   ` [PATCH v2 11/19] tests: turn off git-daemon tests if FIFOs are not available Johannes Schindelin
2016-01-26 14:35   ` [PATCH v2 12/19] mingw: skip test in t1508 that fails due to path conversion Johannes Schindelin
2016-01-26 22:02     ` Junio C Hamano
2016-01-27  8:50       ` Johannes Schindelin
2016-01-27 20:23         ` Junio C Hamano
2016-01-28  7:58           ` Johannes Schindelin
2016-01-26 14:35   ` [PATCH v2 13/19] mingw: fix t9700's assumption about directory separators Johannes Schindelin
2016-01-26 14:35   ` [PATCH v2 14/19] mingw: work around pwd issues in the tests Johannes Schindelin
2016-01-26 14:35   ` [PATCH v2 15/19] Avoid absolute path in t0008 Johannes Schindelin
2016-01-26 14:35   ` [PATCH v2 16/19] mingw: mark t9100's test cases with appropriate prereqs Johannes Schindelin
2016-01-26 21:50     ` Junio C Hamano
2016-01-27 16:02       ` Johannes Schindelin
2016-01-26 14:35   ` [PATCH v2 17/19] mingw: avoid illegal filename in t9118 Johannes Schindelin
2016-01-26 14:35   ` [PATCH v2 18/19] mingw: handle the missing POSIXPERM prereq in t9124 Johannes Schindelin
2016-01-26 22:05     ` Junio C Hamano
2016-01-27  9:20       ` Johannes Schindelin
2016-01-26 14:35   ` [PATCH v2 19/19] mingw: do not bother to test funny file names Johannes Schindelin
2016-01-26 20:03     ` Eric Sunshine
2016-01-26 20:24       ` Junio C Hamano
2016-01-27  8:33       ` Johannes Schindelin
2016-01-26 22:12   ` [PATCH v2 00/19] Let Git's tests pass on Windows Junio C Hamano
2016-01-27  8:38     ` Johannes Schindelin
2016-01-27 20:24       ` Junio C Hamano
2016-01-27 16:19   ` [PATCH v3 00/20] " Johannes Schindelin
2016-01-27 16:19     ` [PATCH v3 01/20] mingw: let's use gettext with MSYS2 Johannes Schindelin
2016-01-27 16:19     ` [PATCH v3 02/20] mingw: do not trust MSYS2's MinGW gettext.sh Johannes Schindelin
2016-01-27 16:19     ` [PATCH v3 03/20] Git.pm: stop assuming that absolute paths start with a slash Johannes Schindelin
2016-01-27 16:19     ` [PATCH v3 04/20] mingw: factor out Windows specific environment setup Johannes Schindelin
2016-01-27 16:19     ` [PATCH v3 05/20] mingw: prepare the TMPDIR environment variable for shell scripts Johannes Schindelin
2016-01-27 16:19     ` [PATCH v3 06/20] mingw: try to delete target directory before renaming Johannes Schindelin
2016-01-27 16:19     ` [PATCH v3 07/20] mingw: let lstat() fail with errno == ENOTDIR when appropriate Johannes Schindelin
2016-01-27 16:19     ` [PATCH v3 08/20] mingw: fix t5601-clone.sh Johannes Schindelin
2016-01-27 16:19     ` [PATCH v3 09/20] mingw: accomodate t0060-path-utils for MSYS2 Johannes Schindelin
2016-01-27 16:19     ` [PATCH v3 10/20] mingw: disable mkfifo-based tests Johannes Schindelin
2016-01-27 16:19     ` [PATCH v3 11/20] tests: turn off git-daemon tests if FIFOs are not available Johannes Schindelin
2016-01-28  8:21       ` Eric Sunshine
2016-01-28  8:40         ` Johannes Schindelin
2016-01-28 21:35           ` Junio C Hamano
2016-01-27 16:19     ` [PATCH v3 12/20] mingw: skip test in t1508 that fails due to path conversion Johannes Schindelin
2016-01-27 16:19     ` [PATCH v3 13/20] mingw: fix t9700's assumption about directory separators Johannes Schindelin
2016-01-27 16:19     ` [PATCH v3 14/20] mingw: work around pwd issues in the tests Johannes Schindelin
2016-01-27 16:20     ` [PATCH v3 15/20] Avoid absolute path in t0008 Johannes Schindelin
2016-01-27 16:20     ` [PATCH v3 16/20] mingw: mark t9100's test cases with appropriate prereqs Johannes Schindelin
2016-01-27 16:33       ` Johannes Schindelin
2016-01-27 16:20     ` [PATCH v3 17/20] mingw: avoid illegal filename in t9118 Johannes Schindelin
2016-01-27 16:20     ` [PATCH v3 18/20] mingw: handle the missing POSIXPERM prereq in t9124 Johannes Schindelin
2016-01-27 16:20     ` [PATCH v3 19/20] mingw: skip a test in t9130 that cannot pass on Windows Johannes Schindelin
2016-01-27 16:20     ` [PATCH v3 20/20] mingw: do not bother to test funny file names Johannes Schindelin
2016-01-28  8:42     ` [PATCH v3 00/20] Let Git's tests pass on Windows Eric Sunshine
2016-01-28  8:57       ` Johannes Schindelin
2016-01-28 21:37         ` Junio C Hamano

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=xmqqa8nubekj.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=johannes.schindelin@gmx$(echo .)de \
    --cc=thomas.braun@byte-physics$(echo .)de \
    /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