{"thread":{"id":"39800","subject":"No one understands diff3 \"Temporary merge branch\" conflict markers","startedAt":"2015-07-07T14:55:05Z","lastAt":"2015-07-07T21:44:38Z","messageCount":4,"participants":["Edward Anderson","Matthieu Moy","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"265734","messageId":"CAOMsSXQVJsd0h1fnNMEJ5+cKpxbeF9mHraXva-wr6Y2zBCADbg@mail.gmail.com","threadId":"39800","inReplyTo":null,"subject":"No one understands diff3 \"Temporary merge branch\" conflict markers","fromName":"Edward Anderson","fromEmail":"nilbus@nilbus.com","sentAt":"2015-07-07T14:55:05Z","receivedAt":"2015-07-07T14:55:05Z","isPatch":false,"sender":{"key":"nilbus@nilbus.com","avatar":null},"body":"I have the diff3 conflictstyle enabled and want to be able to\nunderstand how to understand its output when there are criss-cross\nmerges requiring temporary merge branches.  Eg:\n\n    <<<<<<< HEAD\n      print(A);\n    ||||||| merged common ancestors\n    <<<<<<< Temporary merge branch 1\n      print(B);\n    =======\n      print(C);\n    >>>>>>> feature\n\nI have combed Google but have been able to find no good explanation.\nPeople are hitting these and asking for help, but the suggestions\nare weak and fail to explain anything.\n\n\nBackground\n----------\n\nOur workflow often causes criss-cross merges. In this more extreme\n(but real) case, I have 16 ambiguously \"best\" merge bases:\n\n    $ git merge-base --all HEAD feature | wc -l\n    16\n\nThe 'resolve' strategy isn't an option for this many merge-bases:\n\n    $ git merge -s resolve feature\n    fatal: I cannot read more than 8 trees\n    Merge with strategy resolve failed.\n\nThat brings me back to the recursive strategy.\n\nhttps://lwn.net/Articles/210045/ \"Branching and merging with git\"\ndescribes what a criss-cross merge is and how the recursive strategy\ndeals with them by merging the multiple merge-bases to form a single\ntemporary merge base.\n\n> The classic confusing case is called a \"criss-cross merge\", and\n> looks like this:\n>\n>          o--b-o-o--B\n>         /    \\ /\n>  o--o--o      X\n>         \\    / \\\n>          o--a-o-o--A\n>\n> There are two common ancestors of A and B, marked a and b in the\n> graph above.  And they're not the same.  You could use either one\n> and get reasonable results, but how to choose?\n>\n> The details are too advanced for this discussion, but the default\n> \"recursive\" merge strategy that git uses solves the answer by\n> merging a and b into a temporary commit and using *that* as the\n> merge base.\n>\n> Of course, a and b could have the same problem, so merging them\n> could require another merge of still-older commits.  This is why the\n> algorithm is called \"recursive.\"\n\nWith the diff3 conflictstyle showing the merged common ancestors and\nthe recursive merge strategy, the merged common ancestors may have a\nconflict\n\nConflict #1\n-----------\n\n     1 <<<<<<< HEAD\n     2     unless admin\n     3       fail Unauthorized.new(\"Admin only\")\n     4     end\n     5 ||||||| merged common ancestors\n     6 <<<<<<< Temporary merge branch 1\n     7     unless admin\n     8       fail Unauthorized.new(\"Admin only\")\n     9     end\n    10 ||||||| merged common ancestors\n    11     unless admin\n    12       fail Unauthorized.new\n    13     end\n    14 =======\n    15     fail Unauthorized.new unless admin\n    16 >>>>>>> Temporary merge branch 2\n    17 =======\n    18     unless admin\n    19         fail Unauthorized.new(\"Admin only\")\n    20       fail Unauthorized.new\n    21     end\n    22 >>>>>>> feature\n\nIt seems lines 6-16 are a conflict that occurred when merging the\nmerge-bases.  That conflict could be resolved by merging the change in\nTemporary merge branch 1 (add \"Admin only\") with Temporary merge\nbranch 2 (convert multi-line unless to single-line) as this:\n\n           fail Unauthorized.new(\"Admin only\") unless admin\n\nThus, you might thing that you could refactor the above conflict to be\nthis:\n\n     1 <<<<<<< HEAD\n     2     unless admin\n     3       fail Unauthorized.new(\"Admin only\")\n     4     end\n     5 ||||||| merged common ancestors\n     6     fail Unauthorized.new(\"Admin only\") unless admin\n     7 =======\n     8     unless admin\n     9         fail Unauthorized.new(\"Admin only\")\n    10       fail Unauthorized.new\n    11     end\n    12 >>>>>>> feature\n\nHowever this would be resolved by merging HEAD's apparent change\n(break single-line unless onto multiple lines) and feature's change,\nwhich appears to be a botched up conflict resolution.\n\nObviously I'm reading this wrong. What is the correct way?\n\nConflict #2\n------------\n\n     1 <<<<<<< HEAD\n     2     unless feature.enabled_for_user?(UserIdLookup.new(params).user_id)\n     3       fail Unauthorized.new(\"Requires setting #{label}.\")\n     4 ||||||| merged common ancestors\n     5 <<<<<<< Temporary merge branch 1\n     6     unless feature.enabled_for_user?(params[:user_id])\n     7       fail Unauthorized.new(\"Requires setting #{label}.\")\n     8 =======\n     9     unless feature.enabled_for_user?(params[:user_id])\n    10       fail Unauthorized.new(\"Requires setting #{label}.\")\n    11 >>>>>>> feature\n\nThis is the full conflict, and it doesn't seem to balance. In this\ncase, there's only one Temporary merge branch. Can the line marker on\nline 5 just be ignored?  If so, then this is easy to resolve, but I am\nleft puzzled as to why the sections in lines 6-7 and 9-10 are\nidentical.  Is the Temporary merge branch 1 marker here simply because\nother conflicts had multiple Temporary merge branches that had to be\nlabeled?  Or is the 0 lines between lines 4 and 5 a significant\ndeletion?\n\n---\n\nThe git community needs some light and understanding shed on this\ntopic, on how these conflicts are to be read and understood. I'm\nlooking for something that can be applied methodically. If better\nunderstanding the recursive merge strategy is necessary, that is fine.\nHowever suggestions like \"just resolve it to how you think it should\nbe\" or \"talk to the authors\" will not be appreciated, as they do not\nlend understanding regarding these conflict markers nor where the code\nthey delineate comes from.\n\nDoes anyone out there understand this stuff?\n\nThanks,\nEdward\n"},{"id":"265751","messageId":"vpqbnfondsr.fsf@anie.imag.fr","threadId":"39800","inReplyTo":"CAOMsSXQVJsd0h1fnNMEJ5+cKpxbeF9mHraXva-wr6Y2zBCADbg@mail.gmail.com","subject":"Re: No one understands diff3 \"Temporary merge branch\" conflict markers","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-07T16:16:36Z","receivedAt":"2015-07-07T16:16:36Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Edward Anderson <nilbus@nilbus.com> writes:\n\n> I have the diff3 conflictstyle enabled and want to be able to\n> understand how to understand its output when there are criss-cross\n> merges requiring temporary merge branches.  Eg:\n>\n>     <<<<<<< HEAD\n>       print(A);\n>     ||||||| merged common ancestors\n>     <<<<<<< Temporary merge branch 1\n>       print(B);\n>     =======\n>       print(C);\n>     >>>>>>> feature\n\nI guess you are seeing the result of the recursive-merge.\n\n>> The details are too advanced for this discussion, but the default\n>> \"recursive\" merge strategy that git uses solves the answer by\n>> merging a and b into a temporary commit and using *that* as the\n>> merge base.\n\nThat is the point. We don't have a good common ancestor, so Git builds\none by merging the common ancestors. Then, two things can happen:\n\n* The merge of the common ancestors is conflict-free. Then, we get a\n  \"sane\" common ancestor.\n\n* The merge has conflicts. In this case, the common ancestor that Git\n  built has conflict markers. It is not a big issue, since when merging\n  A, B, and ancestor(A, B), the result of the merge is either A or B,\n  but never comes from ancestor(A, B). So, you never get to see the\n  temporary ancestor(A, B), *except* when you request the common\n  ancestor in the merge conflict.\n\nIt gets nasty since you get recursive merge conflicts, but you don't see\nthe recursivity. Let me try to indent your conflict:\n\n 1 <<<<<<< HEAD\n 2     unless admin\n 3       fail Unauthorized.new(\"Admin only\")\n 4     end\n 5 ||||||| merged common ancestors\n 6         <<<<<<< Temporary merge branch 1\n 7             unless admin\n 8               fail Unauthorized.new(\"Admin only\")\n 9             end\n10         ||||||| merged common ancestors\n11             unless admin\n12               fail Unauthorized.new\n13             end\n14         =======\n15             fail Unauthorized.new unless admin\n16         >>>>>>> Temporary merge branch 2\n17 =======\n18     unless admin\n19         fail Unauthorized.new(\"Admin only\")\n20       fail Unauthorized.new\n21     end\n22 >>>>>>> feature\n\n> It seems lines 6-16 are a conflict that occurred when merging the\n> merge-bases.\n\nYes.\n\n> That conflict could be resolved by merging the change in Temporary\n> merge branch 1 (add \"Admin only\") with Temporary merge branch 2\n> (convert multi-line unless to single-line) as this:\n>\n>            fail Unauthorized.new(\"Admin only\") unless admin\n\nThat is probably what you would do if you resolved the conflict\nmanually, but while merging the common ancestors, Git found an ancestor\nof an ancestor that was different from both ancestors being merged, and\nthere was a conflict. Asking you to resolve this conflict would be\nessentially a loss of time since Git knows that the result won't appear\nin the final merge, but only in the merge base.\n\n 1 <<<<<<< HEAD\n 2     unless feature.enabled_for_user?(UserIdLookup.new(params).user_id)\n 3       fail Unauthorized.new(\"Requires setting #{label}.\")\n 4 ||||||| merged common ancestors\n 5         <<<<<<< Temporary merge branch 1\n 6             unless feature.enabled_for_user?(params[:user_id])\n 7               fail Unauthorized.new(\"Requires setting #{label}.\")\n 8 =======\n 9     unless feature.enabled_for_user?(params[:user_id])\n10       fail Unauthorized.new(\"Requires setting #{label}.\")\n11 >>>>>>> feature\n\n> This is the full conflict, and it doesn't seem to balance.\n\nRight: I guess the merge-base was stg like\n\n<<<<<<< Temporary merge branch 1\n    unless feature.enabled_for_user?(params[:user_id])\n      fail Unauthorized.new(\"Requires setting #{label}.\")\n|||||||\nblabla 1\n=======\nblabla 2\n>>>>>>> Temporary merge branch 2\n\nBut then, the actual merge happens, using this as merge-base. A conflict\noccurs when the commits being merged and the merge-base are all\ndifferent. In your case, I guess the commits being merged were identical\non the next different hunks (the line \"blablabla 1\" probably was in both\ncommits being merged, which allowed the merge algorithm to move to the\nnext hunk), and there were no conflict in this hunk, hence you don't see\nthe merge base.\n\nI hope this helps on the \"light and understanding\" part of your\nquestion. Now, as of \"what to do when I get this?\", I would say: the\nrecursive merge-base was computed internally, but not really meant to be\nshown to the user. You should probably ignore it and resolve the merge\nby looking only at the 2 sides of the conflict (\"ours\" and \"theirs\").\nSorry, this is probably not the answer you expected, but it's the best I\ncan give ;-).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"265756","messageId":"xmqqzj373lrn.fsf@gitster.dls.corp.google.com","threadId":"39800","inReplyTo":"vpqbnfondsr.fsf@anie.imag.fr","subject":"Re: No one understands diff3 \"Temporary merge branch\" conflict markers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-07T17:44:44Z","receivedAt":"2015-07-07T17:44:44Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n> ... I would say: the\n> recursive merge-base was computed internally, but not really meant to be\n> shown to the user.\n\nI wonder if the output becomes easier to read if we unconditionally\nturned off diff3-style for inner merges.\n"},{"id":"265773","messageId":"vpqd203lk1l.fsf@anie.imag.fr","threadId":"39800","inReplyTo":"xmqqzj373lrn.fsf@gitster.dls.corp.google.com","subject":"Re: No one understands diff3 \"Temporary merge branch\" conflict markers","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-07T21:44:38Z","receivedAt":"2015-07-07T21:44:38Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n>\n>> ... I would say: the\n>> recursive merge-base was computed internally, but not really meant to be\n>> shown to the user.\n>\n> I wonder if the output becomes easier to read if we unconditionally\n> turned off diff3-style for inner merges.\n\nOr replace the whole conflict markers with \"Sorry, cannot compute a\nmerge base\" text when doing the recursive merge to build the merge base.\n\nI don't know that area well enough to have a real opinion.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"}]}