From: Phillip Wood <phillip.wood@dunelm.org.uk>
While a rebase is stopped for the user to edit a commit message it can be convenient for them to also edit the todo list. The scripted version of rebase supported this but the C version does not. We already check to see if the todo list has been updated by an exec command so extend this to rewords and squashes. It only costs a single stat call to do this so it should not affect the speed of the rebase (especially as it has just stopped for the user to edit a message)
Note that for squashes the editor may be opened on a different pick to the squash itself as we edit the message at the end of a chain fixups and squashes.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
sequencer.c | 42 ++++++++++++++++++++++++-------------
t/t3429-rebase-edit-todo.sh | 21 ++++++++++++++++++-
2 files changed, 47 insertions(+), 16 deletions(-)
Show changes to 2 files +47 −16
sequencer.c, t/t3429-rebase-edit-todo.sh
diff --git a/sequencer.c b/sequencer.c
index 085777f4a1..a13e1a7466 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1773,7 +1773,7 @@ static int do_pick_commit(struct repository *r,
enum todo_command command,
struct commit *commit,
struct replay_opts *opts,
- int final_fixup)
+ int final_fixup, int *check_todo)
{
unsigned int flags = opts->edit ? EDIT_MSG : 0;
const char *msg_file = opts->edit ? NULL : git_path_merge_msg(r);
@@ -1999,11 +1999,14 @@ static int do_pick_commit(struct repository *r,
res = do_commit(r, msg_file, author, opts, flags);
else
res = error(_("unable to parse commit author"));
- if (!res && reword)
+ *check_todo = !!(flags & EDIT_MSG);
+ if (!res && reword) {
fast_forward_edit:
res = run_git_commit(r, NULL, opts, EDIT_MSG |
VERIFY_MSG | AMEND_MSG |
(flags & ALLOW_EMPTY));
+ *check_todo = 1;
+ }
}
@@ -3821,6 +3824,7 @@ static int pick_commits(struct repository *r,
while (todo_list->current < todo_list->nr) {
struct todo_item *item = todo_list->items + todo_list->current;
const char *arg = todo_item_get_arg(todo_list, item);
+ int check_todo = 0;
if (save_todo(todo_list, opts))
return -1;
@@ -3859,7 +3863,8 @@ static int pick_commits(struct repository *r,
command_to_string(item->command), NULL),
1);
res = do_pick_commit(r, item->command, item->commit,
- opts, is_final_fixup(todo_list));
+ opts, is_final_fixup(todo_list),
+ &check_todo);
if (is_rebase_i(opts) && res < 0) {
/* Reschedule */
advise(_(rescheduled_advice),
@@ -3916,7 +3921,6 @@ static int pick_commits(struct repository *r,
} else if (item->command == TODO_EXEC) {
char *end_of_arg = (char *)(arg + item->arg_len);
int saved = *end_of_arg;
- struct stat st;
if (!opts->verbose)
term_clear_line();
@@ -3927,17 +3931,8 @@ static int pick_commits(struct repository *r,
if (res) {
if (opts->reschedule_failed_exec)
reschedule = 1;
- } else if (stat(get_todo_path(opts), &st))
- res = error_errno(_("could not stat '%s'"),
- get_todo_path(opts));
- else if (match_stat_data(&todo_list->stat, &st)) {
- /* Reread the todo file if it has changed. */
- todo_list_release(todo_list);
- if (read_populate_todo(r, todo_list, opts))
- res = -1; /* message was printed */
- /* `current` will be incremented below */
- todo_list->current = -1;
}
+ check_todo = 1;
} else if (item->command == TODO_LABEL) {
if ((res = do_label(r, arg, item->arg_len)))
reschedule = 1;
@@ -3973,6 +3968,20 @@ static int pick_commits(struct repository *r,
item->commit,
arg, item->arg_len,
opts, res, 0);
+ } else if (check_todo && !res) {
+ struct stat st;
+
+ if (stat(get_todo_path(opts), &st)) {
+ res = error_errno(_("could not stat '%s'"),
+ get_todo_path(opts));
+ } else if (match_stat_data(&todo_list->stat, &st)) {
+ /* Reread the todo file if it has changed. */
+ todo_list_release(todo_list);
+ if (read_populate_todo(r, todo_list, opts))
+ res = -1; /* message was printed */
+ /* `current` will be incremented below */
+ todo_list->current = -1;
+ }
}
todo_list->current++;
@@ -4299,9 +4308,12 @@ static int single_pick(struct repository *r,
struct commit *cmit,
struct replay_opts *opts)
{
+ int check_todo;
+
setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
return do_pick_commit(r, opts->action == REPLAY_PICK ?
- TODO_PICK : TODO_REVERT, cmit, opts, 0);
+ TODO_PICK : TODO_REVERT, cmit, opts, 0,
+ &check_todo);
}
int sequencer_pick_revisions(struct repository *r,
diff --git a/t/t3429-rebase-edit-todo.sh b/t/t3429-rebase-edit-todo.sh
index 76f6d306ea..8739cb60a7 100755
--- a/t/t3429-rebase-edit-todo.sh
+++ b/t/t3429-rebase-edit-todo.sh
@@ -3,9 +3,15 @@
test_description='rebase should reread the todo file if an exec modifies it'
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
+test_expect_success 'setup' '
+ test_commit first file &&
+ test_commit second file &&
+ test_commit third file
+'
test_expect_success 'rebase exec modifies rebase-todo' '
- test_commit initial &&
todo=.git/rebase-merge/git-rebase-todo &&
git rebase HEAD -x "echo exec touch F >>$todo" &&
test -e F
@@ -33,4 +39,17 @@ test_expect_success SHA1 'loose object cache vs re-reading todo list' '
git rebase HEAD -x "./append-todo.sh 5 6"
'
+test_expect_success 'todo is re-read after reword and squash' '
+ write_script reword-editor.sh <<-\EOS &&
+ GIT_SEQUENCE_EDITOR="echo \"exec echo $(cat file) >>actual\" >>" \
+ git rebase --edit-todo
+ EOS
+
+ test_write_lines first third >expected &&
+ set_fake_editor &&
+ GIT_SEQUENCE_EDITOR="$EDITOR" FAKE_LINES="reword 1 squash 2 fixup 3" \
+ GIT_EDITOR=./reword-editor.sh git rebase -i --root third &&
+ test_cmp expected actual
+'
+
test_done
