Merge branch 'nd/clear-gitenv-upon-use-of-alias'

The automatic typo correction applied to an alias was broken
with a recent change already in 'master'.

* nd/clear-gitenv-upon-use-of-alias:
  restore_env(): free the saved environment variable once we are done
  git: simplify environment save/restore logic
  git: protect against unbalanced calls to {save,restore}_env()
  git: remove an early return from save_env_before_alias()

git.c

@@ -25,14 +25,14 @@ static const char *env_names[] = {
 	GIT_PREFIX_ENVIRONMENT
 };
 static char *orig_env[4];
-static int saved_env_before_alias;
+static int save_restore_env_balance;
 
 static void save_env_before_alias(void)
 {
 	int i;
-	if (saved_env_before_alias)
-		return;
-	saved_env_before_alias = 1;
+
+	assert(save_restore_env_balance == 0);
+	save_restore_env_balance = 1;
 	orig_cwd = xgetcwd();
 	for (i = 0; i < ARRAY_SIZE(env_names); i++) {
 		orig_env[i] = getenv(env_names[i]);
@@ -44,17 +44,22 @@ static void save_env_before_alias(void)
 static void restore_env(int external_alias)
 {
 	int i;
+
+	assert(save_restore_env_balance == 1);
+	save_restore_env_balance = 0;
 	if (!external_alias && orig_cwd && chdir(orig_cwd))
 		die_errno("could not move to %s", orig_cwd);
 	free(orig_cwd);
 	for (i = 0; i < ARRAY_SIZE(env_names); i++) {
 		if (external_alias &&
 		    !strcmp(env_names[i], GIT_PREFIX_ENVIRONMENT))
 			continue;
-		if (orig_env[i])
+		if (orig_env[i]) {
 			setenv(env_names[i], orig_env[i], 1);
-		else
+			free(orig_env[i]);
+		} else {
 			unsetenv(env_names[i]);
+		}
 	}
 }
 
@@ -531,16 +536,8 @@ static void handle_builtin(int argc, const char **argv)
 	}
 
 	builtin = get_builtin(cmd);
-	if (builtin) {
-		/*
-		 * XXX: if we can figure out cases where it is _safe_
-		 * to do, we can avoid spawning a new process when
-		 * saved_env_before_alias is true
-		 * (i.e. setup_git_dir* has been run once)
-		 */
-		if (!saved_env_before_alias)
-			exit(run_builtin(builtin, argc, argv));
-	}
+	if (builtin)
+		exit(run_builtin(builtin, argc, argv));
 }
 
 static void execv_dashed_external(const char **argv)
@@ -584,8 +581,17 @@ static int run_argv(int *argcp, const char ***argv)
 	int done_alias = 0;
 
 	while (1) {
-		/* See if it's a builtin */
-		handle_builtin(*argcp, *argv);
+		/*
+		 * If we tried alias and futzed with our environment,
+		 * it no longer is safe to invoke builtins directly in
+		 * general.  We have to spawn them as dashed externals.
+		 *
+		 * NEEDSWORK: if we can figure out cases
+		 * where it is safe to do, we can avoid spawning a new
+		 * process.
+		 */
+		if (!done_alias)
+			handle_builtin(*argcp, *argv);
 
 		/* .. then try the external ones */
 		execv_dashed_external(*argv);

t/t9003-help-autocorrect.sh

@@ -0,0 +1,52 @@
+#!/bin/sh
+
+test_description='help.autocorrect finding a match'
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	# An alias
+	git config alias.lgf "log --format=%s --first-parent" &&
+
+	# A random user-defined command
+	write_script git-distimdistim <<-EOF &&
+		echo distimdistim was called
+	EOF
+
+	PATH="$PATH:." &&
+	export PATH &&
+
+	git commit --allow-empty -m "a single log entry" &&
+
+	# Sanity check
+	git lgf >actual &&
+	echo "a single log entry" >expect &&
+	test_cmp expect actual &&
+
+	git distimdistim >actual &&
+	echo "distimdistim was called" >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'autocorrect showing candidates' '
+	git config help.autocorrect 0 &&
+
+	test_must_fail git lfg 2>actual &&
+	sed -e "1,/^Did you mean this/d" actual | grep lgf &&
+
+	test_must_fail git distimdist 2>actual &&
+	sed -e "1,/^Did you mean this/d" actual | grep distimdistim
+'
+
+test_expect_success 'autocorrect running commands' '
+	git config help.autocorrect -1 &&
+
+	git lfg >actual &&
+	echo "a single log entry" >expect &&
+	test_cmp expect actual &&
+
+	git distimdist >actual &&
+	echo "distimdistim was called" >expect &&
+	test_cmp expect actual
+'
+
+test_done