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

Hi Junio,

On Mon, 25 Jan 2016, Johannes Schindelin wrote:

On Sun, 24 Jan 2016, Junio C Hamano wrote:

quoted

Johannes Schindelin [off-list ref] writes:

quoted

From: Thomas Braun <redacted>

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 <redacted>
Signed-off-by: Johannes Schindelin <redacted>
---

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?

As Eric points out, this is not so much a behavior on Git as of the MSYS2
Bash. In fact, if you call `git.exe checkout -b @/at-test` from a cmd
window, it works just as advertised.

But your comment made me inspect the entire t9100 again, wondering why
things work when we copy the contents instead of symlinking them. And you
know what, even if I could have sworn that I verified for every patch in
this series that it is actually necessary to pass the test suite, it is
*not* necessary.

So I backed it out and it won't be part of v2 anymore.

Whoops. This was meant to be a comment on your comment on 12/19. I'll
reply to the appropriate mail...

As to the patch 13/19 that we are discussing here, I agree that it is
better to simply skip the test with the offending argument. See

	https://github.com/dscho/git/commit/ca5edbe

Ciao,
Dscho