{"thread":{"id":"47538","subject":"[PATCH] rebase -p: fix quoting when calling `git merge`","startedAt":"2018-01-04T21:32:17Z","lastAt":"2018-01-05T21:15:22Z","messageCount":3,"participants":["Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"335868","messageId":"8cafe900ce97d885c58f4a7f02e748d2ae3b02a2.1515101305.git.johannes.schindelin@gmx.de","threadId":"47538","inReplyTo":null,"subject":"[PATCH] rebase -p: fix quoting when calling `git merge`","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-01-04T21:31:52Z","receivedAt":"2018-01-04T21:32:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"It has been reported that strategy arguments are not passed to `git\nmerge` correctly when rebasing interactively, preserving merges.\n\nThe reason is that the strategy arguments are already quoted, and then\nquoted again.\n\nThis fixes https://github.com/git-for-windows/git/issues/1321\n\nOriginal-patch-by: Kim Gybels <kgybels@infogroep.be>\nAlso-reported-by: Matwey V. Kornilov <matwey.kornilov@gmail.com>\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n git-rebase--interactive.sh |  9 ++++++---\n t/t3418-rebase-continue.sh | 14 ++++++++++++++\n 2 files changed, 20 insertions(+), 3 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex e3f5a0abf3c..085aa068cbb 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -392,9 +392,12 @@ pick_one_preserving_merges () {\n \t\t\tnew_parents=${new_parents# $first_parent}\n \t\t\tmerge_args=\"--no-log --no-ff\"\n \t\t\tif ! do_with_author output eval \\\n-\t\t\t'git merge ${gpg_sign_opt:+\"$gpg_sign_opt\"} \\\n-\t\t\t\t$allow_rerere_autoupdate $merge_args \\\n-\t\t\t\t$strategy_args -m \"$msg_content\" $new_parents'\n+\t\t\t\tgit merge ${gpg_sign_opt:+$(git rev-parse \\\n+\t\t\t\t\t--sq-quote \"$gpg_sign_opt\")} \\\n+\t\t\t\t$allow_rerere_autoupdate \"$merge_args\" \\\n+\t\t\t\t\"$strategy_args\" \\\n+\t\t\t\t-m $(git rev-parse --sq-quote \"$msg_content\") \\\n+\t\t\t\t\"$new_parents\"\n \t\t\tthen\n \t\t\t\tprintf \"%s\\n\" \"$msg_content\" > \"$GIT_DIR\"/MERGE_MSG\n \t\t\t\tdie_with_patch $sha1 \"$(eval_gettext \"Error redoing merge \\$sha1\")\"\ndiff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh\nindex fcfdd197bd3..7c91a85f43a 100755\n--- a/t/t3418-rebase-continue.sh\n+++ b/t/t3418-rebase-continue.sh\n@@ -74,6 +74,20 @@ test_expect_success 'rebase --continue remembers merge strategy and options' '\n \ttest -f funny.was.run\n '\n \n+test_expect_success 'rebase passes merge strategy options correctly' '\n+\trm -fr .git/rebase-* &&\n+\tgit reset --hard commit-new-file-F3-on-topic-branch &&\n+\ttest_commit theirs-to-merge &&\n+\tgit reset --hard HEAD^ &&\n+\ttest_commit some-commit &&\n+\ttest_tick &&\n+\tgit merge --no-ff theirs-to-merge &&\n+\tFAKE_LINES=\"1 edit 2 3\" git rebase -i -f -p -m \\\n+\t\t-s recursive --strategy-option=theirs HEAD~2 &&\n+\ttest_commit force-change &&\n+\tgit rebase --continue\n+'\n+\n test_expect_success 'setup rerere database' '\n \trm -fr .git/rebase-* &&\n \tgit reset --hard commit-new-file-F3-on-topic-branch &&\n\nbase-commit: 1eaabe34fc6f486367a176207420378f587d3b48\n-- \n2.15.1.windows.2.391.g0b42e3c56de\n\nPublished-As: https://github.com/dscho/git/releases/tag/rebase-strategy-opts-v1\nFetch-It-Via: git fetch https://github.com/dscho/git rebase-strategy-opts-v1\n"},{"id":"335998","messageId":"xmqqvaggaw11.fsf@gitster.mtv.corp.google.com","threadId":"47538","inReplyTo":"8cafe900ce97d885c58f4a7f02e748d2ae3b02a2.1515101305.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH] rebase -p: fix quoting when calling `git merge`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-05T20:30:18Z","receivedAt":"2018-01-05T20:30:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <johannes.schindelin@gmx.de> writes:\n\n>  git-rebase--interactive.sh |  9 ++++++---\n>  t/t3418-rebase-continue.sh | 14 ++++++++++++++\n>  2 files changed, 20 insertions(+), 3 deletions(-)\n>\n> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> index e3f5a0abf3c..085aa068cbb 100644\n> --- a/git-rebase--interactive.sh\n> +++ b/git-rebase--interactive.sh\n> @@ -392,9 +392,12 @@ pick_one_preserving_merges () {\n>  \t\t\tnew_parents=${new_parents# $first_parent}\n>  \t\t\tmerge_args=\"--no-log --no-ff\"\n>  \t\t\tif ! do_with_author output eval \\\n> -\t\t\t'git merge ${gpg_sign_opt:+\"$gpg_sign_opt\"} \\\n> -\t\t\t\t$allow_rerere_autoupdate $merge_args \\\n> -\t\t\t\t$strategy_args -m \"$msg_content\" $new_parents'\n> +\t\t\t\tgit merge ${gpg_sign_opt:+$(git rev-parse \\\n> +\t\t\t\t\t--sq-quote \"$gpg_sign_opt\")} \\\n> +\t\t\t\t$allow_rerere_autoupdate \"$merge_args\" \\\n> +\t\t\t\t\"$strategy_args\" \\\n> +\t\t\t\t-m $(git rev-parse --sq-quote \"$msg_content\") \\\n> +\t\t\t\t\"$new_parents\"\n\nMakes sense.  I should have been more careful when reviewing\ndb2b3b820e2.\n\nThanks.\n"},{"id":"336012","messageId":"nycvar.QRO.7.76.6.1801052212000.1337@wbunaarf-fpuvaqryva.tvgsbejvaqbjf.bet","threadId":"47538","inReplyTo":"xmqqvaggaw11.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] rebase -p: fix quoting when calling `git merge`","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-01-05T21:14:59Z","receivedAt":"2018-01-05T21:15:22Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Fri, 5 Jan 2018, Junio C Hamano wrote:\n\n> Johannes Schindelin <johannes.schindelin@gmx.de> writes:\n> \n> >  git-rebase--interactive.sh |  9 ++++++---\n> >  t/t3418-rebase-continue.sh | 14 ++++++++++++++\n> >  2 files changed, 20 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> > index e3f5a0abf3c..085aa068cbb 100644\n> > --- a/git-rebase--interactive.sh\n> > +++ b/git-rebase--interactive.sh\n> > @@ -392,9 +392,12 @@ pick_one_preserving_merges () {\n> >  \t\t\tnew_parents=${new_parents# $first_parent}\n> >  \t\t\tmerge_args=\"--no-log --no-ff\"\n> >  \t\t\tif ! do_with_author output eval \\\n> > -\t\t\t'git merge ${gpg_sign_opt:+\"$gpg_sign_opt\"} \\\n> > -\t\t\t\t$allow_rerere_autoupdate $merge_args \\\n> > -\t\t\t\t$strategy_args -m \"$msg_content\" $new_parents'\n> > +\t\t\t\tgit merge ${gpg_sign_opt:+$(git rev-parse \\\n> > +\t\t\t\t\t--sq-quote \"$gpg_sign_opt\")} \\\n> > +\t\t\t\t$allow_rerere_autoupdate \"$merge_args\" \\\n> > +\t\t\t\t\"$strategy_args\" \\\n> > +\t\t\t\t-m $(git rev-parse --sq-quote \"$msg_content\") \\\n> > +\t\t\t\t\"$new_parents\"\n> \n> Makes sense.  I should have been more careful when reviewing\n> db2b3b820e2.\n\nDon't be so harsh on yourself. This bug was apparently not a big deal for\nfour years (which can feel like a *very* long time, can't it?).\n\nBesides, the way code review happens in this project is not really\nconducive to intense testing of the code: mere patch review will never be\nas effective as patch review combined with hands-on testing of code paths\nthat may look like they could hide some unexpected bugs.\n\nCiao,\nDscho\n"}]}