Re: [PATCH 3/5] ssh: 'auto' variant to select between 'ssh' and 'simple'

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Oct 25, 2017 at 5:51 AM, Johannes Schindelin
<Johannes.Schindelin@xxxxxx> wrote:
> Hi Jonathan,
>
> [I only saw that you replied to 3/5 with v2 after writing this reply, but
> it would apply to v2's 3/5, too]
>
> On Mon, 23 Oct 2017, Jonathan Nieder wrote:
>
>> diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
>> index 86811a0c35..fd94dd40d2 100755
>> --- a/t/t5601-clone.sh
>> +++ b/t/t5601-clone.sh
>> @@ -384,6 +384,20 @@ test_expect_success 'uplink is treated as simple' '
>>       expect_ssh myhost src
>>  '
>>
>> +test_expect_success 'OpenSSH-like uplink is treated as ssh' '
>> +     write_script "$TRASH_DIRECTORY/uplink" <<-EOF &&
>> +     if test "\$1" = "-G"
>> +     then
>> +             exit 0
>> +     fi &&
>> +     exec "\$TRASH_DIRECTORY/ssh$X" "\$@"
>> +     EOF
>> +     GIT_SSH="$TRASH_DIRECTORY/uplink" &&
>> +     export GIT_SSH &&
>> +     git clone "[myhost:123]:src" ssh-bracket-clone-sshlike-uplink &&
>> +     expect_ssh "-p 123" myhost src
>> +'
>> +
>>  test_expect_success 'plink is treated specially (as putty)' '
>>       copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink" &&
>>       git clone "[myhost:123]:src" ssh-bracket-clone-plink-0 &&
>
> This breaks on Windows. And it is not immediately obvious how so, so let
> me explain:
>
> As you know, on Windows there is no executable flag. There is the .exe
> file extension to indicate an executable (and .com and .bat and .cmd are
> also handled as executable, at least as executable script).
>
> Now, what happens if you call "abc" in the MSYS2 Bash and there is no
> script called "abc" but an executable called "abc.exe" in the PATH? Why,
> of course we execute that executable. It has to, because if "abc.exe"
> would be renamed into "abc", it would not work any longer.
>
> That is also the reason why that "copy_ssh_wrapper" helper function
> automatically appends that .exe file suffix on Windows: it has to.
>
> Every workaround breaks down at some point, and this workaround is no
> exception. What should the MSYS2 Bash do if asked to overwrite "abc" and
> there is only an "abc.exe"? It actually overwrites "abc.exe" and moves on.
>
> And this is where we are here: the previous test case copied the ssh
> wrapper as "uplink". Except on Windows, it is "uplink.exe". And your newly
> introduced test case overwrites it. And then it tells Git specifically to
> look for "uplink", and Git does *not* append that .exe suffix
> automatically as the MSYS2 Bash would do, because git.exe is not intended
> to work MSYS2-like.
>
> As a consequence, the test fails. Could you please squash in this, to fix
> the test on Windows?

This explanation is in detail and would even make a good commit message
for a follow up commit. (Squashing just that line would loose the explanation
as I suspect the original commit will not dedicate so much text to
this single line)

>
> -- snipsnap --
> diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
> index ec4b17bca62..1afcbd00617 100755
> --- a/t/t5601-clone.sh
> +++ b/t/t5601-clone.sh
> @@ -393,6 +393,7 @@ test_expect_success 'simple does not support port' '
>
>  test_expect_success 'uplink is treated as simple' '
>         copy_ssh_wrapper_as "$TRASH_DIRECTORY/uplink" &&
> +       test_when_finished "rm \"$TRASH_DIRECTORY/uplink$X\"" &&
>         test_must_fail git clone "[myhost:123]:src" ssh-bracket-clone-uplink &&
>         git clone "myhost:src" ssh-clone-uplink &&
>         expect_ssh myhost src
>



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux