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

From: Junio C Hamano <[email-masked]>
Date: Sun, 24 Jan 2016 18:03:40 -0800

Johannes Schindelin <[email-masked]> writes:

> From: Thomas Braun <[email-masked]>
>
> 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 <[email-masked]>
> Signed-off-by: Johannes Schindelin <[email-masked]>
> ---

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