Re: [PATCH 4/7] commit: fix patch hunk editing with "commit -p -m"
Torsten Bögershausen writes: >> +int run_hook_with_custom_index(const char *index_file, const char *name, >> ...) >> +{ >> +const char *hook_env[3] = { NULL }; >> +char index[PATH_MAX]; > Sorry being late with the review. > > Recently some effort has been put to replace the > "PATH_MAX/snprintf() combo" with code using strbuf. > > So I think for new developed code it could make sense to avoid > PATH_MAX from the start. Yes but because this is a compatibility wrapper for an existing function that does the string[PATH_MAX] thing already, it would be clearer to have such a conversion as a separate step. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/7] commit: fix patch hunk editing with "commit -p -m"
On 03/18/2014 11:00 AM, Benoit Pierre wrote: 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 --- builtin/checkout.c | 8 builtin/clone.c | 4 ++-- builtin/commit.c| 35 --- builtin/gc.c| 2 +- builtin/merge.c | 6 +++--- commit.h| 3 +++ run-command.c | 44 run-command.h | 6 +- t/t7514-commit-patch.sh | 4 ++-- 9 files changed, 80 insertions(+), 32 deletions(-) [] index 3914d9c..75abc47 100644 --- 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]; (Please see below) - 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]; Sorry being late with the review. Recently some effort has been put to replace the "PATH_MAX/snprintf() combo" with code using strbuf. So I think for new developed code it could make sense to avoid PATH_MAX from the start. To my knowledge mkpathdup() from path.c can be used, (or are there better ways ?) and the whole function will look like below (not tested) + 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; +} int run_hook_with_custom_index(const char *index_file, const char *name, ...) { const char *hook_env[3] = { NULL }; char index = mkpathdup("GIT_INDEX_FILE=%s", index_file); va_list args; int ret; hook_env[0] = index; va_start(args, name); ret = run_hook_ve(hook_env, name, args); va_end(args); free(index); return ret; } diff --git a/run-command.h b/run-command.h index 6b985af..88460f9 100644 --- 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_CMD2 /*If this is to be git sub-command */ diff --git a/t/t7514-commit-patch.sh b/t/t7514-commit-patch.sh index 41dd37a..998a210 100755 --- 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 && -- To unsubscribe from this list: send the line "unsubscribe
[PATCH 4/7] 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 --- builtin/checkout.c | 8 builtin/clone.c | 4 ++-- builtin/commit.c| 35 --- builtin/gc.c| 2 +- builtin/merge.c | 6 +++--- commit.h| 3 +++ run-command.c | 44 run-command.h | 6 +- t/t7514-commit-patch.sh | 4 ++-- 9 files changed, 80 insertions(+), 32 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index ada51fa..1b86d9c 100644 --- 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 43e772c..9b3c04d 100644 --- 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 3783bca..68a90b3 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -610,7 +610,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) { @@ -867,8 +867,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) { @@ -884,7 +884,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; } @@ -1068,8 +1068,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) @@ -1450,6 +1448,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; @@ -1674,7 +1695,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-co
[PATCH 4/7] 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 --- builtin/checkout.c | 8 builtin/clone.c | 4 ++-- builtin/commit.c| 35 --- builtin/gc.c| 2 +- builtin/merge.c | 6 +++--- commit.h| 3 +++ run-command.c | 44 run-command.h | 6 +- t/t7513-commit-patch.sh | 4 ++-- 9 files changed, 80 insertions(+), 32 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index ada51fa..1b86d9c 100644 --- 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 43e772c..9b3c04d 100644 --- 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 3783bca..68a90b3 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -610,7 +610,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) { @@ -867,8 +867,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) { @@ -884,7 +884,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; } @@ -1068,8 +1068,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) @@ -1450,6 +1448,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; @@ -1674,7 +1695,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-co
Re: [PATCH 4/7] commit: fix patch hunk editing with "commit -p -m"
Benoit Pierre writes: > diff --git a/builtin/checkout.c b/builtin/checkout.c > index 5df3837..da423b2 100644 > --- 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); Funny indentation. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/7] commit: fix patch hunk editing with "commit -p -m"
On Tue, Mar 11, 2014 at 06:56:02PM +0100, Benoit Pierre wrote: > According to the original commit, the change to GIT_EDITOR is only > here for hooks: > > commit 406400ce4f69e79b544dd3539a71b85d99331820 > Author: Paolo Bonzini > Date: Tue Feb 5 11:01:45 2008 +0100 > > git-commit: set GIT_EDITOR=: if editor will not be launched > > This is a preparatory patch that provides a simple way for the future > prepare-commit-msg hook to discover if the editor will be launched. > > Signed-off-by: Junio C Hamano > > So there is really no reason to set it earlier, and not just in the > hook subprocess environment. Ah, you're right. I was thinking that our invocation of launch_editor also respected it. It does, but we never get there at all because use_editor is set to 0. So yeah, it really is just needed for the hook. Your patch, even though it is a bigger change, keeps the setting to the minimal area, which is cleaner and more maintainable in the long run. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/7] commit: fix patch hunk editing with "commit -p -m"
On Mon, Mar 10, 2014 at 9:07 PM, Jeff King wrote: > On Mon, Mar 10, 2014 at 07:49:34PM +0100, Benoit Pierre wrote: > >> Don't change git environment: move the GIT_EDITOR=":" override to the >> hook command subprocess, like it's already done for GIT_INDEX_FILE. >> [...] > > This is a lot of change, and in some ways I think it is making things > better overall. However, the simplest fix for this is basically to move > the setting of GIT_EDITOR down to after we prepare the index. > > Jun Hao (cc'd) has been preparing a series for this based on the > Bloomberg git hackday a few weeks ago, but it hasn't been sent to the > list yet. > > Commits are here: > > https://github.com/bloomberg/git/compare/commit-patch-allow-hunk-editing > > if you care to look. I'm not sure which solution is technically > superior, but it's worth considering both. > > I regret not encouraging Jun to post to the list sooner, as we might > have avoided some duplicated effort. But that's a sunk cost, and we > should pick up whichever is the best for the project. Replying here instead of to Jun Hao message (I'm not subscribed to the mailing list, so I did not received it): Jun Hao wrote: > I like the idea that the environment setting should be done in one > place. Just not sure run_hook is the right place tho. If user doesn't have > any hook setup, will this kick in? > One more question, will this work for dry run? Or dry run doesn't matter > in this case? According to the original commit, the change to GIT_EDITOR is only here for hooks: commit 406400ce4f69e79b544dd3539a71b85d99331820 Author: Paolo Bonzini Date: Tue Feb 5 11:01:45 2008 +0100 git-commit: set GIT_EDITOR=: if editor will not be launched This is a preparatory patch that provides a simple way for the future prepare-commit-msg hook to discover if the editor will be launched. Signed-off-by: Junio C Hamano So there is really no reason to set it earlier, and not just in the hook subprocess environment. Regarding dry run: the bug is present, and my patch fix it too (but is missing a test for this). As to which patch is better: it's really not for me to decide! It's a question for the maintainer(s), Jun Hao patch is sure much smaller and simpler, mine is more involved and I believe cleaner in the long term: there is no risk of another part of the commit process to be impacted by the change of environment. Also note that my patch changes the merge builtin too: GIT_EDITOR will not be overriden if an editor will be launched (when used with --edit). -- A: Because it destroys the flow of conversation. Q: Why is top posting dumb? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/7] commit: fix patch hunk editing with "commit -p -m"
Jeff King writes: > On Mon, Mar 10, 2014 at 07:49:34PM +0100, Benoit Pierre wrote: > >> 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 >> --- >> builtin/checkout.c| 8 +++ >> builtin/clone.c | 4 ++-- >> builtin/commit.c | 35 --- >> builtin/gc.c | 2 +- >> builtin/merge.c | 6 +++--- >> commit.h | 3 +++ >> run-command.c | 44 >> --- >> run-command.h | 6 +- >> t/t7513-commit_-p_-m_hunk_edit.sh | 2 +- >> 9 files changed, 79 insertions(+), 31 deletions(-) > > This is a lot of change, and in some ways I think it is making things > better overall. However, the simplest fix for this is basically to move > the setting of GIT_EDITOR down to after we prepare the index. > > Jun Hao (cc'd) has been preparing a series for this based on the > Bloomberg git hackday a few weeks ago, but it hasn't been sent to the > list yet. > > Commits are here: > > https://github.com/bloomberg/git/compare/commit-patch-allow-hunk-editing > > if you care to look. I'm not sure which solution is technically > superior, but it's worth considering both. > > I regret not encouraging Jun to post to the list sooner, as we might > have avoided some duplicated effort. But that's a sunk cost, and we > should pick up whichever is the best for the project. > > -Peff I like the idea that the environment setting should be done in one place. Just not sure run_hook is the right place tho. If user doesn't have any hook setup, will this kick in? One more question, will this work for dry run? Or dry run doesn't matter in this case? Thanks - Jun -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/7] commit: fix patch hunk editing with "commit -p -m"
On Mon, Mar 10, 2014 at 07:49:34PM +0100, Benoit Pierre wrote: > 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 > --- > builtin/checkout.c| 8 +++ > builtin/clone.c | 4 ++-- > builtin/commit.c | 35 --- > builtin/gc.c | 2 +- > builtin/merge.c | 6 +++--- > commit.h | 3 +++ > run-command.c | 44 > --- > run-command.h | 6 +- > t/t7513-commit_-p_-m_hunk_edit.sh | 2 +- > 9 files changed, 79 insertions(+), 31 deletions(-) This is a lot of change, and in some ways I think it is making things better overall. However, the simplest fix for this is basically to move the setting of GIT_EDITOR down to after we prepare the index. Jun Hao (cc'd) has been preparing a series for this based on the Bloomberg git hackday a few weeks ago, but it hasn't been sent to the list yet. Commits are here: https://github.com/bloomberg/git/compare/commit-patch-allow-hunk-editing if you care to look. I'm not sure which solution is technically superior, but it's worth considering both. I regret not encouraging Jun to post to the list sooner, as we might have avoided some duplicated effort. But that's a sunk cost, and we should pick up whichever is the best for the project. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/7] 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 --- builtin/checkout.c| 8 +++ builtin/clone.c | 4 ++-- builtin/commit.c | 35 --- builtin/gc.c | 2 +- builtin/merge.c | 6 +++--- commit.h | 3 +++ run-command.c | 44 --- run-command.h | 6 +- t/t7513-commit_-p_-m_hunk_edit.sh | 2 +- 9 files changed, 79 insertions(+), 31 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 5df3837..da423b2 100644 --- 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 43e772c..9b3c04d 100644 --- 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 3767478..baf1fc0 100644 --- 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);