[RFC PATCH 6/9] sequencer: add a parameter to sequencer_continue() to accept a todo list

From: Alban Gruin <alban.gruin@gmail.com>
To: git@vger.kernel.org
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Phillip Wood <phillip.wood@dunelm.org.uk>,
	Alban Gruin <alban.gruin@gmail.com>
Subject: [RFC PATCH 6/9] sequencer: add a parameter to sequencer_continue() to accept a todo list
Date: Wed, 17 Jul 2019 16:39:15 +0200	[thread overview]
Message-ID: <20190717143918.7406-7-alban.gruin@gmail.com> (raw)
In-Reply-To: <20190717143918.7406-1-alban.gruin@gmail.com>

As it is called by sequencer_continue() or after an exec command,
read_populate_todo() is a great place to check for dropped commits in
the todo list, but complete_action() (a caller of sequencer_continue())
already does.  Double-checking would show the message twice.

This adds a parameter to sequencer_continue() to accept a todo list.  If
a valid list is provided, read_populate_todo() won't be called.
complete_action() is modified to pass its todo list to
sequencer_continue().

This also avoids reloading the todo list from the disk just after
releasing it.

Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
---
 builtin/rebase.c |  2 +-
 builtin/revert.c |  2 +-
 sequencer.c      | 26 +++++++++++++++-----------
 sequencer.h      |  3 ++-
 4 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 89fc4b8153..205916ca11 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -393,7 +393,7 @@ static int run_rebase_interactive(struct rebase_options *opts,
 	case ACTION_CONTINUE: {
 		struct replay_opts replay_opts = get_replay_opts(opts);
 
-		ret = sequencer_continue(the_repository, &replay_opts);
+		ret = sequencer_continue(the_repository, &replay_opts, NULL);
 		break;
 	}
 	case ACTION_EDIT_TODO:
diff --git a/builtin/revert.c b/builtin/revert.c
index 4e71b2f2aa..45a5c6217d 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -207,7 +207,7 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)
 		return ret;
 	}
 	if (cmd == 'c')
-		return sequencer_continue(the_repository, opts);
+		return sequencer_continue(the_repository, opts, NULL);
 	if (cmd == 'a')
 		return sequencer_rollback(the_repository, opts);
 	return sequencer_pick_revisions(the_repository, opts);
diff --git a/sequencer.c b/sequencer.c
index d66b80d49f..3fb15ff8d9 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4147,9 +4147,11 @@ static int commit_staged_changes(struct repository *r,
 	return 0;
 }
 
-int sequencer_continue(struct repository *r, struct replay_opts *opts)
+int sequencer_continue(struct repository *r, struct replay_opts *opts,
+		       struct todo_list *p_todo_list)
 {
-	struct todo_list todo_list = TODO_LIST_INIT;
+	struct todo_list todo_list = TODO_LIST_INIT,
+		*ptr_todo = (p_todo_list) ? p_todo_list : &todo_list;
 	int res;
 
 	if (read_and_refresh_cache(r, opts))
@@ -4158,13 +4160,13 @@ int sequencer_continue(struct repository *r, struct replay_opts *opts)
 	if (read_populate_opts(opts))
 		return -1;
 	if (is_rebase_i(opts)) {
-		if ((res = read_populate_todo(r, &todo_list, opts)))
+		if (!p_todo_list && (res = read_populate_todo(r, &todo_list, opts)))
 			goto release_todo_list;
-		if (commit_staged_changes(r, opts, &todo_list))
+		if (commit_staged_changes(r, opts, ptr_todo))
 			return -1;
 	} else if (!file_exists(get_todo_path(opts)))
 		return continue_single_pick(r);
-	else if ((res = read_populate_todo(r, &todo_list, opts)))
+	else if (!p_todo_list && (res = read_populate_todo(r, &todo_list, opts)))
 		goto release_todo_list;
 
 	if (!is_rebase_i(opts)) {
@@ -4179,18 +4181,18 @@ int sequencer_continue(struct repository *r, struct replay_opts *opts)
 			res = error_dirty_index(r, opts);
 			goto release_todo_list;
 		}
-		todo_list.current++;
+		ptr_todo->current++;
 	} else if (file_exists(rebase_path_stopped_sha())) {
 		struct strbuf buf = STRBUF_INIT;
 		struct object_id oid;
 
 		if (read_oneliner(&buf, rebase_path_stopped_sha(), 1) &&
 		    !get_oid_committish(buf.buf, &oid))
-			record_in_rewritten(&oid, peek_command(&todo_list, 0));
+			record_in_rewritten(&oid, peek_command(ptr_todo, 0));
 		strbuf_release(&buf);
 	}
 
-	res = pick_commits(r, &todo_list, opts);
+	res = pick_commits(r, ptr_todo, opts);
 release_todo_list:
 	todo_list_release(&todo_list);
 	return res;
@@ -5025,15 +5027,17 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
 		return error_errno(_("could not write '%s'"), todo_file);
 	}
 
-	todo_list_release(&new_todo);
-
 	if (checkout_onto(r, opts, onto_name, &oid, orig_head))
 		return -1;
 
 	if (require_clean_work_tree(r, "rebase", "", 1, 1))
 		return -1;
 
-	return sequencer_continue(r, opts);
+	todo_list_write_total_nr(&new_todo);
+	res = sequencer_continue(r, opts, &new_todo);
+	todo_list_release(&new_todo);
+
+	return res;
 }
 
 struct subject2item_entry {
diff --git a/sequencer.h b/sequencer.h
index 3d0b68c34c..858816f94a 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -127,7 +127,8 @@ const char *todo_item_get_arg(struct todo_list *todo_list,
 void sequencer_init_config(struct replay_opts *opts);
 int sequencer_pick_revisions(struct repository *repo,
 			     struct replay_opts *opts);
-int sequencer_continue(struct repository *repo, struct replay_opts *opts);
+int sequencer_continue(struct repository *repo, struct replay_opts *opts,
+		       struct todo_list *p_todo_list);
 int sequencer_rollback(struct repository *repo, struct replay_opts *opts);
 int sequencer_remove_state(struct replay_opts *opts);
 
-- 
2.22.0

