git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: Git Mailing List <git@vger.kernel.org>
Cc: "Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Elijah Newren" <newren@gmail.com>,
	"Duy Nguyen" <pclouds@gmail.com>,
	"Alban Gruin" <alban.gruin@gmail.com>,
	"Josh Steadmon" <steadmon@google.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Phillip Wood" <phillip.wood@dunelm.org.uk>
Subject: [PATCH v1 12/12] rebase -i: run without forking rebase--interactive
Date: Wed, 17 Apr 2019 15:30:44 +0100	[thread overview]
Message-ID: <20190417143044.17655-13-phillip.wood123@gmail.com> (raw)
In-Reply-To: <20190417143044.17655-1-phillip.wood123@gmail.com>

From: Phillip Wood <phillip.wood@dunelm.org.uk>

When the builtin rebase starts an interactive rebase it parses the
options and then repackages them and forks
`rebase--interactive`. Separate the option parsing in
cmd_rebase__interactive() from the business logic to allow interactive
rebases can be run without forking `rebase__interactive` by calling
run_rebase_interactive() directly.

Starting interactive rebases without forking makes it easy to debug
the sequencer without worrying about attaching to child
processes. Ævar has also reported that some of the rebase perf tests
are 30% faster [1].

This patch also makes it easy to remove cmd_rebase__interactive() in
the future when git-legacy-rebase.sh and
git-rebase--preserve-merges.sh are retired.

[1] https://public-inbox.org/git/87y359cfjj.fsf@evledraar.gmail.com/

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 builtin/rebase.c | 221 ++++++++++++++++++-----------------------------
 1 file changed, 86 insertions(+), 135 deletions(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 0020e1ecf6..cd233c6118 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -316,7 +316,9 @@ static int do_interactive_rebase(struct rebase_options *opts, unsigned flags)
 				&revisions, &shortrevisions))
 		return -1;
 
-	if (init_basic_state(&replay, opts->head_name, opts->onto, head_hash)) {
+	if (init_basic_state(&replay,
+			     opts->head_name ? opts->head_name : "detached HEAD",
+			     opts->onto, head_hash)) {
 		free(revisions);
 		free(shortrevisions);
 
@@ -359,6 +361,77 @@ static int do_interactive_rebase(struct rebase_options *opts, unsigned flags)
 	return ret;
 }
 
+static int run_rebase_interactive(struct rebase_options *opts,
+				  enum action command)
+{
+	unsigned flags = 0;
+	int abbreviate_commands = 0, ret = 0;
+
+	git_config_get_bool("rebase.abbreviatecommands", &abbreviate_commands);
+
+	flags |= opts->keep_empty ? TODO_LIST_KEEP_EMPTY : 0;
+	flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0;
+	flags |= opts->rebase_merges ? TODO_LIST_REBASE_MERGES : 0;
+	flags |= opts->rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0;
+	flags |= command == ACTION_SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0;
+
+	switch (command) {
+	case ACTION_NONE: {
+		if (!opts->onto && !opts->upstream)
+			die(_("a base commit must be provided with --upstream or --onto"));
+
+		ret = do_interactive_rebase(opts, flags);
+		break;
+	}
+	case ACTION_SKIP: {
+		struct string_list merge_rr = STRING_LIST_INIT_DUP;
+
+		rerere_clear(the_repository, &merge_rr);
+	}
+		/* fallthrough */
+	case ACTION_CONTINUE: {
+		struct replay_opts replay_opts = get_replay_opts(opts);
+
+		ret = sequencer_continue(the_repository, &replay_opts);
+		break;
+	}
+	case ACTION_EDIT_TODO:
+		ret = edit_todo_file(flags);
+		break;
+	case ACTION_SHOW_CURRENT_PATCH: {
+		struct child_process cmd = CHILD_PROCESS_INIT;
+
+		cmd.git_cmd = 1;
+		argv_array_pushl(&cmd.args, "show", "REBASE_HEAD", "--", NULL);
+		ret = run_command(&cmd);
+
+		break;
+	}
+	case ACTION_SHORTEN_OIDS:
+	case ACTION_EXPAND_OIDS:
+		ret = transform_todo_file(flags);
+		break;
+	case ACTION_CHECK_TODO_LIST:
+		ret = check_todo_list_from_file(the_repository);
+		break;
+	case ACTION_REARRANGE_SQUASH:
+		ret = rearrange_squash_in_todo_file();
+		break;
+	case ACTION_ADD_EXEC: {
+		struct string_list commands = STRING_LIST_INIT_DUP;
+
+		split_exec_commands(opts->cmd, &commands);
+		ret = add_exec_commands(&commands);
+		string_list_clear(&commands, 0);
+		break;
+	}
+	default:
+		BUG("invalid command '%d'", command);
+	}
+
+	return ret;
+}
+
 static const char * const builtin_rebase_interactive_usage[] = {
 	N_("git rebase--interactive [<options>]"),
 	NULL
@@ -367,8 +440,6 @@ static const char * const builtin_rebase_interactive_usage[] = {
 int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)
 {
 	struct rebase_options opts = REBASE_OPTIONS_INIT;
-	unsigned flags = 0;
-	int abbreviate_commands = 0, ret = 0;
 	struct object_id squash_onto = null_oid;
 	enum action command = ACTION_NONE;
 	struct option options[] = {
@@ -433,8 +504,6 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)
 
 	opts.rebase_cousins = -1;
 
-	git_config_get_bool("rebase.abbreviatecommands", &abbreviate_commands);
-
 	if (argc == 1)
 		usage_with_options(builtin_rebase_interactive_usage, options);
 
@@ -444,71 +513,11 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)
 	if (!is_null_oid(&squash_onto))
 		opts.squash_onto = &squash_onto;
 
-	flags |= opts.keep_empty ? TODO_LIST_KEEP_EMPTY : 0;
-	flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0;
-	flags |= opts.rebase_merges ? TODO_LIST_REBASE_MERGES : 0;
-	flags |= opts.rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0;
-	flags |= command == ACTION_SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0;
-
 	if (opts.rebase_cousins >= 0 && !opts.rebase_merges)
 		warning(_("--[no-]rebase-cousins has no effect without "
 			  "--rebase-merges"));
 
-	switch (command) {
-	case ACTION_NONE: {
-		if (!opts.onto && !opts.upstream)
-			die(_("a base commit must be provided with --upstream or --onto"));
-
-		ret = do_interactive_rebase(&opts, flags);
-		break;
-	}
-	case ACTION_SKIP: {
-		struct string_list merge_rr = STRING_LIST_INIT_DUP;
-
-		rerere_clear(the_repository, &merge_rr);
-	}
-		/* fallthrough */
-	case ACTION_CONTINUE: {
-		struct replay_opts replay_opts = get_replay_opts(&opts);
-
-		ret = sequencer_continue(the_repository, &replay_opts);
-		break;
-	}
-	case ACTION_EDIT_TODO:
-		ret = edit_todo_file(flags);
-		break;
-	case ACTION_SHOW_CURRENT_PATCH: {
-		struct child_process cmd = CHILD_PROCESS_INIT;
-
-		cmd.git_cmd = 1;
-		argv_array_pushl(&cmd.args, "show", "REBASE_HEAD", "--", NULL);
-		ret = run_command(&cmd);
-
-		break;
-	}
-	case ACTION_SHORTEN_OIDS:
-	case ACTION_EXPAND_OIDS:
-		ret = transform_todo_file(flags);
-		break;
-	case ACTION_CHECK_TODO_LIST:
-		ret = check_todo_list_from_file(the_repository);
-		break;
-	case ACTION_REARRANGE_SQUASH:
-		ret = rearrange_squash_in_todo_file();
-		break;
-	case ACTION_ADD_EXEC: {
-		struct string_list commands = STRING_LIST_INIT_DUP;
-
-		split_exec_commands(opts.cmd, &commands);
-		ret = add_exec_commands(&commands);
-		string_list_clear(&commands, 0);
-		break;
-	}
-	default:
-		BUG("invalid command '%d'", command);
-	}
-
-	return !!ret;
+	return !!run_rebase_interactive(&opts, command);
 }
 
 static int use_builtin_rebase(void)
@@ -1071,7 +1080,7 @@ static int run_am(struct rebase_options *opts)
 	return status;
 }
 
-static int run_specific_rebase(struct rebase_options *opts)
+static int run_specific_rebase(struct rebase_options *opts, enum action action)
 {
 	const char *argv[] = { NULL, NULL };
 	struct strbuf script_snippet = STRBUF_INIT, buf = STRBUF_INIT;
@@ -1080,77 +1089,19 @@ static int run_specific_rebase(struct rebase_options *opts)
 
 	if (opts->type == REBASE_INTERACTIVE) {
 		/* Run builtin interactive rebase */
-		struct child_process child = CHILD_PROCESS_INIT;
-
-		argv_array_pushf(&child.env_array, "GIT_CHERRY_PICK_HELP=%s",
-				 resolvemsg);
+		setenv("GIT_CHERRY_PICK_HELP", resolvemsg, 1);
 		if (!(opts->flags & REBASE_INTERACTIVE_EXPLICIT)) {
-			argv_array_push(&child.env_array,
-					"GIT_SEQUENCE_EDITOR=:");
+			setenv("GIT_SEQUENCE_EDITOR", ":", 1);
 			opts->autosquash = 0;
 		}
+		if (opts->gpg_sign_opt) {
+			/* remove the leading "-S" */
+			char *tmp = xstrdup(opts->gpg_sign_opt + 2);
+			free(opts->gpg_sign_opt);
+			opts->gpg_sign_opt = tmp;
+		}
 
-		child.git_cmd = 1;
-		argv_array_push(&child.args, "rebase--interactive");
-
-		if (opts->action)
-			argv_array_pushf(&child.args, "--%s", opts->action);
-		if (opts->keep_empty)
-			argv_array_push(&child.args, "--keep-empty");
-		if (opts->rebase_merges)
-			argv_array_push(&child.args, "--rebase-merges");
-		if (opts->rebase_cousins)
-			argv_array_push(&child.args, "--rebase-cousins");
-		if (opts->autosquash)
-			argv_array_push(&child.args, "--autosquash");
-		if (opts->flags & REBASE_VERBOSE)
-			argv_array_push(&child.args, "--verbose");
-		if (opts->flags & REBASE_FORCE)
-			argv_array_push(&child.args, "--no-ff");
-		if (opts->restrict_revision)
-			argv_array_pushf(&child.args,
-					 "--restrict-revision=^%s",
-					 oid_to_hex(&opts->restrict_revision->object.oid));
-		if (opts->upstream)
-			argv_array_pushf(&child.args, "--upstream=%s",
-					 oid_to_hex(&opts->upstream->object.oid));
-		if (opts->onto)
-			argv_array_pushf(&child.args, "--onto=%s",
-					 oid_to_hex(&opts->onto->object.oid));
-		if (opts->squash_onto)
-			argv_array_pushf(&child.args, "--squash-onto=%s",
-					 oid_to_hex(opts->squash_onto));
-		if (opts->onto_name)
-			argv_array_pushf(&child.args, "--onto-name=%s",
-					 opts->onto_name);
-		argv_array_pushf(&child.args, "--head-name=%s",
-				 opts->head_name ?
-				 opts->head_name : "detached HEAD");
-		if (opts->strategy)
-			argv_array_pushf(&child.args, "--strategy=%s",
-					 opts->strategy);
-		if (opts->strategy_opts)
-			argv_array_pushf(&child.args, "--strategy-opts=%s",
-					 opts->strategy_opts);
-		if (opts->switch_to)
-			argv_array_pushf(&child.args, "--switch-to=%s",
-					 opts->switch_to);
-		if (opts->cmd)
-			argv_array_pushf(&child.args, "--cmd=%s", opts->cmd);
-		if (opts->allow_empty_message)
-			argv_array_push(&child.args, "--allow-empty-message");
-		if (opts->allow_rerere_autoupdate == RERERE_AUTOUPDATE)
-			argv_array_push(&child.args, "--rerere-autoupdate");
-		else if (opts->allow_rerere_autoupdate == RERERE_NOAUTOUPDATE)
-			argv_array_push(&child.args, "--no-rerere-autoupdate");
-		if (opts->gpg_sign_opt)
-			argv_array_push(&child.args, opts->gpg_sign_opt);
-		if (opts->signoff)
-			argv_array_push(&child.args, "--signoff");
-		if (opts->reschedule_failed_exec)
-			argv_array_push(&child.args, "--reschedule-failed-exec");
-
-		status = run_command(&child);
+		status = run_rebase_interactive(opts, action);
 		goto finished_rebase;
 	}
 
@@ -2211,7 +2162,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 	options.revisions = revisions.buf;
 
 run_rebase:
-	ret = !!run_specific_rebase(&options);
+	ret = !!run_specific_rebase(&options, action);
 
 cleanup:
 	strbuf_release(&revisions);
-- 
2.21.0


      parent reply	other threads:[~2019-04-17 14:33 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-19 19:03 [RFC PATCH 00/11] rebase -i run without forking rebase--interactive Phillip Wood
2019-03-19 19:03 ` [RFC PATCH 01/11] sequencer: always discard index after checkout Phillip Wood
2019-03-20  1:50   ` Duy Nguyen
2019-03-21 14:35     ` Phillip Wood
2019-03-19 19:03 ` [RFC PATCH 02/11] rebase: rename write_basic_state() Phillip Wood
2019-03-19 19:03 ` [RFC PATCH 03/11] rebase: use OPT_RERERE_AUTOUPDATE() Phillip Wood
2019-03-19 19:03 ` [RFC PATCH 04/11] rebase -i: combine rebase--interactive.c with rebase.c Phillip Wood
2019-03-19 19:03 ` [RFC PATCH 05/11] rebase -i: remove duplication Phillip Wood
2019-03-19 19:03 ` [RFC PATCH 06/11] rebase -i: use struct commit when parsing options Phillip Wood
2019-03-19 19:03 ` [RFC PATCH 07/11] rebase -i: use struct object_id for squash_onto Phillip Wood
2019-03-19 19:03 ` [RFC PATCH 08/11] rebase -i: use struct rebase_options to parse args Phillip Wood
2019-03-21  4:21   ` Junio C Hamano
2019-03-21 14:59     ` Phillip Wood
2019-03-22  3:34       ` Junio C Hamano
2019-03-21 21:13   ` Alban Gruin
2019-04-10 19:16     ` Phillip Wood
2019-03-19 19:03 ` [RFC PATCH 09/11] rebase -i: use struct rebase_options in do_interactive_rebase() Phillip Wood
2019-03-19 19:03 ` [RFC PATCH 10/11] rebase: use a common action enum Phillip Wood
2019-03-19 20:24   ` Ævar Arnfjörð Bjarmason
2019-03-21 14:43     ` Phillip Wood
2019-03-19 19:03 ` [RFC PATCH 11/11] rebase -i: run without forking rebase--interactive Phillip Wood
2019-03-20 20:50 ` [RFC PATCH 00/11] rebase -i " Josh Steadmon
2019-03-20 23:05 ` Ævar Arnfjörð Bjarmason
2019-03-21 14:40   ` Phillip Wood
2019-03-21  1:44 ` Junio C Hamano
2019-04-17 14:30 ` [PATCH v1 00/12] Run rebase -i " Phillip Wood
2019-04-17 14:30   ` [PATCH v1 01/12] sequencer: always discard index after checkout Phillip Wood
2019-04-17 14:30   ` [PATCH v1 02/12] rebase: don't translate trace strings Phillip Wood
2019-04-19  5:53     ` Junio C Hamano
2019-04-25 17:47       ` Phillip Wood
2019-04-17 14:30   ` [PATCH v1 03/12] rebase: rename write_basic_state() Phillip Wood
2019-04-17 14:30   ` [PATCH v1 04/12] rebase: use OPT_RERERE_AUTOUPDATE() Phillip Wood
2019-04-17 14:30   ` [PATCH v1 05/12] rebase -i: combine rebase--interactive.c with rebase.c Phillip Wood
2019-04-17 14:30   ` [PATCH v1 06/12] rebase -i: remove duplication Phillip Wood
2019-04-17 14:30   ` [PATCH v1 07/12] rebase -i: use struct commit when parsing options Phillip Wood
2019-04-17 14:30   ` [PATCH v1 08/12] rebase -i: use struct object_id for squash_onto Phillip Wood
2019-04-17 14:30   ` [PATCH v1 09/12] rebase -i: use struct rebase_options to parse args Phillip Wood
2019-04-17 14:30   ` [PATCH v1 10/12] rebase -i: use struct rebase_options in do_interactive_rebase() Phillip Wood
2019-04-17 14:30   ` [PATCH v1 11/12] rebase: use a common action enum Phillip Wood
2019-04-17 14:30   ` Phillip Wood [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190417143044.17655-13-phillip.wood123@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=alban.gruin@gmail.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=newren@gmail.com \
    --cc=pclouds@gmail.com \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=steadmon@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).