diff mbox series

[RFC,08/11] rebase -i: use struct rebase_options to parse args

Message ID 20190319190317.6632-9-phillip.wood123@gmail.com (mailing list archive)
State New, archived
Headers show
Series rebase -i run without forking rebase--interactive | expand

Commit Message

Phillip Wood March 19, 2019, 7:03 p.m. UTC
From: Phillip Wood <phillip.wood@dunelm.org.uk>

In order to run `rebase -i` without forking `rebase--interactive` it
will be convenient to use the same structure when parsing the options in
cmd_rebase() and cmd_rebase__interactive().

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

Comments

Junio C Hamano March 21, 2019, 4:21 a.m. UTC | #1
Phillip Wood <phillip.wood123@gmail.com> writes:

> +static struct replay_opts get_replay_opts(const struct rebase_options *opts)
> +{
> +	struct replay_opts replay = REPLAY_OPTS_INIT;
> +
> +	sequencer_init_config(&replay);
> +
> +	replay.action = REPLAY_INTERACTIVE_REBASE;
> +	replay.signoff = opts->signoff;
> +	replay.allow_ff = !(opts->flags & REBASE_FORCE);
> +	if (opts->allow_rerere_autoupdate)
> +		replay.allow_rerere_auto = opts->allow_rerere_autoupdate;
> +	replay.allow_empty = 1;
> +	replay.allow_empty_message = opts->allow_empty_message;
> +	replay.verbose = opts->flags & REBASE_VERBOSE;
> +	replay.reschedule_failed_exec = opts->reschedule_failed_exec;
> +	replay.gpg_sign = xstrdup_or_null(opts->gpg_sign_opt);
> +	replay.strategy = opts->strategy;
> +
> +	return replay;
> +}

This calls init_config() and then sets .action; does it revert to
what dl/merge-cleanup-scissors-fix wants to do, which flipped the
order to fix some bug?  It is a bit hard to tell.

Unfortunately because of the earlier huge code movement the changes
to _this_ file does not conflict and cleanly merges, but because the
other file is removed by this series while a topic in flight updates
it, the semantic conflict like this luckily gets discovered.

Especially since this is still an RFC, I'd preferred to see it
without moving around the code too much (instead, exporting some
symbols that need to be visible with each other after renaming them
to more appropriate names that are fit in the global namespace).
Phillip Wood March 21, 2019, 2:59 p.m. UTC | #2
Hi Junio

On 21/03/2019 04:21, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
>> +static struct replay_opts get_replay_opts(const struct rebase_options *opts)
>> +{
>> +	struct replay_opts replay = REPLAY_OPTS_INIT;
>> +
>> +	sequencer_init_config(&replay);
>> +
>> +	replay.action = REPLAY_INTERACTIVE_REBASE;
>> +	replay.signoff = opts->signoff;
>> +	replay.allow_ff = !(opts->flags & REBASE_FORCE);
>> +	if (opts->allow_rerere_autoupdate)
>> +		replay.allow_rerere_auto = opts->allow_rerere_autoupdate;
>> +	replay.allow_empty = 1;
>> +	replay.allow_empty_message = opts->allow_empty_message;
>> +	replay.verbose = opts->flags & REBASE_VERBOSE;
>> +	replay.reschedule_failed_exec = opts->reschedule_failed_exec;
>> +	replay.gpg_sign = xstrdup_or_null(opts->gpg_sign_opt);
>> +	replay.strategy = opts->strategy;
>> +
>> +	return replay;
>> +}
> 
> This calls init_config() and then sets .action; does it revert to
> what dl/merge-cleanup-scissors-fix wants to do, which flipped the
> order to fix some bug?  It is a bit hard to tell.

dl/merge-cleanup-scissors-fix changes sequencer_init_config() to depend 
on the value of action so action should to be set first. What is in pu 
at the moment in not quite right (though I'm not sure what the practical 
implications are as it looks like the rebase tests are passing[1]).

[1] https://travis-ci.org/git/git/jobs/509332448
> Unfortunately because of the earlier huge code movement the changes
> to _this_ file does not conflict and cleanly merges, but because the
> other file is removed by this series while a topic in flight updates
> it, the semantic conflict like this luckily gets discovered.
> 
> Especially since this is still an RFC, I'd preferred to see it
> without moving around the code too much (instead, exporting some
> symbols that need to be visible with each other after renaming them
> to more appropriate names that are fit in the global namespace).

I'm happy to try doing that, though the textual conflict would be in a 
different place in the file to the semantic conflict but at least they'd 
both be in the same file. I think it would only need to share the 
definitions of struct rebase_options, enum rebase_action and the 
declaration of run_rebase_interactive(). Would you be happy with the 
addition of builtin/rebase.h (there don't seem to be another headers in 
that directory). We could leave rebase--interactive.c around until 
rebase--preserve-merges.sh is finally removed.

Best Wishes

Phillip
Alban Gruin March 21, 2019, 9:13 p.m. UTC | #3
Hi Phillip,

It’s nice to see your work on this on the list.

Le 19/03/2019 à 20:03, Phillip Wood a écrit :
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
> 
> In order to run `rebase -i` without forking `rebase--interactive` it
> will be convenient to use the same structure when parsing the options in
> cmd_rebase() and cmd_rebase__interactive().
> 
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>  builtin/rebase.c | 203 ++++++++++++++++++++++++++---------------------
>  1 file changed, 112 insertions(+), 91 deletions(-)
> 
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index c93f2aa629..33a2495032 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -50,6 +50,73 @@ enum rebase_type {
>  	REBASE_PRESERVE_MERGES
>  };
>  
> +struct rebase_options {
> +	enum rebase_type type;
> +	const char *state_dir;
> +	struct commit *upstream;
> +	const char *upstream_name;
> +	const char *upstream_arg;
> +	char *head_name;
> +	struct object_id orig_head;
> +	struct commit *onto;
> +	const char *onto_name;
> +	const char *revisions;
> +	const char *switch_to;
> +	int root;
> +	struct object_id *squash_onto;
> +	struct commit *restrict_revision;
> +	int dont_finish_rebase;
> +	enum {
> +		REBASE_NO_QUIET = 1<<0,
> +		REBASE_VERBOSE = 1<<1,
> +		REBASE_DIFFSTAT = 1<<2,
> +		REBASE_FORCE = 1<<3,
> +		REBASE_INTERACTIVE_EXPLICIT = 1<<4,
> +	} flags;
> +	struct argv_array git_am_opts;
> +	const char *action;
> +	int signoff;
> +	int allow_rerere_autoupdate;
> +	int keep_empty;
> +	int autosquash;
> +	char *gpg_sign_opt;
> +	int autostash;
> +	char *cmd;
> +	int allow_empty_message;
> +	int rebase_merges, rebase_cousins;
> +	char *strategy, *strategy_opts;
> +	struct strbuf git_format_patch_opt;
> +	int reschedule_failed_exec;
> +};
> +
> +#define REBASE_OPTIONS_INIT {			  	\
> +		.type = REBASE_UNSPECIFIED,	  	\
> +		.flags = REBASE_NO_QUIET, 		\
> +		.git_am_opts = ARGV_ARRAY_INIT,		\
> +		.git_format_patch_opt = STRBUF_INIT	\
> +	}
> +
> +static struct replay_opts get_replay_opts(const struct rebase_options *opts)
> +{
> +	struct replay_opts replay = REPLAY_OPTS_INIT;
> +
> +	sequencer_init_config(&replay);
> +
> +	replay.action = REPLAY_INTERACTIVE_REBASE;
> +	replay.signoff = opts->signoff;
> +	replay.allow_ff = !(opts->flags & REBASE_FORCE);
> +	if (opts->allow_rerere_autoupdate)
> +		replay.allow_rerere_auto = opts->allow_rerere_autoupdate;
> +	replay.allow_empty = 1;
> +	replay.allow_empty_message = opts->allow_empty_message;
> +	replay.verbose = opts->flags & REBASE_VERBOSE;
> +	replay.reschedule_failed_exec = opts->reschedule_failed_exec;
> +	replay.gpg_sign = xstrdup_or_null(opts->gpg_sign_opt);
> +	replay.strategy = opts->strategy;
> +
> +	return replay;
> +}
> +

I wonder if `struct rebase_options` and `struct replay_options` could be
merged, or at least have `replay_options` used in `rebase_options`,
instead of converting one to the other.  I think it would make things
simpler and cleaner, but I don’t know how hard it would be, or if my
assumption is correct.

Cheers,
Alban
Junio C Hamano March 22, 2019, 3:34 a.m. UTC | #4
Phillip Wood <phillip.wood123@gmail.com> writes:

> Hi Junio
>
> On 21/03/2019 04:21, Junio C Hamano wrote:
>> Phillip Wood <phillip.wood123@gmail.com> writes:
>>
>>> +static struct replay_opts get_replay_opts(const struct rebase_options *opts)
>>> +{
>>> +	struct replay_opts replay = REPLAY_OPTS_INIT;
>>> +
>>> +	sequencer_init_config(&replay);
>>> +
>>> +	replay.action = REPLAY_INTERACTIVE_REBASE;
>>> +	replay.signoff = opts->signoff;
>>> +	replay.allow_ff = !(opts->flags & REBASE_FORCE);
>>> +	if (opts->allow_rerere_autoupdate)
>>> +		replay.allow_rerere_auto = opts->allow_rerere_autoupdate;
>>> +	replay.allow_empty = 1;
>>> +	replay.allow_empty_message = opts->allow_empty_message;
>>> +	replay.verbose = opts->flags & REBASE_VERBOSE;
>>> +	replay.reschedule_failed_exec = opts->reschedule_failed_exec;
>>> +	replay.gpg_sign = xstrdup_or_null(opts->gpg_sign_opt);
>>> +	replay.strategy = opts->strategy;
>>> +
>>> +	return replay;
>>> +}
>>
>> This calls init_config() and then sets .action; does it revert to
>> what dl/merge-cleanup-scissors-fix wants to do, which flipped the
>> order to fix some bug?  It is a bit hard to tell.
>
> dl/merge-cleanup-scissors-fix changes sequencer_init_config() to
> depend on the value of action so action should to be set first. What
> is in pu at the moment in not quite right (though I'm not sure what
> the practical implications are as it looks like the rebase tests are
> passing[1]).
>
> [1] https://travis-ci.org/git/git/jobs/509332448

Yes, it actually is the reason why I left the seemingly-wrong merge
in 'pu' on purpose to see if we have enough test coverage.

> they'd both be in the same file. I think it would only need to share
> the definitions of struct rebase_options, enum rebase_action and the
> declaration of run_rebase_interactive(). Would you be happy with the
> addition of builtin/rebase.h (there don't seem to be another headers
> in that directory). We could leave rebase--interactive.c around until
> rebase--preserve-merges.sh is finally removed.

I think dl/merge-cleanup-scissors-fix is getting solid enough, so a
better alternative may be to base this on top of

	reset e902e9bcae ;# The second batch
	merge ag/sequencer-reduce-rewriting-todo
	merge dl/merge-cleanup-scissors-fix

instead of basing it only on ag/sequencer-reduce-rewriting-todo.

Thanks.
Phillip Wood April 10, 2019, 7:16 p.m. UTC | #5
Hi Alban

sorry for the slow reply, I think you're probably off-list for a while 
by now

On 21/03/2019 21:13, Alban Gruin wrote:
> Hi Phillip,
> 
> It’s nice to see your work on this on the list.
> 
> Le 19/03/2019 à 20:03, Phillip Wood a écrit :
>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>
>> In order to run `rebase -i` without forking `rebase--interactive` it
>> will be convenient to use the same structure when parsing the options in
>> cmd_rebase() and cmd_rebase__interactive().
>>
>> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>> ---
>>   builtin/rebase.c | 203 ++++++++++++++++++++++++++---------------------
>>   1 file changed, 112 insertions(+), 91 deletions(-)
>>
>> diff --git a/builtin/rebase.c b/builtin/rebase.c
>> index c93f2aa629..33a2495032 100644
>> --- a/builtin/rebase.c
>> +++ b/builtin/rebase.c
>> @@ -50,6 +50,73 @@ enum rebase_type {
>>   	REBASE_PRESERVE_MERGES
>>   };
>>   
>> +struct rebase_options {
>> +	enum rebase_type type;
>> +	const char *state_dir;
>> +	struct commit *upstream;
>> +	const char *upstream_name;
>> +	const char *upstream_arg;
>> +	char *head_name;
>> +	struct object_id orig_head;
>> +	struct commit *onto;
>> +	const char *onto_name;
>> +	const char *revisions;
>> +	const char *switch_to;
>> +	int root;
>> +	struct object_id *squash_onto;
>> +	struct commit *restrict_revision;
>> +	int dont_finish_rebase;
>> +	enum {
>> +		REBASE_NO_QUIET = 1<<0,
>> +		REBASE_VERBOSE = 1<<1,
>> +		REBASE_DIFFSTAT = 1<<2,
>> +		REBASE_FORCE = 1<<3,
>> +		REBASE_INTERACTIVE_EXPLICIT = 1<<4,
>> +	} flags;
>> +	struct argv_array git_am_opts;
>> +	const char *action;
>> +	int signoff;
>> +	int allow_rerere_autoupdate;
>> +	int keep_empty;
>> +	int autosquash;
>> +	char *gpg_sign_opt;
>> +	int autostash;
>> +	char *cmd;
>> +	int allow_empty_message;
>> +	int rebase_merges, rebase_cousins;
>> +	char *strategy, *strategy_opts;
>> +	struct strbuf git_format_patch_opt;
>> +	int reschedule_failed_exec;
>> +};
>> +
>> +#define REBASE_OPTIONS_INIT {			  	\
>> +		.type = REBASE_UNSPECIFIED,	  	\
>> +		.flags = REBASE_NO_QUIET, 		\
>> +		.git_am_opts = ARGV_ARRAY_INIT,		\
>> +		.git_format_patch_opt = STRBUF_INIT	\
>> +	}
>> +
>> +static struct replay_opts get_replay_opts(const struct rebase_options *opts)
>> +{
>> +	struct replay_opts replay = REPLAY_OPTS_INIT;
>> +
>> +	sequencer_init_config(&replay);
>> +
>> +	replay.action = REPLAY_INTERACTIVE_REBASE;
>> +	replay.signoff = opts->signoff;
>> +	replay.allow_ff = !(opts->flags & REBASE_FORCE);
>> +	if (opts->allow_rerere_autoupdate)
>> +		replay.allow_rerere_auto = opts->allow_rerere_autoupdate;
>> +	replay.allow_empty = 1;
>> +	replay.allow_empty_message = opts->allow_empty_message;
>> +	replay.verbose = opts->flags & REBASE_VERBOSE;
>> +	replay.reschedule_failed_exec = opts->reschedule_failed_exec;
>> +	replay.gpg_sign = xstrdup_or_null(opts->gpg_sign_opt);
>> +	replay.strategy = opts->strategy;
>> +
>> +	return replay;
>> +}
>> +
> 
> I wonder if `struct rebase_options` and `struct replay_options` could be
> merged, or at least have `replay_options` used in `rebase_options`,
> instead of converting one to the other.  I think it would make things
> simpler and cleaner, but I don’t know how hard it would be, or if my
> assumption is correct.

I did consider doing that, but there are a few subtle differences in the 
way the options are stored in each struct (eg the gpg option has a 
leading -S in struct rebase_options but not struct replay_options) and I 
think it would be a bit of a faff to align them as it would mean messing 
with the code uses them and the code that reads/writes the various state 
files (we cannot change the on-disk format without breaking things - 
sometimes users start a rebase with one version of git bundled with 
something like tig and then continue on the command-line with a 
different version). I'd prefer to leave it as a future cleanup once this 
series has been merged.

Best Wishes

Phillip
> 
> Cheers,
> Alban
>
diff mbox series

Patch

diff --git a/builtin/rebase.c b/builtin/rebase.c
index c93f2aa629..33a2495032 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -50,6 +50,73 @@  enum rebase_type {
 	REBASE_PRESERVE_MERGES
 };
 
+struct rebase_options {
+	enum rebase_type type;
+	const char *state_dir;
+	struct commit *upstream;
+	const char *upstream_name;
+	const char *upstream_arg;
+	char *head_name;
+	struct object_id orig_head;
+	struct commit *onto;
+	const char *onto_name;
+	const char *revisions;
+	const char *switch_to;
+	int root;
+	struct object_id *squash_onto;
+	struct commit *restrict_revision;
+	int dont_finish_rebase;
+	enum {
+		REBASE_NO_QUIET = 1<<0,
+		REBASE_VERBOSE = 1<<1,
+		REBASE_DIFFSTAT = 1<<2,
+		REBASE_FORCE = 1<<3,
+		REBASE_INTERACTIVE_EXPLICIT = 1<<4,
+	} flags;
+	struct argv_array git_am_opts;
+	const char *action;
+	int signoff;
+	int allow_rerere_autoupdate;
+	int keep_empty;
+	int autosquash;
+	char *gpg_sign_opt;
+	int autostash;
+	char *cmd;
+	int allow_empty_message;
+	int rebase_merges, rebase_cousins;
+	char *strategy, *strategy_opts;
+	struct strbuf git_format_patch_opt;
+	int reschedule_failed_exec;
+};
+
+#define REBASE_OPTIONS_INIT {			  	\
+		.type = REBASE_UNSPECIFIED,	  	\
+		.flags = REBASE_NO_QUIET, 		\
+		.git_am_opts = ARGV_ARRAY_INIT,		\
+		.git_format_patch_opt = STRBUF_INIT	\
+	}
+
+static struct replay_opts get_replay_opts(const struct rebase_options *opts)
+{
+	struct replay_opts replay = REPLAY_OPTS_INIT;
+
+	sequencer_init_config(&replay);
+
+	replay.action = REPLAY_INTERACTIVE_REBASE;
+	replay.signoff = opts->signoff;
+	replay.allow_ff = !(opts->flags & REBASE_FORCE);
+	if (opts->allow_rerere_autoupdate)
+		replay.allow_rerere_auto = opts->allow_rerere_autoupdate;
+	replay.allow_empty = 1;
+	replay.allow_empty_message = opts->allow_empty_message;
+	replay.verbose = opts->flags & REBASE_VERBOSE;
+	replay.reschedule_failed_exec = opts->reschedule_failed_exec;
+	replay.gpg_sign = xstrdup_or_null(opts->gpg_sign_opt);
+	replay.strategy = opts->strategy;
+
+	return replay;
+}
+
 static int add_exec_commands(struct string_list *commands)
 {
 	const char *todo_file = rebase_path_todo();
@@ -265,32 +332,30 @@  static const char * const builtin_rebase_interactive_usage[] = {
 
 int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)
 {
-	struct replay_opts opts = REPLAY_OPTS_INIT;
-	unsigned flags = 0, keep_empty = 0, rebase_merges = 0, autosquash = 0;
-	int abbreviate_commands = 0, rebase_cousins = -1, ret = 0;
-	const char *onto_name = NULL, *head_name = NULL, *switch_to = NULL,
-		*cmd = NULL;
-	struct commit *onto = NULL, *upstream = NULL, *restrict_revision = NULL;
+	struct rebase_options opts = REBASE_OPTIONS_INIT;
+	unsigned flags = 0;
+	int abbreviate_commands = 0, ret = 0;
 	struct object_id squash_onto = null_oid;
-	struct object_id *squash_onto_opt = NULL;
 	struct string_list commands = STRING_LIST_INIT_DUP;
-	char *raw_strategies = NULL;
 	enum {
 		NONE = 0, CONTINUE, SKIP, EDIT_TODO, SHOW_CURRENT_PATCH,
 		SHORTEN_OIDS, EXPAND_OIDS, CHECK_TODO_LIST, REARRANGE_SQUASH, ADD_EXEC
 	} command = 0;
 	struct option options[] = {
-		OPT_BOOL(0, "ff", &opts.allow_ff, N_("allow fast-forward")),
-		OPT_BOOL(0, "keep-empty", &keep_empty, N_("keep empty commits")),
+		OPT_NEGBIT(0, "ff", &opts.flags, N_("allow fast-forward"),
+			   REBASE_FORCE),
+		OPT_BOOL(0, "keep-empty", &opts.keep_empty, N_("keep empty commits")),
 		OPT_BOOL(0, "allow-empty-message", &opts.allow_empty_message,
 			 N_("allow commits with empty messages")),
-		OPT_BOOL(0, "rebase-merges", &rebase_merges, N_("rebase merge commits")),
-		OPT_BOOL(0, "rebase-cousins", &rebase_cousins,
+		OPT_BOOL(0, "rebase-merges", &opts.rebase_merges, N_("rebase merge commits")),
+		OPT_BOOL(0, "rebase-cousins", &opts.rebase_cousins,
 			 N_("keep original branch points of cousins")),
-		OPT_BOOL(0, "autosquash", &autosquash,
+		OPT_BOOL(0, "autosquash", &opts.autosquash,
 			 N_("move commits that begin with squash!/fixup!")),
 		OPT_BOOL(0, "signoff", &opts.signoff, N_("sign commits")),
-		OPT__VERBOSE(&opts.verbose, N_("be verbose")),
+		OPT_BIT('v', "verbose", &opts.flags,
+			N_("display a diffstat of what changed upstream"),
+			REBASE_NO_QUIET | REBASE_VERBOSE | REBASE_DIFFSTAT),
 		OPT_CMDMODE(0, "continue", &command, N_("continue rebase"),
 			    CONTINUE),
 		OPT_CMDMODE(0, "skip", &command, N_("skip commit"), SKIP),
@@ -308,86 +373,86 @@  int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)
 			N_("rearrange fixup/squash lines"), REARRANGE_SQUASH),
 		OPT_CMDMODE(0, "add-exec-commands", &command,
 			N_("insert exec commands in todo list"), ADD_EXEC),
-		{ OPTION_CALLBACK, 0, "onto", &onto, N_("onto"), N_("onto"),
+		{ OPTION_CALLBACK, 0, "onto", &opts.onto, N_("onto"), N_("onto"),
 		  PARSE_OPT_NONEG, parse_opt_commit, 0 },
-		{ OPTION_CALLBACK, 0, "restrict-revision", &restrict_revision,
+		{ OPTION_CALLBACK, 0, "restrict-revision", &opts.restrict_revision,
 		  N_("restrict-revision"), N_("restrict revision"),
 		  PARSE_OPT_NONEG, parse_opt_commit, 0 },
 		{ OPTION_CALLBACK, 0, "squash-onto", &squash_onto, N_("squash-onto"),
 		  N_("squash onto"), PARSE_OPT_NONEG, parse_opt_object_id, 0 },
-		{ OPTION_CALLBACK, 0, "upstream", &upstream, N_("upstream"),
+		{ OPTION_CALLBACK, 0, "upstream", &opts.upstream, N_("upstream"),
 		  N_("the upstream commit"), PARSE_OPT_NONEG, parse_opt_commit,
 		  0 },
-		OPT_STRING(0, "head-name", &head_name, N_("head-name"), N_("head name")),
-		{ OPTION_STRING, 'S', "gpg-sign", &opts.gpg_sign, N_("key-id"),
+		OPT_STRING(0, "head-name", &opts.head_name, N_("head-name"), N_("head name")),
+		{ OPTION_STRING, 'S', "gpg-sign", &opts.gpg_sign_opt, N_("key-id"),
 			N_("GPG-sign commits"),
 			PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
 		OPT_STRING(0, "strategy", &opts.strategy, N_("strategy"),
 			   N_("rebase strategy")),
-		OPT_STRING(0, "strategy-opts", &raw_strategies, N_("strategy-opts"),
+		OPT_STRING(0, "strategy-opts", &opts.strategy_opts, N_("strategy-opts"),
 			   N_("strategy options")),
-		OPT_STRING(0, "switch-to", &switch_to, N_("switch-to"),
+		OPT_STRING(0, "switch-to", &opts.switch_to, N_("switch-to"),
 			   N_("the branch or commit to checkout")),
-		OPT_STRING(0, "onto-name", &onto_name, N_("onto-name"), N_("onto name")),
-		OPT_STRING(0, "cmd", &cmd, N_("cmd"), N_("the command to run")),
-		OPT_RERERE_AUTOUPDATE(&opts.allow_rerere_auto),
+		OPT_STRING(0, "onto-name", &opts.onto_name, N_("onto-name"), N_("onto name")),
+		OPT_STRING(0, "cmd", &opts.cmd, N_("cmd"), N_("the command to run")),
+		OPT_RERERE_AUTOUPDATE(&opts.allow_rerere_autoupdate),
 		OPT_BOOL(0, "reschedule-failed-exec", &opts.reschedule_failed_exec,
 			 N_("automatically re-schedule any `exec` that fails")),
 		OPT_END()
 	};
 
-	sequencer_init_config(&opts);
+	opts.rebase_cousins = -1;
+
 	git_config_get_bool("rebase.abbreviatecommands", &abbreviate_commands);
 
-	opts.action = REPLAY_INTERACTIVE_REBASE;
-	opts.allow_ff = 1;
-	opts.allow_empty = 1;
-
 	if (argc == 1)
 		usage_with_options(builtin_rebase_interactive_usage, options);
 
 	argc = parse_options(argc, argv, NULL, options,
 			builtin_rebase_interactive_usage, PARSE_OPT_KEEP_ARGV0);
 
-	opts.gpg_sign = xstrdup_or_null(opts.gpg_sign);
-
 	if (!is_null_oid(&squash_onto))
-		squash_onto_opt = &squash_onto;
+		opts.squash_onto = &squash_onto;
 
-	flags |= keep_empty ? TODO_LIST_KEEP_EMPTY : 0;
+	flags |= opts.keep_empty ? TODO_LIST_KEEP_EMPTY : 0;
 	flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0;
-	flags |= rebase_merges ? TODO_LIST_REBASE_MERGES : 0;
-	flags |= rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0;
+	flags |= opts.rebase_merges ? TODO_LIST_REBASE_MERGES : 0;
+	flags |= opts.rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0;
 	flags |= command == SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0;
 
-	if (rebase_cousins >= 0 && !rebase_merges)
+	if (opts.rebase_cousins >= 0 && !opts.rebase_merges)
 		warning(_("--[no-]rebase-cousins has no effect without "
 			  "--rebase-merges"));
 
-	if (cmd && *cmd) {
-		string_list_split(&commands, cmd, '\n', -1);
+	if (opts.cmd && *opts.cmd) {
+		string_list_split(&commands, opts.cmd, '\n', -1);
 
 		/* rebase.c adds a new line to cmd after every command,
 		 * so here the last command is always empty */
 		string_list_remove_empty_items(&commands, 0);
 	}
 
 	switch (command) {
-	case NONE:
-		if (!onto && !upstream)
+	case NONE: {
+		struct replay_opts replay_opts = get_replay_opts(&opts);
+		if (!opts.onto && !opts.upstream)
 			die(_("a base commit must be provided with --upstream or --onto"));
 
-		ret = do_interactive_rebase(&opts, flags, switch_to, upstream, onto,
-					    onto_name, squash_onto_opt, head_name, restrict_revision,
-					    raw_strategies, &commands, autosquash);
+		ret = do_interactive_rebase(&replay_opts, flags, opts.switch_to, opts.upstream, opts.onto,
+					    opts.onto_name, opts.squash_onto, opts.head_name, opts.restrict_revision,
+					    opts.strategy_opts, &commands, opts.autosquash);
 		break;
+	}
 	case SKIP: {
 		struct string_list merge_rr = STRING_LIST_INIT_DUP;
 
 		rerere_clear(the_repository, &merge_rr);
+	}
 		/* fallthrough */
-	case CONTINUE:
-		ret = sequencer_continue(the_repository, &opts);
+	case CONTINUE: {
+		struct replay_opts replay_opts = get_replay_opts(&opts);
+
+		ret = sequencer_continue(the_repository, &replay_opts);
 		break;
 	}
 	case EDIT_TODO:
@@ -446,45 +511,6 @@  static int use_builtin_rebase(void)
 	return ret;
 }
 
-struct rebase_options {
-	enum rebase_type type;
-	const char *state_dir;
-	struct commit *upstream;
-	const char *upstream_name;
-	const char *upstream_arg;
-	char *head_name;
-	struct object_id orig_head;
-	struct commit *onto;
-	const char *onto_name;
-	const char *revisions;
-	const char *switch_to;
-	int root;
-	struct object_id *squash_onto;
-	struct commit *restrict_revision;
-	int dont_finish_rebase;
-	enum {
-		REBASE_NO_QUIET = 1<<0,
-		REBASE_VERBOSE = 1<<1,
-		REBASE_DIFFSTAT = 1<<2,
-		REBASE_FORCE = 1<<3,
-		REBASE_INTERACTIVE_EXPLICIT = 1<<4,
-	} flags;
-	struct argv_array git_am_opts;
-	const char *action;
-	int signoff;
-	int allow_rerere_autoupdate;
-	int keep_empty;
-	int autosquash;
-	char *gpg_sign_opt;
-	int autostash;
-	char *cmd;
-	int allow_empty_message;
-	int rebase_merges, rebase_cousins;
-	char *strategy, *strategy_opts;
-	struct strbuf git_format_patch_opt;
-	int reschedule_failed_exec;
-};
-
 static int is_interactive(struct rebase_options *opts)
 {
 	return opts->type == REBASE_INTERACTIVE ||
@@ -1380,13 +1406,7 @@  static int check_exec_cmd(const char *cmd)
 
 int cmd_rebase(int argc, const char **argv, const char *prefix)
 {
-	struct rebase_options options = {
-		.type = REBASE_UNSPECIFIED,
-		.flags = REBASE_NO_QUIET,
-		.git_am_opts = ARGV_ARRAY_INIT,
-		.allow_empty_message = 1,
-		.git_format_patch_opt = STRBUF_INIT,
-	};
+	struct rebase_options options = REBASE_OPTIONS_INIT;
 	const char *branch_name;
 	int ret, flags, total_argc, in_progress = 0;
 	int ok_to_skip_pre_rebase = 0;
@@ -1540,6 +1560,7 @@  int cmd_rebase(int argc, const char **argv, const char *prefix)
 	trace_repo_setup(prefix);
 	setup_work_tree();
 
+	options.allow_empty_message = 1;
 	git_config(rebase_config, &options);
 
 	strbuf_reset(&buf);