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

[PATCH 13/18] revert: Make pick_commits functionally act on a commit list

From
Ramkumar Ramachandra <artagnon@gmail.com>
Date
Jul 19, 2011, 17:17 UTC
Message-ID
<1311095876-3098-14-git-send-email-artagnon@gmail.com>
In-Reply-To
<1311095876-3098-1-git-send-email-artagnon@gmail.com>

Apart from its central objective of calling into the picking mechanism, pick_commits creates a sequencer directory, prepares a todo list, and even acts upon the "--reset" subcommand. This makes for a bad API since the central worry of callers is to figure out whether or not any conflicts were encountered during the cherry picking. The current API is like:

if (pick_commits(opts) < 0)
   print "Something failed, we're not sure what"

So, change pick_commits so that it's only responsible for picking commits in a loop and reporting any errors, leaving the rest to a new function called pick_revisions. Consequently, the API of pick_commits becomes much clearer:

act_on_subcommand(opts->subcommand);
todo_list = prepare_todo_list();
if (pick_commits(todo_list, opts) < 0)
   print "Error encountered while picking commits"

Now, callers can easily call-in to the cherry-picking machinery by constructing an arbitrary todo list along with some options.

Helped-by: Jonathan Nieder <jrnider@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 builtin/revert.c |   46 ++++++++++++++++++++++++++++++----------------
 1 files changed, 30 insertions(+), 16 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index a8ac5a1..b648448 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -851,11 +851,9 @@ static void save_opts(struct replay_opts *opts)
 	strbuf_release(&buf);
 }
 
-static int pick_commits(struct replay_opts *opts)
+static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
 {
-	struct commit_list *todo_list = NULL;
 	struct strbuf buf = STRBUF_INIT;
-	unsigned char sha1[20];
 	struct commit_list *cur;
 	int res;
 
@@ -865,17 +863,6 @@ static int pick_commits(struct replay_opts *opts)
 				opts->record_origin || opts->edit));
 	read_and_refresh_cache(opts);
 
-	walk_revs_populate_todo(&todo_list, opts);
-	create_seq_dir();
-	if (get_sha1("HEAD", sha1)) {
-		if (opts->action == REVERT)
-			return error(_("Can't revert as initial commit"));
-		return error(_("Can't cherry-pick into empty head"));
-	} else
-		save_head(sha1_to_hex(sha1));
-	save_opts(opts);
-	save_todo(todo_list, opts);
-
 	for (cur = todo_list; cur; cur = cur->next) {
 		save_todo(cur, opts);
 		res = do_pick_commit(cur->item, opts);
@@ -892,6 +879,27 @@ static int pick_commits(struct replay_opts *opts)
 	return 0;
 }
 
+static int pick_revisions(struct replay_opts *opts)
+{
+	struct commit_list *todo_list = NULL;
+	unsigned char sha1[20];
+
+	read_and_refresh_cache(opts);
+
+	walk_revs_populate_todo(&todo_list, opts);
+	create_seq_dir();
+	if (get_sha1("HEAD", sha1)) {
+		if (opts->action == REVERT)
+			return error(_("Can't revert as initial commit"));
+		return error(_("Can't cherry-pick into empty head"));
+	} else
+		save_head(sha1_to_hex(sha1));
+	save_opts(opts);
+	save_todo(todo_list, opts);
+
+	return pick_commits(todo_list, opts);
+}
+
 int cmd_revert(int argc, const char **argv, const char *prefix)
 {
 	int res;
@@ -903,7 +911,13 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
 	opts.action = REVERT;
 	git_config(git_default_config, NULL);
 	parse_args(argc, argv, &opts);
-	res = pick_commits(&opts);
+
+	/*
+	 * Decide what to do depending on the arguments; a fresh
+	 * cherry-pick should be handled differently from an existing
+	 * one that is being continued
+	 */
+	res = pick_revisions(&opts);
 	if (res < 0)
 		die(_("revert failed"));
 	return res;
@@ -918,7 +932,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
 	opts.action = CHERRY_PICK;
 	git_config(git_default_config, NULL);
 	parse_args(argc, argv, &opts);
-	res = pick_commits(&opts);
+	res = pick_revisions(&opts);
 	if (res < 0)
 		die(_("cherry-pick failed"));
 	return res;
-- 
1.7.4.rc1.7.g2cf08.dirty
Previous: Ramkumar RamachandraNext: Ramkumar Ramachandra
Message 24 of 34 in “[GSoC update] Sequencer for inclusion v2”
  1. Ramkumar RamachandraJul 19, 2011
  2. 01/18 advice: Introduce error_resolve_conflictRamkumar Ramachandra, Jul 19, 2011
  3. Phil HordJul 21, 2011
  4. Jeff KingJul 22, 2011
  5. Ramkumar RamachandraJul 24, 2011
  6. Junio C HamanoJul 24, 2011
  7. Phil HordJul 25, 2011
  8. 02/18 config: Introduce functions to write non-standard fileRamkumar Ramachandra, Jul 19, 2011
  9. Jeff KingJul 19, 2011
  10. Jonathan NiederJul 19, 2011
  11. Junio C HamanoJul 19, 2011
  12. Jeff KingJul 19, 2011
  13. Ramkumar RamachandraJul 24, 2011
  14. 03/18 revert: Simplify and inline add_message_to_msgRamkumar Ramachandra, Jul 19, 2011
  15. 04/18 revert: Don't check lone argument in get_encodingRamkumar Ramachandra, Jul 19, 2011
  16. 05/18 revert: Rename no_replay to record_originRamkumar Ramachandra, Jul 19, 2011
  17. 06/18 revert: Propogate errors upwards from do_pick_commitRamkumar Ramachandra, Jul 19, 2011
  18. 07/18 revert: Eliminate global "commit" variableRamkumar Ramachandra, Jul 19, 2011
  19. 08/18 revert: Introduce struct to keep command-line optionsRamkumar Ramachandra, Jul 19, 2011
  20. 09/18 revert: Separate cmdline parsing from functional codeRamkumar Ramachandra, Jul 19, 2011
  21. 10/18 revert: Don't create invalid replay_opts in parse_argsRamkumar Ramachandra, Jul 19, 2011
  22. 11/18 revert: Save data for continuing after conflict resolutionRamkumar Ramachandra, Jul 19, 2011
  23. 12/18 revert: Save command-line options for continuing operationRamkumar Ramachandra, Jul 19, 2011
  24. 13/18 revert: Make pick_commits functionally act on a commit listRamkumar Ramachandra, Jul 19, 2011
  25. 14/18 revert: Introduce --reset to remove sequencer stateRamkumar Ramachandra, Jul 19, 2011
  26. 15/18 reset: Make reset remove the sequencer stateRamkumar Ramachandra, Jul 19, 2011
  27. 16/18 revert: Remove sequencer state when no commits are pendingRamkumar Ramachandra, Jul 19, 2011
  28. 17/18 revert: Don't implictly stomp pending sequencer operationRamkumar Ramachandra, Jul 19, 2011
  29. 18/18 revert: Introduce --continue to continue the operationRamkumar Ramachandra, Jul 19, 2011
  30. Junio C HamanoJul 19, 2011
  31. Ramkumar RamachandraJul 20, 2011
  32. Junio C HamanoJul 19, 2011
  33. Ramkumar RamachandraJul 24, 2011
  34. Junio C HamanoJul 24, 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.