From cb7fb9ed4277f3e1e9203777d4de0805cf29f873 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 22 Mar 2017 16:01:19 +0100 Subject: [PATCH 1/3] t7504: document regression: reword no longer calls commit-msg The `reword` command of an interactive rebase used to call the commit-msg hooks, but that regressed when we switched to the rebase--helper backed by the sequencer. Noticed by Sebastian Schuberth. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/t7504-commit-msg-hook.sh | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/t/t7504-commit-msg-hook.sh b/t/t7504-commit-msg-hook.sh index 8728db61d3..c3d9ab02a3 100755 --- a/t/t7504-commit-msg-hook.sh +++ b/t/t7504-commit-msg-hook.sh @@ -220,4 +220,21 @@ test_expect_success "hook doesn't edit commit message (editor)" ' ' +# set up fake editor to replace `pick` by `reword` +cat > reword-editor <<'EOF' +#!/bin/sh +mv "$1" "$1".bup && +sed 's/^pick/reword/' <"$1".bup >"$1" +EOF +chmod +x reword-editor +REWORD_EDITOR="$(pwd)/reword-editor" +export REWORD_EDITOR + +test_expect_failure 'hook is called for reword during `rebase -i`' ' + + GIT_SEQUENCE_EDITOR="\"$REWORD_EDITOR\"" git rebase -i HEAD^ && + commit_msg_is "new message" + +' + test_done From 789b3effec89e6f336726de109eb8be9dc60b7a5 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 23 Mar 2017 17:07:11 +0100 Subject: [PATCH 2/3] sequencer: make commit options more extensible So far every time we need to tweak the behaviour of run_git_commit() we have been adding a "int" parameter to it. As the function gains parameters and different callsites having different needs, this is becoming a maintenance burden. When a new knob needs to be added to address a specific need for a single callsite, all the other callsites need to add a "no, I do not want anything special with respect to the new knob" argument. Consolidate the existing four parameters into a flag word to make it more maintainable, as we will be adding a new one to the mix soon. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- sequencer.c | 48 ++++++++++++++++++++++++++---------------------- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/sequencer.c b/sequencer.c index 8183a83c1f..677e6ef764 100644 --- a/sequencer.c +++ b/sequencer.c @@ -602,6 +602,11 @@ N_("you have staged changes in your working tree\n" "\n" " git rebase --continue\n"); +#define ALLOW_EMPTY (1<<0) +#define EDIT_MSG (1<<1) +#define AMEND_MSG (1<<2) +#define CLEANUP_MSG (1<<3) + /* * If we are cherry-pick, and if the merge did not result in * hand-editing, we will hit this commit and inherit the original @@ -615,8 +620,7 @@ N_("you have staged changes in your working tree\n" * author metadata. */ static int run_git_commit(const char *defmsg, struct replay_opts *opts, - int allow_empty, int edit, int amend, - int cleanup_commit_message) + unsigned int flags) { struct child_process cmd = CHILD_PROCESS_INIT; const char *value; @@ -624,7 +628,7 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts, cmd.git_cmd = 1; if (is_rebase_i(opts)) { - if (!edit) { + if (!(flags & EDIT_MSG)) { cmd.stdout_to_stderr = 1; cmd.err = -1; } @@ -640,7 +644,7 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts, argv_array_push(&cmd.args, "commit"); argv_array_push(&cmd.args, "-n"); - if (amend) + if ((flags & AMEND_MSG)) argv_array_push(&cmd.args, "--amend"); if (opts->gpg_sign) argv_array_pushf(&cmd.args, "-S%s", opts->gpg_sign); @@ -648,16 +652,16 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts, argv_array_push(&cmd.args, "-s"); if (defmsg) argv_array_pushl(&cmd.args, "-F", defmsg, NULL); - if (cleanup_commit_message) + if ((flags & CLEANUP_MSG)) argv_array_push(&cmd.args, "--cleanup=strip"); - if (edit) + if ((flags & EDIT_MSG)) argv_array_push(&cmd.args, "-e"); - else if (!cleanup_commit_message && + else if (!(flags & CLEANUP_MSG) && !opts->signoff && !opts->record_origin && git_config_get_value("commit.cleanup", &value)) argv_array_push(&cmd.args, "--cleanup=verbatim"); - if (allow_empty) + if ((flags & ALLOW_EMPTY)) argv_array_push(&cmd.args, "--allow-empty"); if (opts->allow_empty_message) @@ -926,14 +930,14 @@ static void record_in_rewritten(struct object_id *oid, static int do_pick_commit(enum todo_command command, struct commit *commit, struct replay_opts *opts, int final_fixup) { - int edit = opts->edit, cleanup_commit_message = 0; - const char *msg_file = edit ? NULL : git_path_merge_msg(); + unsigned int flags = opts->edit ? EDIT_MSG : 0; + const char *msg_file = opts->edit ? NULL : git_path_merge_msg(); unsigned char head[20]; struct commit *base, *next, *parent; const char *base_label, *next_label; struct commit_message msg = { NULL, NULL, NULL, NULL }; struct strbuf msgbuf = STRBUF_INIT; - int res, unborn = 0, amend = 0, allow = 0; + int res, unborn = 0, allow; if (opts->no_commit) { /* @@ -991,7 +995,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit, opts); if (res || command != TODO_REWORD) goto leave; - edit = amend = 1; + flags |= EDIT_MSG | AMEND_MSG; msg_file = NULL; goto fast_forward_edit; } @@ -1046,15 +1050,15 @@ static int do_pick_commit(enum todo_command command, struct commit *commit, } if (command == TODO_REWORD) - edit = 1; + flags |= EDIT_MSG; else if (is_fixup(command)) { if (update_squash_messages(command, commit, opts)) return -1; - amend = 1; + flags |= AMEND_MSG; if (!final_fixup) msg_file = rebase_path_squash_msg(); else if (file_exists(rebase_path_fixup_msg())) { - cleanup_commit_message = 1; + flags |= CLEANUP_MSG; msg_file = rebase_path_fixup_msg(); } else { const char *dest = git_path("SQUASH_MSG"); @@ -1064,7 +1068,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit, rebase_path_squash_msg(), dest); unlink(git_path("MERGE_MSG")); msg_file = dest; - edit = 1; + flags |= EDIT_MSG; } } @@ -1123,11 +1127,11 @@ static int do_pick_commit(enum todo_command command, struct commit *commit, if (allow < 0) { res = allow; goto leave; - } + } else if (allow) + flags |= ALLOW_EMPTY; if (!opts->no_commit) fast_forward_edit: - res = run_git_commit(msg_file, opts, allow, edit, amend, - cleanup_commit_message); + res = run_git_commit(msg_file, opts, flags); if (!res && final_fixup) { unlink(rebase_path_fixup_msg()); @@ -2154,7 +2158,7 @@ static int continue_single_pick(void) static int commit_staged_changes(struct replay_opts *opts) { - int amend = 0; + unsigned int flags = ALLOW_EMPTY | EDIT_MSG; if (has_unstaged_changes(1)) return error(_("cannot rebase: You have unstaged changes.")); @@ -2184,10 +2188,10 @@ static int commit_staged_changes(struct replay_opts *opts) "--continue' again.")); strbuf_release(&rev); - amend = 1; + flags |= AMEND_MSG; } - if (run_git_commit(rebase_path_message(), opts, 1, 1, amend, 0)) + if (run_git_commit(rebase_path_message(), opts, flags)) return error(_("could not commit staged changes.")); unlink(rebase_path_amend()); return 0; From b92ff6e81429c6a2636b4d6d26ab9ce5d4890fe8 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 23 Mar 2017 17:07:17 +0100 Subject: [PATCH 3/3] sequencer: allow the commit-msg hooks to run during a `reword` The `reword` command used to call `git commit` in a manner that asks for the prepare-commit-msg and commit-msg hooks to do their thing. Converting that part of the interactive rebase to C code introduced the regression where those hooks were no longer run. Let's fix this. Note: the flag is called `VERIFY_MSG` instead of the more intuitive `RUN_COMMIT_MSG_HOOKS` to indicate that the flag suppresses the `--no-verify` flag (which may do other things in the future in addition to suppressing the commit message hooks, too). Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- sequencer.c | 8 ++++++-- t/t7504-commit-msg-hook.sh | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/sequencer.c b/sequencer.c index 677e6ef764..d53ca44e5e 100644 --- a/sequencer.c +++ b/sequencer.c @@ -606,6 +606,7 @@ N_("you have staged changes in your working tree\n" #define EDIT_MSG (1<<1) #define AMEND_MSG (1<<2) #define CLEANUP_MSG (1<<3) +#define VERIFY_MSG (1<<4) /* * If we are cherry-pick, and if the merge did not result in @@ -642,8 +643,9 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts, } argv_array_push(&cmd.args, "commit"); - argv_array_push(&cmd.args, "-n"); + if (!(flags & VERIFY_MSG)) + argv_array_push(&cmd.args, "-n"); if ((flags & AMEND_MSG)) argv_array_push(&cmd.args, "--amend"); if (opts->gpg_sign) @@ -996,6 +998,8 @@ static int do_pick_commit(enum todo_command command, struct commit *commit, if (res || command != TODO_REWORD) goto leave; flags |= EDIT_MSG | AMEND_MSG; + if (command == TODO_REWORD) + flags |= VERIFY_MSG; msg_file = NULL; goto fast_forward_edit; } @@ -1050,7 +1054,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit, } if (command == TODO_REWORD) - flags |= EDIT_MSG; + flags |= EDIT_MSG | VERIFY_MSG; else if (is_fixup(command)) { if (update_squash_messages(command, commit, opts)) return -1; diff --git a/t/t7504-commit-msg-hook.sh b/t/t7504-commit-msg-hook.sh index c3d9ab02a3..88d4cda299 100755 --- a/t/t7504-commit-msg-hook.sh +++ b/t/t7504-commit-msg-hook.sh @@ -230,7 +230,7 @@ chmod +x reword-editor REWORD_EDITOR="$(pwd)/reword-editor" export REWORD_EDITOR -test_expect_failure 'hook is called for reword during `rebase -i`' ' +test_expect_success 'hook is called for reword during `rebase -i`' ' GIT_SEQUENCE_EDITOR="\"$REWORD_EDITOR\"" git rebase -i HEAD^ && commit_msg_is "new message"