commit: fix patch hunk editing with "commit -p -m"

Don't change git environment: move the GIT_EDITOR=":" override to the
hook command subprocess, like it's already done for GIT_INDEX_FILE.

Signed-off-by: Benoit Pierre <benoit.pierre@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 5df3837e3102e2f7d432ec11bb4685535bfaa0ac..624bfbad7c76dea54ce90e62f0717157f622e2e5 100644 (file)
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -53,10 +53,10 @@ struct checkout_opts {
 static int post_checkout_hook(struct commit *old, struct commit *new,
                              int changed)
 {
-       return run_hook(NULL, "post-checkout",
-                       sha1_to_hex(old ? old->object.sha1 : null_sha1),
-                       sha1_to_hex(new ? new->object.sha1 : null_sha1),
-                       changed ? "1" : "0", NULL);
+       return run_hook_le(NULL, "post-checkout",
+                          sha1_to_hex(old ? old->object.sha1 : null_sha1),
+                          sha1_to_hex(new ? new->object.sha1 : null_sha1),
+                          changed ? "1" : "0", NULL);
        /* "new" can be NULL when checking out from the index before
           a commit exists. */
 

diff --git a/builtin/clone.c b/builtin/clone.c
index 43e772ccdbaba3172f7ee1a969f92b300cb9d5a1..9b3c04d914b3d318f723c2d3ded403fca9503c07 100644 (file)
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -660,8 +660,8 @@ static int checkout(void)
            commit_locked_index(lock_file))
                die(_("unable to write new index file"));
 
-       err |= run_hook(NULL, "post-checkout", sha1_to_hex(null_sha1),
-                       sha1_to_hex(sha1), "1", NULL);
+       err |= run_hook_le(NULL, "post-checkout", sha1_to_hex(null_sha1),
+                          sha1_to_hex(sha1), "1", NULL);
 
        if (!err && option_recursive)
                err = run_command_v_opt(argv_submodule, RUN_GIT_CMD);

diff --git a/builtin/commit.c b/builtin/commit.c
index 3767478c6ddb02cf8a4a418f8911102aeaacfbe8..baf1fc0fff2b1d910eb470797f7605a6a904bd60 100644 (file)
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -612,7 +612,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
        /* This checks and barfs if author is badly specified */
        determine_author_info(author_ident);
 
-       if (!no_verify && run_hook(index_file, "pre-commit", NULL))
+       if (!no_verify && run_commit_hook(use_editor, index_file, "pre-commit", NULL))
                return 0;
 
        if (squash_message) {
@@ -866,8 +866,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
                return 0;
        }
 
-       if (run_hook(index_file, "prepare-commit-msg",
-                    git_path(commit_editmsg), hook_arg1, hook_arg2, NULL))
+       if (run_commit_hook(use_editor, index_file, "prepare-commit-msg",
+                           git_path(commit_editmsg), hook_arg1, hook_arg2, NULL))
                return 0;
 
        if (use_editor) {
@@ -883,7 +883,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
        }
 
        if (!no_verify &&
-           run_hook(index_file, "commit-msg", git_path(commit_editmsg), NULL)) {
+           run_commit_hook(use_editor, index_file, "commit-msg", git_path(commit_editmsg), NULL)) {
                return 0;
        }
 
@@ -1067,8 +1067,6 @@ static int parse_and_validate_options(int argc, const char *argv[],
                use_editor = 0;
        if (0 <= edit_flag)
                use_editor = edit_flag;
-       if (!use_editor)
-               setenv("GIT_EDITOR", ":", 1);
 
        /* Sanity check options */
        if (amend && !current_head)
@@ -1445,6 +1443,29 @@ static int run_rewrite_hook(const unsigned char *oldsha1,
        return finish_command(&proc);
 }
 
+int run_commit_hook(int editor_is_used, const char *index_file, const char *name, ...)
+{
+       const char *hook_env[3] =  { NULL };
+       char index[PATH_MAX];
+       va_list args;
+       int ret;
+
+       snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
+       hook_env[0] = index;
+
+       /*
+        * Let the hook know that no editor will be launched.
+        */
+       if (!editor_is_used)
+               hook_env[1] = "GIT_EDITOR=:";
+
+       va_start(args, name);
+       ret = run_hook_ve(hook_env, name, args);
+       va_end(args);
+
+       return ret;
+}
+
 int cmd_commit(int argc, const char **argv, const char *prefix)
 {
        static struct wt_status s;
@@ -1669,7 +1690,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
                     "not exceeded, and then \"git reset HEAD\" to recover."));
 
        rerere(0);
-       run_hook(get_index_file(), "post-commit", NULL);
+       run_commit_hook(use_editor, get_index_file(), "post-commit", NULL);
        if (amend && !no_post_rewrite) {
                struct notes_rewrite_cfg *cfg;
                cfg = init_copy_notes_for_rewrite("amend");

diff --git a/builtin/gc.c b/builtin/gc.c
index c19545d49e217400ee1736183c870557b9c9d54b..7fa717a3b6ab08bb5b4d5549eeb96f07460f95e6 100644 (file)
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -179,7 +179,7 @@ static int need_to_gc(void)
        else if (!too_many_loose_objects())
                return 0;
 
-       if (run_hook(NULL, "pre-auto-gc", NULL))
+       if (run_hook_le(NULL, "pre-auto-gc", NULL))
                return 0;
        return 1;
 }

diff --git a/builtin/merge.c b/builtin/merge.c
index e576a7fdc619303a65e859920a1ab6936db2626c..67f312d877c0db248d1268ea8bda29eb61fe4ca4 100644 (file)
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -421,7 +421,7 @@ static void finish(struct commit *head_commit,
        }
 
        /* Run a post-merge hook */
-       run_hook(NULL, "post-merge", squash ? "1" : "0", NULL);
+       run_hook_le(NULL, "post-merge", squash ? "1" : "0", NULL);
 
        strbuf_release(&reflog_message);
 }
@@ -821,8 +821,8 @@ static void prepare_to_commit(struct commit_list *remoteheads)
        if (0 < option_edit)
                strbuf_commented_addf(&msg, _(merge_editor_comment), comment_line_char);
        write_merge_msg(&msg);
-       if (run_hook(get_index_file(), "prepare-commit-msg",
-                    git_path("MERGE_MSG"), "merge", NULL, NULL))
+       if (run_commit_hook(1, get_index_file(), "prepare-commit-msg",
+                           git_path("MERGE_MSG"), "merge", NULL))
                abort_commit(remoteheads, NULL);
        if (0 < option_edit) {
                if (launch_editor(git_path("MERGE_MSG"), NULL, NULL))

diff --git a/commit.h b/commit.h
index 16d9c4351395ac55d6856d8d92d24ce7064df974..8d97a5caa3bd3ff5686f68998102527a3876bc4f 100644 (file)
--- a/commit.h
+++ b/commit.h
@@ -304,4 +304,7 @@ extern void check_commit_signature(const struct commit* commit, struct signature
 
 int compare_commits_by_commit_date(const void *a_, const void *b_, void *unused);
 
+LAST_ARG_MUST_BE_NULL
+extern int run_commit_hook(int editor_is_used, const char *index_file, const char *name, ...);
+
 #endif /* COMMIT_H */

diff --git a/run-command.c b/run-command.c
index 3914d9c5117f29651b56772da143f0a226494a62..75abc478c6da75471e5fdbb55c74efdfcf3ad160 100644 (file)
--- a/run-command.c
+++ b/run-command.c
@@ -760,13 +760,11 @@ char *find_hook(const char *name)
        return path;
 }
 
-int run_hook(const char *index_file, const char *name, ...)
+int run_hook_ve(const char *const *env, const char *name, va_list args)
 {
        struct child_process hook;
        struct argv_array argv = ARGV_ARRAY_INIT;
-       const char *p, *env[2];
-       char index[PATH_MAX];
-       va_list args;
+       const char *p;
        int ret;
 
        p = find_hook(name);
@@ -775,23 +773,45 @@ int run_hook(const char *index_file, const char *name, ...)
 
        argv_array_push(&argv, p);
 
-       va_start(args, name);
        while ((p = va_arg(args, const char *)))
                argv_array_push(&argv, p);
-       va_end(args);
 
        memset(&hook, 0, sizeof(hook));
        hook.argv = argv.argv;
+       hook.env = env;
        hook.no_stdin = 1;
        hook.stdout_to_stderr = 1;
-       if (index_file) {
-               snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
-               env[0] = index;
-               env[1] = NULL;
-               hook.env = env;
-       }
 
        ret = run_command(&hook);
        argv_array_clear(&argv);
        return ret;
 }
+
+int run_hook_le(const char *const *env, const char *name, ...)
+{
+       va_list args;
+       int ret;
+
+       va_start(args, name);
+       ret = run_hook_ve(env, name, args);
+       va_end(args);
+
+       return ret;
+}
+
+int run_hook_with_custom_index(const char *index_file, const char *name, ...)
+{
+       const char *hook_env[3] =  { NULL };
+       char index[PATH_MAX];
+       va_list args;
+       int ret;
+
+       snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
+       hook_env[0] = index;
+
+       va_start(args, name);
+       ret = run_hook_ve(hook_env, name, args);
+       va_end(args);
+
+       return ret;
+}

diff --git a/run-command.h b/run-command.h
index 6b985afd07a5b58ecadad8859ae90b3430ccb4c2..88460f9bfeeb5c0aa08cff9994ab2b0bfa2ad933 100644 (file)
--- a/run-command.h
+++ b/run-command.h
@@ -47,7 +47,11 @@ int run_command(struct child_process *);
 
 extern char *find_hook(const char *name);
 LAST_ARG_MUST_BE_NULL
-extern int run_hook(const char *index_file, const char *name, ...);
+extern int run_hook_le(const char *const *env, const char *name, ...);
+extern int run_hook_ve(const char *const *env, const char *name, va_list args);
+
+LAST_ARG_MUST_BE_NULL
+extern int run_hook_with_custom_index(const char *index_file, const char *name, ...);
 
 #define RUN_COMMAND_NO_STDIN 1
 #define RUN_GIT_CMD         2  /*If this is to be git sub-command */

diff --git a/t/t7514-commit-patch.sh b/t/t7514-commit-patch.sh
index 41dd37af755c690dcd20cc30fae5e6992f5a6b13..998a2103c7440f00788ec4c0ef607f8c48781660 100755 (executable)
--- a/t/t7514-commit-patch.sh
+++ b/t/t7514-commit-patch.sh
@@ -15,7 +15,7 @@ test_expect_success 'setup (initial)' '
        git commit -m commit1
 '
 
-test_expect_failure 'edit hunk "commit -p -m message"' '
+test_expect_success 'edit hunk "commit -p -m message"' '
        test_when_finished "rm -f editor_was_started" &&
        rm -f editor_was_started &&
        echo more >>file &&
@@ -23,7 +23,7 @@ test_expect_failure 'edit hunk "commit -p -m message"' '
        test -r editor_was_started
 '
 
-test_expect_failure 'edit hunk "commit --dry-run -p -m message"' '
+test_expect_success 'edit hunk "commit --dry-run -p -m message"' '
        test_when_finished "rm -f editor_was_started" &&
        rm -f editor_was_started &&
        echo more >>file &&