{"thread":{"id":"10223","subject":"inexplicable failure to merge recursively across cherry-picks","startedAt":"2007-10-10T01:55:45Z","lastAt":"2007-10-11T22:33:04Z","messageCount":11,"participants":["martin f krafft","Linus Torvalds","David Kastrup","David Brown","Miklos Vajna","Sam Vilain"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"55326","messageId":"20071010015545.GA17336@lapse.madduck.net","threadId":"10223","inReplyTo":null,"subject":"inexplicable failure to merge recursively across cherry-picks","fromName":"martin f krafft","fromEmail":"madduck@madduck.net","sentAt":"2007-10-10T01:55:45Z","receivedAt":"2007-10-10T01:55:45Z","isPatch":false,"sender":{"key":"madduck@madduck.net","avatar":null},"body":"Hi folks,\n\nI hope this is not a daily series of mine, being confused about Git\nmerging, but I've run my head against a wall again and before\nI crush my skull, I'd prefer to reach out to you to help me regain\nan understanding.\n\nThis is about the new mdadm for Debian packaging effort. You can\nclone from git://git.debian.org/git/pkg-mdadm/mdadm-new.git and see\nthe repo at\nhttp://git.debian.org/?p=pkg-mdadm/mdadm-new.git;a=summary. Do not\ntrack as this repo is subject to change.\n\nMy master branch was last merged with upstream's mdadm-2.6.2 tag\n(commit 263a535). Since then, I've committed a couple of changes to\nmaster including three cherry-picks from upstream since mdadm-2.6.2.\n\nI tagged upstream at the point which I want to merge into master:\nmdadm-2.6.3+200709292116+4450e59. When I merge that tag into master,\nI get a merge conflict on Monitor.c:\n\n <<<<<<< HEAD:Monitor.c\n if (mse->devnum != MAXINT &&\n =======\n if (mse->devnum != INT_MAX &&\n >>>>>>> upstream:Monitor.c\n\nThere are five commits between mdadm-2.6.2 and\nmdadm-2.6.3+200709292116+4450e59 that affect Monitor.c:\n\n 01d9299\n e4dc510\n* 66f8bbb\n 98127a6\n 4450e59\n\nThe third commit, the one with the asterisk is the one that\nI cherry-picked (as 845eef9); the other two cherries I picked do not\ntouch Monitor.c.\n\nThe fifth/last commit (4450e59) is the one responsible for the\nchange which seems to cause the conflict. It is the *only* commit\nsince the common ancestor of *both* branches that touches the\nconflicting lines.\n\nThe fourth commit (98127a6) inserts a single line at the top of the\nfile, so that's nothing that would cause a conflict.\n\nTo be honest, I can't explain it. But I didn't give up.\n\nI branched master2 off 845eef9b~1, cherry-picked the first two\ncommits that touch Monitor.c, cherry-picked all the commits\n845eef9b..master into master2 and merge upstream...\n\n... to get exactly the same conflict in exactly the same line in\nexactly the same file.\n\nWhat is going on. Am I seriously overestimating Git's merging\ncapacities, or do I have a bug in my brain?\n\n-- \nmartin; (greetings from the heart of the sun.)\n \\____ echo mailto: !#^.\"<*>\"|tr \"<*> mailto:\" net@madduck\n \n\"the only difference between shakespeare and you\n was the size of his idiom list -- not the size of his vocabulary.\"\n -- alan perlis\n \nspamtraps: madduck.bogus@madduck.net\n"},{"id":"55328","messageId":"alpine.LFD.0.999.0710091926560.3838@woody.linux-foundation.org","threadId":"10223","inReplyTo":"20071010015545.GA17336@lapse.madduck.net","subject":"Re: inexplicable failure to merge recursively across cherry-picks","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-10-10T02:54:21Z","receivedAt":"2007-10-10T02:54:21Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 10 Oct 2007, martin f krafft wrote:\n> \n> There are five commits between mdadm-2.6.2 and\n> mdadm-2.6.3+200709292116+4450e59 that affect Monitor.c:\n> \n> 01d9299\n> e4dc510\n> * 66f8bbb\n> 98127a6\n> 4450e59\n> \n> The third commit, the one with the asterisk is the one that\n> I cherry-picked (as 845eef9); the other two cherries I picked do not\n> touch Monitor.c.\n\nSide note - run\n\n\tgitk --merge\n\nwhen you have a merge conflict, and it will basically show you the thing \ngraphically (ie history as it is relevant to the merge, and only to the \nfiles that get conflicts).\n\nBut basically, both sides have modified the code *around* that line, and \nthey have modified it differently.\n\nDo this in your partial merge tree on 'master':\n\n\tgit diff ...mdadm-2.6.3+200709292116+4450e59 Monitor.c\n\tgit diff mdadm-2.6.3+200709292116+4450e59... Monitor.c\n\nwhich will show you the diff from the common base ancestor. And in \nparticular, it will show how one branch did this:\n\n\t@@ -399,9 +401,8 @@ int Monitor(mddev_dev_t devlist,\n\t struct mdstat_ent *mse;\n\t for (mse=mdstat; mse; mse=mse->next)\n\t if (mse->devnum != MAXINT &&\n\t- (strcmp(mse->level, \"raid1\")==0 ||\n\t- strcmp(mse->level, \"raid5\")==0 ||\n\t- strcmp(mse->level, \"multipath\")==0)\n\t+ (strcmp(mse->level, \"raid0\")!=0 &&\n\t+ strcmp(mse->level, \"linear\")!=0)\n\t ) {\n\t struct state *st = malloc(sizeof *st);\n\t mdu_array_info_t array;\n\nand the other one did\n\n\t@@ -398,10 +402,9 @@ int Monitor(mddev_dev_t devlist,\n\t if (scan) {\n\t struct mdstat_ent *mse;\n\t for (mse=mdstat; mse; mse=mse->next)\n\t- if (mse->devnum != MAXINT &&\n\t- (strcmp(mse->level, \"raid1\")==0 ||\n\t- strcmp(mse->level, \"raid5\")==0 ||\n\t- strcmp(mse->level, \"multipath\")==0)\n\t+ if (mse->devnum != INT_MAX &&\n\t+ (strcmp(mse->level, \"raid0\")!=0 &&\n\t+ strcmp(mse->level, \"linear\")!=0)\n\t ) {\n\t struct state *st = malloc(sizeof *st);\n\t mdu_array_info_t array;\n\nAnd now maybe git's behaviour makes more sense. See? You basically had two \ndifferent branches that made *almost* the same changes to the same area, \nbut not quite. So how is git to know which one was the *right* one to \npick? The one that changed the \"if (mse->devnum != MAXINT &&\" line, or the \none that left it alone?\n\n> I branched master2 off 845eef9b~1, cherry-picked the first two\n> commits that touch Monitor.c, cherry-picked all the commits\n> 845eef9b..master into master2 and merge upstream...\n\nCherry-picking is immaterial. It doesn't matter how the changes come into \nthe tree. It doesn't matter what the history is. The only thing git cares \nabout is the content, and the end result.\n\nGit knows that the two branches got to two different end results. They \nwere identical except for that one line, and it asks you to say which \nbranch was \"right\" wrt that one line.\n\nIn other words, git never looks at individual commits when trying to \nmerge. It doesn't try to figure out what the \"meaning\" of the changes are, \nit purely looks at the content.\n\nAnd btw, it *has* to work that way, because if you don't work that way, \nthen you get different results depending on which path the development \ntook (eg you might get different results if something was considered a \n\"revert\", for example, or if something was split up into two patches on \none side but not the other etc etc).\n\nBut in this case it's pretty obvious: the commit from one side (the one \nthat changes MAXINT->INT_MAX: \"Monitor.c s/MAXINT/INT_MAX/g\") merged fine \ninto the result *except* for that one section that had touched the same \ngeneral area for other reasons. And that one area was seen as a conflict \nbecause of those other reasons being in the same hunk.\n\nAnd yes, in this case the \"other reasons\" happened to be cherry-picked and \nthus \"the same\" on both sides, but that doesn't mean that they should have \nbeen considered in any way special: the result of cherry-picking is \ncontext-dependent and is a part of the history of the target, and does not \nequate any kind of \"identity\" with the source. Cherry-picking is 100% \nequivalent to re-doing the commit entirely, and for all git knows, there \nwas a reason why it wasn't done together with that s/MAXINT/INT_MAX/g \nchange.\n\n(Not that git even thinks in those terms - git literally just says: \"I \ncannot resolve the _content_ independently and without understanding the \nhistory and thinking behind the differences, so you'd better help me\")\n\n\t\t\tLinus\n"},{"id":"55358","messageId":"20071010102528.GB20390@lapse.madduck.net","threadId":"10223","inReplyTo":"alpine.LFD.0.999.0710091926560.3838@woody.linux-foundation.org","subject":"Re: inexplicable failure to merge recursively across cherry-picks","fromName":"martin f krafft","fromEmail":"madduck@madduck.net","sentAt":"2007-10-10T10:25:28Z","receivedAt":"2007-10-10T10:25:28Z","isPatch":false,"sender":{"key":"madduck@madduck.net","avatar":null},"body":"also sprach Linus Torvalds [2007.10.10.0354 +0100]:\n> Cherry-picking is immaterial. It doesn't matter how the changes\n> come into the tree. It doesn't matter what the history is. The\n> only thing git cares about is the content, and the end result.\n\nThis is the part I over-estimated. I thought that Git would figure\nout that commits 1-3 had been merged into the target and thus apply,\nin sequence, only the commits from the source which had not been\nmerged.\n\nMany thanks (again), Linus! Looking forward to your next content\nmanager; you know, the one with artificial intelligence built in!\nYou could call it \"wit\" :)\n\n-- \nmartin; (greetings from the heart of the sun.)\n \\____ echo mailto: !#^.\"<*>\"|tr \"<*> mailto:\" net@madduck\n \ndies ist eine manuell generierte email. sie beinhaltet\ntippfehler und ist auch ohne großbuchstaben gültig.\n \nspamtraps: madduck.bogus@madduck.net\n"},{"id":"55359","messageId":"858x6bw975.fsf@lola.goethe.zz","threadId":"10223","inReplyTo":"20071010102528.GB20390@lapse.madduck.net","subject":"Re: inexplicable failure to merge recursively across cherry-picks","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-10-10T10:33:18Z","receivedAt":"2007-10-10T10:33:18Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"martin f krafft writes:\n\n> also sprach Linus Torvalds [2007.10.10.0354 +0100]:\n>> Cherry-picking is immaterial. It doesn't matter how the changes\n>> come into the tree. It doesn't matter what the history is. The\n>> only thing git cares about is the content, and the end result.\n>\n> This is the part I over-estimated. I thought that Git would figure\n> out that commits 1-3 had been merged into the target and thus apply,\n> in sequence, only the commits from the source which had not been\n> merged.\n>\n> Many thanks (again), Linus! Looking forward to your next content\n> manager; you know, the one with artificial intelligence built in!\n> You could call it \"wit\" :)\n\nWell, there is also an obvious name choice when the distinguishing\ninnovation is a well-rounded feature set, but it would cause a name\ncollision for the equivalent of \"tig\".\n\n-- \nDavid Kastrup, Kriemhildstr. 15, 44793 Bochum\n"},{"id":"55378","messageId":"alpine.LFD.0.999.0710100808150.3838@woody.linux-foundation.org","threadId":"10223","inReplyTo":"20071010102528.GB20390@lapse.madduck.net","subject":"Re: inexplicable failure to merge recursively across cherry-picks","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-10-10T15:25:15Z","receivedAt":"2007-10-10T15:25:15Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 10 Oct 2007, martin f krafft wrote:\n> also sprach Linus Torvalds [2007.10.10.0354 +0100]:\n> > Cherry-picking is immaterial. It doesn't matter how the changes\n> > come into the tree. It doesn't matter what the history is. The\n> > only thing git cares about is the content, and the end result.\n> \n> This is the part I over-estimated. I thought that Git would figure\n> out that commits 1-3 had been merged into the target and thus apply,\n> in sequence, only the commits from the source which had not been\n> merged.\n\nYes, *some* SCM's have tried to do that. In particular, the ones that are \n\"patch-based\" tend to think that patches are \"identical\" regardless of \nwhere they are, and while re-ordering of them is a special event, it's not \nsomethign that changes the fundamental 'ID' of the patch.\n\nFor example, I think the darcs \"patch algebra\" works that way.\n\nIt's a really horrible model. Not only doesn't it scale, but it leads to \nvarious very strange linkages between patches, and it fails the most \nimportant part: it means that merges get different results just because \npeople are doing the same changes two different ways.\n\n> Many thanks (again), Linus! Looking forward to your next content\n> manager; you know, the one with artificial intelligence built in!\n> You could call it \"wit\" :)\n\nWell, the git model is really largely the reverse: the system is supposed \nto be as *stupid* as humanly possible, but:\n\n - make it predictable exactly because it's stupid and doesn't do anything \n even half-ways smart.\n\n This is part of the \"it doesn't matter *how* you got to a particular \n state, git will always do the same thing regardless of whether you \n moved an existing patch around or whether you re-did the changes as \n (possibly more than one) new and unrelated commits\".\n\n - conflicts aren't bad - they're *good*. Trying to aggressively resolve \n them automatically when two branches have done slightly different \n things in the same area is stupid and just results in more problems.\n\n Instead, git tries to do what I don't think *anybody* else has done: \n make the conflicts easy to resolve, by allowing you to work with them \n in your normal working tree, and still giving you a lot of tools to \n help you see what's going on.\n\nSo git doesn't try to avoid conflicts per se: the merge strategies are \nfundamentally pretty simple (rename detection and the whole \"recursive \nmerge\" thing may not be simple code, but the concepts are pretty \nstraightforward), and they handle all the really *obvious* cases, but at \nthe same time, I feel strongly that anything even half-way subtle should \nnot be left to the SCM - the SCM should show it and make it really easy \nfor the user to then fix it up.\n\nSide note: even with a totally obvious three-way merge, with absolutely \nzero conflicts even remotely close to each other, you can have the merge \nalgorithm generate a good merge that doesn't actually *work*.\n\nFor example, it's happened a few times that one branch renames a structure \nmember name (and changes all the uses) and another branch adds new code \nthat uses the old member name. The end result: the code will *merge* fine, \nand there are zero conflicts in the content, because all the changes were \ntotally disjoint, but the end result doesn't actually work or even \ncompile!\n\nSo no merge strategy is ever perfect. The git approach is to be simple and \npredictable, and also to make it easy to fix up (ie even if you get the \nabove kind of automatic merge problem, if you catch it in compiling, you \ncan fix it up, and do a \"git commit --amend\" to fix up the merge itself \nbefore you push it out).\n\n\t\t\tLinus\n"},{"id":"55381","messageId":"20071010154831.GA19226@old.davidb.org","threadId":"10223","inReplyTo":"alpine.LFD.0.999.0710100808150.3838@woody.linux-foundation.org","subject":"Re: inexplicable failure to merge recursively across cherry-picks","fromName":"David Brown","fromEmail":"git@davidb.org","sentAt":"2007-10-10T15:48:31Z","receivedAt":"2007-10-10T15:48:31Z","isPatch":false,"sender":{"key":"git@davidb.org","avatar":"https://gravatar.com/avatar/94c86a2938470a74c2eac5e2b69afc0871f79a660295c02219597aba8cb101c1?d=mp&s=160"},"body":"On Wed, Oct 10, 2007 at 08:25:15AM -0700, Linus Torvalds wrote:\n\n>Yes, *some* SCM's have tried to do that. In particular, the ones that are \n>\"patch-based\" tend to think that patches are \"identical\" regardless of \n>where they are, and while re-ordering of them is a special event, it's not \n>somethign that changes the fundamental 'ID' of the patch.\n>\n>For example, I think the darcs \"patch algebra\" works that way.\n>\n>It's a really horrible model. Not only doesn't it scale, but it leads to \n>various very strange linkages between patches, and it fails the most \n>important part: it means that merges get different results just because \n>people are doing the same changes two different ways.\n\nActually, specifically darcs, different merges _always_ result in the same\ndata. It's a fundamental part of is patch algebra. No matter what order\nyou apply a given set of patches, even with conflicts and reordering, you\nalways get the same result, or no result. Conflicts are \"resolved\" by\ninserting conflict markers in the file, ordered by the patch ID. It\ndoesn't matter which order you apply them in, you get the same markers.\nThen there will be a merge patch which fixes the markers that someone could\napply, no matter what order the applied the previous patches.\n\nDarcs breaks down in a few places, though.\n\n - The no result. Sometimes, it just can't figure out how to reorder\n patches. Even worse, occasionally, the implementation will fail to\n terminate try to figure this out. There isn't much to do at this\n point, except manually apply the patch, hence generating a new patch\n ID.\n\n - It doesn't scale well.\n\nThe strange linkages between patches could be thought of as a feature,\nsince it is basically constraining the order that the patches can be\napplied in.\n\nThere is a darcs-git project that tries to do the darcs things on top of\ngit.\n\nDave\n"},{"id":"55396","messageId":"20071010190757.GP23810@genesis.frugalware.org","threadId":"10223","inReplyTo":"20071010154831.GA19226@old.davidb.org","subject":"Re: inexplicable failure to merge recursively across cherry-picks","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2007-10-10T19:07:57Z","receivedAt":"2007-10-10T19:07:57Z","isPatch":false,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Wed, Oct 10, 2007 at 08:48:31AM -0700, David Brown wrote:\n> On Wed, Oct 10, 2007 at 08:25:15AM -0700, Linus Torvalds wrote:\n\n> >Yes, *some* SCM's have tried to do that. In particular, the ones that are \"patch-based\" tend to think that patches are \"identical\" regardless of where they are, and while re-ordering of them is a special event, it's not somethign that changes the fundamental 'ID' of the patch.\n\n> >For example, I think the darcs \"patch algebra\" works that way.\n\n> >It's a really horrible model. Not only doesn't it scale, but it leads to various very strange linkages between patches, and it fails the most important part: it means that merges get different results just because people are doing the same changes two different ways.\n\n> Actually, specifically darcs, different merges _always_ result in the same\n> data. It's a fundamental part of is patch algebra. No matter what order\n> you apply a given set of patches, even with conflicts and reordering, you\n> always get the same result, or no result. Conflicts are \"resolved\" by\n> inserting conflict markers in the file, ordered by the patch ID. It\n> doesn't matter which order you apply them in, you get the same markers.\n> Then there will be a merge patch which fixes the markers that someone could\n> apply, no matter what order the applied the previous patches.\n\n> Darcs breaks down in a few places, though.\n\n> - The no result. Sometimes, it just can't figure out how to reorder\n> patches. Even worse, occasionally, the implementation will fail to\n> terminate try to figure this out. There isn't much to do at this\n> point, except manually apply the patch, hence generating a new patch\n> ID.\n\n> - It doesn't scale well.\n\n> The strange linkages between patches could be thought of as a feature,\n> since it is basically constraining the order that the patches can be\n> applied in.\n\n> There is a darcs-git project that tries to do the darcs things on top of\n> git.\n\n> Dave\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\nthanks,\n- VMiklos\n"},{"id":"55401","messageId":"alpine.LFD.0.999.0710101231130.20690@woody.linux-foundation.org","threadId":"10223","inReplyTo":"20071010190757.GP23810@genesis.frugalware.org","subject":"Re: inexplicable failure to merge recursively across cherry-picks","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-10-10T19:35:42Z","receivedAt":"2007-10-10T19:35:42Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 10 Oct 2007, Miklos Vajna wrote:\n> \n> Actually, specifically darcs, different merges _always_ result in the same\n> data.\n\nNo they don't. You don't understand the problem.\n\nYes, different merges WITH THE SAME PATCHES always result in the same \ndata.\n\nBut that's not a realistic - or even very interesting - schenario.\n\nWhat's much more common is that the same problem gets solved slightly \ndifferently in two different branches. For example, maybe somebody does it \nas two different patches - where the second one fixes a bug in the first \nfix. And another person does the same fix, but without the bug in the \nfirst place.\n\nSee? A patch-based system gets confused by those kinds of issues (or they \nturn into various special cases). And that is fundamentally why you MUST \nNOT take history into account (where \"history\" is some series of \nindividual patches).\n\nYes, history is interesting for historical reasons, and to explain what \nthe context was, but in many ways, history is exactly the *wrong* thing to \nuse when it comes to merging. You should look at the end result, since \npeople can - and do - come to the same result through different ways.\n\n\t\t\tLinus\n"},{"id":"55440","messageId":"20071011000826.GQ23810@genesis.frugalware.org","threadId":"10223","inReplyTo":"20071010154831.GA19226@old.davidb.org","subject":"Re: inexplicable failure to merge recursively across cherry-picks","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2007-10-11T00:08:26Z","receivedAt":"2007-10-11T00:08:26Z","isPatch":false,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"[ ehh, sorry for my previous mail, i must be doing something wrong.. ]\n\nOn Wed, Oct 10, 2007 at 08:48:31AM -0700, David Brown wrote:\n> There is a darcs-git project that tries to do the darcs things on top of\n> git.\n\nactually it's broken, according to its author:\n\nhttp://www.mail-archive.com/darcs-users@darcs.net/msg03161.html\n\nthough i loved darcs, but yes - it scales horribly\n\n- VMiklos\n"},{"id":"55513","messageId":"470E9AD5.2090002@vilain.net","threadId":"10223","inReplyTo":"alpine.LFD.0.999.0710100808150.3838@woody.linux-foundation.org","subject":"Re: inexplicable failure to merge recursively across cherry-picks","fromName":"Sam Vilain","fromEmail":"sam@vilain.net","sentAt":"2007-10-11T21:51:17Z","receivedAt":"2007-10-11T21:51:17Z","isPatch":false,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"Linus Torvalds wrote:\n> So git doesn't try to avoid conflicts per se: the merge strategies are \n> fundamentally pretty simple (rename detection and the whole \"recursive \n> merge\" thing may not be simple code, but the concepts are pretty \n> straightforward), and they handle all the really *obvious* cases, but at \n> the same time, I feel strongly that anything even half-way subtle should \n> not be left to the SCM - the SCM should show it and make it really easy \n> for the user to then fix it up.\n\nThis is true. However I think there are some obvious places for\nimprovement that does look at the file history, when the regular\nalgorithm fails;\n\n1. do a --cherry-pick rev-list on just the file being merged and see if\nall the changes on one side disappear, in which case just take the result.\n\n2. see if the files were identical at some point, in which case use a\nnew merge base for that file based on the changes since that revision.\n\nI actually thought #2 was already the way recursive worked!\n\nSam.\n"},{"id":"55517","messageId":"alpine.LFD.0.999.0710111455220.20690@woody.linux-foundation.org","threadId":"10223","inReplyTo":"470E9AD5.2090002@vilain.net","subject":"Re: inexplicable failure to merge recursively across cherry-picks","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-10-11T22:33:04Z","receivedAt":"2007-10-11T22:33:04Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 12 Oct 2007, Sam Vilain wrote:\n> \n> 1. do a --cherry-pick rev-list on just the file being merged and see if\n> all the changes on one side disappear, in which case just take the result.\n> \n> 2. see if the files were identical at some point, in which case use a\n> new merge base for that file based on the changes since that revision.\n> \n> I actually thought #2 was already the way recursive worked!\n\nActually, I think both of these are fundamentally wrong.\n\nThe reason is that you talk about \"the file\".\n\nAnything that is based on per-file heuristics is going to mean that you \nuse a history that is not necessarily compatible with the _other_ files in \nthe project.\n\nI agree 100% that per-file history information is going to find more \nthings to merge automatically. But the point I was trying to make was that \n\"automatic merges\" aren't always *good*. \n\nI realize that pretty much every single theoretical merge algorithm out \nthere tries to make merges happen automatically as much as possible, but \nthey all en dup having strange issues.\n\nFor example, take a patch that cherry-picked into mainline from a \ndevelopment branch, but that partly depended on some support-feature that \nwasn't in mainline yet. So then there is another patch that removes part \nof that patch from mainline. So mainline is fine.\n\nNow, three months later, the development branch is stable, and is fully \nmerged. What happens?\n\nGit will largely get this right. Git will look at the last *global* common \nbase, and will just look at the contents, and do a reasonable job. Yes, \nthere will probably be conflicts (because both the development branch and \nthe mainline ended up touching the same parts of the files thanks to the \ncherry-pick, and yet mainline has some added hacks on top to disable it), \nbut on the whole that's exactly what you want!\n\n(Alternatively, maybe the \"remove the part that wasn't supported yet\" \nended up meaning that that particular part of the patch was excised \nentirely from mainline, and there was no conflict at all, and git just \nmerged the new stuff from the development branch cleanly! So I'm not \nsaying that it *has* to conflict, I'm just saying that it might have).\n\nIn other words: git always \"does the right thing\". Assuming both branches \nare stable and working, git does a very reasonable thing. It's obviously \nnot always the thing people may *want*, but it's guaranteed to be a \nreasonable and simple guess, and there's no way it's \"too clever for its \nown good, and just screwed the pooch entirely\".\n\nIn contrast, your suggested merge strategy would be HORRIBLY BROKEN!\n\nWhy? Because it doesn't look at the *common* history to the project, it \nlooks at some per-file state that is totally bogus and has no relevance. \nThink it through: what happens if there were files with the same content \n(because of the cherry-pick), and then the file history for one of the \nbranches was later changed to disable something because the support for it \nwasn't in the \"whole history\"?\n\nRight: the final merge will contain that change! Because there as a time \nwhere the file was identical (the cherry-pick), so you're taking all the \nlater changes to that file (the undo)!\n\nNotice? Totally the wrong thing to do!\n\nSo this is a classic case of trying to make \"easier\" merges, but where the \nwhole approach is totally broken! You simply MUST NOT add logic like that. \nIt's a lot better to give a conflict, than to try to be \"clever\", and \nsilently do the wrong thing.\n\nYes, you can be really stupid, and silently do the wrong thing too, but if \nyou're stupid, at least the \"silent wrong thing\" is never really subtle, \nit's pretty much guaranteed to easily explainable. And the good news is \nthat you didn't have a complicated and fragile algorithm just to get the \nwrong answer.\n\n(Put another way: if you are always going to have situations where you get \nthe wrong answer, you'd better take the simple and stupid algorithm, \nbecause people are more likely to then be able to _predict_ that wrong \nanswer and are thus more ready to handle it!)\n\nSo being clever really is the wrong thing to do. And using history that \nisn't global and true history (ie just looking at one file, and deciding \nthat matching that one file \"means\" something) is fundamnetally broken.\n\nIn fact, in general, individual pieces of history are totally worthless. \nThe fact that some individual change was done in one branch doesn't really \ntell you *anything*. The reason that change was done may be implied by all \nthe previous changes, or conversely, later changes may have undone the \nchange, so any merge algorithm that starts to look at individual commits \nis likely to be pure and utter crap - exactly because it's starting to \nmake decisions based on local information that may not be valid in the big \npicture.\n\n(Where the \"big picture\" may be either about \"space\" - other files - or \nabout \"time\" - other commits, that simply mean that the individual changes \nof one commit are meaningless on their own).\n\nBtw, one thing to note is how well the simple and stupid git merge \nstrategy works. It turns out that doing things with the \"big picture\" \nmodel actually does work really well. People think that they need \n\"finegrained history\" to make good merges, but I think most people who \nhave actually done a fair number of merges with git have noticed that it's \nactually pretty dang painless.\n\nBut to be honest, there are cases where git isn't being very helpful. In \nparticular, I think there *are* things that git could probably be more \nhelpful with, but looking at local history is not one of them, I think.\n\nSo here are some suggestions on things I think we could improve on:\n\n - I think it would be wonderful to have a helper tool for handling \n conflicts. \n\n In particular, while I don't think per-file history is good for \n resolving conflicts *automatically*, I actually do think that per-file \n history can be a good way to *manually* resolve conflicts.\n\n In other words, it you have a conflict, I think it would be wonderful \n to have some git-gui-like thing that can show the history (with \n patches) for that file, and basically combine a three-way graphical \n merge *with* some per-commit information where you can say \"choose the \n thing that that commit did for this conflict\".\n\n So I think git already has tools to help resolve conflicts, and I \n personally love doing \"gitk --merge\" or \"git log -p --merge\" when a \n conflict does happen, but I think some smart GUI person could do \n something even much better!\n\n And notice how I think that it's *really* wrong to use per-file history \n automatically, but that I think it's not wrong at all to use it when \n there's a human that says \"ok, obviously pick that case\". Things that \n are horrible when they cause subtle and automatic resolves can be very \n good when they cause subtle resolves that a human looked at!\n\n - I suspect we have issues with common whitespace changes, where we again \n could probably help people resolve whitespace changes etc better. \n\n Again, I don't think those are necessarily things you want to do \n automatically, but I know from personal experience that handling things \n like one side having done a re-indent can be *really* annoying, just \n because you end up doing tons of mindless stuff when you fix up all the \n totally idiotic and usually trivial conflicts.\n\n.. and I'm sure there are other things we could do better too, but the \nabove two are things that while they haven't happened for me for the \nkernel (probably because we have learnt how to not cause them over the \nyears), I've seen them in other places.\n\nAnd yes, the above two suggestions fall solidly in the \"conflicts aren't \nbad per se, but you want to make the tool really help you resolve them!\" \ncamp. \n\n\t\t\tLinus\n"}]}