From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 17 Feb 2016 18:13:31 +0000 (-0800)
Subject: Merge branch 'nd/clear-gitenv-upon-use-of-alias'
X-Git-Tag: v2.8.0-rc0~52
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/dbda66b0e283494c35958316c9b1c47b6e6c9f59?ds=inline;hp=-c

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()
---

dbda66b0e283494c35958316c9b1c47b6e6c9f59
diff --combined git.c
index da278c3d41,93f569d064..6c64c9430e
--- a/git.c
+++ b/git.c
@@@ -25,14 -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,6 -44,9 +44,9 @@@
  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);
@@@ -51,10 -54,12 +54,12 @@@
  		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]);
+ 		}
  	}
  }
  
@@@ -372,7 -377,6 +377,7 @@@ static int run_builtin(struct cmd_struc
  
  static struct cmd_struct commands[] = {
  	{ "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
 +	{ "am", cmd_am, RUN_SETUP | NEED_WORK_TREE },
  	{ "annotate", cmd_annotate, RUN_SETUP },
  	{ "apply", cmd_apply, RUN_SETUP_GENTLY },
  	{ "archive", cmd_archive },
@@@ -419,7 -423,7 +424,7 @@@
  	{ "index-pack", cmd_index_pack, RUN_SETUP_GENTLY },
  	{ "init", cmd_init_db },
  	{ "init-db", cmd_init_db },
 -	{ "interpret-trailers", cmd_interpret_trailers, RUN_SETUP },
 +	{ "interpret-trailers", cmd_interpret_trailers, RUN_SETUP_GENTLY },
  	{ "log", cmd_log, RUN_SETUP },
  	{ "ls-files", cmd_ls_files, RUN_SETUP },
  	{ "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY },
@@@ -448,7 -452,6 +453,7 @@@
  	{ "pickaxe", cmd_blame, RUN_SETUP },
  	{ "prune", cmd_prune, RUN_SETUP },
  	{ "prune-packed", cmd_prune_packed, RUN_SETUP },
 +	{ "pull", cmd_pull, RUN_SETUP | NEED_WORK_TREE },
  	{ "push", cmd_push, RUN_SETUP },
  	{ "read-tree", cmd_read_tree, RUN_SETUP },
  	{ "receive-pack", cmd_receive_pack },
@@@ -472,7 -475,6 +477,7 @@@
  	{ "stage", cmd_add, RUN_SETUP | NEED_WORK_TREE },
  	{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
  	{ "stripspace", cmd_stripspace },
 +	{ "submodule--helper", cmd_submodule__helper, RUN_SETUP },
  	{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
  	{ "tag", cmd_tag, RUN_SETUP },
  	{ "unpack-file", cmd_unpack_file, RUN_SETUP },
@@@ -531,16 -533,8 +536,8 @@@ static void handle_builtin(int argc, co
  	}
  
  	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 -578,17 +581,17 @@@ static int run_argv(int *argcp, const c
  	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);