{"thread":{"id":"27148","subject":"[PATCH] git-rebase--interactive.sh: preserve-merges fails on merges created with no-ff","startedAt":"2011-04-21T03:38:00Z","lastAt":"2011-04-28T04:35:55Z","messageCount":6,"participants":["Andrew Wong","Junio C Hamano","Christian Couder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"166178","messageId":"1303357080-25840-1-git-send-email-andrew.kw.w@gmail.com","threadId":"27148","inReplyTo":null,"subject":"[PATCH] git-rebase--interactive.sh: preserve-merges fails on merges created with no-ff","fromName":"Andrew Wong","fromEmail":"andrew.kw.w@gmail.com","sentAt":"2011-04-21T03:38:00Z","receivedAt":"2011-04-21T03:38:00Z","isPatch":true,"sender":{"key":"andrew.kw.w@gmail.com","avatar":"https://avatars.githubusercontent.com/u/489311?v=4"},"body":"'git rebase' uses 'git merge' to preserve merges (-p).  This preserves\nthe original merge commit correctly, except when the original merge\ncommit was created by 'git merge --no-ff'.  In this case, 'git rebase'\nwill fail to preserve the merge, because during 'git rebase', 'git\nmerge' will simply fast-forward and skip the commit.  For example:\n\n               B\n              / \\\n             A---M\n            /\n    ---o---O---P---Q\n\nIf we try to rebase M onto P, we lose the merge commit and this happens:\n\n                 A---B\n                /\n    ---o---O---P---Q\n\nTo correct this, we simply do a \"no fast-forward\" on all merge commits\nwhen rebasing.  Since by the time we decided to do a 'git merge' inside\n'git rebase', it means there was a merge originally, so 'git merge'\nshould always create a merge commit regardless of what the merge\nbranches look like. This way, when rebase M onto P from the above\nexample, we get:\n\n                   B\n                  / \\\n                 A---M\n                /\n    ---o---O---P---Q\n\nSigned-off-by: Andrew Wong <andrew.kw.w@gmail.com>\n---\n git-rebase--interactive.sh |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 5873ba4..c308529 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -339,7 +339,7 @@ pick_one_preserving_merges () {\n \t\t\t# No point in merging the first parent, that's HEAD\n \t\t\tnew_parents=${new_parents# $first_parent}\n \t\t\tif ! do_with_author output \\\n-\t\t\t\tgit merge $STRATEGY -m \"$msg\" $new_parents\n+\t\t\t\tgit merge --no-ff $STRATEGY -m \"$msg\" $new_parents\n \t\t\tthen\n \t\t\t\tprintf \"%s\\n\" \"$msg\" > \"$GIT_DIR\"/MERGE_MSG\n \t\t\t\tdie_with_patch $sha1 \"Error redoing merge $sha1\"\n-- \n1.7.2.2\n"},{"id":"166441","messageId":"4DB77E53.7070206@sohovfx.com","threadId":"27148","inReplyTo":"1303357080-25840-1-git-send-email-andrew.kw.w@gmail.com","subject":"Re: [PATCH] git-rebase--interactive.sh: preserve-merges fails on merges created with no-ff","fromName":"Andrew Wong","fromEmail":"andrew.w@sohovfx.com","sentAt":"2011-04-27T02:24:19Z","receivedAt":"2011-04-27T02:24:19Z","isPatch":true,"sender":{"key":"andrew.w@sohovfx.com","avatar":null},"body":"Could someone please take a look at this patch?\n"},{"id":"166451","messageId":"7v1v0ob851.fsf@alter.siamese.dyndns.org","threadId":"27148","inReplyTo":"4DB77E53.7070206@sohovfx.com","subject":"Re: [PATCH] git-rebase--interactive.sh: preserve-merges fails on merges created with no-ff","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-27T05:00:58Z","receivedAt":"2011-04-27T05:00:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andrew Wong <andrew.w@sohovfx.com> writes:\n\n> Could someone please take a look at this patch?\n\nI took a look at it when you sent it out, and found it so obviously and\ntrivially correct that I expected that others will soon say it looked\nobviously the right thing to do.  So I decided to wait until that to\nhappen before applying it.\n\nBut nobody said anything, and forgot about it.\n\nLet's see if it happens soon enough this time ;-).  Thanks for a reminder.\n"},{"id":"166471","messageId":"BANLkTi=Cu2nXiLaOT0v-Zwz6uGg=UKuyfg@mail.gmail.com","threadId":"27148","inReplyTo":"7v1v0ob851.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-rebase--interactive.sh: preserve-merges fails on merges created with no-ff","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2011-04-27T08:15:35Z","receivedAt":"2011-04-27T08:15:35Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wed, Apr 27, 2011 at 7:00 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Andrew Wong <andrew.w@sohovfx.com> writes:\n>\n>> Could someone please take a look at this patch?\n>\n> I took a look at it when you sent it out, and found it so obviously and\n> trivially correct that I expected that others will soon say it looked\n> obviously the right thing to do.\n\nMe too!\n\n>  So I decided to wait until that to\n> happen before applying it.\n>\n> But nobody said anything, and forgot about it.\n>\n> Let's see if it happens soon enough this time ;-).  Thanks for a reminder.\n\nThanks from me too,\nChristian.\n"},{"id":"166534","messageId":"7vfwp38uwf.fsf@alter.siamese.dyndns.org","threadId":"27148","inReplyTo":"4DB77E53.7070206@sohovfx.com","subject":"Re: [PATCH] git-rebase--interactive.sh: preserve-merges fails on merges created with no-ff","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-27T17:29:52Z","receivedAt":"2011-04-27T17:29:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andrew Wong <andrew.w@sohovfx.com> writes:\n\n> Could someone please take a look at this patch?\n\nCan you add a test for this change, perhaps to either t3409 or t3414?\n"},{"id":"166597","messageId":"1303965355-3393-1-git-send-email-andrew.kw.w@gmail.com","threadId":"27148","inReplyTo":"7vfwp38uwf.fsf@alter.siamese.dyndns.org","subject":"[PATCH v2] git-rebase--interactive.sh: preserve-merges fails on merges created with no-ff","fromName":"Andrew Wong","fromEmail":"andrew.kw.w@gmail.com","sentAt":"2011-04-28T04:35:55Z","receivedAt":"2011-04-28T04:35:55Z","isPatch":true,"sender":{"key":"andrew.kw.w@gmail.com","avatar":"https://avatars.githubusercontent.com/u/489311?v=4"},"body":"'git rebase' uses 'git merge' to preserve merges (-p).  This preserves\nthe original merge commit correctly, except when the original merge\ncommit was created by 'git merge --no-ff'.  In this case, 'git rebase'\nwill fail to preserve the merge, because during 'git rebase', 'git\nmerge' will simply fast-forward and skip the commit.  For example:\n\n               B\n              / \\\n             A---M\n            /\n    ---o---O---P---Q\n\nIf we try to rebase M onto P, we lose the merge commit and this happens:\n\n                 A---B\n                /\n    ---o---O---P---Q\n\nTo correct this, we simply do a \"no fast-forward\" on all merge commits\nwhen rebasing.  Since by the time we decided to do a 'git merge' inside\n'git rebase', it means there was a merge originally, so 'git merge'\nshould always create a merge commit regardless of what the merge\nbranches look like. This way, when rebase M onto P from the above\nexample, we get:\n\n                   B\n                  / \\\n                 A---M\n                /\n    ---o---O---P---Q\n\nSigned-off-by: Andrew Wong <andrew.kw.w@gmail.com>\n---\n git-rebase--interactive.sh        |    2 +-\n t/t3409-rebase-preserve-merges.sh |   32 +++++++++++++++++++++++++++++++-\n 2 files changed, 32 insertions(+), 2 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 5873ba4..c308529 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -339,7 +339,7 @@ pick_one_preserving_merges () {\n \t\t\t# No point in merging the first parent, that's HEAD\n \t\t\tnew_parents=${new_parents# $first_parent}\n \t\t\tif ! do_with_author output \\\n-\t\t\t\tgit merge $STRATEGY -m \"$msg\" $new_parents\n+\t\t\t\tgit merge --no-ff $STRATEGY -m \"$msg\" $new_parents\n \t\t\tthen\n \t\t\t\tprintf \"%s\\n\" \"$msg\" > \"$GIT_DIR\"/MERGE_MSG\n \t\t\t\tdie_with_patch $sha1 \"Error redoing merge $sha1\"\ndiff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh\nindex 19341e5..08201e2 100755\n--- a/t/t3409-rebase-preserve-merges.sh\n+++ b/t/t3409-rebase-preserve-merges.sh\n@@ -27,7 +27,17 @@ export GIT_AUTHOR_EMAIL\n #    \\\n #     B2       <-- origin/topic\n #\n-# In both cases, 'topic' is rebased onto 'origin/topic'.\n+# Clone 3 (no-ff merge):\n+#\n+# A1--A2--B3   <-- origin/master\n+#  \\\n+#   B1------M  <-- topic\n+#    \\     /\n+#     \\--A3    <-- topic2\n+#      \\\n+#       B2     <-- origin/topic\n+#\n+# In all cases, 'topic' is rebased onto 'origin/topic'.\n \n test_expect_success 'setup for merge-preserving rebase' \\\n \t'echo First > A &&\n@@ -61,6 +71,16 @@ test_expect_success 'setup for merge-preserving rebase' \\\n \t\tgit commit -m \"Merge origin/master into topic\"\n \t) &&\n \n+\tgit clone ./. clone3 &&\n+\t(\n+\t\tcd clone3 &&\n+\t\tgit checkout -b topic2 origin/topic &&\n+\t\techo Sixth > A &&\n+\t\tgit commit -a -m \"Modify A3\" &&\n+\t\tgit checkout -b topic origin/topic &&\n+\t\tgit merge --no-ff topic2\n+\t) &&\n+\n \tgit checkout topic &&\n \techo Fourth >> B &&\n \tgit commit -a -m \"Modify B2\"\n@@ -93,4 +113,14 @@ test_expect_success '--continue works after a conflict' '\n \t)\n '\n \n+test_expect_success 'rebase -p preserves no-ff merges' '\n+\t(\n+\tcd clone3 &&\n+\tgit fetch &&\n+\tgit rebase -p origin/topic &&\n+\ttest 3 = $(git rev-list --all --pretty=oneline | grep \"Modify A\" | wc -l) &&\n+\ttest 1 = $(git rev-list --all --pretty=oneline | grep \"Merge branch\" | wc -l)\n+\t)\n+'\n+\n test_done\n-- \n1.7.2.2\n"}]}