From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 19 May 2015 20:17:54 +0000 (-0700)
Subject: Merge branch 'bc/connect-plink'
X-Git-Tag: v2.5.0-rc0~110
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/7a4f891329527826e6f1c80be2b77cfbfee06190?ds=inline;hp=-c

Merge branch 'bc/connect-plink'

The connection initiation code for "ssh" transport tried to absorb
differences between the stock "ssh" and Putty-supplied "plink" and
its derivatives, but the logic to tell that we are using "plink"
variants were too loose and falsely triggered when "plink" appeared
anywhere in the path (e.g. "/home/me/bin/uplink/ssh").

* bc/connect-plink:
connect: improve check for plink to reduce false positives
t5601: fix quotation error leading to skipped tests
connect: simplify SSH connection code path
---

7a4f891329527826e6f1c80be2b77cfbfee06190
diff --combined connect.c
index 391d21192f,1f7b7694f6..c0144d859a
--- a/connect.c
+++ b/connect.c
@@@ -310,8 -310,6 +310,8 @@@ static void get_host_and_port(char **ho
  		if (end != colon + 1 && *end == '\0' && 0 <= portnr && portnr < 65536) {
  			*colon = 0;
  			*port = colon + 1;
 +		} else if (!colon[1]) {
 +			*colon = 0;
  		}
  	}
  }
@@@ -724,7 -722,7 +724,7 @@@ struct child_process *git_connect(int f
  		conn->in = conn->out = -1;
  		if (protocol == PROTO_SSH) {
  			const char *ssh;
- 			int putty;
+ 			int putty, tortoiseplink = 0;
  			char *ssh_host = hostandport;
  			const char *port = NULL;
  			get_host_and_port(&ssh_host, &port);
@@@ -741,30 -739,41 +741,42 @@@
  
  				free(hostandport);
  				free(path);
 +				free(conn);
  				return NULL;
+ 			}
+ 
+ 			ssh = getenv("GIT_SSH_COMMAND");
+ 			if (ssh) {
+ 				conn->use_shell = 1;
+ 				putty = 0;
  			} else {
- 				ssh = getenv("GIT_SSH_COMMAND");
- 				if (ssh) {
- 					conn->use_shell = 1;
- 					putty = 0;
- 				} else {
- 					ssh = getenv("GIT_SSH");
- 					if (!ssh)
- 						ssh = "ssh";
- 					putty = !!strcasestr(ssh, "plink");
- 				}
- 
- 				argv_array_push(&conn->args, ssh);
- 				if (putty && !strcasestr(ssh, "tortoiseplink"))
- 					argv_array_push(&conn->args, "-batch");
- 				if (port) {
- 					/* P is for PuTTY, p is for OpenSSH */
- 					argv_array_push(&conn->args, putty ? "-P" : "-p");
- 					argv_array_push(&conn->args, port);
- 				}
- 				argv_array_push(&conn->args, ssh_host);
+ 				const char *base;
+ 				char *ssh_dup;
+ 
+ 				ssh = getenv("GIT_SSH");
+ 				if (!ssh)
+ 					ssh = "ssh";
+ 
+ 				ssh_dup = xstrdup(ssh);
+ 				base = basename(ssh_dup);
+ 
+ 				tortoiseplink = !strcasecmp(base, "tortoiseplink") ||
+ 					!strcasecmp(base, "tortoiseplink.exe");
+ 				putty = !strcasecmp(base, "plink") ||
+ 					!strcasecmp(base, "plink.exe") || tortoiseplink;
+ 
+ 				free(ssh_dup);
+ 			}
+ 
+ 			argv_array_push(&conn->args, ssh);
+ 			if (tortoiseplink)
+ 				argv_array_push(&conn->args, "-batch");
+ 			if (port) {
+ 				/* P is for PuTTY, p is for OpenSSH */
+ 				argv_array_push(&conn->args, putty ? "-P" : "-p");
+ 				argv_array_push(&conn->args, port);
  			}
+ 			argv_array_push(&conn->args, ssh_host);
  		} else {
  			/* remove repo-local variables from the environment */
  			conn->env = local_repo_env;
diff --combined t/t5601-clone.sh
index 1befc453a3,731c09c418..bfdaf75966
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@@ -296,6 -296,12 +296,12 @@@ setup_ssh_wrapper () 
  	'
  }
  
+ copy_ssh_wrapper_as () {
+ 	cp "$TRASH_DIRECTORY/ssh-wrapper" "$1" &&
+ 	GIT_SSH="$1" &&
+ 	export GIT_SSH
+ }
+ 
  expect_ssh () {
  	test_when_finished '
  		(cd "$TRASH_DIRECTORY" && rm -f ssh-expect && >ssh-output)
@@@ -332,9 -338,36 +338,36 @@@ test_expect_success !MINGW,!CYGWIN 'clo
  
  test_expect_success 'bracketed hostnames are still ssh' '
  	git clone "[myhost:123]:src" ssh-bracket-clone &&
- 	expect_ssh myhost '-p 123' src
+ 	expect_ssh "-p 123" myhost src
+ '
+ 
+ test_expect_success 'uplink is not treated as putty' '
+ 	copy_ssh_wrapper_as "$TRASH_DIRECTORY/uplink" &&
+ 	git clone "[myhost:123]:src" ssh-bracket-clone-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 &&
+ 	expect_ssh "-P 123" myhost src
  '
  
+ test_expect_success 'plink.exe is treated specially (as putty)' '
+ 	copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink.exe" &&
+ 	git clone "[myhost:123]:src" ssh-bracket-clone-plink-1 &&
+ 	expect_ssh "-P 123" myhost src
+ '
+ 
+ test_expect_success 'tortoiseplink is like putty, with extra arguments' '
+ 	copy_ssh_wrapper_as "$TRASH_DIRECTORY/tortoiseplink" &&
+ 	git clone "[myhost:123]:src" ssh-bracket-clone-plink-2 &&
+ 	expect_ssh "-batch -P 123" myhost src
+ '
+ 
+ # Reset the GIT_SSH environment variable for clone tests.
+ setup_ssh_wrapper
+ 
  counter=0
  # $1 url
  # $2 none|host
@@@ -387,17 -420,14 +420,17 @@@ d
  done
  
  #with ssh:// scheme
 -test_expect_success 'clone ssh://host.xz/home/user/repo' '
 -	test_clone_url "ssh://host.xz/home/user/repo" host.xz "/home/user/repo"
 -'
 -
 -# from home directory
 -test_expect_success 'clone ssh://host.xz/~repo' '
 -	test_clone_url "ssh://host.xz/~repo" host.xz "~repo"
 +#ignore trailing colon
 +for tcol in "" :
 +do
 +	test_expect_success "clone ssh://host.xz$tcol/home/user/repo" '
 +		test_clone_url "ssh://host.xz$tcol/home/user/repo" host.xz /home/user/repo
 +	'
 +	# from home directory
 +	test_expect_success "clone ssh://host.xz$tcol/~repo" '
 +	test_clone_url "ssh://host.xz$tcol/~repo" host.xz "~repo"
  '
 +done
  
  # with port number
  test_expect_success 'clone ssh://host.xz:22/home/user/repo' '
@@@ -410,9 -440,9 +443,9 @@@ test_expect_success 'clone ssh://host.x
  '
  
  #IPv6
 -for tuah in ::1 [::1] user@::1 user@[::1] [user@::1]
 +for tuah in ::1 [::1] [::1]: user@::1 user@[::1] user@[::1]: [user@::1] [user@::1]:
  do
 -	ehost=$(echo $tuah | tr -d "[]")
 +	ehost=$(echo $tuah | sed -e "s/1]:/1]/ "| tr -d "[]")
  	test_expect_success "clone ssh://$tuah/home/user/repo" "
  	  test_clone_url ssh://$tuah/home/user/repo $ehost /home/user/repo
  	"