rebase -i: check for updated todo after squash and reword

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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Phillip Wood committed Aug 19, 2019 at 02:18 UTC a47ba3c77713d04953e0630f8e7109084329078a
2 files changed +47 -16
sequencer.c
+27 -15
@@ -1773,7 +1773,7 @@ static int do_pick_commit(struct repository *r,
1773 enum todo_command command,
1774 struct commit *commit,
1775 struct replay_opts *opts,
1776 - int final_fixup)
1776 + int final_fixup, int *check_todo)
1777 {
1778 unsigned int flags = opts->edit ? EDIT_MSG : 0;
1779 const char *msg_file = opts->edit ? NULL : git_path_merge_msg(r);
@@ -1999,11 +1999,14 @@ static int do_pick_commit(struct repository *r,
1999 res = do_commit(r, msg_file, author, opts, flags);
2000 else
2001 res = error(_("unable to parse commit author"));
2002 - if (!res && reword)
2002 + *check_todo = !!(flags & EDIT_MSG);
2003 + if (!res && reword) {
2004 fast_forward_edit:
2005 res = run_git_commit(r, NULL, opts, EDIT_MSG |
2006 VERIFY_MSG | AMEND_MSG |
2007 (flags & ALLOW_EMPTY));
2008 + *check_todo = 1;
2009 + }
2010 }
2011
2012
@@ -3711,6 +3714,7 @@ static int pick_commits(struct repository *r,
3714 while (todo_list->current < todo_list->nr) {
3715 struct todo_item *item = todo_list->items + todo_list->current;
3716 const char *arg = todo_item_get_arg(todo_list, item);
3717 + int check_todo = 0;
3718
3719 if (save_todo(todo_list, opts))
3720 return -1;
@@ -3749,7 +3753,8 @@ static int pick_commits(struct repository *r,
3753 command_to_string(item->command), NULL),
3754 1);
3755 res = do_pick_commit(r, item->command, item->commit,
3752 - opts, is_final_fixup(todo_list));
3756 + opts, is_final_fixup(todo_list),
3757 + &check_todo);
3758 if (is_rebase_i(opts) && res < 0) {
3759 /* Reschedule */
3760 advise(_(rescheduled_advice),
@@ -3806,7 +3811,6 @@ static int pick_commits(struct repository *r,
3811 } else if (item->command == TODO_EXEC) {
3812 char *end_of_arg = (char *)(arg + item->arg_len);
3813 int saved = *end_of_arg;
3809 - struct stat st;
3814
3815 if (!opts->verbose)
3816 term_clear_line();
@@ -3817,17 +3821,8 @@ static int pick_commits(struct repository *r,
3821 if (res) {
3822 if (opts->reschedule_failed_exec)
3823 reschedule = 1;
3820 - } else if (stat(get_todo_path(opts), &st))
3821 - res = error_errno(_("could not stat '%s'"),
3822 - get_todo_path(opts));
3823 - else if (match_stat_data(&todo_list->stat, &st)) {
3824 - /* Reread the todo file if it has changed. */
3825 - todo_list_release(todo_list);
3826 - if (read_populate_todo(r, todo_list, opts))
3827 - res = -1; /* message was printed */
3828 - /* `current` will be incremented below */
3829 - todo_list->current = -1;
3824 }
3825 + check_todo = 1;
3826 } else if (item->command == TODO_LABEL) {
3827 if ((res = do_label(r, arg, item->arg_len)))
3828 reschedule = 1;
@@ -3863,6 +3858,20 @@ static int pick_commits(struct repository *r,
3858 item->commit,
3859 arg, item->arg_len,
3860 opts, res, 0);
3861 + } else if (check_todo && !res) {
3862 + struct stat st;
3863 +
3864 + if (stat(get_todo_path(opts), &st)) {
3865 + res = error_errno(_("could not stat '%s'"),
3866 + get_todo_path(opts));
3867 + } else if (match_stat_data(&todo_list->stat, &st)) {
3868 + /* Reread the todo file if it has changed. */
3869 + todo_list_release(todo_list);
3870 + if (read_populate_todo(r, todo_list, opts))
3871 + res = -1; /* message was printed */
3872 + /* `current` will be incremented below */
3873 + todo_list->current = -1;
3874 + }
3875 }
3876
3877 todo_list->current++;
@@ -4189,9 +4198,12 @@ static int single_pick(struct repository *r,
4198 struct commit *cmit,
4199 struct replay_opts *opts)
4200 {
4201 + int check_todo;
4202 +
4203 setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
4204 return do_pick_commit(r, opts->action == REPLAY_PICK ?
4194 - TODO_PICK : TODO_REVERT, cmit, opts, 0);
4205 + TODO_PICK : TODO_REVERT, cmit, opts, 0,
4206 + &check_todo);
4207 }
4208
4209 int sequencer_pick_revisions(struct repository *r,
t/t3429-rebase-edit-todo.sh
+20 -1
@@ -3,9 +3,15 @@
3 test_description='rebase should reread the todo file if an exec modifies it'
4
5 . ./test-lib.sh
6 +. "$TEST_DIRECTORY"/lib-rebase.sh
7 +
8 +test_expect_success 'setup' '
9 + test_commit first file &&
10 + test_commit second file &&
11 + test_commit third file
12 +'
13
14 test_expect_success 'rebase exec modifies rebase-todo' '
8 - test_commit initial &&
15 todo=.git/rebase-merge/git-rebase-todo &&
16 git rebase HEAD -x "echo exec touch F >>$todo" &&
17 test -e F
@@ -33,4 +39,17 @@ test_expect_success SHA1 'loose object cache vs re-reading todo list' '
39 git rebase HEAD -x "./append-todo.sh 5 6"
40 '
41
42 +test_expect_success 'todo is re-read after reword and squash' '
43 + write_script reword-editor.sh <<-\EOS &&
44 + GIT_SEQUENCE_EDITOR="echo \"exec echo $(cat file) >>actual\" >>" \
45 + git rebase --edit-todo
46 + EOS
47 +
48 + test_write_lines first third >expected &&
49 + set_fake_editor &&
50 + GIT_SEQUENCE_EDITOR="$EDITOR" FAKE_LINES="reword 1 squash 2 fixup 3" \
51 + GIT_EDITOR=./reword-editor.sh git rebase -i --root third &&
52 + test_cmp expected actual
53 +'
54 +
55 test_done