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

d95138e6 (setup: set env $GIT_WORK_TREE when work tree is set, like
$GIT_DIR, 2015-06-26) attempted to work around a glitch in alias
handling by overwriting GIT_WORK_TREE environment variable to
affect subprocesses when set_git_work_tree() gets called, which
resulted in a rather unpleasant regression to "clone" and "init".
Try to address the same issue by always restoring the environment
and respawning the real underlying command when handling alias.

* nd/clear-gitenv-upon-use-of-alias:
run-command: don't warn on SIGPIPE deaths
git.c: make sure we do not leak GIT_* to alias scripts
setup.c: re-fix d95138e (setup: set env $GIT_WORK_TREE when ..
git.c: make it clear save_env() is for alias handling only

diff --git a/git.c b/git.c
index 6ed824cacfccddcb01104835b45ab934ff3f443e..da278c3d41ed308581ffec46e3487e7670898d41 100644 (file)
--- a/git.c
+++ b/git.c
@@ -25,14 +25,14 @@ static const char *env_names[] = {
        GIT_PREFIX_ENVIRONMENT
 };
 static char *orig_env[4];
-static int saved_environment;
+static int saved_env_before_alias;
 
-static void save_env(void)
+static void save_env_before_alias(void)
 {
        int i;
-       if (saved_environment)
+       if (saved_env_before_alias)
                return;
-       saved_environment = 1;
+       saved_env_before_alias = 1;
        orig_cwd = xgetcwd();
        for (i = 0; i < ARRAY_SIZE(env_names); i++) {
                orig_env[i] = getenv(env_names[i]);
@@ -41,13 +41,16 @@ static void save_env(void)
        }
 }
 
-static void restore_env(void)
+static void restore_env(int external_alias)
 {
        int i;
-       if (orig_cwd && chdir(orig_cwd))
+       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])
                        setenv(env_names[i], orig_env[i], 1);
                else
@@ -226,14 +229,14 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 static int handle_alias(int *argcp, const char ***argv)
 {
        int envchanged = 0, ret = 0, saved_errno = errno;
-       const char *subdir;
        int count, option_count;
        const char **new_argv;
        const char *alias_command;
        char *alias_string;
        int unused_nongit;
 
-       subdir = setup_git_directory_gently(&unused_nongit);
+       save_env_before_alias();
+       setup_git_directory_gently(&unused_nongit);
 
        alias_command = (*argv)[0];
        alias_string = alias_lookup(alias_command);
@@ -243,6 +246,7 @@ static int handle_alias(int *argcp, const char ***argv)
                        int argc = *argcp, i;
 
                        commit_pager_choice();
+                       restore_env(1);
 
                        /* build alias_argv */
                        alias_argv = xmalloc(sizeof(*alias_argv) * (argc + 1));
@@ -291,8 +295,7 @@ static int handle_alias(int *argcp, const char ***argv)
                ret = 1;
        }
 
-       if (subdir && chdir(subdir))
-               die_errno("Cannot change to '%s'", subdir);
+       restore_env(0);
 
        errno = saved_errno;
 
@@ -307,7 +310,6 @@ static int handle_alias(int *argcp, const char ***argv)
  * RUN_SETUP for reading from the configuration file.
  */
 #define NEED_WORK_TREE         (1<<3)
-#define NO_SETUP               (1<<4)
 
 struct cmd_struct {
        const char *cmd;
@@ -389,7 +391,7 @@ static struct cmd_struct commands[] = {
        { "cherry", cmd_cherry, RUN_SETUP },
        { "cherry-pick", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },
        { "clean", cmd_clean, RUN_SETUP | NEED_WORK_TREE },
-       { "clone", cmd_clone, NO_SETUP },
+       { "clone", cmd_clone },
        { "column", cmd_column, RUN_SETUP_GENTLY },
        { "commit", cmd_commit, RUN_SETUP | NEED_WORK_TREE },
        { "commit-tree", cmd_commit_tree, RUN_SETUP },
@@ -415,8 +417,8 @@ static struct cmd_struct commands[] = {
        { "hash-object", cmd_hash_object },
        { "help", cmd_help },
        { "index-pack", cmd_index_pack, RUN_SETUP_GENTLY },
-       { "init", cmd_init_db, NO_SETUP },
-       { "init-db", cmd_init_db, NO_SETUP },
+       { "init", cmd_init_db },
+       { "init-db", cmd_init_db },
        { "interpret-trailers", cmd_interpret_trailers, RUN_SETUP_GENTLY },
        { "log", cmd_log, RUN_SETUP },
        { "ls-files", cmd_ls_files, RUN_SETUP },
@@ -530,9 +532,13 @@ static void handle_builtin(int argc, const char **argv)
 
        builtin = get_builtin(cmd);
        if (builtin) {
-               if (saved_environment && (builtin->option & NO_SETUP))
-                       restore_env();
-               else
+               /*
+                * 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));
        }
 }
@@ -590,7 +596,6 @@ static int run_argv(int *argcp, const char ***argv)
                 */
                if (done_alias)
                        break;
-               save_env();
                if (!handle_alias(argcp, argv))
                        break;
                done_alias = 1;

diff --git a/run-command.c b/run-command.c
index 51fd72c4273c3ee59246285e40c9a5f9bd0e2f98..cdf01845790849f863aef4195a68e12fa19ca543 100644 (file)
--- a/run-command.c
+++ b/run-command.c
@@ -247,7 +247,7 @@ static int wait_or_whine(pid_t pid, const char *argv0, int in_signal)
                error("waitpid is confused (%s)", argv0);
        } else if (WIFSIGNALED(status)) {
                code = WTERMSIG(status);
-               if (code != SIGINT && code != SIGQUIT)
+               if (code != SIGINT && code != SIGQUIT && code != SIGPIPE)
                        error("%s died of signal %d", argv0, code);
                /*
                 * This return value is chosen so that code & 0xff

diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index f91bbcfc853a47e0a314160b79fc4816178f1723..295aa5949a13baeb8cafff0a6782f6e49e68a075 100755 (executable)
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -87,6 +87,23 @@ test_expect_success 'plain nested in bare through aliased command' '
        check_config bare-ancestor-aliased.git/plain-nested/.git false unset
 '
 
+test_expect_success 'No extra GIT_* on alias scripts' '
+       (
+               env | sed -ne "/^GIT_/s/=.*//p" &&
+               echo GIT_PREFIX &&        # setup.c
+               echo GIT_TEXTDOMAINDIR    # wrapper-for-bin.sh
+       ) | sort | uniq >expected &&
+       cat <<-\EOF >script &&
+       #!/bin/sh
+       env | sed -ne "/^GIT_/s/=.*//p" | sort >actual
+       exit 0
+       EOF
+       chmod 755 script &&
+       git config alias.script \!./script &&
+       ( mkdir sub && cd sub && git script ) &&
+       test_cmp expected actual
+'
+
 test_expect_success 'plain with GIT_WORK_TREE' '
        mkdir plain-wt &&
        test_must_fail env GIT_WORK_TREE="$(pwd)/plain-wt" git init plain-wt

diff --git a/t/t0002-gitfile.sh b/t/t0002-gitfile.sh
index 3afe0125c99f037b9f144806c00435dcba3f93ea..9670e8cbe6cb9a9faa3519b0f11dc16713496188 100755 (executable)
--- a/t/t0002-gitfile.sh
+++ b/t/t0002-gitfile.sh
@@ -99,7 +99,7 @@ test_expect_success 'check rev-list' '
        test "$SHA" = "$(git rev-list HEAD)"
 '
 
-test_expect_failure 'setup_git_dir twice in subdir' '
+test_expect_success 'setup_git_dir twice in subdir' '
        git init sgd &&
        (
                cd sgd &&

diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 9b34f3c615df5080085b5f8cf956d2bac099d9d4..31b46582d79889c4d85879e7dedfc2c618b274d1 100755 (executable)
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -65,6 +65,29 @@ test_expect_success 'clone respects GIT_WORK_TREE' '
 
 '
 
+test_expect_success 'clone from hooks' '
+
+       test_create_repo r0 &&
+       cd r0 &&
+       test_commit initial &&
+       cd .. &&
+       git init r1 &&
+       cd r1 &&
+       cat >.git/hooks/pre-commit <<-\EOF &&
+       #!/bin/sh
+       git clone ../r0 ../r2
+       exit 1
+       EOF
+       chmod u+x .git/hooks/pre-commit &&
+       : >file &&
+       git add file &&
+       test_must_fail git commit -m invoke-hook &&
+       cd .. &&
+       test_cmp r0/.git/HEAD r2/.git/HEAD &&
+       test_cmp r0/initial.t r2/initial.t
+
+'
+
 test_expect_success 'clone creates intermediate directories' '
 
        git clone src long/path/to/dst &&