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

[PATCH 16/18] revert: Don't implictly stomp pending sequencer operation

From
Ramkumar Ramachandra <artagnon@gmail.com>
Date
Jul 28, 2011, 16:52 UTC
Message-ID
<1311871951-3497-17-git-send-email-artagnon@gmail.com>
In-Reply-To
<1311871951-3497-1-git-send-email-artagnon@gmail.com>

Protect the user from forgetting about a pending sequencer operation by immediately erroring out when an existing cherry-pick or revert operation is in progress like:

  $ git cherry-pick foo
  ... conflict ...
  $ git cherry-pick moo
  error: .git/sequencer already exists
  hint: A cherry-pick or revert is in progress
  hint: Use --reset to forget about it
  fatal: cherry-pick failed

A naive version of this would break the following established ways of working:

  $ git cherry-pick foo
  ... conflict ...
  $ git reset --hard  # I actually meant "moo" when I said "foo"
  $ git cherry-pick moo
  $ git cherry-pick foo
  ... conflict ...
  $ git commit # commit the resolution
  $ git cherry-pick moo # New operation

However, the previous patches "reset: Make reset remove the sequencer state" and "revert: Remove sequencer state when no commits are pending" make sure that this does not happen.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 builtin/revert.c                |   30 +++++++++++++++++++++++++-----
 t/t3510-cherry-pick-sequence.sh |   10 ++++++++++
 2 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 80a2ef7..e173c71 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -66,6 +66,15 @@ struct replay_opts {
 
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
 
+static void fatal(const char *advice, ...)
+{
+	va_list params;
+
+	va_start(params, advice);
+	vreportf("fatal: ", advice, params);
+	va_end(params);
+}
+
 static const char *action_name(const struct replay_opts *opts)
 {
 	return opts->action == REVERT ? "revert" : "cherry-pick";
@@ -673,13 +682,15 @@ static void walk_revs_populate_todo(struct commit_list **todo_list,
 		next = commit_list_append(commit, next);
 }
 
-static void create_seq_dir(void)
+static int create_seq_dir(void)
 {
 	const char *seq_dir = git_path(SEQ_DIR);
 
-	if (!(file_exists(seq_dir) && is_directory(seq_dir))
-		&& mkdir(seq_dir, 0777) < 0)
+	if (file_exists(seq_dir))
+		return error(_("%s already exists."), seq_dir);
+	else if (mkdir(seq_dir, 0777) < 0)
 		die_errno(_("Could not create sequencer directory '%s'."), seq_dir);
+	return 0;
 }
 
 static void save_head(const char *head)
@@ -798,9 +809,18 @@ static int pick_revisions(struct replay_opts *opts)
 		remove_sequencer_state(1);
 		return 0;
 	} else {
-		/* Start a new cherry-pick/ revert sequence */
+		/*
+		 * Start a new cherry-pick/ revert sequence; but
+		 * first, make sure that an existing one isn't in
+		 * progress
+		 */
+
 		walk_revs_populate_todo(&todo_list, opts);
-		create_seq_dir();
+		if (create_seq_dir() < 0) {
+			fatal(_("A cherry-pick or revert is in progress."));
+			advise(_("Use --reset to forget about it"));
+			exit(128);
+		}
 		if (get_sha1("HEAD", sha1)) {
 			if (opts->action == REVERT)
 				die(_("Can't revert as initial commit"));
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 528f270..342cab3 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -110,4 +110,14 @@ test_expect_success 'cherry-pick cleans up sequencer state when one commit is le
 	test_cmp expect actual
 '
 
+test_expect_success 'cherry-pick does not implicitly stomp an existing operation' '
+	git cherry-pick --reset &&
+	pristine_detach initial &&
+	test_must_fail git cherry-pick base..anotherpick &&
+	test-chmtime -v +0 .git/sequencer >expect &&
+	test_must_fail git cherry-pick unrelatedpick &&
+	test-chmtime -v +0 .git/sequencer >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
1.7.4.rc1.7.g2cf08.dirty
Previous: Ramkumar RamachandraNext: Ramkumar Ramachandra
Message 28 of 36 in “Sequencer for inclusion v4”
  1. 00/18 Sequencer for inclusion v4Ramkumar Ramachandra, Jul 28, 2011
  2. 01/18 advice: Introduce error_resolve_conflictRamkumar Ramachandra, Jul 28, 2011
  3. 02/18 config: Introduce functions to write non-standard fileRamkumar Ramachandra, Jul 28, 2011
  4. Jonathan NiederJul 28, 2011
  5. Ramkumar RamachandraJul 28, 2011
  6. 03/18 revert: Simplify and inline add_message_to_msgRamkumar Ramachandra, Jul 28, 2011
  7. 04/18 revert: Don't check lone argument in get_encodingRamkumar Ramachandra, Jul 28, 2011
  8. 05/18 revert: Rename no_replay to record_originRamkumar Ramachandra, Jul 28, 2011
  9. 06/18 revert: Eliminate global "commit" variableRamkumar Ramachandra, Jul 28, 2011
  10. 07/18 revert: Introduce struct to keep command-line optionsRamkumar Ramachandra, Jul 28, 2011
  11. 08/18 revert: Separate cmdline parsing from functional codeRamkumar Ramachandra, Jul 28, 2011
  12. 09/18 revert: Don't create invalid replay_opts in parse_argsRamkumar Ramachandra, Jul 28, 2011
  13. 10/18 revert: Save data for continuing after conflict resolutionRamkumar Ramachandra, Jul 28, 2011
  14. Jonathan NiederJul 28, 2011
  15. Ramkumar RamachandraJul 28, 2011
  16. 11/18 revert: Save command-line options for continuing operationRamkumar Ramachandra, Jul 28, 2011
  17. Christian CouderJul 31, 2011
  18. Ramkumar RamachandraAug 1, 2011
  19. 12/18 revert: Make pick_commits functionally act on a commit listRamkumar Ramachandra, Jul 28, 2011
  20. Christian CouderJul 31, 2011
  21. Christian CouderJul 31, 2011
  22. Ramkumar RamachandraAug 1, 2011
  23. 13/18 revert: Introduce --reset to remove sequencer stateRamkumar Ramachandra, Jul 28, 2011
  24. Christian CouderJul 31, 2011
  25. Ramkumar RamachandraAug 1, 2011
  26. 14/18 reset: Make reset remove the sequencer stateRamkumar Ramachandra, Jul 28, 2011
  27. 15/18 revert: Remove sequencer state when no commits are pendingRamkumar Ramachandra, Jul 28, 2011
  28. 16/18 revert: Don't implictly stomp pending sequencer operationRamkumar Ramachandra, Jul 28, 2011
  29. 17/18 revert: Introduce --continue to continue the operationRamkumar Ramachandra, Jul 28, 2011
  30. Christian CouderJul 31, 2011
  31. Ramkumar RamachandraAug 1, 2011
  32. Jonathan NiederAug 3, 2011
  33. Junio C HamanoAug 3, 2011
  34. Ramkumar RamachandraAug 3, 2011
  35. 18/18 revert: Propogate errors upwards from do_pick_commitRamkumar Ramachandra, Jul 28, 2011
  36. Ramkumar RamachandraJul 28, 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.