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

[RFC PATCH 7/9] rebase-interactive: todo_list_check() also uses the done list

From
Alban Gruin <alban.gruin@gmail.com>
Date
Jul 17, 2019, 14:39 UTC
Message-ID
<20190717143918.7406-8-alban.gruin@gmail.com>
In-Reply-To
<20190717143918.7406-1-alban.gruin@gmail.com>

todo_list_check() works by checking if every commit in old_todo (the backup list) is also present in new_todo (the todo list to check).

This works only when no commits have been picked (ie. right after the initial edit). In other cases, the backup list will contain one or several commits that have been applied and are no longer in the todo list. Let's take this todo list, for instance:

	pick 123abc An interesting change
	break
	pick cba321 A boring change

After executing the `break' command, the backup list will contain the original todo list, while the todo list will only contain the last `pick' insn. Even if commit 123abc has been picked up, in this case, todo_list_check() will fail because it is not part of the todo list.

There is another list containing every command that have been executed: the "done" list. In our example, after the `break' command, the "done" list will contain the first commit (123abc). Comparing the backup list against the todo list and the done list will fix this problem.

This changes todo_list_check() to compare old_todo against new_todo _and_ the done list. This should be useful to check the todo list when resuming a rebase or reloading the todo list after an `exec' command, but not for `--edit-todo' (the todo list is copied before edition). But, for the sake of safety, it is still done in the next commit.

This also adds a function, todo_list_check_against_backup(), to load the done list and the backup list, and call todo_list_check().

Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
---
 rebase-interactive.c | 57 ++++++++++++++++++++++++++++++++++++++------
 rebase-interactive.h |  6 ++++-
 sequencer.c          |  4 ++--
 3 files changed, 57 insertions(+), 10 deletions(-)
diff --git a/rebase-interactive.c b/rebase-interactive.c
index aa18ae82b7..c7dea85553 100644
--- a/rebase-interactive.c
+++ b/rebase-interactive.c
@@ -6,6 +6,8 @@
 #include "commit-slab.h"
 #include "config.h"
 
+static GIT_PATH_FUNC(rebase_path_done, "rebase-merge/done")
+
 enum missing_commit_check_level {
 	MISSING_COMMIT_CHECK_IGNORE = 0,
 	MISSING_COMMIT_CHECK_WARN,
@@ -124,13 +126,27 @@ int edit_todo_list(struct repository *r, struct todo_list *todo_list,
 }
 
 define_commit_slab(commit_seen, unsigned char);
+
+static void mark_commits_as_seen(struct commit_seen *commit_seen,
+				 struct todo_list *list)
+{
+	int i;
+
+	for (i = 0; i < list->nr; i++) {
+		struct commit *commit = list->items[i].commit;
+		if (commit)
+			*commit_seen_at(commit_seen, commit) = 1;
+	}
+}
+
 /*
  * Check if the user dropped some commits by mistake
  * Behaviour determined by rebase.missingCommitsCheck.
  * Check if there is an unrecognized command or a
  * bad SHA-1 in a command.
  */
-int todo_list_check(struct todo_list *old_todo, struct todo_list *new_todo)
+int todo_list_check(struct todo_list *old_todo, struct todo_list *new_todo,
+		    struct todo_list *done)
 {
 	enum missing_commit_check_level check_level = get_missing_commit_check_level();
 	struct strbuf missing = STRBUF_INIT;
@@ -142,12 +158,11 @@ int todo_list_check(struct todo_list *old_todo, struct todo_list *new_todo)
 	if (check_level == MISSING_COMMIT_CHECK_IGNORE)
 		goto leave_check;
 
-	/* Mark the commits in git-rebase-todo as seen */
-	for (i = 0; i < new_todo->nr; i++) {
-		struct commit *commit = new_todo->items[i].commit;
-		if (commit)
-			*commit_seen_at(&commit_seen, commit) = 1;
-	}
+	/* Mark the commits in git-rebase-todo and git-rebase-done as
+	   seen */
+	mark_commits_as_seen(&commit_seen, new_todo);
+	if (done)
+		mark_commits_as_seen(&commit_seen, done);
 
 	/* Find commits in git-rebase-todo.backup yet unseen */
 	for (i = old_todo->nr - 1; i >= 0; i--) {
@@ -187,3 +202,31 @@ int todo_list_check(struct todo_list *old_todo, struct todo_list *new_todo)
 	clear_commit_seen(&commit_seen);
 	return res;
 }
+
+int todo_list_check_against_backup(struct repository *r,
+				   struct todo_list *todo_list)
+{
+	struct todo_list done = TODO_LIST_INIT, initial = TODO_LIST_INIT;
+	int res;
+
+	if (strbuf_read_file(&done.buf, rebase_path_done(), 0) < 0 && errno != ENOENT)
+		return error(_("could not read '%s'"), rebase_path_done());
+
+	strbuf_read_file(&done.buf, rebase_path_done(), 0);
+	todo_list_parse_insn_buffer(r, done.buf.buf, &done);
+
+	if (strbuf_read_file(&initial.buf, rebase_path_todo_backup(), 0) < 0 &&
+	    errno != ENOENT) {
+		todo_list_release(&done);
+		return error(_("could not read '%s'"), rebase_path_done());
+	}
+
+	todo_list_parse_insn_buffer(r, initial.buf.buf, &initial);
+
+	res = todo_list_check(&initial, todo_list, &done);
+
+	todo_list_release(&done);
+	todo_list_release(&initial);
+
+	return res;
+}
diff --git a/rebase-interactive.h b/rebase-interactive.h
index 44dbb06311..ac401cda25 100644
--- a/rebase-interactive.h
+++ b/rebase-interactive.h
@@ -11,6 +11,10 @@ void append_todo_help(unsigned keep_empty, int command_count,
 int edit_todo_list(struct repository *r, struct todo_list *todo_list,
 		   struct todo_list *new_todo, const char *shortrevisions,
 		   const char *shortonto, unsigned flags);
-int todo_list_check(struct todo_list *old_todo, struct todo_list *new_todo);
+
+int todo_list_check(struct todo_list *old_todo, struct todo_list *new_todo,
+		    struct todo_list *done);
+int todo_list_check_against_backup(struct repository *r,
+				   struct todo_list *todo_list);
 
 #endif
diff --git a/sequencer.c b/sequencer.c
index 3fb15ff8d9..0638c92f12 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4898,7 +4898,7 @@ int check_todo_list_from_file(struct repository *r)
 	if (!res)
 		res = todo_list_parse_insn_buffer(r, new_todo.buf.buf, &new_todo);
 	if (!res)
-		res = todo_list_check(&old_todo, &new_todo);
+		res = todo_list_check(&old_todo, &new_todo, NULL);
 	if (res)
 		fprintf(stderr, _(edit_todo_list_advice));
 out:
@@ -5008,7 +5008,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
 	}
 
 	if (todo_list_parse_insn_buffer(r, new_todo.buf.buf, &new_todo) ||
-	    todo_list_check(todo_list, &new_todo)) {
+	    todo_list_check(todo_list, &new_todo, NULL)) {
 		fprintf(stderr, _(edit_todo_list_advice));
 		checkout_onto(r, opts, onto_name, &onto->object.oid, orig_head);
 		todo_list_release(&new_todo);

-- 
2.22.0
Previous: Alban GruinNext: Alban Gruin
Message 17 of 57 in “rebase -i: extend rebase.missingCommitsCheck to `--edit-todo' and co.”
  1. 0/9 rebase -i: extend rebase.missingCommitsCheck to `--edit-todo' and co.Alban Gruin, Jul 17, 2019
  2. 1/9 t3404: demonstrate that --edit-todo does not check for dropped commitsAlban Gruin, Jul 17, 2019
  3. Junio C HamanoJul 18, 2019
  4. Alban GruinJul 19, 2019
  5. Junio C HamanoJul 19, 2019
  6. 2/9 t3429: demonstrate that rebase exec does not check for dropped commitsAlban Gruin, Jul 17, 2019
  7. 3/9 sequencer: update `total_nr' when adding an item to a todo listAlban Gruin, Jul 17, 2019
  8. Junio C HamanoJul 18, 2019
  9. Alban GruinJul 19, 2019
  10. 4/9 sequencer: update `done_nr' when skipping commands in a todo listAlban Gruin, Jul 17, 2019
  11. Junio C HamanoJul 18, 2019
  12. Alban GruinJul 19, 2019
  13. 5/9 sequencer: move the code writing total_nr on the disk to a new functionAlban Gruin, Jul 17, 2019
  14. Junio C HamanoJul 18, 2019
  15. Alban GruinJul 19, 2019
  16. 6/9 sequencer: add a parameter to sequencer_continue() to accept a todo listAlban Gruin, Jul 17, 2019
  17. 7/9 rebase-interactive: todo_list_check() also uses the done listAlban Gruin, Jul 17, 2019
  18. 8/9 rebase-interactive: warn if commit is dropped with --edit-todoAlban Gruin, Jul 17, 2019
  19. 9/9 sequencer: have read_populate_todo() check for dropped commitsAlban Gruin, Jul 17, 2019
  20. Phillip WoodJul 24, 2019
  21. Johannes SchindelinJul 25, 2019
  22. Alban GruinJul 25, 2019
  23. Phillip WoodJul 29, 2019
  24. Alban GruinSep 24, 2019
  25. 0/2 rebase -i: extend rebase.missingCommitsCheck to `--edit-todo'Alban Gruin, Nov 4, 2019
  26. 1/2 t3404: demonstrate that --edit-todo does not check for dropped commitsAlban Gruin, Nov 4, 2019
  27. 2/2 rebase-interactive: warn if commit is dropped with --edit-todoAlban Gruin, Nov 4, 2019
  28. Phillip WoodNov 5, 2019
  29. 0/2 rebase -i: extend rebase.missingCommitsCheckAlban Gruin, Dec 2, 2019
  30. 1/2 sequencer: move check_todo_list_from_file() to rebase-interactive.cAlban Gruin, Dec 2, 2019
  31. Johannes SchindelinDec 6, 2019
  32. 2/2 rebase-interactive: warn if commit is dropped with `rebase --edit-todo'Alban Gruin, Dec 2, 2019
  33. Junio C HamanoDec 4, 2019
  34. Phillip WoodDec 9, 2019
  35. Alban GruinJan 9, 2020
  36. Phillip WoodJan 10, 2020
  37. Alban GruinJan 10, 2020
  38. Phillip WoodJan 11, 2020
  39. Phillip WoodDec 9, 2019
  40. Junio C HamanoDec 4, 2019
  41. Alban GruinDec 5, 2019
  42. Phillip WoodDec 6, 2019
  43. Johannes SchindelinDec 6, 2019
  44. 0/2 rebase -i: extend rebase.missingCommitsCheckAlban Gruin, Jan 11, 2020
  45. 1/2 sequencer: move check_todo_list_from_file() to rebase-interactive.cAlban Gruin, Jan 11, 2020
  46. 2/2 rebase-interactive: warn if commit is dropped with `rebase --edit-todo'Alban Gruin, Jan 11, 2020
  47. Phillip WoodJan 19, 2020
  48. Alban GruinJan 25, 2020
  49. 0/2 rebase -i: extend rebase.missingCommitsCheckAlban Gruin, Jan 25, 2020
  50. 1/2 sequencer: move check_todo_list_from_file() to rebase-interactive.cAlban Gruin, Jan 25, 2020
  51. 2/2 rebase-interactive: warn if commit is dropped with `rebase --edit-todo'Alban Gruin, Jan 25, 2020
  52. Johannes SchindelinJan 26, 2020
  53. Alban GruinJan 27, 2020
  54. Johannes SchindelinJan 28, 2020
  55. 0/2 rebase -i: extend rebase.missingCommitsCheckAlban Gruin, Jan 28, 2020
  56. 1/2 sequencer: move check_todo_list_from_file() to rebase-interactive.cAlban Gruin, Jan 28, 2020
  57. 2/2 rebase-interactive: warn if commit is dropped with `rebase --edit-todo'Alban Gruin, Jan 28, 2020

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.