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

[PATCH 5/5] sequencer: Remove sequencer state after final commit

From
Ramkumar Ramachandra <artagnon@gmail.com>
Date
Aug 10, 2011, 09:55 UTC
Message-ID
<1312970151-18906-6-git-send-email-artagnon@gmail.com>
In-Reply-To
<1312970151-18906-1-git-send-email-artagnon@gmail.com>

Since d3f4628e (revert: Remove sequencer state when no commits are pending, 2011-07-06), the sequencer removes the sequencer state before the final commit is actually completed. This design is inherently flawed, as it will not allow the user to abort the sequencer operation at that stage. Instead, write and expose a new function to count the number of commits left in the instruction sheet; use this in builtin/commit.c to remove the sequencer state when a commit has successfully completed and there is only one instruction left in the sheet.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 builtin/commit.c                |    5 +++++
 builtin/revert.c                |   12 +-----------
 sequencer.c                     |   14 ++++++++++++++
 sequencer.h                     |    1 +
 t/t3510-cherry-pick-sequence.sh |    4 ++--
 5 files changed, 23 insertions(+), 13 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index e1af9b1..7ee4269 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -26,6 +26,7 @@
 #include "unpack-trees.h"
 #include "quote.h"
 #include "submodule.h"
+#include "sequencer.h"
 
 static const char * const builtin_commit_usage[] = {
 	"git commit [options] [--] <filepattern>...",
@@ -1521,6 +1522,10 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	unlink(git_path("MERGE_MODE"));
 	unlink(git_path("SQUASH_MSG"));
 
+	/* Remove sequencer state if we just finished the last insn */
+	if (sequencer_count_todo() == 1)
+		remove_sequencer_state(1);
+
 	if (commit_index_files())
 		die (_("Repository has been updated, but unable to write\n"
 		     "new_index file. Check that disk is not full or quota is\n"
diff --git a/builtin/revert.c b/builtin/revert.c
index 491c10a..718ba86 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -581,18 +581,8 @@ static int pick_commits(struct replay_insn_list *todo_list,
 		sequencer_save_todo(cur);
 		opts->action = cur->action;
 		res = do_pick_commit(cur->operand, opts);
-		if (res) {
-			if (!cur->next && res > 0)
-				/*
-				 * A conflict was encountered while
-				 * picking the last commit.  The
-				 * sequencer state is useless now --
-				 * the user simply needs to resolve
-				 * the conflict and commit
-				 */
-				remove_sequencer_state(0);
+		if (res)
 			return res;
-		}
 	}
 
 	/*
diff --git a/sequencer.c b/sequencer.c
index 2993846..0ad1da6 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -326,3 +326,17 @@ void sequencer_save_opts(struct replay_opts *opts)
 							opts->xopts[i], "^$", 0);
 	}
 }
+
+int sequencer_count_todo(void)
+{
+	struct replay_insn_list *todo_list = NULL;
+	struct replay_insn_list *cur;
+	int insn_count = 0;
+
+	if (!file_exists(git_path(SEQ_TODO_FILE)))
+		return 0;
+	sequencer_read_todo(&todo_list);
+	for (cur = todo_list; cur; cur = cur->next)
+		insn_count += 1;
+	return insn_count;
+}
diff --git a/sequencer.h b/sequencer.h
index a5b951d..ca133e2 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -68,5 +68,6 @@ int sequencer_create_dir(void);
 void sequencer_save_head(const char *head);
 void sequencer_save_todo(struct replay_insn_list *todo_list);
 void sequencer_save_opts(struct replay_opts *opts);
+int sequencer_count_todo(void);
 
 #endif
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index f85372c..832baa0 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -82,13 +82,13 @@ test_expect_success '--reset cleans up sequencer state' '
 	test_path_is_missing .git/sequencer
 '
 
-test_expect_success 'cherry-pick cleans up sequencer state when one commit is left' '
+test_expect_success 'final commit cleans up sequencer state' '
 	pristine_detach initial &&
 	test_must_fail git cherry-pick base..picked &&
-	test_path_is_missing .git/sequencer &&
 	echo "resolved" >foo &&
 	git add foo &&
 	git commit &&
+	test_path_is_missing .git/sequencer &&
 	{
 		git rev-list HEAD |
 		git diff-tree --root --stdin |
-- 
1.7.6.351.gb35ac.dirty
Previous: Ramkumar Ramachandra
Message 16 of 16 in “Some preliminary work based on sequencer-stable”
  1. 0/5 Some preliminary work based on sequencer-stableRamkumar Ramachandra, Aug 10, 2011
  2. 1/5 revert: Don't remove the sequencer state on errorRamkumar Ramachandra, Aug 10, 2011
  3. 2/5 sequencer.h: Move data structuresRamkumar Ramachandra, Aug 10, 2011
  4. 3/5 revert: Allow mixed pick and revert instructionsRamkumar Ramachandra, Aug 10, 2011
  5. Jonathan NiederAug 10, 2011
  6. Ramkumar RamachandraAug 11, 2011
  7. Ramkumar RamachandraAug 11, 2011
  8. Jonathan NiederAug 11, 2011
  9. 4/5 sequencer: Expose code that handles files in .git/sequencerRamkumar Ramachandra, Aug 10, 2011
  10. Jonathan NiederAug 10, 2011
  11. Ramkumar RamachandraAug 10, 2011
  12. Jonathan NiederAug 10, 2011
  13. Ramkumar RamachandraAug 11, 2011
  14. Jonathan NiederAug 11, 2011
  15. Ramkumar RamachandraAug 11, 2011
  16. 5/5 sequencer: Remove sequencer state after final commitRamkumar Ramachandra, Aug 10, 2011

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.