Hi Elijah,
On Wed, 27 Jun 2018, Elijah Newren wrote:
> git-rebase.sh wrote strategy options to .git/rebase/merge/strategy_opts
> in the following format:
> '--ours' '--renormalize'
> Note the double spaces.
>
> git-rebase--interactive uses sequencer.c to parse that file, and
> sequencer.c used split_cmdline() to get the individual strategy options.
> After splitting, sequencer.c prefixed each "option" with a double dash,
> so, concatenating all its options would result in:
> -- --ours -- --renormalize
>
> So, when it ended up calling try_merge_strategy(), that in turn would run
> git merge-$strategy -- --ours -- --renormalize $merge_base -- $head $remote
>
> instead of the expected/desired
> git merge-$strategy --ours --renormalize $merge_base -- $head $remote
>
> Remove the extra spaces so that when it goes through split_cmdline() we end
> up with the desired command line.
>
> Signed-off-by: Elijah Newren <newren@xxxxxxxxx>
> ---
> git-rebase.sh | 2 +-
> sequencer.c | 7 ++++++-
> t/t3418-rebase-continue.sh | 2 +-
> 3 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/git-rebase.sh b/git-rebase.sh
> index 19bdebb480..f3b10c7f62 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -328,7 +328,7 @@ do
> do_merge=t
> ;;
> --strategy-option=*)
> - strategy_opts="$strategy_opts $(git rev-parse --sq-quote "--${1#--strategy-option=}")"
> + strategy_opts="$strategy_opts $(git rev-parse --sq-quote "--${1#--strategy-option=}" | sed -e s/^.//)"
Didn't you mean to use "s/^ *//" instead?
> do_merge=t
> test -z "$strategy" && strategy=recursive
> ;;
> diff --git a/sequencer.c b/sequencer.c
> index 5354d4d51e..ef9237c814 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -2206,6 +2206,7 @@ static int populate_opts_cb(const char *key, const char *value, void *data)
> static void read_strategy_opts(struct replay_opts *opts, struct strbuf *buf)
> {
> int i;
> + char *strategy_opts_string;
>
> strbuf_reset(buf);
> if (!read_oneliner(buf, rebase_path_strategy(), 0))
> @@ -2214,7 +2215,11 @@ static void read_strategy_opts(struct replay_opts *opts, struct strbuf *buf)
> if (!read_oneliner(buf, rebase_path_strategy_opts(), 0))
> return;
>
> - opts->xopts_nr = split_cmdline(buf->buf, (const char ***)&opts->xopts);
> + strategy_opts_string = buf->buf;
> + if (*strategy_opts_string == ' ')
I think that this would ideally even be a `while` instead of an `if`.
> + strategy_opts_string++;
> + opts->xopts_nr = split_cmdline(strategy_opts_string,
> + (const char ***)&opts->xopts);
> for (i = 0; i < opts->xopts_nr; i++) {
> const char *arg = opts->xopts[i];
>
> diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
> index 11546d6e14..c145dbac38 100755
> --- a/t/t3418-rebase-continue.sh
> +++ b/t/t3418-rebase-continue.sh
> @@ -74,7 +74,7 @@ test_expect_success 'rebase --continue remembers merge strategy and options' '
> test -f funny.was.run
> '
>
> -test_expect_failure 'rebase -i --continue handles merge strategy and options' '
> +test_expect_success 'rebase -i --continue handles merge strategy and options' '
> rm -fr .git/rebase-* &&
> git reset --hard commit-new-file-F2-on-topic-branch &&
> test_commit "commit-new-file-F3-on-topic-branch-for-dash-i" F3 32 &&
Thanks!
Dscho
