{"thread":{"id":"15343","subject":"Merge seems to get confused by (reverted) cherry-picks","startedAt":"2008-09-03T07:20:11Z","lastAt":"2008-09-03T17:10:29Z","messageCount":9,"participants":["Björn Steinbrink","Junio C Hamano","Ittay Dror","Andreas Ericsson","Johan Herland","Linus Torvalds","Mike Hommey"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"89587","messageId":"20080903072011.GA14252@atjola.homenet","threadId":"15343","inReplyTo":null,"subject":"Merge seems to get confused by (reverted) cherry-picks","fromName":"Björn Steinbrink","fromEmail":"b.steinbrink@gmx.de","sentAt":"2008-09-03T07:20:11Z","receivedAt":"2008-09-03T07:20:11Z","isPatch":false,"sender":{"key":"b.steinbrink@gmx.de","avatar":"https://avatars.githubusercontent.com/u/230962?v=4"},"body":"Hi,\n\n\"git merge\" produces a (IMHO) wrong result, when a commit from the\nbranch that is to be merged in was cherry-picked into the current branch\nand later reverted on the original branch. Basically ignoring the\nrevert.\n\nExample:\nmkdir gmt\ncd gmt\ngit init\n\n/bin/echo -e \"1\\n2\\n3\\n4\\n5\\n6\\n\" > file\ngit add file\ngit commit -m init\n\n/bin/echo -e \"1\\n2\\n3\\na\\n4\\n5\\n6\\n\" > file\ngit add file\ngit commit -m add\n\ngit revert --no-edit HEAD\n\ngit checkout -b test HEAD~2\nsleep 1 # Avoid race\ngit cherry-pick master^\ngit merge master\n\n\nThat last \"git merge\" call happily tells:\nAlready uptodate!\nMerge made by recursive.\n\nAnd \"file\" still contains the \"a\" that was added in the second commit.\n\nSeems broken to me, of course I want that revert to \"show up\" in the\nmerge result, probably as a conflict for me to resolve.\n\nBjörn\n"},{"id":"89588","messageId":"20080903072556.GA18449@atjola.homenet","threadId":"15343","inReplyTo":"20080903072011.GA14252@atjola.homenet","subject":"Re: Merge seems to get confused by (reverted) cherry-picks","fromName":"Björn Steinbrink","fromEmail":"b.steinbrink@gmx.de","sentAt":"2008-09-03T07:25:56Z","receivedAt":"2008-09-03T07:25:56Z","isPatch":false,"sender":{"key":"b.steinbrink@gmx.de","avatar":"https://avatars.githubusercontent.com/u/230962?v=4"},"body":"On 2008.09.03 09:20:11 +0200, Björn Steinbrink wrote:\n> Hi,\n> \n> \"git merge\" produces a (IMHO) wrong result, when a commit from the\n> branch that is to be merged in was cherry-picked into the current branch\n> and later reverted on the original branch. Basically ignoring the\n> revert.\n> \n> Example:\n> mkdir gmt\n> cd gmt\n> git init\n> \n> /bin/echo -e \"1\\n2\\n3\\n4\\n5\\n6\\n\" > file\n> git add file\n> git commit -m init\n> \n> /bin/echo -e \"1\\n2\\n3\\na\\n4\\n5\\n6\\n\" > file\n> git add file\n> git commit -m add\n> \n> git revert --no-edit HEAD\n> \n> git checkout -b test HEAD~2\n> sleep 1 # Avoid race\n> git cherry-pick master^\n> git merge master\n> \n> \n> That last \"git merge\" call happily tells:\n> Already uptodate!\n> Merge made by recursive.\n> \n> And \"file\" still contains the \"a\" that was added in the second commit.\n> \n> Seems broken to me, of course I want that revert to \"show up\" in the\n> merge result, probably as a conflict for me to resolve.\n\nI knew I would forget something... That's with:\n$ git --version\ngit version 1.6.0.1.196.g01914\n\nBjörn\n"},{"id":"89590","messageId":"7vprnld5ws.fsf@gitster.siamese.dyndns.org","threadId":"15343","inReplyTo":"20080903072011.GA14252@atjola.homenet","subject":"Re: Merge seems to get confused by (reverted) cherry-picks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-09-03T07:50:43Z","receivedAt":"2008-09-03T07:50:43Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Björn Steinbrink writes:\n\n> \"git merge\" produces a (IMHO) wrong result, when a commit from the\n> branch that is to be merged in was cherry-picked into the current branch\n> and later reverted on the original branch. Basically ignoring the\n> revert.\n\nThere are a few issues around 3-way merge.\n\nOne thing is, what happened in between the common ancestor and the final\nresults on histories before you initiate the merges does not matter. When\ndoing a 3-way merge, you look only at three endpoints: your final state,\ntheir final state and the common ancestor between the two.\n\nYour history looks like this:\n\n 123a456\n 3-----------?\n / /\n / /\n 0-----1-----2\n 123456 123456\n\nDuring the development between 0..2, your undecision might have caused the\ncontents of the file fluctuate back and forth many number of times, but in\nthe end result, you (the one who went 0..1..2) decided that for your\ndevelopment, you do not have to change the contents of that path. The\npath might have been a xyzzy.h file, and you once added an extern decl of\na function because you thought a function in xyzzy.c might be needed by\nanother file frotz.c, but it turned out that the function can stay private\nand you removed that extern decl from xyzzy.h and ended up in the same\nstate.\n\nBut the other person who built the history 0..3 decided that having the\nchange is better than not having it. Perhaps his code does use the\nfunction from some other place and needs an extern.\n\nYou are merging the two histories, which means by definition you trust\nyour decision and the other guys decision with equal weight. And here is\nanother thing.\n\nWhen you compare the path in question at 0 and 2, you see they are\nidentical. And you are interpreting that \"I say they MUST STAY THE SAME,\nwhile they say they want to change it some way, that is a conflict\".\n\nBut in 3-way merge context, you do not interpret the fact that something\nis identical between 0..2 as \"they MUST STAY THE SAME\". Instead, you read\nit as \"My history does not care what happens to that path -- if the other\nguy wants to change it, I'll happily take it.\"\n\n Note. I am not claiming that the above interpretation will always\n match what you would expect. I am just explaining how the underlying\n concept of 3-way merge works in general. If you think about it in a\n realistic context, such as the \"extern in xyzzy.h you did not need to\n add but the other guy needed to have\", you'll realize that more often\n than not, \"I do not care and let the other guy decide\" interpretation\n results in a more useful result.\n\nThat essentially boils down to three rules:\n\n (0) If both of you did not change anything, the final result won't have\n any change (obvious);\n\n (1) If you decided you do not have a need to change a path, but the other\n one saw a need, you take the change;\n\n (2) If you and the other one both wanted to change a path but in a\n different way, you need to merge at the contents level.\n\nAnd you are seeing rule (1) in action.\n"},{"id":"89595","messageId":"20080903083739.GA18653@atjola.homenet","threadId":"15343","inReplyTo":"7vprnld5ws.fsf@gitster.siamese.dyndns.org","subject":"Re: Merge seems to get confused by (reverted) cherry-picks","fromName":"Björn Steinbrink","fromEmail":"b.steinbrink@gmx.de","sentAt":"2008-09-03T08:37:39Z","receivedAt":"2008-09-03T08:37:39Z","isPatch":false,"sender":{"key":"b.steinbrink@gmx.de","avatar":"https://avatars.githubusercontent.com/u/230962?v=4"},"body":"On 2008.09.03 00:50:43 -0700, Junio C Hamano wrote:\n> But in 3-way merge context, you do not interpret the fact that something\n> is identical between 0..2 as \"they MUST STAY THE SAME\". Instead, you read\n> it as \"My history does not care what happens to that path -- if the other\n> guy wants to change it, I'll happily take it.\"\n> \n> Note. I am not claiming that the above interpretation will always\n> match what you would expect. I am just explaining how the underlying\n> concept of 3-way merge works in general. If you think about it in a\n> realistic context, such as the \"extern in xyzzy.h you did not need to\n> add but the other guy needed to have\", you'll realize that more often\n> than not, \"I do not care and let the other guy decide\" interpretation\n> results in a more useful result.\n> \n> That essentially boils down to three rules:\n> \n> (0) If both of you did not change anything, the final result won't have\n> any change (obvious);\n> \n> (1) If you decided you do not have a need to change a path, but the other\n> one saw a need, you take the change;\n> \n> (2) If you and the other one both wanted to change a path but in a\n> different way, you need to merge at the contents level.\n> \n> And you are seeing rule (1) in action.\n\nOK, so that basically means \"if you cherry-pick, you better make sure\nthat you don't have to revert or just get your fine-toothed comb ready\nwhen you merge later\", right? Any advice on how to deal with such a\nsituation?\n\nThe actual use case I had was somewhat like this:\nSome new code got added in commit A on the master branch as it was\nintended for the next major release. Then it became obvious that it\nneeds to go into a minor release, so it got cherry-picked into the\nmaintainance branch. For some reason that new code then got moved into a\ndifferent file. That move happened on the maintainance branch (ie.\nimmediate bugfix). And then came the regular merge of the maintainance\nbranch into the master branch, which ended up with the code being\nduplicated.\n\n(Well, actually, it's a two-level maintainance branch setup, but you get\nthe idea...)\n\nOf course, in a perfect world the first commit would never have been\ncherry-picked and would've been originally made on the maintainance\nbranch, and the problem would have never shown up. But that's not the\ncase and I'm not omniscient, so I would have surely missed the problem,\nif it wasn't for that two-level setup, which for this merge meant that\nthe merge result should have been equal to the branch being merged, so\nit was easy to spot.\n\nBjörn\n"},{"id":"89605","messageId":"48BE56BD.6050805@gmail.com","threadId":"15343","inReplyTo":"7vprnld5ws.fsf@gitster.siamese.dyndns.org","subject":"Re: Merge seems to get confused by (reverted) cherry-picks","fromName":"Ittay Dror","fromEmail":"ittay.dror@gmail.com","sentAt":"2008-09-03T09:19:57Z","receivedAt":"2008-09-03T09:19:57Z","isPatch":false,"sender":{"key":"ittay.dror@gmail.com","avatar":"https://gravatar.com/avatar/0dc47ece3354a295768075bb5afa328d49cda722c9f3100029ada80ee4561e22?d=mp&s=160"},"body":"Note: codeville tried to implement a merge algorithm that considers the \nhistory to decide what the user wants to do: \nhttp://revctrl.org/PreciseCodevilleMerge. Maybe worth while exploring?\n\nIttay\n\nJunio C Hamano wrote:\n> Björn Steinbrink writes:\n>\n> \n>> \"git merge\" produces a (IMHO) wrong result, when a commit from the\n>> branch that is to be merged in was cherry-picked into the current branch\n>> and later reverted on the original branch. Basically ignoring the\n>> revert.\n>> \n>\n> There are a few issues around 3-way merge.\n>\n> One thing is, what happened in between the common ancestor and the final\n> results on histories before you initiate the merges does not matter. When\n> doing a 3-way merge, you look only at three endpoints: your final state,\n> their final state and the common ancestor between the two.\n>\n> Your history looks like this:\n>\n> 123a456\n> 3-----------?\n> / /\n> / /\n> 0-----1-----2\n> 123456 123456\n>\n> During the development between 0..2, your undecision might have caused the\n> contents of the file fluctuate back and forth many number of times, but in\n> the end result, you (the one who went 0..1..2) decided that for your\n> development, you do not have to change the contents of that path. The\n> path might have been a xyzzy.h file, and you once added an extern decl of\n> a function because you thought a function in xyzzy.c might be needed by\n> another file frotz.c, but it turned out that the function can stay private\n> and you removed that extern decl from xyzzy.h and ended up in the same\n> state.\n>\n> But the other person who built the history 0..3 decided that having the\n> change is better than not having it. Perhaps his code does use the\n> function from some other place and needs an extern.\n>\n> You are merging the two histories, which means by definition you trust\n> your decision and the other guys decision with equal weight. And here is\n> another thing.\n>\n> When you compare the path in question at 0 and 2, you see they are\n> identical. And you are interpreting that \"I say they MUST STAY THE SAME,\n> while they say they want to change it some way, that is a conflict\".\n>\n> But in 3-way merge context, you do not interpret the fact that something\n> is identical between 0..2 as \"they MUST STAY THE SAME\". Instead, you read\n> it as \"My history does not care what happens to that path -- if the other\n> guy wants to change it, I'll happily take it.\"\n>\n> Note. I am not claiming that the above interpretation will always\n> match what you would expect. I am just explaining how the underlying\n> concept of 3-way merge works in general. If you think about it in a\n> realistic context, such as the \"extern in xyzzy.h you did not need to\n> add but the other guy needed to have\", you'll realize that more often\n> than not, \"I do not care and let the other guy decide\" interpretation\n> results in a more useful result.\n>\n> That essentially boils down to three rules:\n>\n> (0) If both of you did not change anything, the final result won't have\n> any change (obvious);\n>\n> (1) If you decided you do not have a need to change a path, but the other\n> one saw a need, you take the change;\n>\n> (2) If you and the other one both wanted to change a path but in a\n> different way, you need to merge at the contents level.\n>\n> And you are seeing rule (1) in action.\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n>\n> \n\n-- \nIttay Dror \nTikal \nTikal Project \n\n\n-- \n--\nIttay Dror \n"},{"id":"89617","messageId":"48BE6F37.3070302@op5.se","threadId":"15343","inReplyTo":"48BE56BD.6050805@gmail.com","subject":"Re: Merge seems to get confused by (reverted) cherry-picks","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2008-09-03T11:04:23Z","receivedAt":"2008-09-03T11:04:23Z","isPatch":false,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Ittay Dror wrote:\n> Note: codeville tried to implement a merge algorithm that considers the \n> history to decide what the user wants to do: \n> http://revctrl.org/PreciseCodevilleMerge. Maybe worth while exploring?\n> \n\nI believe that one would still fail for this case, as \"ImplicitUndo\"\nisn't included.\n\nBesides that, I'd rather have something simple that does something\npredictable than something complex that pushes the \"gets it right\"\nfigure from 98% to 99.2%, while failing in completely annoying and\nsurprising ways sometimes. Someone or something still has to verify\nthat the merge produced something sane.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"89618","messageId":"200809031316.05336.johan@herland.net","threadId":"15343","inReplyTo":"48BE56BD.6050805@gmail.com","subject":"Re: Merge seems to get confused by (reverted) cherry-picks","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-09-03T11:16:05Z","receivedAt":"2008-09-03T11:16:05Z","isPatch":false,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Wednesday 03 September 2008, Ittay Dror wrote:\n> Note: codeville tried to implement a merge algorithm that considers\n> the history to decide what the user wants to do:\n> http://revctrl.org/PreciseCodevilleMerge. Maybe worth while\n> exploring?\n\nYou haven't been here long, have you? ;)\n\nThere was an infamous mailing list discussion between Bram Cohen (who \ncreated PreciseCodevilleMerge), and Linus Torvalds back in 2005, \ndiscussing merge strategies. It is well recounted here:\n\nhttp://wincent.com/a/about/wincent/weblog/archives/2007/07/a_look_back_bra.php\n\nAlso, nowadays, even Bram himself seems to have conceded \nPreciseCodevilleMerge in favour of traditional 3-way merge, at least if \nyou look at item #3 in the following posting from his blog:\n\nhttp://bramcohen.livejournal.com/52148.html\n\n\n\n...Johan\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"89636","messageId":"alpine.LFD.1.10.0809030741190.3259@nehalem.linux-foundation.org","threadId":"15343","inReplyTo":"20080903083739.GA18653@atjola.homenet","subject":"Re: Merge seems to get confused by (reverted) cherry-picks","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-09-03T15:26:10Z","receivedAt":"2008-09-03T15:26:10Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 3 Sep 2008, Björn Steinbrink wrote:\n> \n> OK, so that basically means \"if you cherry-pick, you better make sure\n> that you don't have to revert or just get your fine-toothed comb ready\n> when you merge later\", right? Any advice on how to deal with such a\n> situation?\n\nWell, it's not actually even really related to cherry-picks in particular, \nalthough yes, cherry-picks and reverts are perhaps the simplest case to \nexplain.\n\nAt a more fundamental level, it's about git _not_ caring about any \nindividual commit in the history, and only caring about the \"big picture\". \n\nIn this, git is fairly consistent - the same way that git never cares \nabout any individual _file_, but always merges the whole tree, it also \nnever really cares about any individual _commit_, but always the whole \nhistory.\n\nSo it really doesn't matter if one commit undid another - the only thing \nthat matters as far as git merging is concerned is what the final set of \nchanges were from the common base point.\n\nNow, there's nothing to say that git _couldn't_ try to look at individual \ncommits when deciding how to merge, but I actually think it's \nfundamentally wrong to do so, for pretty much the same reason I think it's \nfundamentally wrong to try to encode rename information.\n\nThe fact is, should it really matter whether something was \"reverted\", or \nwhether multiple gradual changes made ot go back to what it used to be? \nGit says no, an considers the two to be totally equivalent in the end. \nexactly the same way that git doesn't matter whether you first created a \nnew file and later deleted a similar old file, or whether you renamed it. \n\nThe only thing that matters is the end result.\n\nSo if you have one branch that first does \"A\", and then does \"revert A\", \nas far as git is concerned, that branch didn't do anything at all when it \ncomes to data.\n\nSo when you merge it with another branch that does \"A\" too, the end result \nis that the merged contents will have A. That's fairly easy to understand \nif you just think of git as caring about the whole end result and just \ndoing a three-way merge at the end points (which is what it does), but I \nwould also like to explain why I think it's fundamentally the _right_ \nthing to do, not just an \"implementation detail because it's simpler\".\n\nWhen one branch does the \"revert A\" does that really mean that A was bad? \nNo. It could mean that A was \"unnecessary\" or \"not quite ready\". The \nrevert, after all, was literally done in the context of _that_ branch, and \nthe reasons may well have been totally private to that branch. Git doesn't \nknow, and git shouldn't care - the only thing that should matter is the \nend result.\n\nTo really hammer this point in, let's say that \"A\" was really doing two \ndifferent things - A1 and A2 - to two different files. And let's further \nsay that it had been cherry-picked because the one branch needed just a \npart of it - A1. And then later, in a fit of cleanup madness, that branch \nundid A2, because it really didn't need it.\n\nWhen you merge the two branches, what do you expect? You could argue that \nyou would expect A2 to be undone in the original branch too. It was, after \nall, a partial revert. But I think you'll agree that it's not at all \n\"obvious\" any more.\n\nIn fact, I will argue that it would be horribly _wrong_, because the \nbranch that undid part of A could have done it two different ways:\n\n - it could do the \"cherry-pick A\" as one commit and the \"undo A2\" as \n another (as I implied above)\n\n - but it could equally well have done a \"cherry-pick just part of A\", and \n done it as just one commit (perhaps because it noticed that A2 didn't \n even _compile_ within the context of that branch, and did an '--amend' \n to fix it up rather than create a new commit.\n\nSee? Shouldn't both really act the same? Should it really make a \ndifference to what git does if there was a cherry-pick and a partial \nrevert, or a partial cherry-pick? Should _how_ you do something matter \nmore than the end result? HELL NO!\n\nAnd is the \"partial revert\" really any different from the \"total revert\"?\n\nNow, if you're a math person, think of the \"limit behavior\" as A2 \napproaches all of A. The final end result is that you were to revert _all_ \nof A. Should that limit case be fundamentally different from the case of \nA2 being just a _part_ of A? What should the logic be? What if all of A \nwas reverted except for the whitespace cleanups that it did (almost by \nmistake?)\n\nSo in the end, the answer is that git doesn't care about individual \ncommits, because caring about individual commits is totally crazy. Git \n_remembers_ them, of course, and it can _show_ you them when you merge, \nbut the actual end result depends purely on the *state* of the merge (and \nthe \"big picture\" of the history, ie where the branches join etc), not of \nthe small details of how you got to that state.\n\nAnd that is fundamentally the only sane thing to do.\n\nHere's another final thought to leave you with:\n\n - what if the other branch had decided that instead of reverting it, it \n could just do a \"git rebase -i\" _without_ it, because that other branch \n had never been exposed to anybody else?\n\nSee? The \"how you got to some state\" really must be immaterial in a stable \nmerge strategy. I realize that I'm at odds with some SCM people on this, \nbut I'm ok with that, because I also realize that all those other SCM \npeople are just _stupid_.\n\n\t\t\tLinus\n"},{"id":"89650","messageId":"20080903171028.GA1657@glandium.org","threadId":"15343","inReplyTo":"200809031316.05336.johan@herland.net","subject":"Re: Merge seems to get confused by (reverted) cherry-picks","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2008-09-03T17:10:29Z","receivedAt":"2008-09-03T17:10:29Z","isPatch":false,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Wed, Sep 03, 2008 at 01:16:05PM +0200, Johan Herland wrote:\n> On Wednesday 03 September 2008, Ittay Dror wrote:\n> > Note: codeville tried to implement a merge algorithm that considers\n> > the history to decide what the user wants to do:\n> > http://revctrl.org/PreciseCodevilleMerge. Maybe worth while\n> > exploring?\n> \n> You haven't been here long, have you? ;)\n> \n> There was an infamous mailing list discussion between Bram Cohen (who \n> created PreciseCodevilleMerge), and Linus Torvalds back in 2005, \n> discussing merge strategies. It is well recounted here:\n> \n> http://wincent.com/a/about/wincent/weblog/archives/2007/07/a_look_back_bra.php\n\nI quite agree with the footnote statement, except on one thing: it would\nmake it easier to have additional tools to help with such merge\nresolutions automatically.\n\nWhile resolving a rename/delete merge conflict is not a big deal,\nresolving function moves from one file to another (possibly new) file is\nanother story.\n\nMike\n"}]}