diff mbox series

[GSoC,v2,1/1] rebase -i: add --ignore-whitespace flag

Message ID 20190718185514.20108-2-rohit.ashiwal265@gmail.com (mailing list archive)
State New, archived
Headers show
Series rebase -i: support --ignore-whitespace | expand

Commit Message

Rohit Ashiwal July 18, 2019, 6:55 p.m. UTC
There are two backends available for rebasing, viz, the am and the
interactive. Naturally, there shall be some features that are
implemented in one but not in the other. One such flag is
--ignore-whitespace which indicates merge mechanism to treat lines
with only whitespace changes as unchanged. Wire the interactive
rebase to also understand the --ignore-whitespace flag by
translating it to -Xignore-space-change.

Signed-off-by: Rohit Ashiwal <rohit.ashiwal265@gmail.com>
---
 Documentation/git-rebase.txt            |  9 +++-
 builtin/rebase.c                        | 26 ++++++++--
 sequencer.h                             |  1 +
 t/t3422-rebase-incompatible-options.sh  |  1 -
 t/t3431-rebase-options-compatibility.sh | 66 +++++++++++++++++++++++++
 5 files changed, 97 insertions(+), 6 deletions(-)
 create mode 100755 t/t3431-rebase-options-compatibility.sh

Comments

Junio C Hamano July 19, 2019, 9:33 p.m. UTC | #1
Rohit Ashiwal <rohit.ashiwal265@gmail.com> writes:

>  t/t3431-rebase-options-compatibility.sh | 66 +++++++++++++++++++++++++

Isn't 3431 already taken?

>  5 files changed, 97 insertions(+), 6 deletions(-)
>  create mode 100755 t/t3431-rebase-options-compatibility.sh
> ...
> +	git checkout --orphan master &&
> +	q_to_tab >file <<-EOF &&
> +	line 1
> +	        line 2
> +	line 3
> +	EOF

This will trigger "indent-with-space".  Instead of using q-to-tab,
perhaps something like this would be more appropriate (not limited
to this piece, but also other ones in this script that actually do
use Q to visualize a tab)?

	sed -e "s/^|//" >file <<-EOF &&
	|line 1
	|        line 2
	|line 3
	EOF
Phillip Wood July 22, 2019, 10 a.m. UTC | #2
Hi Rohit,

It's good to see another patch reducing the differences between the
rebase back ends.

On 18/07/2019 19:55, Rohit Ashiwal wrote:
> There are two backends available for rebasing, viz, the am and the
> interactive. Naturally, there shall be some features that are
> implemented in one but not in the other. One such flag is
> --ignore-whitespace which indicates merge mechanism to treat lines
> with only whitespace changes as unchanged. Wire the interactive
> rebase to also understand the --ignore-whitespace flag by
> translating it to -Xignore-space-change.
> 
> Signed-off-by: Rohit Ashiwal <rohit.ashiwal265@gmail.com>
> ---
>  Documentation/git-rebase.txt            |  9 +++-
>  builtin/rebase.c                        | 26 ++++++++--
>  sequencer.h                             |  1 +
>  t/t3422-rebase-incompatible-options.sh  |  1 -
>  t/t3431-rebase-options-compatibility.sh | 66 +++++++++++++++++++++++++
>  5 files changed, 97 insertions(+), 6 deletions(-)
>  create mode 100755 t/t3431-rebase-options-compatibility.sh
> 
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 5e4e927647..eda52ed824 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -371,8 +371,13 @@ If either <upstream> or --root is given on the command line, then the
>  default is `--no-fork-point`, otherwise the default is `--fork-point`.
>  
>  --ignore-whitespace::
> +	This flag is either passed to the 'git apply' program
> +	(see linkgit:git-apply[1]), or to 'git merge' program
> +	(see linkgit:git-merge[1]) as `-Xignore-space-change`,
> +	depending on which backend is selected by other options.
> +
>  --whitespace=<option>::
> -	These flag are passed to the 'git apply' program
> +	This flag is passed to the 'git apply' program
>  	(see linkgit:git-apply[1]) that applies the patch.
>  +
>  See also INCOMPATIBLE OPTIONS below.
> @@ -520,7 +525,6 @@ The following options:
>   * --committer-date-is-author-date
>   * --ignore-date
>   * --whitespace
> - * --ignore-whitespace
>   * -C
>  
>  are incompatible with the following options:
> @@ -543,6 +547,7 @@ In addition, the following pairs of options are incompatible:
>   * --preserve-merges and --interactive
>   * --preserve-merges and --signoff
>   * --preserve-merges and --rebase-merges
> + * --rebase-merges and --ignore-whitespace
>   * --rebase-merges and --strategy
>   * --rebase-merges and --strategy-option
>  
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index db6ca9bd7d..afe376c3fe 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -79,6 +79,7 @@ struct rebase_options {
>  	int allow_rerere_autoupdate;
>  	int keep_empty;
>  	int autosquash;
> +	int ignore_whitespace;
>  	char *gpg_sign_opt;
>  	int autostash;
>  	char *cmd;
> @@ -376,6 +377,17 @@ static int run_rebase_interactive(struct rebase_options *opts,
>  	flags |= opts->rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0;
>  	flags |= command == ACTION_SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0;
>  
> +	if (opts->ignore_whitespace) {
> +		struct strbuf buf = STRBUF_INIT;
> +
> +		if (opts->strategy_opts)
> +			strbuf_addstr(&buf, opts->strategy_opts);
> +
> +		strbuf_addstr(&buf, " --ignore-space-change");
> +		free(opts->strategy_opts);
> +		opts->strategy_opts = strbuf_detach(&buf, NULL);
> +	}
> +

I think this would fit better in get_replay_opts()

>  	switch (command) {
>  	case ACTION_NONE: {
>  		if (!opts->onto && !opts->upstream)
> @@ -489,6 +501,8 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)
>  		{ OPTION_STRING, 'S', "gpg-sign", &opts.gpg_sign_opt, N_("key-id"),
>  			N_("GPG-sign commits"),
>  			PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
> +		OPT_BOOL(0, "ignore-whitespace", &opts.ignore_whitespace,
> +			 N_("ignore changes in whitespace")),

As with the other patch is this actually going to be used by
rebase--preserve-merges.sh?

>  		OPT_STRING(0, "strategy", &opts.strategy, N_("strategy"),
>  			   N_("rebase strategy")),
>  		OPT_STRING(0, "strategy-opts", &opts.strategy_opts, N_("strategy-opts"),
> @@ -511,6 +525,8 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)
>  	argc = parse_options(argc, argv, NULL, options,
>  			builtin_rebase_interactive_usage, PARSE_OPT_KEEP_ARGV0);
>  
> +	opts.strategy_opts = xstrdup_or_null(opts.strategy_opts);
> +
>  	if (!is_null_oid(&squash_onto))
>  		opts.squash_onto = &squash_onto;
>  
> @@ -954,6 +970,8 @@ static int run_am(struct rebase_options *opts)
>  	am.git_cmd = 1;
>  	argv_array_push(&am.args, "am");
>  
> +	if (opts->ignore_whitespace)
> +		argv_array_push(&am.args, "--ignore-whitespace");
>  	if (opts->action && !strcmp("continue", opts->action)) {
>  		argv_array_push(&am.args, "--resolved");
>  		argv_array_pushf(&am.args, "--resolvemsg=%s", resolvemsg);
> @@ -1401,9 +1419,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>  			PARSE_OPT_NOARG, NULL, REBASE_DIFFSTAT },
>  		OPT_BOOL(0, "signoff", &options.signoff,
>  			 N_("add a Signed-off-by: line to each commit")),
> -		OPT_PASSTHRU_ARGV(0, "ignore-whitespace", &options.git_am_opts,
> -				  NULL, N_("passed to 'git am'"),
> -				  PARSE_OPT_NOARG),
>  		OPT_PASSTHRU_ARGV(0, "committer-date-is-author-date",
>  				  &options.git_am_opts, NULL,
>  				  N_("passed to 'git am'"), PARSE_OPT_NOARG),
> @@ -1411,6 +1426,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>  				  N_("passed to 'git am'"), PARSE_OPT_NOARG),
>  		OPT_PASSTHRU_ARGV('C', NULL, &options.git_am_opts, N_("n"),
>  				  N_("passed to 'git apply'"), 0),
> +		OPT_BOOL(0, "ignore-whitespace", &options.ignore_whitespace,
> +			 N_("ignore changes in whitespace")),
>  		OPT_PASSTHRU_ARGV(0, "whitespace", &options.git_am_opts,
>  				  N_("action"), N_("passed to 'git apply'"), 0),
>  		OPT_BIT('f', "force-rebase", &options.flags,
> @@ -1821,6 +1838,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>  	}
>  
>  	if (options.rebase_merges) {
> +		if (options.ignore_whitespace)
> +			die(_("cannot combine '--rebase-merges' with "
> +			      "'--ignore-whitespace'"));

I was going to ask why we cannot just pass -Xignore-space-change when
making the merge but the lines below seem to show we don't support merge
options yet.

>  		if (strategy_options.nr)
>  			die(_("cannot combine '--rebase-merges' with "
>  			      "'--strategy-option'"));
> diff --git a/sequencer.h b/sequencer.h
> index 0c494b83d4..303047a133 100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -43,6 +43,7 @@ struct replay_opts {
>  	int verbose;
>  	int quiet;
>  	int reschedule_failed_exec;
> +	int ignore_whitespace;

Is this new field used anywhere - we add -Xignore-space-change to
replay_opts.xopts so why do we need this as well?

Best Wishes

Phillip

>  
>  	int mainline;
>  
> diff --git a/t/t3422-rebase-incompatible-options.sh b/t/t3422-rebase-incompatible-options.sh
> index a5868ea152..4342f79eea 100755
> --- a/t/t3422-rebase-incompatible-options.sh
> +++ b/t/t3422-rebase-incompatible-options.sh
> @@ -61,7 +61,6 @@ test_rebase_am_only () {
>  }
>  
>  test_rebase_am_only --whitespace=fix
> -test_rebase_am_only --ignore-whitespace
>  test_rebase_am_only --committer-date-is-author-date
>  test_rebase_am_only -C4
>  
> diff --git a/t/t3431-rebase-options-compatibility.sh b/t/t3431-rebase-options-compatibility.sh
> new file mode 100755
> index 0000000000..f38ae6f5fc
> --- /dev/null
> +++ b/t/t3431-rebase-options-compatibility.sh
> @@ -0,0 +1,66 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2019 Rohit Ashiwal
> +#
> +
> +test_description='tests to ensure compatibility between am and interactive backends'
> +
> +. ./test-lib.sh
> +
> +# This is a special case in which both am and interactive backends
> +# provide the same outputs. It was done intentionally because
> +# --ignore-whitespace both the backends fall short of optimal
> +# behaviour.
> +test_expect_success 'setup' '
> +	git checkout -b topic &&
> +	q_to_tab >file <<-EOF &&
> +	line 1
> +	Qline 2
> +	line 3
> +	EOF
> +	git add file &&
> +	git commit -m "add file" &&
> +	q_to_tab >file <<-EOF &&
> +	line 1
> +	new line 2
> +	line 3
> +	EOF
> +	git commit -am "update file" &&
> +	git tag side &&
> +
> +	git checkout --orphan master &&
> +	q_to_tab >file <<-EOF &&
> +	line 1
> +	        line 2
> +	line 3
> +	EOF
> +	git add file &&
> +	git commit -m "add file" &&
> +	git tag main
> +'
> +
> +test_expect_success '--ignore-whitespace works with am backend' '
> +	cat >expect <<-EOF &&
> +	line 1
> +	new line 2
> +	line 3
> +	EOF
> +	test_must_fail git rebase main side &&
> +	git rebase --abort &&
> +	git rebase --ignore-whitespace main side &&
> +	test_cmp expect file
> +'
> +
> +test_expect_success '--ignore-whitespace works with interactive backend' '
> +	cat >expect <<-EOF &&
> +	line 1
> +	new line 2
> +	line 3
> +	EOF
> +	test_must_fail git rebase --merge main side &&
> +	git rebase --abort &&
> +	git rebase --merge --ignore-whitespace main side &&
> +	test_cmp expect file
> +'
> +
> +test_done
>
Rohit Ashiwal July 23, 2019, 7:58 p.m. UTC | #3
Hi Phillip

On Mon, 22 Jul 2019 11:00:40 +0100 Phillip Wood <phillip.wood123@gmail.com> wrote:
> 
> [...]
> >
> > +	if (opts->ignore_whitespace) {
> > +		struct strbuf buf = STRBUF_INIT;
> > +
> > +		if (opts->strategy_opts)
> > +			strbuf_addstr(&buf, opts->strategy_opts);
> > +
> > +		strbuf_addstr(&buf, " --ignore-space-change");
> > +		free(opts->strategy_opts);
> > +		opts->strategy_opts = strbuf_detach(&buf, NULL);
> > +	}
> > +
> 
> I think this would fit better in get_replay_opts()

Agreed, I'll move this to get_replay_opts().

> [...]
> 
> > @@ -489,6 +501,8 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)
> >  		{ OPTION_STRING, 'S', "gpg-sign", &opts.gpg_sign_opt, N_("key-id"),
> >  			N_("GPG-sign commits"),
> >  			PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
> > +		OPT_BOOL(0, "ignore-whitespace", &opts.ignore_whitespace,
> > +			 N_("ignore changes in whitespace")),
> 
> As with the other patch is this actually going to be used by
> rebase--preserve-merges.sh?

I added this just for the completness. Is there any discussion on
dropping rebase--interactive as command and may be lib'fying it while
deprecating rebase--preserve-merges?

> [...]
> 
> > @@ -43,6 +43,7 @@ struct replay_opts {
> >  	int verbose;
> >  	int quiet;
> >  	int reschedule_failed_exec;
> > +	int ignore_whitespace;
> 
> Is this new field used anywhere - we add -Xignore-space-change to
> replay_opts.xopts so why do we need this as well?

Ah! I just realised cmd_rebase__interactive use rebase_options and
not replay_options. I too think this field is not required.

Thanks
Rohit
Rohit Ashiwal July 23, 2019, 7:59 p.m. UTC | #4
Hi Junio

On Fri, 19 Jul 2019 14:33:49 -0700 Junio C Hamano <gitster@pobox.com> wrote:
> 
> >  t/t3431-rebase-options-compatibility.sh | 66 +++++++++++++++++++++++++
> 
> Isn't 3431 already taken?

It is not in git/git[1].

> >  5 files changed, 97 insertions(+), 6 deletions(-)
> >  create mode 100755 t/t3431-rebase-options-compatibility.sh
> > ...
> > +	git checkout --orphan master &&
> > +	q_to_tab >file <<-EOF &&
> > +	line 1
> > +	        line 2
> > +	line 3
> > +	EOF
> 
> This will trigger "indent-with-space".  Instead of using q-to-tab,
> perhaps something like this would be more appropriate (not limited
> to this piece, but also other ones in this script that actually do
> use Q to visualize a tab)?
> 
> 	sed -e "s/^|//" >file <<-EOF &&
> 	|line 1
> 	|        line 2
> 	|line 3
> 	EOF
> 

Oh! My mistake, I should have used cat instead.

Thanks
Rohit

[1]: https://github.com/git/git/tree/master/t
Junio C Hamano July 23, 2019, 8:57 p.m. UTC | #5
Rohit Ashiwal <rohit.ashiwal265@gmail.com> writes:

> Hi Junio
>
> On Fri, 19 Jul 2019 14:33:49 -0700 Junio C Hamano <gitster@pobox.com> wrote:
>> 
>> >  t/t3431-rebase-options-compatibility.sh | 66 +++++++++++++++++++++++++
>> 
>> Isn't 3431 already taken?
>
> It is not in git/git[1].

That's a very selfish attitude ;-)  Pay attention to what others are
doing while you are working on this topic (hint: do not limit your
check to 'master', but make sure your changes also work well when
merged to 'next' and 'pu'---make a trial merge or two and run test
suite, for example).

>> > +	git checkout --orphan master &&
>> > +	q_to_tab >file <<-EOF &&
>> > +	line 1
>> > +	        line 2
>> > +	line 3
>> > +	EOF
>> 
>> This will trigger "indent-with-space".  Instead of using q-to-tab,
>> perhaps something like this would be more appropriate (not limited
>> to this piece, but also other ones in this script that actually do
>> use Q to visualize a tab)?
>> 
>> 	sed -e "s/^|//" >file <<-EOF &&
>> 	|line 1
>> 	|        line 2
>> 	|line 3
>> 	EOF
>> 
>
> Oh! My mistake, I should have used cat instead.

I am not sure how you would avoid indent-with-space with "cat".
With the use of "sed" like I showed above to spell the left margin
explicitly out, we can---I am not saying it's the only way, but I do
not think of a clean way to do the same with "cat".
Elijah Newren July 23, 2019, 9:01 p.m. UTC | #6
On Tue, Jul 23, 2019 at 1:01 PM Rohit Ashiwal
<rohit.ashiwal265@gmail.com> wrote:
> On Mon, 22 Jul 2019 11:00:40 +0100 Phillip Wood <phillip.wood123@gmail.com> wrote:
> > [...]
> >
> > > @@ -489,6 +501,8 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)
> > >             { OPTION_STRING, 'S', "gpg-sign", &opts.gpg_sign_opt, N_("key-id"),
> > >                     N_("GPG-sign commits"),
> > >                     PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
> > > +           OPT_BOOL(0, "ignore-whitespace", &opts.ignore_whitespace,
> > > +                    N_("ignore changes in whitespace")),
> >
> > As with the other patch is this actually going to be used by
> > rebase--preserve-merges.sh?
>
> I added this just for the completness. Is there any discussion on
> dropping rebase--interactive as command and may be lib'fying it while
> deprecating rebase--preserve-merges?

preserve-merges is already deprecated, see
https://public-inbox.org/git/pull.158.git.gitgitgadget@gmail.com/ and
the output of:
   git grep deprecated -- '*rebase*'

It's also instructive to take a look at
https://public-inbox.org/git/xmqqk1rrm8ic.fsf@gitster-ct.c.googlers.com/,
which talks about how git-rebase--preserve-merges.sh came to be; from
reading that, it looks like the whole point of making
rebase--preserve-merges.sh a separate script was to avoid the effort
needed to libify it.  As such, you probably just want to avoid
breaking it or even changing it at all until it can be deleted.
Anything that only the preserve-rebases backend uses (such as direct
invocations of rebase--interactive, according to what Phillip said
elsewhere in this thread), are probably better left alone as much as
possible, with us instead documenting that preserve-merges lacks any
new capabilities you are otherwise adding to rebase.

Not sure if that answers all your questions, though.
Johannes Schindelin July 24, 2019, 11:14 a.m. UTC | #7
Hi Elijah & Rohit,

On Tue, 23 Jul 2019, Elijah Newren wrote:

> On Tue, Jul 23, 2019 at 1:01 PM Rohit Ashiwal
> <rohit.ashiwal265@gmail.com> wrote:
> > On Mon, 22 Jul 2019 11:00:40 +0100 Phillip Wood <phillip.wood123@gmail.com> wrote:
> > > [...]
> > >
> > > > @@ -489,6 +501,8 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)
> > > >             { OPTION_STRING, 'S', "gpg-sign", &opts.gpg_sign_opt, N_("key-id"),
> > > >                     N_("GPG-sign commits"),
> > > >                     PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
> > > > +           OPT_BOOL(0, "ignore-whitespace", &opts.ignore_whitespace,
> > > > +                    N_("ignore changes in whitespace")),
> > >
> > > As with the other patch is this actually going to be used by
> > > rebase--preserve-merges.sh?
> >
> > I added this just for the completness. Is there any discussion on
> > dropping rebase--interactive as command and may be lib'fying it while
> > deprecating rebase--preserve-merges?
>
> preserve-merges is already deprecated, see
> https://public-inbox.org/git/pull.158.git.gitgitgadget@gmail.com/ and
> the output of:
>    git grep deprecated -- '*rebase*'
>
> It's also instructive to take a look at
> https://public-inbox.org/git/xmqqk1rrm8ic.fsf@gitster-ct.c.googlers.com/,
> which talks about how git-rebase--preserve-merges.sh came to be; from
> reading that, it looks like the whole point of making
> rebase--preserve-merges.sh a separate script was to avoid the effort
> needed to libify it.  As such, you probably just want to avoid
> breaking it or even changing it at all until it can be deleted.
> Anything that only the preserve-rebases backend uses (such as direct
> invocations of rebase--interactive, according to what Phillip said
> elsewhere in this thread), are probably better left alone as much as
> possible, with us instead documenting that preserve-merges lacks any
> new capabilities you are otherwise adding to rebase.

Indeed, libifying or even as much as enhancing it is a lot of love lost
at this stage.

> Not sure if that answers all your questions, though.

I am sure that Rohit has a lot more questions that are not answered by
this email (maybe even "What is the meaning of life?"), but one
particular (not quite asked) question that I would like to answer here
is: what does it take to actually drop `--preserve-merges`?

To answer that question, I prepared the `drop-rebase-p` branch at
https://github.com/dscho/git/commits/drop-rebase-p, essentially dropping
`git-rebase--preserve--merges.sh` and addressing the fall-out.

The biggest issue seems to be `--rebase-merges`' lack of support for
merge strategies other than the default, recursive merge one. To address
that, I implemented this patch:
https://github.com/dscho/git/commit/afa39b4fbb3ef5a53ccb9ae1f4376be87f570110

There are a couple other patches that need to broken out into their own
patch series, I just did not get to that yet.

Ciao,
Dscho
diff mbox series

Patch

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 5e4e927647..eda52ed824 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -371,8 +371,13 @@  If either <upstream> or --root is given on the command line, then the
 default is `--no-fork-point`, otherwise the default is `--fork-point`.
 
 --ignore-whitespace::
+	This flag is either passed to the 'git apply' program
+	(see linkgit:git-apply[1]), or to 'git merge' program
+	(see linkgit:git-merge[1]) as `-Xignore-space-change`,
+	depending on which backend is selected by other options.
+
 --whitespace=<option>::
-	These flag are passed to the 'git apply' program
+	This flag is passed to the 'git apply' program
 	(see linkgit:git-apply[1]) that applies the patch.
 +
 See also INCOMPATIBLE OPTIONS below.
@@ -520,7 +525,6 @@  The following options:
  * --committer-date-is-author-date
  * --ignore-date
  * --whitespace
- * --ignore-whitespace
  * -C
 
 are incompatible with the following options:
@@ -543,6 +547,7 @@  In addition, the following pairs of options are incompatible:
  * --preserve-merges and --interactive
  * --preserve-merges and --signoff
  * --preserve-merges and --rebase-merges
+ * --rebase-merges and --ignore-whitespace
  * --rebase-merges and --strategy
  * --rebase-merges and --strategy-option
 
diff --git a/builtin/rebase.c b/builtin/rebase.c
index db6ca9bd7d..afe376c3fe 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -79,6 +79,7 @@  struct rebase_options {
 	int allow_rerere_autoupdate;
 	int keep_empty;
 	int autosquash;
+	int ignore_whitespace;
 	char *gpg_sign_opt;
 	int autostash;
 	char *cmd;
@@ -376,6 +377,17 @@  static int run_rebase_interactive(struct rebase_options *opts,
 	flags |= opts->rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0;
 	flags |= command == ACTION_SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0;
 
+	if (opts->ignore_whitespace) {
+		struct strbuf buf = STRBUF_INIT;
+
+		if (opts->strategy_opts)
+			strbuf_addstr(&buf, opts->strategy_opts);
+
+		strbuf_addstr(&buf, " --ignore-space-change");
+		free(opts->strategy_opts);
+		opts->strategy_opts = strbuf_detach(&buf, NULL);
+	}
+
 	switch (command) {
 	case ACTION_NONE: {
 		if (!opts->onto && !opts->upstream)
@@ -489,6 +501,8 @@  int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)
 		{ OPTION_STRING, 'S', "gpg-sign", &opts.gpg_sign_opt, N_("key-id"),
 			N_("GPG-sign commits"),
 			PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
+		OPT_BOOL(0, "ignore-whitespace", &opts.ignore_whitespace,
+			 N_("ignore changes in whitespace")),
 		OPT_STRING(0, "strategy", &opts.strategy, N_("strategy"),
 			   N_("rebase strategy")),
 		OPT_STRING(0, "strategy-opts", &opts.strategy_opts, N_("strategy-opts"),
@@ -511,6 +525,8 @@  int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, NULL, options,
 			builtin_rebase_interactive_usage, PARSE_OPT_KEEP_ARGV0);
 
+	opts.strategy_opts = xstrdup_or_null(opts.strategy_opts);
+
 	if (!is_null_oid(&squash_onto))
 		opts.squash_onto = &squash_onto;
 
@@ -954,6 +970,8 @@  static int run_am(struct rebase_options *opts)
 	am.git_cmd = 1;
 	argv_array_push(&am.args, "am");
 
+	if (opts->ignore_whitespace)
+		argv_array_push(&am.args, "--ignore-whitespace");
 	if (opts->action && !strcmp("continue", opts->action)) {
 		argv_array_push(&am.args, "--resolved");
 		argv_array_pushf(&am.args, "--resolvemsg=%s", resolvemsg);
@@ -1401,9 +1419,6 @@  int cmd_rebase(int argc, const char **argv, const char *prefix)
 			PARSE_OPT_NOARG, NULL, REBASE_DIFFSTAT },
 		OPT_BOOL(0, "signoff", &options.signoff,
 			 N_("add a Signed-off-by: line to each commit")),
-		OPT_PASSTHRU_ARGV(0, "ignore-whitespace", &options.git_am_opts,
-				  NULL, N_("passed to 'git am'"),
-				  PARSE_OPT_NOARG),
 		OPT_PASSTHRU_ARGV(0, "committer-date-is-author-date",
 				  &options.git_am_opts, NULL,
 				  N_("passed to 'git am'"), PARSE_OPT_NOARG),
@@ -1411,6 +1426,8 @@  int cmd_rebase(int argc, const char **argv, const char *prefix)
 				  N_("passed to 'git am'"), PARSE_OPT_NOARG),
 		OPT_PASSTHRU_ARGV('C', NULL, &options.git_am_opts, N_("n"),
 				  N_("passed to 'git apply'"), 0),
+		OPT_BOOL(0, "ignore-whitespace", &options.ignore_whitespace,
+			 N_("ignore changes in whitespace")),
 		OPT_PASSTHRU_ARGV(0, "whitespace", &options.git_am_opts,
 				  N_("action"), N_("passed to 'git apply'"), 0),
 		OPT_BIT('f', "force-rebase", &options.flags,
@@ -1821,6 +1838,9 @@  int cmd_rebase(int argc, const char **argv, const char *prefix)
 	}
 
 	if (options.rebase_merges) {
+		if (options.ignore_whitespace)
+			die(_("cannot combine '--rebase-merges' with "
+			      "'--ignore-whitespace'"));
 		if (strategy_options.nr)
 			die(_("cannot combine '--rebase-merges' with "
 			      "'--strategy-option'"));
diff --git a/sequencer.h b/sequencer.h
index 0c494b83d4..303047a133 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -43,6 +43,7 @@  struct replay_opts {
 	int verbose;
 	int quiet;
 	int reschedule_failed_exec;
+	int ignore_whitespace;
 
 	int mainline;
 
diff --git a/t/t3422-rebase-incompatible-options.sh b/t/t3422-rebase-incompatible-options.sh
index a5868ea152..4342f79eea 100755
--- a/t/t3422-rebase-incompatible-options.sh
+++ b/t/t3422-rebase-incompatible-options.sh
@@ -61,7 +61,6 @@  test_rebase_am_only () {
 }
 
 test_rebase_am_only --whitespace=fix
-test_rebase_am_only --ignore-whitespace
 test_rebase_am_only --committer-date-is-author-date
 test_rebase_am_only -C4
 
diff --git a/t/t3431-rebase-options-compatibility.sh b/t/t3431-rebase-options-compatibility.sh
new file mode 100755
index 0000000000..f38ae6f5fc
--- /dev/null
+++ b/t/t3431-rebase-options-compatibility.sh
@@ -0,0 +1,66 @@ 
+#!/bin/sh
+#
+# Copyright (c) 2019 Rohit Ashiwal
+#
+
+test_description='tests to ensure compatibility between am and interactive backends'
+
+. ./test-lib.sh
+
+# This is a special case in which both am and interactive backends
+# provide the same outputs. It was done intentionally because
+# --ignore-whitespace both the backends fall short of optimal
+# behaviour.
+test_expect_success 'setup' '
+	git checkout -b topic &&
+	q_to_tab >file <<-EOF &&
+	line 1
+	Qline 2
+	line 3
+	EOF
+	git add file &&
+	git commit -m "add file" &&
+	q_to_tab >file <<-EOF &&
+	line 1
+	new line 2
+	line 3
+	EOF
+	git commit -am "update file" &&
+	git tag side &&
+
+	git checkout --orphan master &&
+	q_to_tab >file <<-EOF &&
+	line 1
+	        line 2
+	line 3
+	EOF
+	git add file &&
+	git commit -m "add file" &&
+	git tag main
+'
+
+test_expect_success '--ignore-whitespace works with am backend' '
+	cat >expect <<-EOF &&
+	line 1
+	new line 2
+	line 3
+	EOF
+	test_must_fail git rebase main side &&
+	git rebase --abort &&
+	git rebase --ignore-whitespace main side &&
+	test_cmp expect file
+'
+
+test_expect_success '--ignore-whitespace works with interactive backend' '
+	cat >expect <<-EOF &&
+	line 1
+	new line 2
+	line 3
+	EOF
+	test_must_fail git rebase --merge main side &&
+	git rebase --abort &&
+	git rebase --merge --ignore-whitespace main side &&
+	test_cmp expect file
+'
+
+test_done