Re: [PATCH 13/19] mingw: outsmart MSYS2's path substitution in t1508

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 25, 2016, 02:03 UTC
Message-ID: <xmqqa8nubekj.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1a4477f951edc9f58a24163d3935a7b35a3f14b2.1453650173.git.johannes.schindelin@gmx.de>

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

Show 14 quoted lines

> 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?

Show 40 quoted lines

>  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}"