git_connect: clarify conn->use_shell flagjk/connect-clear-env

When executing user-specified programs, we generally always
want to use a shell, for flexibility and consistency. One
big exception is executing $GIT_SSH, which for historical
reasons must not use a shell.

Once upon a time the logic in git_connect looked like:

  if (protocol == PROTO_SSH) {
	  ... setup ssh ...
  } else {
	  ... setup local connection ...
	  conn->use_shell = 1;
  }

But over time the PROTO_SSH block has grown, and the "local"
block has shrunk so that it contains only conn->use_shell;
it's easy to miss at the end of the large block.  Moreover,
PROTO_SSH now also sometimes sets use_shell, when the new
GIT_SSH_COMMAND is used.

Let's just set conn->use_shell when we're setting up the "conn"
struct, and unset it (with a comment) in the historical GIT_SSH
case.  This will make the flow easier to follow.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

1 files changed, 13 insertions, 9 deletions

diff --git a/connect.c b/connect.c
index 962f9904d4..acd39d70c8 100644
--- a/connect.c
+++ b/connect.c
@@ -723,10 +723,11 @@ struct child_process *git_connect(int fd[2], const char *url,
 
 		/* remove repo-local variables from the environment */
 		conn->env = local_repo_env;
+		conn->use_shell = 1;
 		conn->in = conn->out = -1;
 		if (protocol == PROTO_SSH) {
 			const char *ssh;
-			int putty, tortoiseplink = 0;
+			int putty = 0, tortoiseplink = 0;
 			char *ssh_host = hostandport;
 			const char *port = NULL;
 			get_host_and_port(&ssh_host, &port);
@@ -748,13 +749,17 @@ struct child_process *git_connect(int fd[2], const char *url,
 			}
 
 			ssh = getenv("GIT_SSH_COMMAND");
-			if (ssh) {
-				conn->use_shell = 1;
-				putty = 0;
-			} else {
+			if (!ssh) {
 				const char *base;
 				char *ssh_dup;
 
+				/*
+				 * GIT_SSH is the no-shell version of
+				 * GIT_SSH_COMMAND (and must remain so for
+				 * historical compatibility).
+				 */
+				conn->use_shell = 0;
+
 				ssh = getenv("GIT_SSH");
 				if (!ssh)
 					ssh = "ssh";
@@ -764,8 +769,9 @@ struct child_process *git_connect(int fd[2], const char *url,
 
 				tortoiseplink = !strcasecmp(base, "tortoiseplink") ||
 					!strcasecmp(base, "tortoiseplink.exe");
-				putty = !strcasecmp(base, "plink") ||
-					!strcasecmp(base, "plink.exe") || tortoiseplink;
+				putty = tortoiseplink ||
+					!strcasecmp(base, "plink") ||
+					!strcasecmp(base, "plink.exe");
 
 				free(ssh_dup);
 			}
@@ -779,8 +785,6 @@ struct child_process *git_connect(int fd[2], const char *url,
 				argv_array_push(&conn->args, port);
 			}
 			argv_array_push(&conn->args, ssh_host);
-		} else {
-			conn->use_shell = 1;
 		}
 		argv_array_push(&conn->args, cmd.buf);