ssh: 'simple' variant does not support --port

When trying to connect to an ssh:// URL with port explicitly specified
and the ssh command configured with GIT_SSH does not support such a
setting, it is less confusing to error out than to silently suppress
the port setting and continue.

This requires updating the GIT_SSH setting in t5603-clone-dirname.sh.
That test is about the directory name produced when cloning various
URLs.  It uses an ssh wrapper that ignores all its arguments but does
not declare that it supports a port argument; update it to set
GIT_SSH_VARIANT=ssh to do so.  (Real-life ssh wrappers that pass a
port argument to OpenSSH would also support -G and would not require
such an update.)

Reported-by: William Yan <wyan@google.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Acked-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/connect.c b/connect.c
index c3d1cd37871a50e292f872946ba127e97e62010b..d6033861b35f31aae6c4c93dba7fa5f1c7fac673 100644 (file)
--- a/connect.c
+++ b/connect.c
@@ -967,11 +967,20 @@ static void push_ssh_options(struct argv_array *args, struct argv_array *env,
        if (variant == VARIANT_TORTOISEPLINK)
                argv_array_push(args, "-batch");
 
-       if (port && variant != VARIANT_SIMPLE) {
-               if (variant == VARIANT_SSH)
+       if (port) {
+               switch (variant) {
+               case VARIANT_AUTO:
+                       BUG("VARIANT_AUTO passed to push_ssh_options");
+               case VARIANT_SIMPLE:
+                       die("ssh variant 'simple' does not support setting port");
+               case VARIANT_SSH:
                        argv_array_push(args, "-p");
-               else
+                       break;
+               case VARIANT_PLINK:
+               case VARIANT_PUTTY:
+               case VARIANT_TORTOISEPLINK:
                        argv_array_push(args, "-P");
+               }
 
                argv_array_push(args, port);
        }

diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 9ae1606ee5139af8c11d31ce2a108299435a9081..66784fc8ff2ec8cf24cdb70348e70995c333d233 100755 (executable)
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -378,12 +378,18 @@ test_expect_success 'variant=auto picks based on basename' '
 
 test_expect_success 'simple does not support -4/-6' '
        copy_ssh_wrapper_as "$TRASH_DIRECTORY/simple" &&
-       test_must_fail git clone -4 "[myhost:123]:src" ssh-bracket-clone-simple
+       test_must_fail git clone -4 "myhost:src" ssh-4-clone-simple
+'
+
+test_expect_success 'simple does not support port' '
+       copy_ssh_wrapper_as "$TRASH_DIRECTORY/simple" &&
+       test_must_fail git clone "[myhost:123]:src" ssh-bracket-clone-simple
 '
 
 test_expect_success 'uplink is treated as simple' '
        copy_ssh_wrapper_as "$TRASH_DIRECTORY/uplink" &&
-       git clone "[myhost:123]:src" ssh-bracket-clone-uplink &&
+       test_must_fail git clone "[myhost:123]:src" ssh-bracket-clone-uplink &&
+       git clone "myhost:src" ssh-clone-uplink &&
        expect_ssh myhost src
 '
 

diff --git a/t/t5603-clone-dirname.sh b/t/t5603-clone-dirname.sh
index d5af7581297d6167b82d029a015e367c057ff7af..13b5e5eb9b9fa2aa96e0496eb9fa3e7049fdfb51 100755 (executable)
--- a/t/t5603-clone-dirname.sh
+++ b/t/t5603-clone-dirname.sh
@@ -11,7 +11,9 @@ test_expect_success 'setup ssh wrapper' '
        git upload-pack "$TRASH_DIRECTORY"
        EOF
        GIT_SSH="$TRASH_DIRECTORY/ssh-wrapper" &&
+       GIT_SSH_VARIANT=ssh &&
        export GIT_SSH &&
+       export GIT_SSH_VARIANT &&
        export TRASH_DIRECTORY
 '