{"thread":{"id":"30581","subject":"[PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes","startedAt":"2012-05-21T20:19:09Z","lastAt":"2012-05-25T20:03:21Z","messageCount":16,"participants":["Johannes Sixt","Junio C Hamano","Jonathan Nieder","Martin von Zweigbergk"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"191825","messageId":"4FBAA33D.1080703@kdbg.org","threadId":"30581","inReplyTo":null,"subject":"[PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2012-05-21T20:19:09Z","receivedAt":"2012-05-21T20:19:09Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"When rebase -p had to replay a merge commit, it used to redo the merge.\nBut this has drawbacks:\n\n- When the merge was evil, i.e., contained changes that are in neither of\n  the parents, that change was not preserved.\n\n- The 'git merge' invocation passed the commit message of the old merge\n  commit, but it still obeyed the merge.log option. If it was set, the log\n  ended up twice in the commit message.\n\nReplace the 'git merge' by a cherry-pick that picks the changes that the\nmerge introduces with respect to the first parent.\n\nThe change in t3409 reflects the new situation after a conflicting merge\nis replayed: Earlier, 'git merge' found a conflict in an add/add\nsituation with file B only in stage 2 and 3, now it is a regular three-way\nmerge conflict.\n\nA test in  t3410 fails now with this new implementation. The branch that\nis merged in by the old history and that is to be preserved is already\nmerged into the new upstream. In the old implementation, 'git merge'\ndetected this situation and succeeded with \"Already up to date\", but the\nnew implementation does not notice that it is pointless to keep the merge.\nIt just tries to follow instructions, but fails due to a conflicting\ncherry-pick. This is a corner case whose correct solution should be that\nthe \"pick\" instruction is not generated in the first place.\n\nSigned-off-by: Johannes Sixt <j6t@kdbg.org>\n---\n This is RFC because of the new failure addressed in the last paragraph\n above (see also the art work in the patch context below), how serious\n the failure is, and what to do about it.\n\n See also this recent thread for further motivation:\n http://thread.gmane.org/gmane.comp.version-control.git/194434/focus=194737\n\n git-rebase--interactive.sh                | 12 +++++++-----\n t/t3409-rebase-preserve-merges.sh         |  2 +-\n t/t3410-rebase-preserve-dropped-merges.sh |  4 ++--\n 3 files changed, 10 insertions(+), 8 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 0c19b7c..642daab 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -312,12 +312,14 @@ pick_one_preserving_merges () {\n \t\t\tmsg_content=\"$(commit_message $sha1)\"\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 --no-ff ${strategy:+-s $strategy} -m \\\n-\t\t\t\t\t\"$msg_content\" $new_parents\n+\t\t\tprintf \"%s\\n\" $new_parents >\"$GIT_DIR\"/MERGE_HEAD\n+\t\t\tprintf \"%s\\n\" \"$msg_content\" >\"$GIT_DIR\"/MERGE_MSG\n+\t\t\tif output git cherry-pick -m 1 -n \"$sha1\"\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 \"Error redoing merge $sha1\"\n+\t\t\t\tdo_with_author output git commit --no-verify -F \"$GIT_DIR\"/MERGE_MSG ||\n+\t\t\t\t\tdie_with_patch $sha1 \"Could not replay merge $sha1\"\n+\t\t\telse\n+\t\t\t\tdie_with_patch $sha1 \"Could not pick merge $sha1\"\n \t\t\tfi\n \t\t\techo \"$sha1 $(git rev-parse HEAD^0)\" >> \"$rewritten_list\"\n \t\t\t;;\ndiff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh\nindex 6de4e22..7161b99 100755\n--- a/t/t3409-rebase-preserve-merges.sh\n+++ b/t/t3409-rebase-preserve-merges.sh\n@@ -116,7 +116,7 @@ test_expect_success '--continue works after a conflict' '\n \tcd clone2 &&\n \tgit fetch &&\n \ttest_must_fail git rebase -p origin/topic &&\n-\ttest 2 = $(git ls-files B | wc -l) &&\n+\ttest 3 = $(git ls-files B | wc -l) &&\n \techo Resolved again > B &&\n \ttest_must_fail git rebase --continue &&\n \tgrep \"^@@@ \" .git/rebase-merge/patch &&\ndiff --git a/t/t3410-rebase-preserve-dropped-merges.sh b/t/t3410-rebase-preserve-dropped-merges.sh\nindex 6f73b95..36c69b2 100755\n--- a/t/t3410-rebase-preserve-dropped-merges.sh\n+++ b/t/t3410-rebase-preserve-dropped-merges.sh\n@@ -60,12 +60,12 @@ test_expect_success 'skip same-resolution merges with -p' '\n # A - B - C - D - E\n #   \\             \\ \\\n #     F - G - H -- L2 \\        -->   L2\n #       \\             |                \\\n #         I -- G3 --- J2 -- K2           I -- G3 -- K2\n-# G2 = different changes as G\n-test_expect_success 'keep different-resolution merges with -p' '\n+# G3 = different changes as G\n+test_expect_failure 'keep different-resolution merges with -p' '\n \tgit checkout H &&\n \ttest_must_fail git merge E &&\n \ttest_commit L2 file1 23 &&\n \tgit checkout I &&\n \ttest_commit G3 file1 4 &&\n-- \n1.7.10.2.529.g0c18cfd\n"},{"id":"191918","messageId":"7vobpg3wj6.fsf@alter.siamese.dyndns.org","threadId":"30581","inReplyTo":"4FBAA33D.1080703@kdbg.org","subject":"Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-22T18:23:09Z","receivedAt":"2012-05-22T18:23:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> When rebase -p had to replay a merge commit, it used to redo the merge.\n> But this has drawbacks:\n>\n> - When the merge was evil, i.e., contained changes that are in neither of\n>   the parents, that change was not preserved.\n\nThis is a desiable property, and not necessarily limited to \"evil\" merges\nbut also applies to everyday conflict resolutions.  Replaying the change\nbetween the merge and its first parent is a way to achieve it, but I think\nit also has downsides.  If you are replaying a merge to an updated history\nthat already contains a part of what is merged, some part of the\ndifference between the original merge and its first parent already exists\nin the commit that the will become the first parent of the replayed merge.\n\n> - The 'git merge' invocation passed the commit message of the old merge\n>   commit, but it still obeyed the merge.log option. If it was set, the log\n>   ended up twice in the commit message.\n\nThis should be fixed independent of this patch, no?  Is it a matter of\njust passing --no-log or something, or is there anything more elaborate\nnecessary?\n"},{"id":"191929","messageId":"4FBBE949.8060305@kdbg.org","threadId":"30581","inReplyTo":"7vobpg3wj6.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2012-05-22T19:30:17Z","receivedAt":"2012-05-22T19:30:17Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 22.05.2012 20:23, schrieb Junio C Hamano:\n> Johannes Sixt <j6t@kdbg.org> writes:\n> \n>> When rebase -p had to replay a merge commit, it used to redo the merge.\n>> But this has drawbacks:\n>>\n>> - When the merge was evil, i.e., contained changes that are in neither of\n>>   the parents, that change was not preserved.\n> \n> This is a desiable property, and not necessarily limited to \"evil\" merges\n> but also applies to everyday conflict resolutions.  Replaying the change\n> between the merge and its first parent is a way to achieve it, but I think\n> it also has downsides.  If you are replaying a merge to an updated history\n> that already contains a part of what is merged, some part of the\n> difference between the original merge and its first parent already exists\n> in the commit that the will become the first parent of the replayed merge.\n\nYes, in such a case we need some cleverness from the merge machinery in\ncherry-pick: It has to recognize that some changes were applied on both\nbranches. This might generate new conflicts.\n\nI'll have to see whether this is an issue in practice.\n\nAt any rate, if any difficulties with a particular merge commit can be\nanticipated, it is always possible to replace the\n\n  pick that-merge-commit\n\ninstruction by\n\n  x git merge that-branch\n\n(we could even generate these lines in the instruction sheet as a comment).\n\n>> - The 'git merge' invocation passed the commit message of the old merge\n>>   commit, but it still obeyed the merge.log option. If it was set, the log\n>>   ended up twice in the commit message.\n> \n> This should be fixed independent of this patch, no?  Is it a matter of\n> just passing --no-log or something, or is there anything more elaborate\n> necessary?\n\nFrankly, IMHO[*], rebase -p that is based on git-cherry-pick is so much\nbetter that I don't want to think about which workarounds would be\nneeded to make it work better with git-merge :-)\n\n[*] It is still only an opinion. I haven't gotten around to use the\nupdated rebase -p in the field. Another reason that it's still RFC.\n\nThanks,\n-- Hannes\n"},{"id":"191962","messageId":"20120522233821.GA31667@burratino","threadId":"30581","inReplyTo":"4FBAA33D.1080703@kdbg.org","subject":"Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-05-22T23:38:21Z","receivedAt":"2012-05-22T23:38:21Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Johannes Sixt wrote:\n\n> - The 'git merge' invocation passed the commit message of the old merge\n>   commit, but it still obeyed the merge.log option. If it was set, the log\n>   ended up twice in the commit message.\n\nLikewise for the \"Conflicts:\" paragraph, which should also be pretty\nsimple to fix directly.  I don't think that points directly to a\nfundamental flaw in the current implementation, but it does make it\nseem like no one has been using \"rebase -p\" for anything nontrivial. :)\n\n[...]\n> --- a/git-rebase--interactive.sh\n> +++ b/git-rebase--interactive.sh\n> @@ -312,12 +312,14 @@ pick_one_preserving_merges () {\n>  \t\t\tmsg_content=\"$(commit_message $sha1)\"\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 --no-ff ${strategy:+-s $strategy} -m \\\n> -\t\t\t\t\t\"$msg_content\" $new_parents\n> +\t\t\tprintf \"%s\\n\" $new_parents >\"$GIT_DIR\"/MERGE_HEAD\n> +\t\t\tprintf \"%s\\n\" \"$msg_content\" >\"$GIT_DIR\"/MERGE_MSG\n> +\t\t\tif output git cherry-pick -m 1 -n \"$sha1\"\n>  \t\t\tthen\n> +\t\t\t\tdo_with_author output git commit --no-verify -F \"$GIT_DIR\"/MERGE_MSG ||\n> +\t\t\t\t\tdie_with_patch $sha1 \"Could not replay merge $sha1\"\n> +\t\t\telse\n> -\t\t\t\tprintf \"%s\\n\" \"$msg_content\" > \"$GIT_DIR\"/MERGE_MSG\n> -\t\t\t\tdie_with_patch $sha1 \"Error redoing merge $sha1\"\n> +\t\t\t\tdie_with_patch $sha1 \"Could not pick merge $sha1\"\n>  \t\t\tfi\n\nCute.  The approach looks very sensible to me.\n\nThanks,\nJonathan\n"},{"id":"191992","messageId":"CAOeW2eE9EW3gER7ZDThGABtZ0doNuUb70DnKrnzD+OeWYLO7cQ@mail.gmail.com","threadId":"30581","inReplyTo":"4FBAA33D.1080703@kdbg.org","subject":"Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2012-05-23T15:37:42Z","receivedAt":"2012-05-23T15:37:42Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Mon, May 21, 2012 at 1:19 PM, Johannes Sixt <j6t@kdbg.org> wrote:\n> Replace the 'git merge' by a cherry-pick that picks the changes that the\n> merge introduces with respect to the first parent.\n\nJust a reminder of what I said in [1] (the same thread that you referenced):\n\nI think this will break \"git rebase -p --onto g b f\" on  the below history.\n\n          .-e-.\n         /     \\\n      .-c---d---f\n     /\na---b---g\n\nEven if this wasn't the intended use case, git currently supports it\nand I would personally be a little surprised if no one has gotten used\nto it working. So should we at least check for this case and handle it\nwith \"git merge\" as usual? Or stop supporting it and error out instead\n(and mention in release notes?)?\n\nBtw, with a history without \"internal\" merges, but where the merge\ncommit was generated \"backwards\" so the first parent is not in the\nto-be-rebased history, am I correct in thinking that the \"git\ncherry-pick -m 1\" will fail? Do you think we should consider this case\nor do we consider that too broken to care about?\n\n\nMartin\n\n [1] http://thread.gmane.org/gmane.comp.version-control.git/194434/focus=194786\n"},{"id":"192011","messageId":"7vzk8yzq35.fsf@alter.siamese.dyndns.org","threadId":"30581","inReplyTo":"CAOeW2eE9EW3gER7ZDThGABtZ0doNuUb70DnKrnzD+OeWYLO7cQ@mail.gmail.com","subject":"Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-23T18:53:34Z","receivedAt":"2012-05-23T18:53:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:\n\n> On Mon, May 21, 2012 at 1:19 PM, Johannes Sixt <j6t@kdbg.org> wrote:\n>> Replace the 'git merge' by a cherry-pick that picks the changes that the\n>> merge introduces with respect to the first parent.\n>\n> Just a reminder of what I said in [1] (the same thread that you referenced):\n>\n> I think this will break \"git rebase -p --onto g b f\" on  the below history.\n>\n>           .-e-.\n>          /     \\\n>       .-c---d---f\n>      /\n> a---b---g\n\n\n... where the replayed history would look like this:\n\n                  .-e'.\n                 /     \\\n    a---b---g---c'--d'--f'\n\nand e' and e, even though being \"moral equivalent\", may have different\ndegrees of change since c' and c respectively depending on how changes\nbetween b and c and changes between b and g are related to each other.\n\nIf the change in the dag b..f does not have much to do with the change\nbetween b and g, then the change from c to e would be the same as the\nchange from c' to e', and if the original merge at f resolved conflicts\nbetween e and d (or added an evil change), replaying the difference from e\nto f on top of e' may give a better result.\n\nI think your original point was that the above clean picture would not\nhold if e-b and g-b has interactions.  g-b may contain some change that\ne-b has independently done, in which case e'-g would be made smaller than\ne-b when the conflict is resolved while replaying e to produce e' on top\nof c' (the same applies if you replace e with any commit in the dag\n\"rev-list ^b e d\").  The merge to produce f' may result in conflicts,\nwhether you merge e' and d' or replay f-e on top of e'.\n\nA better way to keep potential \"evil merges\" at f while replaying to\nproduce f' may *not* be by applying the difference f-e on top of e'.\nInstead, you can learn from what 'rerere' does.  That is, to attempt a\nmechanical merge between e and d and call the result (with possible\nconflict markers and all) pre-f.  If you compare pre-f and f, that is the\nresolution and evil change made at f.  When reproducing f', mechanically\nmerge e' and d', call the result (again, with possible conflict markers\nand all) pre-f', and try reproducing f' by a 3-way merge between pre-f,\npre-f' and f.\n"},{"id":"192012","messageId":"4FBD3391.7010905@kdbg.org","threadId":"30581","inReplyTo":"CAOeW2eE9EW3gER7ZDThGABtZ0doNuUb70DnKrnzD+OeWYLO7cQ@mail.gmail.com","subject":"Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2012-05-23T18:59:29Z","receivedAt":"2012-05-23T18:59:29Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 23.05.2012 17:37, schrieb Martin von Zweigbergk:\n> On Mon, May 21, 2012 at 1:19 PM, Johannes Sixt <j6t@kdbg.org> wrote:\n>> Replace the 'git merge' by a cherry-pick that picks the changes that the\n>> merge introduces with respect to the first parent.\n> \n> Just a reminder of what I said in [1] (the same thread that you referenced):\n> \n> I think this will break \"git rebase -p --onto g b f\" on  the below history.\n> \n>           .-e-.\n>          /     \\\n>       .-c---d---f\n>      /\n> a---b---g\n\nThis still works. It is tested by t3411.3.\n\n> Even if this wasn't the intended use case, git currently supports it\n> and I would personally be a little surprised if no one has gotten used\n> to it working. So should we at least check for this case and handle it\n> with \"git merge\" as usual? Or stop supporting it and error out instead\n> (and mention in release notes?)?\n\nYESSS, drop it. It would allow us to remove all sorts of crufty\nwork-arounds for work-arounds. THIS IS NOT 'git sequencer'!\n\n> Btw, with a history without \"internal\" merges, but where the merge\n> commit was generated \"backwards\" so the first parent is not in the\n> to-be-rebased history, am I correct in thinking that the \"git\n> cherry-pick -m 1\" will fail? Do you think we should consider this case\n> or do we consider that too broken to care about?\n\nI think something odd will happen. If in doubt, I would suggest to\nconsider this such an odd case which rebase is not intended for ;-)\n(garbage in - garbage out).\n\n-- Hannes\n"},{"id":"192015","messageId":"CAOeW2eGvEaQYk9KODmLzZuEBu-KhKcQeL4PE-4YHwSgtP0dJfA@mail.gmail.com","threadId":"30581","inReplyTo":"7vzk8yzq35.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2012-05-23T20:41:17Z","receivedAt":"2012-05-23T20:41:17Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Wed, May 23, 2012 at 11:53 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> I think your original point was that the above clean picture would not\n> hold if e-b and g-b has interactions.  g-b may contain some change that\n> e-b has independently done, in which case e'-g would be made smaller than\n> e-b when the conflict is resolved while replaying e to produce e' on top\n> of c' (the same applies if you replace e with any commit in the dag\n> \"rev-list ^b e d\").  The merge to produce f' may result in conflicts,\n> whether you merge e' and d' or replay f-e on top of e'.\n\nRight. Or if e' was changed as a result of a \"edit\" action (I meant to\ninclude '-i' in addition to '-p').\n\n> A better way to keep potential \"evil merges\" at f while replaying to\n> produce f' may *not* be by applying the difference f-e on top of e'.\n> Instead, you can learn from what 'rerere' does.  That is, to attempt a\n> mechanical merge between e and d and call the result (with possible\n> conflict markers and all) pre-f.  If you compare pre-f and f, that is the\n> resolution and evil change made at f.  When reproducing f', mechanically\n> merge e' and d', call the result (again, with possible conflict markers\n> and all) pre-f', and try reproducing f' by a 3-way merge between pre-f,\n> pre-f' and f.\n\nYes, I've had the same idea myself. Anyway, as Johannes said, that's\nprobably something to consider for the sequencer.\n\nMartin\n"},{"id":"192088","messageId":"7vehq9xz7a.fsf@alter.siamese.dyndns.org","threadId":"30581","inReplyTo":"CAOeW2eGvEaQYk9KODmLzZuEBu-KhKcQeL4PE-4YHwSgtP0dJfA@mail.gmail.com","subject":"Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-24T17:31:53Z","receivedAt":"2012-05-24T17:31:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:\n\n> Yes, I've had the same idea myself. Anyway, as Johannes said, that's\n> probably something to consider for the sequencer.\n\nAre you saying that \"rebase -any-variant\" and the sequencer should behave\ndifferently?  It is not immediately obvious to me why it is a good idea.\n"},{"id":"192090","messageId":"CAOeW2eH85+qa2PXS55_xGwH+tpMDMEK76HywfpLTYrv_Dtg49Q@mail.gmail.com","threadId":"30581","inReplyTo":"7vehq9xz7a.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2012-05-24T17:47:57Z","receivedAt":"2012-05-24T17:47:57Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Thu, May 24, 2012 at 10:31 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:\n>\n>> Yes, I've had the same idea myself. Anyway, as Johannes said, that's\n>> probably something to consider for the sequencer.\n>\n> Are you saying that \"rebase -any-variant\" and the sequencer should behave\n> differently?  It is not immediately obvious to me why it is a good idea.\n\nThat's not what I meant to say. I thought the sequencer is supposed to\nreplace much of git-rebase and I thought that's what Johannes was\nreferring to as well when he said not to make git-rebase too\nintelligent. I assumed the reasoning was that any work spent on\ngit-rebase at this point will be thrown away once git-rebase instead\ncalls into the sequencer. But I have not been very involved in the\ndiscussions about the sequencer, so I may very well have misunderstood\nthings. Or are you saying that even if it's true that git-rebase will\neventually call into the sequencer for much of its operation, this\nspecific part (if at all implemented) does not belong in the\nsequencer?\n\nMartin\n"},{"id":"192106","messageId":"7vpq9twdbc.fsf@alter.siamese.dyndns.org","threadId":"30581","inReplyTo":"CAOeW2eH85+qa2PXS55_xGwH+tpMDMEK76HywfpLTYrv_Dtg49Q@mail.gmail.com","subject":"Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-24T20:09:59Z","receivedAt":"2012-05-24T20:09:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:\n\n> ... I assumed the reasoning was that any work spent on\n> git-rebase at this point will be thrown away once git-rebase instead\n> calls into the sequencer....\n\nAh, sorry I mis-read you.\n"},{"id":"192110","messageId":"4FBE9AC7.3010506@kdbg.org","threadId":"30581","inReplyTo":"CAOeW2eH85+qa2PXS55_xGwH+tpMDMEK76HywfpLTYrv_Dtg49Q@mail.gmail.com","subject":"Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2012-05-24T20:32:07Z","receivedAt":"2012-05-24T20:32:07Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 24.05.2012 19:47, schrieb Martin von Zweigbergk:\n> On Thu, May 24, 2012 at 10:31 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:\n>>\n>>> Yes, I've had the same idea myself. Anyway, as Johannes said, that's\n>>> probably something to consider for the sequencer.\n>>\n>> Are you saying that \"rebase -any-variant\" and the sequencer should behave\n>> differently?  It is not immediately obvious to me why it is a good idea.\n> \n> That's not what I meant to say. I thought the sequencer is supposed to\n> replace much of git-rebase and I thought that's what Johannes was\n> referring to as well when he said not to make git-rebase too\n> intelligent.\n\nYou are probably refering to what I said here:\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/194434/focus=195074\n\nWhen I wrote the post, I was not aware that rebase -p *is* indeed able\nto transplant a branchy topic to a new upstream. I was convinced that\nrebase -p can only move a (first-parent) topic line which may have\nmerged in some unrelated other topics. So, you should take it with a\nlarge grain of salt.\n\n-------\n\nToday I was able to use rebase -i -p in the field. I used it to rebuild\nan integration branch (akin to git's pu branch). Guess what? It did not\nwork as expected:\n\nTwo of the topic branches' early parts were already merged in the\nupstream. The instruction sheet had only 'pick' of merge commits for the\ntopics. Except for these two; there, all commits (that were not yet in\nupstream) were offered to pick, including the merge commit.\n\nI started with this:\n\n    A--M--o--o   <- master\n   /  /\n--o--X--Y        <- side branch (partially merged in master)\n   \\     \\\n    R--S--N--T   <- integration (to be rebuilt on master)\n\nI wanted this:\n\n     A--M--o--o\n    /  /       \\\n   /  /          R'--S'--N'--T'\n--o--X--Y---------------´\n\nBut I got this:\n\n    A--M--o--o-------Y'\n   /  /       \\       \\\n--o--X--Y      R'--S'--N'--T'\n\n(Note that this has nothing to do with my patch; the badness happens\nalready before any rebasing begins.)\n\nGah! I'm frustrated. When --preserve-merges was invented, it supported\ntwo very important use-cases:\n\n1. Rebuild an integration branch.\n2. Rebase a topic that merges an 'unrelated side branch'.\n\nThen people came along thinking that \"preserve merges\" means that *any*\nsort of merges should be preserved, including a branchy-and-mergy topic\nlike the example you gave. *Of course* it is much more difficult to\nsupport this case. And sure as hell with all the work-arounds needed to\nsupport it, a good deal of other good functionality became broken\nsubtly. This is why I say that we should drop support for the\ncomplicated cases and resurrect correctness for the simpler, but\nimportant cases.\n\n-- Hannes\n"},{"id":"192125","messageId":"7vaa0xw9dy.fsf@alter.siamese.dyndns.org","threadId":"30581","inReplyTo":"4FBE9AC7.3010506@kdbg.org","subject":"Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-24T21:34:49Z","receivedAt":"2012-05-24T21:34:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> Today I was able to use rebase -i -p in the field. I used it to rebuild\n> an integration branch (akin to git's pu branch). Guess what? It did not\n> work as expected:\n>\n> Two of the topic branches' early parts were already merged in the\n> upstream. The instruction sheet had only 'pick' of merge commits for the\n> topics. Except for these two; there, all commits (that were not yet in\n> upstream) were offered to pick, including the merge commit.\n>\n> I started with this:\n>\n>     A--M--o--o   <- master\n>    /  /\n> --o--X--Y        <- side branch (partially merged in master)\n>    \\     \\\n>     R--S--N--T   <- integration (to be rebuilt on master)\n>\n> I wanted this:\n>\n>      A--M--o--o\n>     /  /       \\\n>    /  /          R'--S'--N'--T'\n> --o--X--Y---------------´\n\nIt is unclear what exact revisions you gave to rebase, but I am assuming\nthat you asked \"rebase --onto master X^ T\" (you can use R^ instead of X^;\nthey refer to the same commit).\n\nIt is straightforward to see what should happen to R and S; they should be\na straight replay of single parent commit on top of 'master'.  \n\nBut it is unclear what should happen to X and Y.\n\nWhen you are rebasing the X^..T sub-DAG, can you say what makes S more\nspecial than Y by looking at the original topology?  Your \"I wanted this\"\npicture depicts S' to be rewritten but Y stays the same.  Why?  \n\nThey are both in the X^..T DAG, and neither of them is merged to 'master'.\nI can sort of see why X and R would be treated differently (X is part of\nmaster, R is not), but I cannot justify why your \"I wanted this\" picture\nreplays S' without replaying Y'.\n\nWhat am I missing?\n\n> But I got this:\n>\n>     A--M--o--o-------Y'\n>    /  /       \\       \\\n> --o--X--Y      R'--S'--N'--T'\n\nWhich is what I would expect, from the \"Y and S play a similar role in the\nsub-DAG X^..T in the original DAG, with respect to master\" point of view.\n"},{"id":"192164","messageId":"4FBFAC19.8030108@kdbg.org","threadId":"30581","inReplyTo":"7vaa0xw9dy.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2012-05-25T15:58:17Z","receivedAt":"2012-05-25T15:58:17Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 24.05.2012 23:34, schrieb Junio C Hamano:\n> Johannes Sixt <j6t@kdbg.org> writes:\n> \n>> Today I was able to use rebase -i -p in the field. I used it to rebuild\n>> an integration branch (akin to git's pu branch). Guess what? It did not\n>> work as expected:\n>>\n>> Two of the topic branches' early parts were already merged in the\n>> upstream. The instruction sheet had only 'pick' of merge commits for the\n>> topics. Except for these two; there, all commits (that were not yet in\n>> upstream) were offered to pick, including the merge commit.\n>>\n>> I started with this:\n>>\n>>     A--M--o--o   <- master\n>>    /  /\n>> --o--X--Y        <- side branch (partially merged in master)\n>>    \\     \\\n>>     R--S--N--T   <- integration (to be rebuilt on master)\n>>\n>> I wanted this:\n>>\n>>      A--M--o--o\n>>     /  /       \\\n>>    /  /          R'--S'--N'--T'\n>> --o--X--Y---------------´\n> \n> It is unclear what exact revisions you gave to rebase, but I am assuming\n> that you asked \"rebase --onto master X^ T\" (you can use R^ instead of X^;\n> they refer to the same commit).\n\nSorry, I should have been more precise. The command was\n\n  git rebase -i -p master T\n\nAn important detail is that the order of parents of N is S, Y. (I\nassumed that this was somewhat clear from the context of my message.)\n\n> It is straightforward to see what should happen to R and S; they should be\n> a straight replay of single parent commit on top of 'master'.  \n> \n> But it is unclear what should happen to X and Y.\n\nNothing. Relabel \"integration\" to \"topic\" in my artwork above. Then X\nand Y are on an \"unrelated side branch\" that is merged into the topic\nwhose tip is at T. The early part of this side branch is already in\nmaster.  In fact, if X were not merged into master, yet, then 'rebase -i\n-p' already works as (I) intended: it leaves X and Y alone.\n\n> When you are rebasing the X^..T sub-DAG, can you say what makes S more\n> special than Y by looking at the original topology?  Your \"I wanted this\"\n> picture depicts S' to be rewritten but Y stays the same.  Why?  \n\nIt is \"not on topic\" (pun intended). It is the second parent of N. Think\nof X and Y as an independent bug fix sub-topic. It is merged into topic\nonly because T depends on the fix.\n\n> They are both in the X^..T DAG, and neither of them is merged to 'master'.\n> I can sort of see why X and R would be treated differently (X is part of\n> master, R is not), but I cannot justify why your \"I wanted this\" picture\n> replays S' without replaying Y'.\n> \n> What am I missing?\n\nFirst-parentship. When a topic or an integration branch is rebased (with\n--preserve-merges), only the first-parent chain should be rewritten.\n\n>> But I got this:\n>>\n>>     A--M--o--o-------Y'\n>>    /  /       \\       \\\n>> --o--X--Y      R'--S'--N'--T'\n> \n> Which is what I would expect, from the \"Y and S play a similar role in the\n> sub-DAG X^..T in the original DAG, with respect to master\" point of view.\n\n-- Hannes\n"},{"id":"192168","messageId":"7vy5ogtcxm.fsf@alter.siamese.dyndns.org","threadId":"30581","inReplyTo":"4FBFAC19.8030108@kdbg.org","subject":"Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-25T16:58:45Z","receivedAt":"2012-05-25T16:58:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> First-parentship. When a topic or an integration branch is rebased (with\n> --preserve-merges), only the first-parent chain should be rewritten.\n\nWhile I think the \"start from commit T and down to where it branches off\nof 'master', rebuild first-parent chain on top of elsewhere\" mode has\nuses, there is no way it can co-exist under the same option name with the\ncurrent \"transplant the DAG as a whole on top of elsewhere, preserving the\ntopology inside the DAG\" mode.  The name \"preserve merges\" clicks with the\nlatter mode better than the former mode, at least to me.  Perhaps the\nother mode can be called \"rebase --first-parent\" or something?\n"},{"id":"192200","messageId":"4FBFE589.1080805@kdbg.org","threadId":"30581","inReplyTo":"7vy5ogtcxm.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2012-05-25T20:03:21Z","receivedAt":"2012-05-25T20:03:21Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 25.05.2012 18:58, schrieb Junio C Hamano:\n> Johannes Sixt <j6t@kdbg.org> writes:\n> \n>> First-parentship. When a topic or an integration branch is rebased (with\n>> --preserve-merges), only the first-parent chain should be rewritten.\n> \n> While I think the \"start from commit T and down to where it branches off\n> of 'master', rebuild first-parent chain on top of elsewhere\" mode has\n> uses, there is no way it can co-exist under the same option name with the\n> current \"transplant the DAG as a whole on top of elsewhere, preserving the\n> topology inside the DAG\" mode.\n\nFair enough.\n\n>  The name \"preserve merges\" clicks with the\n> latter mode better than the former mode, at least to me.  Perhaps the\n> other mode can be called \"rebase --first-parent\" or something?\n\nSounds good. I'll take this route.\n\n-- Hannes\n"}]}