git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 2/3] rebase -i: check for updated todo after squash and reword

From
PGPhillip Wood via GitGitGadget <gitgitgadget@gmail.com>
Date
Aug 19, 2019, 09:18 UTC
Message-ID
<7839a84778d085665da70910d9a5897f3353ff45.1566206300.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.312.git.gitgitgadget@gmail.com>
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(-)
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

-- 
gitgitgadget
Previous: Phillip Wood via GitGitGadgetNext: Phillip Wood via GitGitGadget
Message 3 of 6 in “rebase -i: always update HEAD before rewording”
  1. 0/3 rebase -i: always update HEAD before rewordingPhillip Wood via GitGitGadget, Aug 19, 2019
  2. 1/3 rebase -i: always update HEAD before rewordingPhillip Wood via GitGitGadget, Aug 19, 2019
  3. 2/3 rebase -i: check for updated todo after squash and rewordPhillip Wood via GitGitGadget, Aug 19, 2019
  4. 3/3 sequencer: simplify root commit creationPhillip Wood via GitGitGadget, Aug 19, 2019
  5. Eric SunshineAug 19, 2019
  6. Phillip WoodAug 22, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.