{"thread":{"id":"1631","subject":"baffled again","startedAt":"2005-08-23T22:56:27Z","lastAt":"2005-08-25T05:58:23Z","messageCount":10,"participants":["tony.luck@intel.com","Tony Luck","Linus Torvalds","Junio C Hamano","Daniel Barkalow"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"7680","messageId":"200508232256.j7NMuR1q027892@agluck-lia64.sc.intel.com","threadId":"1631","inReplyTo":null,"subject":"baffled again","fromName":"","fromEmail":"tony.luck@intel.com","sentAt":"2005-08-23T22:56:27Z","receivedAt":"2005-08-23T22:56:27Z","isPatch":false,"sender":{"key":"tony.luck@intel.com","avatar":"https://avatars.githubusercontent.com/u/5446021?v=4"},"body":"So I have another anomaly in my GIT tree. A patch to\nback out a bogus change to arch/ia64/hp/sim/boot/bootloader.c\nin my release branch at commit\n\n 62d75f3753647656323b0365faa43fc1a8f7be97\n\nappears to have been lost when I merged the release branch to\nthe test branch at commit\n\n 0c3e091838f02c537ccab3b6e8180091080f7df2\n\nSo now this file still has this:\n\n/* SSC_WAIT_COMPLETION appears to want this large alignment. gcc < 4\n* seems to give it by default, however gcc > 4 is smarter and may\n* not.\n*/\nstruct disk_stat {\n\tint fd;\n\tunsigned count;\n} __attribute__ ((aligned (16)));\n\nin the test branch, when I think the comment and __attribute__\nshould have been backout out.\n\n-Tony\n\nTree is at rsync://rsync.kernel.org/pub/scm/linux/kernel/git/aegl/linux-2.6.git\n"},{"id":"7699","messageId":"12c511ca050823223333c41857@mail.gmail.com","threadId":"1631","inReplyTo":"200508232256.j7NMuR1q027892@agluck-lia64.sc.intel.com","subject":"Re: baffled again","fromName":"Tony Luck","fromEmail":"tony.luck@gmail.com","sentAt":"2005-08-24T05:33:33Z","receivedAt":"2005-08-24T05:33:33Z","isPatch":false,"sender":{"key":"tony.luck@gmail.com","avatar":null},"body":"I'm at home, and too lazy to log in to work to look at my tree. But I\nhave a theory\nas to what went wrong for me.\n\nAt the start I had a file, same contents in test and release branch.\n\nI applied a patch to release, and pulled to test. So the contents are still\nthe same, both with the patch applied.\n\nNext, I was given a better patch (the first one just masked the real problem\nand happened to make the symptoms go away). This patch touches a\ncompletely different file. So I applied a patch to revert the change\nin release,\nand the new patch.\n\nNow ... when I try to merge release into test, my guess is that GIT is\nlooking at the common ancestor before I touched anything. So when\nit compares the current state of this file it sees that I have the bad patch\nin the test tree, and the release tree has the \"original\" version (which has\nhad the patch applied and reverted ... so the contents are back at the\noriginal state).\n\nSo GIT decides that the test branch has had a patch, and the release\nbranch hasn't ... and so it merges by keeping the version in test.\n\nPlausible?\n\n-Tony\n"},{"id":"7700","messageId":"Pine.LNX.4.58.0508232258170.3317@g5.osdl.org","threadId":"1631","inReplyTo":"12c511ca050823223333c41857@mail.gmail.com","subject":"Re: baffled again","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-08-24T05:58:34Z","receivedAt":"2005-08-24T05:58:34Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 23 Aug 2005, Tony Luck wrote:\n> \n> So GIT decides that the test branch has had a patch, and the release\n> branch hasn't ... and so it merges by keeping the version in test.\n> \n> Plausible?\n\nVery. Sounds like what happened.\n\n\t\tLinus\n"},{"id":"7701","messageId":"7vek8jhk7y.fsf@assigned-by-dhcp.cox.net","threadId":"1631","inReplyTo":"200508232256.j7NMuR1q027892@agluck-lia64.sc.intel.com","subject":"Re: baffled again","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-08-24T07:23:13Z","receivedAt":"2005-08-24T07:23:13Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"tony.luck@intel.com writes:\n\n> So I have another anomaly in my GIT tree. A patch to\n> back out a bogus change to arch/ia64/hp/sim/boot/bootloader.c\n> in my release branch at commit\n>\n> 62d75f3753647656323b0365faa43fc1a8f7be97\n>\n> appears to have been lost when I merged the release branch to\n> the test branch at commit\n>\n> 0c3e091838f02c537ccab3b6e8180091080f7df2\n\n : siamese; git cat-file commit 0c3e091838f02c537ccab3b6e8180091080f7df2 \n tree 61a407356d1e897e0badea552ce69e657cab6108\n parent 7ffacc1a2527c219b834fe226a7a55dc67ca3637\n parent a4cce10492358b33d33bb43f98284c80482037e8\n author Tony Luck 1124808655 -0700\n committer Tony Luck 1124808655 -0700\n\n Pull release into test branch\n\nSo I pulled 7ffacc and a4cce1 from your repository and started\ndigging from there. 7ffacc was the head of \"test\" branch back\nthen, and a4cce1 was the head of \"release\" branch. I checked\nout 7ffacc in the repository and pulled a4cce1 into it, using\nthe GIT with the \"optimum merge-base\" patch.\n\n : siamese; git pull . aegl-release\n Packing 0 objects\n Unpacking 0 objects\n\n * committish: a4cce10492358b33d33bb43f98284c80482037e8\trefs/heads/aegl-release from .\n Trying to find the optimum merge base.\n Trying to merge a4cce10492358b33d33bb43f98284c80482037e8 into 7ffacc1a2527c219b834fe226a7a55dc67ca3637 using c1ffb910f7a4e1e79d462bb359067d97ad1a8a25.\n Simple merge failed, trying Automatic merge\n Auto-merging arch/ia64/sn/kernel/io_init.c.\n Committed merge db376974c0aebb9e99e5cd0bce21088c6a9d927c\n arch/ia64/hp/sim/boot/boot_head.S | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\nIt is using c1ffb9 as the merge base. The problematic path\nin the three trees involved are:\n\n: siamese; git ls-tree -r aegl-test-7ffacc1a | grep arch/ia64/hp/sim/boot/bootloader.c\n100644 blob a7bed60b69f9e8de9a49944e22d03fb388ae93c7\tarch/ia64/hp/sim/boot/bootloader.c\n: siamese; git ls-tree -r aegl-release-a4cce1 | grep arch/ia64/hp/sim/boot/bootloader.c\n100644 blob 51a7b7b4dd0e7c5720683a40637cdb79a31ec4c4\tarch/ia64/hp/sim/boot/bootloader.c\n: siamese; git ls-tree -r aegl-c1ffb9 | grep arch/ia64/hp/sim/boot/bootloader.c\n100644 blob 51a7b7b4dd0e7c5720683a40637cdb79a31ec4c4\tarch/ia64/hp/sim/boot/bootloader.c\n\nSo the file did not change between the merge base and release,\nand test had the change. merge-cache picked the one in the test\nrelease. Your guess in the other message hits the mark.\n\nI wonder what _other_ candidates these two commits have in\ncommon and what would have happened if they were used as the\nbase instead?\n\n : siamese; git merge-base -a aegl-test-7ffacc1a aegl-release-a4cce1\n f6fdd7d9c273bb2a20ab467cb57067494f932fa3\n 3a931d4cca1b6dabe1085cc04e909575df9219ae\n c1ffb910f7a4e1e79d462bb359067d97ad1a8a25\n\nYou can check what variant of the file each of these commits\ncontain. What is happening is:\n\n* the problematic patch 4aec0f is one before 3a931d. Among the\n three merge-base candidates, only 3a931d contains teh wrongly\n patched version.\n\n* the problematic change 4aec0f patch introduces is part of test\n branch, because it was pulled via release.\n\n* the tip of release being merged into test has this patch\n reverted, and the file is exactly the same as before 4aec0f\n patch.\n\nSo three-way trivial merge algorithm says, \"hey, the file did\nnot change between common ancestor and release but it is\ndifferent in test, so the one in the test branch must be the\nmerge result.\"\n\nThis does not have much to do with which common ancestor\nmerge-base chooses. Sorry, I am not sure what is the right way\nto resolve this offhand.\n"},{"id":"7706","messageId":"Pine.LNX.4.63.0508241135120.23242@iabervon.org","threadId":"1631","inReplyTo":"7vek8jhk7y.fsf@assigned-by-dhcp.cox.net","subject":"Re: baffled again","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2005-08-24T16:09:42Z","receivedAt":"2005-08-24T16:09:42Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 24 Aug 2005, Junio C Hamano wrote:\n\n> tony.luck@intel.com writes:\n>\n> > So I have another anomaly in my GIT tree. A patch to\n> > back out a bogus change to arch/ia64/hp/sim/boot/bootloader.c\n> > in my release branch at commit\n> >\n> > 62d75f3753647656323b0365faa43fc1a8f7be97\n> >\n> > appears to have been lost when I merged the release branch to\n> > the test branch at commit\n> >\n> > 0c3e091838f02c537ccab3b6e8180091080f7df2\n>\n> : siamese; git cat-file commit 0c3e091838f02c537ccab3b6e8180091080f7df2\n> tree 61a407356d1e897e0badea552ce69e657cab6108\n> parent 7ffacc1a2527c219b834fe226a7a55dc67ca3637\n> parent a4cce10492358b33d33bb43f98284c80482037e8\n> author Tony Luck 1124808655 -0700\n> committer Tony Luck 1124808655 -0700\n>\n> Pull release into test branch\n>\n> So I pulled 7ffacc and a4cce1 from your repository and started\n> digging from there. 7ffacc was the head of \"test\" branch back\n> then, and a4cce1 was the head of \"release\" branch. I checked\n> out 7ffacc in the repository and pulled a4cce1 into it, using\n> the GIT with the \"optimum merge-base\" patch.\n>\n> : siamese; git pull . aegl-release\n> Packing 0 objects\n> Unpacking 0 objects\n>\n> * committish: a4cce10492358b33d33bb43f98284c80482037e8\trefs/heads/aegl-release from .\n> Trying to find the optimum merge base.\n> Trying to merge a4cce10492358b33d33bb43f98284c80482037e8 into 7ffacc1a2527c219b834fe226a7a55dc67ca3637 using c1ffb910f7a4e1e79d462bb359067d97ad1a8a25.\n> Simple merge failed, trying Automatic merge\n> Auto-merging arch/ia64/sn/kernel/io_init.c.\n> Committed merge db376974c0aebb9e99e5cd0bce21088c6a9d927c\n> arch/ia64/hp/sim/boot/boot_head.S | 2 +-\n> 1 files changed, 1 insertions(+), 1 deletions(-)\n>\n> It is using c1ffb9 as the merge base. The problematic path\n> in the three trees involved are:\n>\n> : siamese; git ls-tree -r aegl-test-7ffacc1a | grep arch/ia64/hp/sim/boot/bootloader.c\n> 100644 blob a7bed60b69f9e8de9a49944e22d03fb388ae93c7\tarch/ia64/hp/sim/boot/bootloader.c\n> : siamese; git ls-tree -r aegl-release-a4cce1 | grep arch/ia64/hp/sim/boot/bootloader.c\n> 100644 blob 51a7b7b4dd0e7c5720683a40637cdb79a31ec4c4\tarch/ia64/hp/sim/boot/bootloader.c\n> : siamese; git ls-tree -r aegl-c1ffb9 | grep arch/ia64/hp/sim/boot/bootloader.c\n> 100644 blob 51a7b7b4dd0e7c5720683a40637cdb79a31ec4c4\tarch/ia64/hp/sim/boot/bootloader.c\n>\n> So the file did not change between the merge base and release,\n> and test had the change. merge-cache picked the one in the test\n> release. Your guess in the other message hits the mark.\n>\n> I wonder what _other_ candidates these two commits have in\n> common and what would have happened if they were used as the\n> base instead?\n>\n> : siamese; git merge-base -a aegl-test-7ffacc1a aegl-release-a4cce1\n> f6fdd7d9c273bb2a20ab467cb57067494f932fa3\n> 3a931d4cca1b6dabe1085cc04e909575df9219ae\n> c1ffb910f7a4e1e79d462bb359067d97ad1a8a25\n>\n> You can check what variant of the file each of these commits\n> contain.\n>\n> What is happening is:\n>\n> * the problematic patch 4aec0f is one before 3a931d. Among the\n> three merge-base candidates, only 3a931d contains teh wrongly\n> patched version.\n>\n> * the problematic change 4aec0f patch introduces is part of test\n> branch, because it was pulled via release.\n>\n> * the tip of release being merged into test has this patch\n> reverted, and the file is exactly the same as before 4aec0f\n> patch.\n>\n> So three-way trivial merge algorithm says, \"hey, the file did\n> not change between common ancestor and release but it is\n> different in test, so the one in the test branch must be the\n> merge result.\"\n>\n> This does not have much to do with which common ancestor\n> merge-base chooses. Sorry, I am not sure what is the right way\n> to resolve this offhand.\n\nIf it picks 3a931d4cca1b6dabe1085cc04e909575df9219ae, it will determine\nthat the file didn't change between that and test, and is different in\nrelease, so the one in release must be right. I believe that the hint that\nsomething is going on is that different common ancestors give\ndifferent trivial merges (as opposed to some giving failure and some\ngiving the same result), and resolving it probably involves identifying\nthat that paths from f6f... and c1f... to release don't keep the same blob\nthrough the middle, despite having the same ends.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"7715","messageId":"Pine.LNX.4.58.0508241140290.3317@g5.osdl.org","threadId":"1631","inReplyTo":"7vek8jhk7y.fsf@assigned-by-dhcp.cox.net","subject":"Re: baffled again","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-08-24T18:47:25Z","receivedAt":"2005-08-24T18:47:25Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 24 Aug 2005, Junio C Hamano wrote:\n> \n> This does not have much to do with which common ancestor\n> merge-base chooses. Sorry, I am not sure what is the right way\n> to resolve this offhand.\n\nI think git did the \"right thing\", it just happened to be the thing that\nTony didn't want. Which makes it the \"wrong thing\", of course, but from a\npurely technical standpoint, I don't think there's anything really wrong\nwith the merge. \n\nBasically, he had two branches, A and B, and both contained the same patch\n(but _not_ the same commit). One undid it, the other did not. There's no\nreal way to say which one is \"correct\", and both cases clearly merge\nperfectly, so both outcomes \"patch applied\" and \"patch reverted\" are\nreally equally valid.\n\nNow, if the shared patch hadn't been a patch, but a shared _commit_, then \nthe thing would have been unambiguous - the shared commit would have been \nthe merge point, and the revert would have clearly undone that shared \ncommit.\n\nWhat does this all mean? It just means that merging doesn't necessarily\neven _have_ \"one right answer\". Automatic merges can be dangerous. The git\n\"global three-way\" merge (global because it bases it's original state on\n_global_ history, rather than local one) is about as safe as it gets (*), \nbut even it can have these ambigious cases that it resolves automatically, \nand not the way you wanted it to.\n\n\t\tLinus\n\n(*) \"safe as it gets\" of course also means \"potentially really annoying\",\nsince it tends to require manual fixups for any even possibly half-way\nambiguous case.\n"},{"id":"7718","messageId":"Pine.LNX.4.58.0508241152240.3317@g5.osdl.org","threadId":"1631","inReplyTo":"Pine.LNX.4.58.0508241140290.3317@g5.osdl.org","subject":"Re: baffled again","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-08-24T18:57:38Z","receivedAt":"2005-08-24T18:57:38Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 24 Aug 2005, Linus Torvalds wrote:\n> \n> Basically, he had two branches, A and B, and both contained the same patch\n> (but _not_ the same commit). One undid it, the other did not. There's no\n> real way to say which one is \"correct\", and both cases clearly merge\n> perfectly, so both outcomes \"patch applied\" and \"patch reverted\" are\n> really equally valid.\n\nIn fact, the case that git selected (\"patch applied\"), is not only the one\nthat is very fundamentally the one git will always select in this kind of\nsituation - in some respects is actually the nicer choice of the two.\n\nWhile it may cause problems (ie the revert was the right thing to do),\nit's at least the state that is less likely to be \"lost\". Having a revert\ndisappear is likely better than having a real change disappear. The\nreaction to the reverted code showing up again is likely \"damn, won't that\nbug ever go away, I fixed it once already\" - but at least people will see \nthat it's fair: \"it was applied twice, so let's revert it twice\".\n\nIn contrast, the reaction to a patch going away is likely just very\nconfusing: you have two people applying it, but only one reverting it will\nrevert both, while the first person who applied it may never have realized \nit got reverted.\n\n\t\tLinus\n"},{"id":"7720","messageId":"Pine.LNX.4.63.0508241504580.23242@iabervon.org","threadId":"1631","inReplyTo":"Pine.LNX.4.58.0508241140290.3317@g5.osdl.org","subject":"Re: baffled again","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2005-08-24T19:33:32Z","receivedAt":"2005-08-24T19:33:32Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 24 Aug 2005, Linus Torvalds wrote:\n\n> Now, if the shared patch hadn't been a patch, but a shared _commit_, then\n> the thing would have been unambiguous - the shared commit would have been\n> the merge point, and the revert would have clearly undone that shared\n> commit.\n\nActually, it was a shared commit\n(4aec0fb12267718c750475f3404337ad13caa8f5), which was (an ancestor of) a\ncandidate merge point, but wasn't the one selected. Since a different one\nwas chosen, it looked to the 3-way merge like a shared patch (since it\nignores the untaken parent in the merges in the history).\n\nThis should be fixable, but it'll require more cleverness in read-tree.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"7735","messageId":"7vzmr63deq.fsf@assigned-by-dhcp.cox.net","threadId":"1631","inReplyTo":"Pine.LNX.4.58.0508241152240.3317@g5.osdl.org","subject":"Re: baffled again","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-08-25T03:26:21Z","receivedAt":"2005-08-25T03:26:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> In fact, the case that git selected (\"patch applied\"), is not only the one\n> that is very fundamentally the one git will always select in this kind of\n> situation - in some respects is actually the nicer choice of the two.\n\nWhile I appreciate the excuse for not taking immediate and hasty\naction, I have two problems with your analysis.\n\n * I am not yet convinced that it is _not_ by accident that git\n ended up choosing the nicer choice of the two.\n\n * Even if it does always choose the nicer choice of the two,\n Tony was lucky (no pun intended). Rather, we were lucky that\n Tony was observant. A careless merger may well have easily\n missed this mismerge (from the human point of view).\n"},{"id":"7742","messageId":"12c511ca05082422584e6b1bfb@mail.gmail.com","threadId":"1631","inReplyTo":"7vzmr63deq.fsf@assigned-by-dhcp.cox.net","subject":"Re: baffled again","fromName":"Tony Luck","fromEmail":"tony.luck@gmail.com","sentAt":"2005-08-25T05:58:23Z","receivedAt":"2005-08-25T05:58:23Z","isPatch":false,"sender":{"key":"tony.luck@gmail.com","avatar":null},"body":"> * Even if it does always choose the nicer choice of the two,\n> Tony was lucky (no pun intended). Rather, we were lucky that\n> Tony was observant. A careless merger may well have easily\n> missed this mismerge (from the human point of view).\n\nActually I can't take credit here. This was a case of the \"many-eyes\" of\nopen source working at its finest ... someone e-mailed me and told me\nthat I should have backed out the old patch before applying the new one.\nWhile typing the e-mail to say that I already had in the release branch,\nI found the problem that it had been \"lost\" in the merge into the test branch.\n\nBut this is a good reminder that merging is not a precise science, and\nthere is more than one plausible merge in many situations ... and while\nGIT will pick the one that you want far more often than not, there is\nthe possibility that it will surprise you. Maybe there should be a note\nto this effect in the tutorial. Git is not magic, nor is it imbued with\nDWIM technology.\n\n-Tony\n"}]}