Merge branch 'nd/clear-gitenv-upon-use-of-alias'
authorJunio C Hamano <gitster@pobox.com>
Wed, 17 Feb 2016 18:13:31 +0000 (10:13 -0800)
committerJunio C Hamano <gitster@pobox.com>
Wed, 17 Feb 2016 18:13:31 +0000 (10:13 -0800)
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()

1  2 
git.c
diff --combined git.c
index da278c3d41ed308581ffec46e3487e7670898d41,93f569d0644263d90babbd7c26e2768b52a84e60..6c64c9430e8e5ae3130b0c97e90bcbad29625b1e
--- 1/git.c
--- 2/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);
                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 },
        { "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 },
        { "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 },
        { "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);