# [PATCH] rebase -p: fix quoting when calling `git merge` 3 messages from 2018-01-04 to 2018-01-05. Participants: Johannes Schindelin, Junio C Hamano. Thread: https://gitlist.dev/t/47538 ## Johannes Schindelin, 2018-01-04 21:31 Subject: [PATCH] rebase -p: fix quoting when calling `git merge` Message-ID: <8cafe900ce97d885c58f4a7f02e748d2ae3b02a2.1515101305.git.johannes.schindelin@gmx.de> URL: https://gitlist.dev/e/8cafe900ce97d885c58f4a7f02e748d2ae3b02a2.1515101305.git.johannes.schindelin%40gmx.de ``` It has been reported that strategy arguments are not passed to `git merge` correctly when rebasing interactively, preserving merges. The reason is that the strategy arguments are already quoted, and then quoted again. This fixes https://github.com/git-for-windows/git/issues/1321 Original-patch-by: Kim Gybels Also-reported-by: Matwey V. Kornilov Signed-off-by: Johannes Schindelin --- git-rebase--interactive.sh | 9 ++++++--- t/t3418-rebase-continue.sh | 14 ++++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index e3f5a0abf3c..085aa068cbb 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -392,9 +392,12 @@ pick_one_preserving_merges () { new_parents=${new_parents# $first_parent} merge_args="--no-log --no-ff" if ! do_with_author output eval \ - 'git merge ${gpg_sign_opt:+"$gpg_sign_opt"} \ - $allow_rerere_autoupdate $merge_args \ - $strategy_args -m "$msg_content" $new_parents' + git merge ${gpg_sign_opt:+$(git rev-parse \ + --sq-quote "$gpg_sign_opt")} \ + $allow_rerere_autoupdate "$merge_args" \ + "$strategy_args" \ + -m $(git rev-parse --sq-quote "$msg_content") \ + "$new_parents" then printf "%s\n" "$msg_content" > "$GIT_DIR"/MERGE_MSG die_with_patch $sha1 "$(eval_gettext "Error redoing merge \$sha1")" diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh index fcfdd197bd3..7c91a85f43a 100755 --- a/t/t3418-rebase-continue.sh +++ b/t/t3418-rebase-continue.sh @@ -74,6 +74,20 @@ test_expect_success 'rebase --continue remembers merge strategy and options' ' test -f funny.was.run ' +test_expect_success 'rebase passes merge strategy options correctly' ' + rm -fr .git/rebase-* && + git reset --hard commit-new-file-F3-on-topic-branch && + test_commit theirs-to-merge && + git reset --hard HEAD^ && + test_commit some-commit && + test_tick && + git merge --no-ff theirs-to-merge && + FAKE_LINES="1 edit 2 3" git rebase -i -f -p -m \ + -s recursive --strategy-option=theirs HEAD~2 && + test_commit force-change && + git rebase --continue +' + test_expect_success 'setup rerere database' ' rm -fr .git/rebase-* && git reset --hard commit-new-file-F3-on-topic-branch && base-commit: 1eaabe34fc6f486367a176207420378f587d3b48 -- 2.15.1.windows.2.391.g0b42e3c56de Published-As: https://github.com/dscho/git/releases/tag/rebase-strategy-opts-v1 Fetch-It-Via: git fetch https://github.com/dscho/git rebase-strategy-opts-v1 ``` ## Junio C Hamano, 2018-01-05 20:30 Subject: Re: [PATCH] rebase -p: fix quoting when calling `git merge` Message-ID: URL: https://gitlist.dev/e/xmqqvaggaw11.fsf%40gitster.mtv.corp.google.com In-Reply-To: <8cafe900ce97d885c58f4a7f02e748d2ae3b02a2.1515101305.git.johannes.schindelin@gmx.de> ``` Johannes Schindelin writes: > git-rebase--interactive.sh | 9 ++++++--- > t/t3418-rebase-continue.sh | 14 ++++++++++++++ > 2 files changed, 20 insertions(+), 3 deletions(-) > > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh > index e3f5a0abf3c..085aa068cbb 100644 > --- a/git-rebase--interactive.sh > +++ b/git-rebase--interactive.sh > @@ -392,9 +392,12 @@ pick_one_preserving_merges () { > new_parents=${new_parents# $first_parent} > merge_args="--no-log --no-ff" > if ! do_with_author output eval \ > - 'git merge ${gpg_sign_opt:+"$gpg_sign_opt"} \ > - $allow_rerere_autoupdate $merge_args \ > - $strategy_args -m "$msg_content" $new_parents' > + git merge ${gpg_sign_opt:+$(git rev-parse \ > + --sq-quote "$gpg_sign_opt")} \ > + $allow_rerere_autoupdate "$merge_args" \ > + "$strategy_args" \ > + -m $(git rev-parse --sq-quote "$msg_content") \ > + "$new_parents" Makes sense. I should have been more careful when reviewing db2b3b820e2. Thanks. ``` ## Johannes Schindelin, 2018-01-05 21:14 Subject: Re: [PATCH] rebase -p: fix quoting when calling `git merge` Message-ID: URL: https://gitlist.dev/e/nycvar.QRO.7.76.6.1801052212000.1337%40wbunaarf-fpuvaqryva.tvgsbejvaqbjf.bet In-Reply-To: ``` Hi Junio, On Fri, 5 Jan 2018, Junio C Hamano wrote: > Johannes Schindelin writes: > > > git-rebase--interactive.sh | 9 ++++++--- > > t/t3418-rebase-continue.sh | 14 ++++++++++++++ > > 2 files changed, 20 insertions(+), 3 deletions(-) > > > > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh > > index e3f5a0abf3c..085aa068cbb 100644 > > --- a/git-rebase--interactive.sh > > +++ b/git-rebase--interactive.sh > > @@ -392,9 +392,12 @@ pick_one_preserving_merges () { > > new_parents=${new_parents# $first_parent} > > merge_args="--no-log --no-ff" > > if ! do_with_author output eval \ > > - 'git merge ${gpg_sign_opt:+"$gpg_sign_opt"} \ > > - $allow_rerere_autoupdate $merge_args \ > > - $strategy_args -m "$msg_content" $new_parents' > > + git merge ${gpg_sign_opt:+$(git rev-parse \ > > + --sq-quote "$gpg_sign_opt")} \ > > + $allow_rerere_autoupdate "$merge_args" \ > > + "$strategy_args" \ > > + -m $(git rev-parse --sq-quote "$msg_content") \ > > + "$new_parents" > > Makes sense. I should have been more careful when reviewing > db2b3b820e2. Don't be so harsh on yourself. This bug was apparently not a big deal for four years (which can feel like a *very* long time, can't it?). Besides, the way code review happens in this project is not really conducive to intense testing of the code: mere patch review will never be as effective as patch review combined with hands-on testing of code paths that may look like they could hide some unexpected bugs. Ciao, Dscho ```