{"thread":{"id":"865","subject":"Handling merge conflicts a bit more gracefully..","startedAt":"2005-06-08T20:55:23Z","lastAt":"2005-06-18T00:26:02Z","messageCount":33,"participants":["Linus Torvalds","Junio C Hamano","Jeff Garzik","Herbert Xu"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"4723","messageId":"Pine.LNX.4.58.0506081336080.2286@ppc970.osdl.org","threadId":"865","inReplyTo":null,"subject":"Handling merge conflicts a bit more gracefully..","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-06-08T20:55:23Z","receivedAt":"2005-06-08T20:55:23Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nOk, Jeff reported that whenever there is a merge conflict, he ends up \nreally punting on it and doing it all with diffs, which clearly meant that \nI had to fix up my silly things for this. Which I think I've done now.\n\nWhat happens now in the case of a merge conflict is:\n - the merge is obviously not committed\n - we do all the successful merges, and update the index file for them\n - for the files that conflict, we force the index to contain the old\n version of the file (ie we remove the merge from the index), and we\n write the (failed) output of the merge into the working directory, and\n we complain loudly:\n\n\tAuto-merging xyzzy.\n\tmerge: warning: conflicts during merge\n\tERROR: Merge conflict in xyzzy.\n\tfatal: merge program failed\n\tAutomatic merge failed, fix up by hand\n\nat which point a normal \"git-diff-files -p xyzzy\" will show the incomplete\nmerge results (as relative to the original BRANCH you started with), and\nin fact you can also do \"git-diff-cache -p MERGE_HEAD xyzzy\" to see the\nsame thing (but relative to the branch you tried to merge).\n\nYou then fix up the merge failure by hand (exactly the way you'd do with \nCVS), and you do a \"git-update-cache xyzzy\" when you're happy with the end \nresult. Then a simple \"git commit\" should do the right thing.\n\nIf you decide that the merge is too hard to undo, you'd do:\n\n\tgit-read-tree -u -m HEAD\n\trm .git/MERGE_HEAD\n\nand use git-checkout-cache judiciously to remove any edits the merge did.\n\nThis is definitely not perfect, but it's a hell of a lot more usable than\nit used to be, and not really worse than what CVS people are used to (and\nusually a lot better, since git will obviously get the origin of a\nthree-way merge right, unlike CVS).\n\nComments? It would be good to have people test this and maybe even write a \nfew automated tests that it all works as expected..\n\n\t\tLinus\n"},{"id":"4730","messageId":"7vis0o30sc.fsf@assigned-by-dhcp.cox.net","threadId":"865","inReplyTo":"Pine.LNX.4.58.0506081336080.2286@ppc970.osdl.org","subject":"Re: Handling merge conflicts a bit more gracefully..","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-06-08T23:07:47Z","receivedAt":"2005-06-08T23:07:47Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">>>>> \"LT\" == Linus Torvalds writes:\n\nLT> What happens now in the case of a merge conflict is:\nLT> - the merge is obviously not committed\nLT> - we do all the successful merges, and update the index file for them\nLT> - for the files that conflict, we force the index to contain the old\nLT> version of the file (ie we remove the merge from the index), and we\nLT> write the (failed) output of the merge into the working directory, and\nLT> we complain loudly:\n\nLT> Comments? It would be good to have people test this and maybe even write a \nLT> few automated tests that it all works as expected..\n\nOK, I'll bite. Other than some minor details, the work tree\nseems to be updated with the result of the merge, either\nsuccessful one or failed one.\n\n2a68a8659f7dc55fd285d235ae2d19e7a8116c30 \\\n(from f9e7750621ca5e067f58a679caff5ff2f9881c4c)\ndiff --git a/git-merge-one-file-script b/git-merge-one-file-script\n--- a/git-merge-one-file-script\n+++ b/git-merge-one-file-script\n@@ -19,22 +19,25 @@ case \"${1:-.}${2:-.}${3:-.}\" in\n # Deleted in both.\n #\n \"$1..\")\n-\techo \"ERROR: $4 is removed in both branches.\"\n-\techo \"ERROR: This is a potential rename conflict.\"\n-\texit 1;;\n+\techo \"WARNING: $4 is removed in both branches.\"\n+\techo \"WARNING: This is a potential rename conflict.\"\n+\texec git-update-cache --remove -- \"$4\" ;;\n\nMaking sure that the path does not exist in the work tree with\ntest -f \"$4\" would be more sensible, before running --remove.\n\n #\n # Deleted in one and unchanged in the other.\n #\n \"$1..\" | \"$1.$1\" | \"$1$1.\")\n \techo \"Removing $4\"\n-\texec git-update-cache --force-remove \"$4\" ;;\n+\trm -f -- \"$4\"\n+\texec git-update-cache --remove -- \"$4\" ;;\n\nMake sure \"$4\" is not a directory, perhaps? At least barf if\nthat 'rm -f -- \"$4\"' fails?\n\n #\n # Modified in both, but differently.\n #\n\n@@ -55,19 +60,21 @@ case \"${1:-.}${2:-.}${3:-.}\" in\n \torig=`git-unpack-file $1`\n \tsrc1=`git-unpack-file $2`\n \tsrc2=`git-unpack-file $3`\n-\tmerge \"$src2\" \"$orig\" \"$src1\"\n+\tmerge -p \"$src1\" \"$orig\" \"$src2\" > \"$4\"\n \tret=$?\n+\trm -f -- \"$orig\" \"$src1\" \"$src2\"\n \tif [ \"$6\" != \"$7\" ]; then\n \t\techo \"ERROR: Permissions $5->$6->$7 don't match.\"\n+\t\tret=1\n \tfi\n \tif [ $ret -ne 0 ]; then\n-\t\techo \"ERROR: Leaving conflict merge in $src2.\"\n+\t\t# Reset the index to the first branch, making\n+\t\t# git-diff-file useful\n+\t\tgit-update-cache --add --cacheinfo \"$6\" \"$2\" \"$4\"\n+\t\techo \"ERROR: Merge conflict in $4.\"\n \t\texit 1\n \tfi\n-\tsha1=`git-write-blob \"$src2\"` || {\n-\t\techo \"ERROR: Leaving conflict merge in $src2.\"\n-\t}\n-\texec git-update-cache --add --cacheinfo \"$6\" $sha1 \"$4\" ;;\n+\texec git-update-cache --add -- \"$4\" ;;\n *)\n \techo \"ERROR: Not handling case $4: $1 -> $2 -> $3\" ;;\n esac\n\nAgain, make sure \"$4\" is not a directory before redirecting into\nit from merge, so that you can tell merge failures from it?\n\n"},{"id":"4732","messageId":"Pine.LNX.4.58.0506081629370.2286@ppc970.osdl.org","threadId":"865","inReplyTo":"7vis0o30sc.fsf@assigned-by-dhcp.cox.net","subject":"Re: Handling merge conflicts a bit more gracefully..","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-06-08T23:35:48Z","receivedAt":"2005-06-08T23:35:48Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 8 Jun 2005, Junio C Hamano wrote:\n> # Deleted in both.\n> \n> Making sure that the path does not exist in the work tree with\n> test -f \"$4\" would be more sensible, before running --remove.\n\nYeah, my (broken) thinking was that since it wasn't in both, it wasn't in \nthe working directory either, but you're right, that's just crazy talk. \nThere could be a stale file there.\n\nMade it do a\n\n\trm -f -- \"$4\" || exit 1\n\ninstead (and changed the other one to do the \"|| exit 1\" too, since you're \nalso obviously right on the directory issue).\n\n> # Modified in both, but differently.\n> +\tmerge -p \"$src1\" \"$orig\" \"$src2\" > \"$4\"\n> \n> Again, make sure \"$4\" is not a directory before redirecting into\n> it from merge, so that you can tell merge failures from it?\n\nHmm.. What's the cleanest way to check for redirection errors, but still\nbe able to distinguish those cleanly from \"merge\" itself returning an\nerror?\n\n\t\t\tLinus\n"},{"id":"4735","messageId":"7vzmu01jmc.fsf@assigned-by-dhcp.cox.net","threadId":"865","inReplyTo":"Pine.LNX.4.58.0506081629370.2286@ppc970.osdl.org","subject":"Re: Handling merge conflicts a bit more gracefully..","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-06-09T00:03:55Z","receivedAt":"2005-06-09T00:03:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">>>>> \"LT\" == Linus Torvalds writes:\n\n>> # Modified in both, but differently.\n>> +\tmerge -p \"$src1\" \"$orig\" \"$src2\" > \"$4\"\n>> \n>> Again, make sure \"$4\" is not a directory before redirecting into\n>> it from merge, so that you can tell merge failures from it?\n\nLT> Hmm.. What's the cleanest way to check for redirection errors, but still\nLT> be able to distinguish those cleanly from \"merge\" itself returning an\nLT> error?\n\nI do not think you can, unless you are willing to parse shell\nerror messages, which I do not want you to be willing to ;-).\n\n : siamese; ls -dlF junk j.py\n ---------- 1 junio junio 845 May 7 2004 j.py\n drwxrwxr-x 2 junio junio 4096 May 4 22:31 junk/\n : siamese; echo foo >j.py ; echo $?\n bash: j.py: Permission denied\n 1\n : siamese; echo foo >junk ; echo $?\n bash: junk: Is a directory\n 1\n\nI think you have a bigger problem of leading paths, BTW.\n\nSince we would want to have the merge result file at that path,\nand not being able to create such is an error, how about doing\ndumb and simple, like:\n\n d=`dirname \"$4\"` &&\n mkdir -p \"$d\" &&\n rm -f -- \"$4\" &&\n : >\"$4\" || {\n echo \"barf\"\n exit 1\n }\n merge -p \"$src1\" \"$orig\" \"$src2\" >\"$4\"\n ret=$?\n\n"},{"id":"4736","messageId":"7voeag1j9y.fsf@assigned-by-dhcp.cox.net","threadId":"865","inReplyTo":"Pine.LNX.4.58.0506081629370.2286@ppc970.osdl.org","subject":"Re: Handling merge conflicts a bit more gracefully..","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-06-09T00:11:21Z","receivedAt":"2005-06-09T00:11:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"While I have your attention, I have been thinking about a\nproblem at lower level than what is being discussed.\n\nConsider the following two command sequences:\n\n (1) git-read-tree -m $H $M\t&& git-write-tree\n\n (2) I=`git-write-tree` &&\n git-read-tree -m $H $I $M &&\n git-merge-cache -o git-merge-one-file-script -a &&\n git-write-tree\n\nI think they should be equivalent in that:\n\n - when (1) refuses to run, (2) should either cause\n git-read-tree to refuse, or at least should result in an\n unmerged cache and git-write-tree phase should fail;\n\n - when (1) succeeds, (2) should also succeed, and the\n resulting tree from both should be the same.\n\n\n"},{"id":"4738","messageId":"Pine.LNX.4.58.0506081738000.2286@ppc970.osdl.org","threadId":"865","inReplyTo":"7vzmu01jmc.fsf@assigned-by-dhcp.cox.net","subject":"Re: Handling merge conflicts a bit more gracefully..","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-06-09T00:41:56Z","receivedAt":"2005-06-09T00:41:56Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 8 Jun 2005, Junio C Hamano wrote:\n> \n> I do not think you can, unless you are willing to parse shell\n> error messages, which I do not want you to be willing to ;-).\n\nYeah, no. \n\n> I think you have a bigger problem of leading paths, BTW.\n\nGotcha. I committed a largely untested fix that hopefully does this all \nright.\n\nI'm currently using your suggested thing (inside a function), but I think \nI'll instead make it do\n\n\tgit-update-cache --add --cacheinfo ... &&\n\t\tgit-checkout-cache -u -f \"$4\"\n\nwhich seems even simpler.\n\n\t\tLinus\n"},{"id":"4741","messageId":"7v4qc8z6gj.fsf@assigned-by-dhcp.cox.net","threadId":"865","inReplyTo":"Pine.LNX.4.58.0506081738000.2286@ppc970.osdl.org","subject":"Re: Handling merge conflicts a bit more gracefully..","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-06-09T01:04:12Z","receivedAt":"2005-06-09T01:04:12Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">>>>> \"LT\" == Linus Torvalds writes:\n\nLT> I'll instead make it do\n\nLT> \tgit-update-cache --add --cacheinfo ... &&\nLT> \t\tgit-checkout-cache -u -f \"$4\"\n\nLT> which seems even simpler.\n\nI like that one much much better. Consistently using\ncheckout-cache -f everywhere is much preferred. It creates the\nleading paths itself, and even nukes interfering files it finds\nwhile creating leading directories, which the verify_path using\nmkdir -p would not give you.\n\n"},{"id":"4742","messageId":"Pine.LNX.4.58.0506081757170.2286@ppc970.osdl.org","threadId":"865","inReplyTo":"7voeag1j9y.fsf@assigned-by-dhcp.cox.net","subject":"Re: Handling merge conflicts a bit more gracefully..","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-06-09T01:08:44Z","receivedAt":"2005-06-09T01:08:44Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 8 Jun 2005, Junio C Hamano wrote:\n> \n> Consider the following two command sequences:\n> \n> (1) git-read-tree -m $H $M\t&& git-write-tree\n> \n> (2) I=`git-write-tree` &&\n> git-read-tree -m $H $I $M &&\n> git-merge-cache -o git-merge-one-file-script -a &&\n> git-write-tree\n> \n> I think they should be equivalent in that:\n> \n> - when (1) refuses to run, (2) should either cause\n> git-read-tree to refuse, or at least should result in an\n> unmerged cache and git-write-tree phase should fail;\n> \n> - when (1) succeeds, (2) should also succeed, and the\n> resulting tree from both should be the same.\n\nI think that sounds reasonable. Is it not the case now?\n\n\t\tLinus\n"},{"id":"4745","messageId":"7vll5kxolo.fsf@assigned-by-dhcp.cox.net","threadId":"865","inReplyTo":"Pine.LNX.4.58.0506081757170.2286@ppc970.osdl.org","subject":"Re: Handling merge conflicts a bit more gracefully..","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-06-09T02:15:15Z","receivedAt":"2005-06-09T02:15:15Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">>>>> \"LT\" == Linus Torvalds writes:\n\nLT> I think that sounds reasonable. Is it not the case now?\n\nWell, except that $I may validly be an empty tree ;-), so not\nquite.\n\nIn case it was not clear, where I am headed is this. I would\nlike to rip out the two-tree \"carry forward\" implementation from\nread-tree, and replace it with:\n\n read_cache() -- current goes to stage0\n read_tree(H) -- H goes to stage1\n read_tree(M) -- M goes to stage3\n for each path\n if it appears in stage0, copy it to stage2\n else if it appears in stage1, copy it to stage2\n threeway_merge() !!\n\nAnd then the resulting possibly unmerged cache can be resolved\nexactly the same way with merge-cache.\n\nThe trouble I feel with the current \"carry forward\" code is that\nwhen it works it does sensible thing, but otherwise does not\nhelp the end user at all. With all the work going into making\nmerge-one-file-script nicer today, I think leveraging three-way\nmerge support for two-tree fast forward case would make a lot\nmore sense than keeping the all-or-nothing carry forward code I\nrecently added to it.\n\nWhen/if that happens, then the current fast-forward code would\nneed to be changed from:\n\n read-tree -m $H $M && echo $M >.git/HEAD\n\nto\n\n read-tree -m $H $M &&\n if unmerged paths in the resulting cache\n then\n merge-cache -o merge-one-file-script -a\n fi &&\n echo $M >.git/HEAD\n\nand the user's local changes since H when fast forwarding to M\nwould be handled with the same workflow as the three-way case.\n\nHmm.\n\n"},{"id":"4747","messageId":"Pine.LNX.4.58.0506081936370.2286@ppc970.osdl.org","threadId":"865","inReplyTo":"7vll5kxolo.fsf@assigned-by-dhcp.cox.net","subject":"Re: Handling merge conflicts a bit more gracefully..","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-06-09T02:48:45Z","receivedAt":"2005-06-09T02:48:45Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 8 Jun 2005, Junio C Hamano wrote:\n> \n> Well, except that $I may validly be an empty tree ;-), so not\n> quite.\n\nYeah, ok, so the fact that we allow missing things in the index (which was \ndebatable to start with) makes for exceptions. \n\nWe could certainly be stricter about the index contents, and require that\nthey match the branch we're merging from exactly, rather than be a subset. \nThat said, I'm not convinced that it's worth it, even if it means that a\ntwo-way merge ends up acceping merges that a three-way one never would.\n\n> In case it was not clear, where I am headed is this. I would\n> like to rip out the two-tree \"carry forward\" implementation from\n> read-tree, and replace it with:\n\nYeah, I see that, I'm just not entirely convinced it's a good idea.\n\nThe thing is, a two-way merge really _is_ very different from a three-way\none, in that it's a fast-forward, and the fact that it allows for things\nthat the more complex \"full\" case wouldn't allow I feel is something of an\nadvantage. And it _can_ allow them exactly because it's not the full case.\n\nI like your concept:\n\n> When/if that happens, then the current fast-forward code would\n> need to be changed from:\n> \n> read-tree -m $H $M && echo $M >.git/HEAD\n> \n> to\n> \n> read-tree -m $H $M &&\n> if unmerged paths in the resulting cache\n> then\n> merge-cache -o merge-one-file-script -a\n> fi &&\n> echo $M >.git/HEAD\n> \n> and the user's local changes since H when fast forwarding to M\n> would be handled with the same workflow as the three-way case.\n\nbut that one doesn't really help the case of stuff he hasn't marked \nup-dated, so I think that's actually a special case that _isn't_ the \nimportant one. I think the case that is more important (and more likely to \nhit people) is when they have something in their working tree that \nconflicts with the merge, and then what you want is really that the \ncurrent \"update\" code do the three-way merge in the working directory, not \nthat it's done on the index file contents.\n\nSo I see where you are coming from, but I don't think the index file is \nthe most important case. The more important case is the one that the \nthree-way merge doesn't handle either!\n\n\t\tLinus\n"},{"id":"4748","messageId":"42A7B28A.9010508@pobox.com","threadId":"865","inReplyTo":"Pine.LNX.4.58.0506081336080.2286@ppc970.osdl.org","subject":"Re: Handling merge conflicts a bit more gracefully..","fromName":"Jeff Garzik","fromEmail":"jgarzik@pobox.com","sentAt":"2005-06-09T03:07:54Z","receivedAt":"2005-06-09T03:07:54Z","isPatch":false,"sender":{"key":"jgarzik@pobox.com","avatar":null},"body":"Linus Torvalds wrote:\n> Comments? It would be good to have people test this and maybe even write a \n> few automated tests that it all works as expected..\n\nI've got a few libata branches I have been putting off updating to the \nlatest kernel, because of merge conflicts ('chs-support' and 'passthru' \nbranches of libata-dev.git).\n\nIf this merge-gracefully stuff is all checked into git.git, I can \ndefinitely give it some real-world testing.\n\n\tJeff\n\n\n"},{"id":"4750","messageId":"Pine.LNX.4.58.0506082110510.2286@ppc970.osdl.org","threadId":"865","inReplyTo":"42A7B28A.9010508@pobox.com","subject":"Re: Handling merge conflicts a bit more gracefully..","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-06-09T04:11:41Z","receivedAt":"2005-06-09T04:11:41Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 8 Jun 2005, Jeff Garzik wrote:\n> \n> If this merge-gracefully stuff is all checked into git.git, I can \n> definitely give it some real-world testing.\n\nYup, all there. Not a _ton_ of testing exactly, but I did actually test\nboth the content conflict case and the \"new directory\" case. At least \nonce.\n\n\t\tLinus\n"},{"id":"4751","messageId":"7vfyvsuoz3.fsf@assigned-by-dhcp.cox.net","threadId":"865","inReplyTo":"Pine.LNX.4.58.0506081936370.2286@ppc970.osdl.org","subject":"Re: Handling merge conflicts a bit more gracefully..","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-06-09T04:35:28Z","receivedAt":"2005-06-09T04:35:28Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">>>>> \"LT\" == Linus Torvalds writes:\n\nLT> Yeah, ok, so the fact that we allow missing things in the\nLT> index (which was debatable to start with) makes for\nLT> exceptions.\n\nNot just that. Another big difference is that we allow _extra_\nthings in the index in two-tree case (i.e. local additions).\nBut I do not think these exceptions are necessarily bad.\n\nAnd you are right that two-tree is _very_ different from\nthree-way merge.\n\nLT> We could certainly be stricter about the index contents, and\nLT> require that they match the branch we're merging from\nLT> exactly, rather than be a subset.\n\nI guess great minds do not always think alike. I was going in\nquite the opposite direction. I vaguely recall saying this\nbefore on this list ;-)\n\nWith the current three-way code, if I rewrite two-way merge\nusing the three-way \"read-tree -m H I-mixed-with-H M\" (emulated\ntwo-tree fast forward, where \"I\" denotes \"tree that would have\nresulted from the original cache\"), it would give quite\ndifferent results from the \"carry forward\" two-way code we have.\nSo in that sense, three-way and two-way are quite different.\n\nI have, however, not convinced myself that this difference is\ncoming from some fundamental difference between two-tree fast\nforward and three-way merge. If desirable results fall out\nnaturally for the \"emulated two-way\" case by handling three-way\ncase more carefully (e.g. not having stricter index requirements\nthan necessary), that would be wonderful. I think, for example,\nthere are places where we have too strict index requirements in\nthree-way merge (grep for '(ALT)' in t/t1000*.sh test file).\n\nI probably am dreaming, though.\n\nLT> I think the case that is more important (and more likely to\nLT> hit people) is when they have something in their working\nLT> tree that conflicts with the merge, and then what you want\nLT> is really that the current \"update\" code do the three-way\nLT> merge in the working directory, not that it's done on the\nLT> index file contents.\n\nLT> ..., but I don't think the index file is the most important\nLT> case. The more important case is the one that the three-way\nLT> merge doesn't handle either!\n\nI agree with all of the above. Their working tree has changes\nfrom H, and merging M into H conflicts with those changes. That\nmeans, although they did not actually make a formal commit, what\nthey have is essentially this:\n\n cache contents\n is here\n v\n ---I---\n / ^work tree contents is here\n --H\n \\\n ----------M\n\nwhich means we are exactly in the same situation as \"merge I and\nM pivoting on H\" three-way merge, with a dirty work tree. Any\nsolution and help we would give to the end-user for the\nthree-way case would automatically help this two-way case,\nwouldn't it?\n\nI do not think index file is important either; maybe I am not\nreally understanding your argument. I fully accept the new\nworld order with today's merge-one-file-script changes, that the\nmerge result will be left in the work tree for the user to\nverify and sort out. What I am trying to do in the above\npicture is to help the end-user forward-porting differences in I\nsince H (along with the work tree changes since I) when doing a\nfast-forward from H to M happens, using the files in the work\ntree.\n\n"},{"id":"4753","messageId":"Pine.LNX.4.58.0506082145160.2286@ppc970.osdl.org","threadId":"865","inReplyTo":"7vfyvsuoz3.fsf@assigned-by-dhcp.cox.net","subject":"Re: Handling merge conflicts a bit more gracefully..","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-06-09T04:54:50Z","receivedAt":"2005-06-09T04:54:50Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 8 Jun 2005, Junio C Hamano wrote:\n> >>>>> \"LT\" == Linus Torvalds writes:\n> \n> LT> Yeah, ok, so the fact that we allow missing things in the\n> LT> index (which was debatable to start with) makes for\n> LT> exceptions.\n> \n> Not just that. Another big difference is that we allow _extra_\n> things in the index in two-tree case (i.e. local additions).\n> But I do not think these exceptions are necessarily bad.\n\nWell, they'd be bad in a three-way merge.\n\nThe reason they aren't bad in a two-way merge is that you don't commit the \nresult - the commits have been done already. \n\nThat's really the big conceptual difference between two-way and three-way: \nnever mind the merge algorithm itself.\n\n(In fact, in many ways, two-way merges are really just the same as a \none-way merge, except it now knows where it came from, so it can do sanity \nchecking).\n\nAs to working tree changes:\n\n> which means we are exactly in the same situation as \"merge I and\n> M pivoting on H\" three-way merge, with a dirty work tree. Any\n> solution and help we would give to the end-user for the\n> three-way case would automatically help this two-way case,\n> wouldn't it?\n\nYes.\n\nIn fact, there's a fairly simple solution, which is to remove the current \ncheck for \"verify_uptodate()\" and instead replace it with the \"update\" \nphase not just writing the file, but actually doing a three-way merge on \nit.\n\nNOTE! This would not affect the resulting _tree_ in any way at all. It \nwould literally only affect how we write out the working directory. Right \nnow we just fail when the working file isn't up-to-date, and that could be \nreplaced with instead doing a\n\n\tmerge W I M\n\nwhere \"W\" is the working file, \"I\" is the index file, and \"M\" is the merge \nresult that we currently just write out directly.\n\nIn the special case of I == M, we already do _exactly_ this: we know that\nsince I=M, the merge will be W, so we don't do the update at all.\n\nSo in fact, doing a 3-way merge is really a generalization of what we\nalready do, and removes a failure case.\n\nNOTE! This 3way merge is fundamentally _different_ from the 3-way merge\nthat is done by \"git-merge-one-file-script\" that we already do. _That_\n3-way merge is done not on the working files, but on the results in the\ntrees, while this new 3way merge would be done purely in the working\ndirectory (ie it wouldn't make sense without the \"-u\" flag).\n\nIf we do this, I'd personally suggest it be another flag, possibly \"-u3\" \ninstead of just plain \"-u\".\n\n\t\tLinus\n"},{"id":"4755","messageId":"7vzmu0t8j5.fsf@assigned-by-dhcp.cox.net","threadId":"865","inReplyTo":"Pine.LNX.4.58.0506082145160.2286@ppc970.osdl.org","subject":"Re: Handling merge conflicts a bit more gracefully..","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-06-09T05:15:58Z","receivedAt":"2005-06-09T05:15:58Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">>>>> \"LT\" == Linus Torvalds writes:\n\nLT> On Wed, 8 Jun 2005, Junio C Hamano wrote:\n>> >>>>> \"LT\" == Linus Torvalds writes:\n>> \nLT> Yeah, ok, so the fact that we allow missing things in the\nLT> index (which was debatable to start with) makes for\nLT> exceptions.\n>> \n>> Not just that. Another big difference is that we allow _extra_\n>> things in the index in two-tree case (i.e. local additions).\n>> But I do not think these exceptions are necessarily bad.\n\nLT> Well, they'd be bad in a three-way merge.\n\nNo question about it. I am not proposing to conditionally\naccept extra entries in 3-way case.\n\nBut when \"read-tree -m H I-mixed-with-H M\" 3-way merge is\nemulating \"read-tree -m H M\", it does not need to accept any\nextra entries in the cache, because in this case \"our head\" tree\nis \"I-mixed-with-H\", which by definition contains everything in\nthe current cache (remember, \"I-mixed-with-H\" is built by\nlooking at each path and if it has stage0 then copy it to stage2\notherwise if it has stage1 then copy it to stage2, after reading\nthe index file into stage0, H into stage1, and M into stage 3).\n\nAnd after such \"3-way merge emulating 2-way fast-forward,\" you\ncan commit---the commit will have a single parent, M, and the\ndifference it contains is the changes the user made while he was\nworking off of H, rebased to M.\n\nOf course this all assumes that we have a perfectly working\nthree-way merge ;-).\n\n"},{"id":"4761","messageId":"7voeagrp11.fsf_-_@assigned-by-dhcp.cox.net","threadId":"865","inReplyTo":"Pine.LNX.4.58.0506081629370.2286@ppc970.osdl.org","subject":"[PATCH 0/3] Handling merge conflicts a bit more gracefully","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-06-09T07:02:34Z","receivedAt":"2005-06-09T07:02:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This series consists of three patches.\n\n [PATCH 1/3] read-tree.c: rename local variables used in 3-way merge code.\n [PATCH 2/3] read-tree -m 3-way: loosen index requirements that is too strict.\n [PATCH 3/3] read-tree -m 3-way: handle more trivial merges internally\n\nYou may have noticed that I already described some \"alternative\nsemantics\" in the 3-way merge test script t1000. This set of\npatches implements some of them, namely the following 5 cases:\n\n O A B result index requirements\n-------------------------------------------------------------------\n 5 missing exists A==B take A must match A, if exists.\n ------------------------------------------------------------------\n 6 exists missing missing remove must not exist.\n ------------------------------------------------------------------\n 8 exists missing O==B remove must not exist.\n ------------------------------------------------------------------\n 10 exists O==A missing remove must match A and be\n up-to-date, if exists.\n ------------------------------------------------------------------\n 14 exists O==A O!=B take B if exists, must either (1)\n match A and be up-to-date,\n or (2) match B.\n-------------------------------------------------------------------\n\nThe first patch is to match the local variable names used in the\nfunctions involved to the names used in the case matrix.\n\nCase #14 is resolved identically as the old code does, but the\nindex requirement old code placed on this case was stricter than\nnecessary. In this case, satisfying the usual rule of \"match A\nand be up-to-date if exists\" is certainly OK, but additionally,\nif the original index matches the tree being merged (without\neven being up-to-date) is also permissible, because there would\nbe no information loss or work-tree clobbering if we allowed it.\nThe second patch in the series corrects this.\n\nCase #5, #6, #8, and #10 were traditionally kept unmerged in the\nindex file when read-tree is done, and resolving them was left\nto the script. By resolving these internally, we can loosen the\nindex requirements without compromising correctness for case #5.\nOther three cases could still be left for the \"script policy\"\nbecause this change does not affect the index requirements for\nthese cases, but it was simple enough to implement them and this\nwould not be too controversial a change. The third patch in the\nseries implements these changes.\n\n"},{"id":"4759","messageId":"7vhdg8roxa.fsf_-_@assigned-by-dhcp.cox.net","threadId":"865","inReplyTo":"7voeagrp11.fsf_-_@assigned-by-dhcp.cox.net","subject":"[PATCH 1/3] read-tree.c: rename local variables used in 3-way merge code.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-06-09T07:04:49Z","receivedAt":"2005-06-09T07:04:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I'd hate to do this, but every time I try to touch this code and\nvalidate what it does against the case matrix in t1000 test, I\nget confused. The variable names are renamed to match the case\nmatrix. Now they are named as:\n\n i -- entry from the index file (formerly known as \"old\")\n o -- merge base (formerly known as \"a\")\n a -- our head (formerly known as \"b\")\n b -- merge head (formerly known as \"c\")\n\nSigned-off-by: Junio C Hamano \n---\n\n read-tree.c | 40 ++++++++++++++++++++--------------------\n 1 files changed, 20 insertions(+), 20 deletions(-)\n\ndiff --git a/read-tree.c b/read-tree.c\n--- a/read-tree.c\n+++ b/read-tree.c\n@@ -40,9 +40,9 @@ static int same(struct cache_entry *a, s\n * This removes all trivial merges that don't change the tree\n * and collapses them to state 0.\n */\n-static struct cache_entry *merge_entries(struct cache_entry *a,\n-\t\t\t\t\t struct cache_entry *b,\n-\t\t\t\t\t struct cache_entry *c)\n+static struct cache_entry *merge_entries(struct cache_entry *o,\n+\t\t\t\t\t struct cache_entry *a,\n+\t\t\t\t\t struct cache_entry *b)\n {\n \t/*\n \t * Ok, all three entries describe the same\n@@ -58,16 +58,16 @@ static struct cache_entry *merge_entries\n \t * The \"all entries exactly the same\" case falls out as\n \t * a special case of any of the \"two same\" cases.\n \t *\n-\t * Here \"a\" is \"original\", and \"b\" and \"c\" are the two\n+\t * Here \"o\" is \"original\", and \"a\" and \"b\" are the two\n \t * trees we are merging.\n \t */\n-\tif (a && b && c) {\n-\t\tif (same(b,c))\n-\t\t\treturn c;\n+\tif (o && a && b) {\n \t\tif (same(a,b))\n-\t\t\treturn c;\n-\t\tif (same(a,c))\n \t\t\treturn b;\n+\t\tif (same(o,a))\n+\t\t\treturn b;\n+\t\tif (same(o,b))\n+\t\t\treturn a;\n \t}\n \treturn NULL;\n }\n@@ -126,29 +126,29 @@ static int merged_entry(struct cache_ent\n \n static int threeway_merge(struct cache_entry *stages[4], struct cache_entry **dst)\n {\n-\tstruct cache_entry *old = stages[0];\n-\tstruct cache_entry *a = stages[1], *b = stages[2], *c = stages[3];\n+\tstruct cache_entry *i = stages[0];\n+\tstruct cache_entry *o = stages[1], *a = stages[2], *b = stages[3];\n \tstruct cache_entry *merge;\n \tint count;\n \n \t/*\n-\t * If we have an entry in the index cache (\"old\"), then we want\n+\t * If we have an entry in the index cache (\"i\"), then we want\n \t * to make sure that it matches any entries in stage 2 (\"first\n-\t * branch\", aka \"b\").\n+\t * branch\", aka \"a\").\n \t */\n-\tif (old) {\n-\t\tif (!b || !same(old, b))\n+\tif (i) {\n+\t\tif (!a || !same(i, a))\n \t\t\treturn -1;\n \t}\n-\tmerge = merge_entries(a, b, c);\n+\tmerge = merge_entries(o, a, b);\n \tif (merge)\n-\t\treturn merged_entry(merge, old, dst);\n-\tif (old)\n-\t\tverify_uptodate(old);\n+\t\treturn merged_entry(merge, i, dst);\n+\tif (i)\n+\t\tverify_uptodate(i);\n \tcount = 0;\n+\tif (o) { *dst++ = o; count++; }\n \tif (a) { *dst++ = a; count++; }\n \tif (b) { *dst++ = b; count++; }\n-\tif (c) { *dst++ = c; count++; }\n \treturn count;\n }\n \n------------\n\n"},{"id":"4758","messageId":"7vbr6growa.fsf_-_@assigned-by-dhcp.cox.net","threadId":"865","inReplyTo":"7voeagrp11.fsf_-_@assigned-by-dhcp.cox.net","subject":"[PATCH 2/3] read-tree -m 3-way: loosen index requirements that is too strict.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-06-09T07:05:25Z","receivedAt":"2005-06-09T07:05:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This patch teaches \"read-tree -m O A B\" that, when only \"the\nother tree\" changed a path, and if the work tree already has\nthat change, we are not in a situation that would clobber the\ncache and the working tree, and lets the merge succeed; this is\ncase #14ALT in t1000 test.\n\nSigned-off-by: Junio C Hamano \n---\n\n read-tree.c | 16 ++++++++++++++++\n t/t1000-read-tree-m-3way.sh | 9 +++++++++\n 2 files changed, 25 insertions(+), 0 deletions(-)\n\ndiff --git a/read-tree.c b/read-tree.c\n--- a/read-tree.c\n+++ b/read-tree.c\n@@ -131,6 +131,22 @@ static int threeway_merge(struct cache_e\n \tstruct cache_entry *merge;\n \tint count;\n \n+\t/* The case #14ALT is special in that it allows \"i\" to match\n+\t * the \"merged branch\", aka \"b\" and even be dirty, as an\n+\t * alternative to the usual 'must match \"a\" and be up-to-date'\n+\t * rule.\n+\t */\n+\tif (o && a && b && same(o, a) && !same(o, b)) {\n+\t\tif (i) {\n+\t\t\tif (same(i, b))\n+\t\t\t\t; /* case #14ALT exception */\n+\t\t\telse if (same(i, a))\n+\t\t\t\tverify_uptodate(i);\n+\t\t\telse\n+\t\t\t\treturn -1;\n+\t\t}\n+\t}\n+\telse /* otherwise the original rule applies */\n \t/*\n \t * If we have an entry in the index cache (\"i\"), then we want\n \t * to make sure that it matches any entries in stage 2 (\"first\ndiff --git a/t/t1000-read-tree-m-3way.sh b/t/t1000-read-tree-m-3way.sh\n--- a/t/t1000-read-tree-m-3way.sh\n+++ b/t/t1000-read-tree-m-3way.sh\n@@ -455,6 +455,15 @@ test_expect_success \\\n git-read-tree -m $tree_O $tree_A $tree_B &&\n check_result\"\n \n+test_expect_success \\\n+ '14ALT - in O && A && B && O==A && O!=B case, matching B is also OK' \\\n+ \"rm -f .git/index NM &&\n+ cp .orig-B/NM NM &&\n+ git-update-cache --add NM &&\n+ echo extra >>NM &&\n+ git-read-tree -m $tree_O $tree_A $tree_B &&\n+ check_result\"\n+\n test_expect_failure \\\n '14 (fail) - must match and be up-to-date in O && A && B && O==A && O!=B case' \\\n \"rm -f .git/index NM &&\n------------\n\n"},{"id":"4760","messageId":"7v64woroui.fsf_-_@assigned-by-dhcp.cox.net","threadId":"865","inReplyTo":"7voeagrp11.fsf_-_@assigned-by-dhcp.cox.net","subject":"[PATCH 3/3] read-tree -m 3-way: handle more trivial merges internally","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-06-09T07:06:29Z","receivedAt":"2005-06-09T07:06:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This patch teaches \"read-tree -m O A B\" that some more trivial\ncases can be handled internally. This allows us to loosen\notherwise too strict index requirements in case #5ALT, where\nboth branches create a new file identically --- the previous\ncode required index to be up-to-date and aborted the merge when\nit is not, but there is no reason to require it to be up-to-date\nin this case.\n\nThe test vector has been updated to match the new behaviour as\nwell.\n\nSigned-off-by: Junio C Hamano \n---\n\n read-tree.c | 16 ++++++++++++++++\n t/t1000-read-tree-m-3way.sh | 27 +++++++++------------------\n 2 files changed, 25 insertions(+), 18 deletions(-)\n\ndiff --git a/read-tree.c b/read-tree.c\n--- a/read-tree.c\n+++ b/read-tree.c\n@@ -69,6 +69,12 @@ static struct cache_entry *merge_entries\n \t\tif (same(o,b))\n \t\t\treturn a;\n \t}\n+\t/* #5ALT */\n+\tif (!o && a && b && same(a,b)) {\n+\t\t/* Match what git-merge-one-file-script does */\n+\t\tprintf(\"Adding %s\\n\", a->name);\n+\t\treturn a;\n+\t}\n \treturn NULL;\n }\n \n@@ -161,6 +167,16 @@ static int threeway_merge(struct cache_e\n \t\treturn merged_entry(merge, i, dst);\n \tif (i)\n \t\tverify_uptodate(i);\n+\n+\t/* #6ALT, #8ALT, and #10ALT */\n+\tif ((o && !a && !b) ||\n+\t (o && !a && b && same(o, b)) ||\n+\t (o && a && !b && same(o, a))) {\n+\t\t/* Match what git-merge-one-file-script does */\n+\t\tprintf(\"Removing %s\\n\", o->name); \n+\t\treturn 0;\n+\t}\n+\n \tcount = 0;\n \tif (o) { *dst++ = o; count++; }\n \tif (a) { *dst++ = a; count++; }\ndiff --git a/t/t1000-read-tree-m-3way.sh b/t/t1000-read-tree-m-3way.sh\n--- a/t/t1000-read-tree-m-3way.sh\n+++ b/t/t1000-read-tree-m-3way.sh\n@@ -75,21 +75,18 @@ In addition:\n . ../lib-read-tree-m-3way.sh\n \n ################################################################\n-# This is the \"no trivial merge unless all three exists\" table.\n+# Trivial \"majority when 3 stages exist\" merge plus #5ALT, #6ALT,\n+# #8ALT, #10ALT trivial merges.\n \n cat >expected <<\\EOF\n 100644 X 2\tAA\n 100644 X 3\tAA\n 100644 X 2\tAN\n-100644 X 1\tDD\n 100644 X 3\tDF\n 100644 X 2\tDF/DF\n 100644 X 1\tDM\n 100644 X 3\tDM\n-100644 X 1\tDN\n-100644 X 3\tDN\n-100644 X 2\tLL\n-100644 X 3\tLL\n+100644 X 0\tLL\n 100644 X 1\tMD\n 100644 X 2\tMD\n 100644 X 1\tMM\n@@ -97,8 +94,6 @@ cat >expected <<\\EOF\n 100644 X 3\tMM\n 100644 X 0\tMN\n 100644 X 3\tNA\n-100644 X 1\tND\n-100644 X 2\tND\n 100644 X 0\tNM\n 100644 X 0\tNN\n 100644 X 0\tSS\n@@ -108,11 +103,8 @@ cat >expected <<\\EOF\n 100644 X 2\tZ/AA\n 100644 X 3\tZ/AA\n 100644 X 2\tZ/AN\n-100644 X 1\tZ/DD\n 100644 X 1\tZ/DM\n 100644 X 3\tZ/DM\n-100644 X 1\tZ/DN\n-100644 X 3\tZ/DN\n 100644 X 1\tZ/MD\n 100644 X 2\tZ/MD\n 100644 X 1\tZ/MM\n@@ -120,8 +112,6 @@ cat >expected <<\\EOF\n 100644 X 3\tZ/MM\n 100644 X 0\tZ/MN\n 100644 X 3\tZ/NA\n-100644 X 1\tZ/ND\n-100644 X 2\tZ/ND\n 100644 X 0\tZ/NM\n 100644 X 0\tZ/NN\n EOF\n@@ -289,23 +279,24 @@ test_expect_failure \\\n git-read-tree -m $tree_O $tree_A $tree_B\"\n \n test_expect_success \\\n- '5 - must match and be up-to-date in !O && A && B && A==B case.' \\\n+ '5 - must match in !O && A && B && A==B case.' \\\n \"rm -f .git/index LL &&\n cp .orig-A/LL LL &&\n git-update-cache --add LL &&\n git-read-tree -m $tree_O $tree_A $tree_B &&\n check_result\"\n \n-test_expect_failure \\\n- '5 (fail) - must match and be up-to-date in !O && A && B && A==B case.' \\\n+test_expect_success \\\n+ '5 - must match in !O && A && B && A==B case.' \\\n \"rm -f .git/index LL &&\n cp .orig-A/LL LL &&\n git-update-cache --add LL &&\n echo extra >>LL &&\n- git-read-tree -m $tree_O $tree_A $tree_B\"\n+ git-read-tree -m $tree_O $tree_A $tree_B &&\n+ check_result\"\n \n test_expect_failure \\\n- '5 (fail) - must match and be up-to-date in !O && A && B && A==B case.' \\\n+ '5 (fail) - must match in !O && A && B && A==B case.' \\\n \"rm -f .git/index LL &&\n cp .orig-A/LL LL &&\n echo extra >>LL &&\n------------\n\n"},{"id":"4790","messageId":"Pine.LNX.4.58.0506090800580.2286@ppc970.osdl.org","threadId":"865","inReplyTo":"7v64woroui.fsf_-_@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 3/3] read-tree -m 3-way: handle more trivial merges internally","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-06-09T15:15:52Z","receivedAt":"2005-06-09T15:15:52Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 9 Jun 2005, Junio C Hamano wrote:\n>\n> This patch teaches \"read-tree -m O A B\" that some more trivial\n> cases can be handled internally.\n\nNo, I think this is quite possibly wrong for several reasons.\n\nFor one, it makes the rest of the system unaware of the deleted files, so \nnothing ever deletes them from the working directory.\n\nAnd that's not entirely trivial to fix either, since the obvious fix \n(which is to just do a\n\n\tif (update)\n\t\tunlink(b->name);\n\nor something like that) is wrong. It's wrong because we must not do the\nupdate until the very end, when we've either merged all entries or we've\nfailed on an entry that couldn't be merged (that's why I did the extra\nCE_UPDATE flag, instead of updating as we go along).\n\nNow, you could fix that by creating a separate list of files to be \ndeleted (so this is not fundamental, it's just more complicated than the \ntrivial case), but that doesn't help, because there's _another_ reason why \nread-tree shouldn't handle these cases.\n\nNamely that read-tree doesn't have a frigging clue about renames, and \nshouldn't have.\n\nBut a real merge program _could_ have a frigging clue, and might notice \npatterns like\n\n - file got modified in one branch, removed in the other\n - a file got added in the other branch\n - \"Hey, that added file looks like the removed one!\"\n - Let's merge the modifications from the first branch into the move of \n the second branch!\n\nSee? Now, git-read-tree won't handle the first case anyway, but your \nchange _does_ make it handle the \"file got added\" case, which means that \nnow the added file is invisible the the \"smart merger\", and the smart \nmerger can't really tell that it was a rename any more.\n\nSo our current stupid file-by-file \"git-merge-cache\" will never do this, \nbut that's a limitation of me being less than the intellectual giant I \nwish I was. So I just do the stupid merges. But I _know_ they are stupid, \nand I would like to leave the door open for somebody else to fix up the \ncases I don't handle.\n\nYou're basically closing that door.\n\nNow, you can (validly) argue that you could still just look at the\noriginal trees (\"git-diff-tree -C $O $M\") and grep for copies/movement and\ndo it by hand _there_ instead of looking at the result of the read-tree, \nand you may well be right. So again, this is not a _fundamental_ problem, \nalthough it's a bit more fundamental than the first one. \n\nSo if you want to convince me that it's better to do the rename detection\noutside of the index file, go wild. Alternatively, you can argue that we\ncan always undo this later, when once we _do_ have rename and copy\ndetection and can try to merge things automatically (what _do_ you do if a\nfile is copied in one branch and modified in the other? Just warn the poor\nuser, I guess).\n\nSo I just need a little convincing that this is a good idea.\n\n\t\tLinus\n"},{"id":"4795","messageId":"7v7jh3phkk.fsf@assigned-by-dhcp.cox.net","threadId":"865","inReplyTo":"Pine.LNX.4.58.0506090800580.2286@ppc970.osdl.org","subject":"Re: [PATCH 3/3] read-tree -m 3-way: handle more trivial merges internally","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-06-09T17:26:35Z","receivedAt":"2005-06-09T17:26:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">>>>> \"LT\" == Linus Torvalds writes:\n\nLT> No, I think this is quite possibly wrong for several reasons.\n\nI agree with everything you said.\n\nI need to regurgitate other points you raised, but one immediate\ncomment on the \"lost remove\" case. The current two-way code has\nthe same brokenness in that it does not unlink removed files\nunder \"-u\". We either need the \"list of files to be removed\",\nor we need to make two-way abort if we see these \"remove\" cases.\n\n"},{"id":"4796","messageId":"Pine.LNX.4.58.0506091033300.2286@ppc970.osdl.org","threadId":"865","inReplyTo":"7v7jh3phkk.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 3/3] read-tree -m 3-way: handle more trivial merges internally","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-06-09T17:37:45Z","receivedAt":"2005-06-09T17:37:45Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 9 Jun 2005, Junio C Hamano wrote:\n> \n> I need to regurgitate other points you raised, but one immediate\n> comment on the \"lost remove\" case. The current two-way code has\n> the same brokenness in that it does not unlink removed files\n> under \"-u\". We either need the \"list of files to be removed\",\n> or we need to make two-way abort if we see these \"remove\" cases.\n\nYes, you're right.\n\nHo humm. I'll think about it. There's no \"next\" pointer in a struct \ncache-struct, and because we use the on-disk layout (good or bad, I dunno, \nbut it does remove the need for copying megabytes of data for some cases) \nwe can't just add one. So to generate a list of \"deleted\" files we'd have \nto make a separate array or something.\n\nNot hard, but it's a bit ugly. I don't see any alternative, though, unless\nwe really do end up using the same \"leave it in the different stages and\nforce people to run git-merge-cache on the result\" thing that the\nthree-way merge does.\n\nThe fact that the three-way merge _might_ also like to remove the entries,\nand that the two-way merge already handles the addition of new files, does\nkind of argue that we should do it. For symmetry witht he \"file add\" case, \nif nothing else.\n\n\t\t\tLinus\n"},{"id":"4801","messageId":"7vaclzclqd.fsf@assigned-by-dhcp.cox.net","threadId":"865","inReplyTo":"Pine.LNX.4.58.0506090800580.2286@ppc970.osdl.org","subject":"Re: [PATCH 3/3] read-tree -m 3-way: handle more trivial merges internally","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-06-09T20:35:06Z","receivedAt":"2005-06-09T20:35:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">>>>> \"LT\" == Linus Torvalds writes:\n\nLT> Namely that read-tree doesn't have a frigging clue about renames, and \nLT> shouldn't have.\n\nLT> But a real merge program _could_ have a frigging clue, and might notice \nLT> patterns like\n\nLT> - file got modified in one branch, removed in the other\nLT> - a file got added in the other branch\nLT> - \"Hey, that added file looks like the removed one!\"\nLT> - Let's merge the modifications from the first branch into the move of \nLT> the second branch!\n\nLT> Now, you can (validly) argue that you could still just look at the\nLT> original trees (\"git-diff-tree -C $O $M\") and grep for copies/movement and\nLT> do it by hand _there_ instead of looking at the result of the read-tree, \nLT> and you may well be right. So again, this is not a _fundamental_ problem, \nLT> although it's a bit more fundamental than the first one. \n\nMy knee-jerk reaction was \"No, I would refuse to make that\nargument, because making the merge mechanism to examine trees\nitself would take us full-circle back to where we started\n[*1*]\".\n\nI agree we can, as the zeroth order approximation, run two\n\"diff-tree -B --find-copies-harder -C\" [*3*, *4*] on (O,A) and\n(O,B) pairs, and compare their output to cover the rename case\n[*2*] you described. I think we also can write a simple program\nthat reads an unmerged index file and do the equivalent of these\ntwo diff-tree commands.\n\nHowever, what I suspect to happen in practice is that the lines\nof development leading to A and B may have so much modification\nto those renamed or copied files since they forked at O that we\nmay not recognize renames or copies as such by only looking at\n(O,A) and (O,B). In order to do a reasonable job while merging,\nwe may end up needing to run \"diff-tree --stdin -B -C\" on the\noutput of \"rev-list O A\" to fully follow the rename/copy trail\n[*5*].\n\nWhat all this means is that the simple three-stage information\nread-tree -m gives us, which is about only three trees, might\nnot be enough to handle renames and copies intelligently, when\nwe need to deal with a pair of trees that have diverged for too\nlong.\n\nOnce we go down this path, arguing against making \"read-tree -m\"\nresults useless for such an intelligent merge logic (because it\nforces the merge logic to look at the trees and commits\ninvolved) ceases to make much sense, because such an intelligent\nmerge logic needs to look at more than three trees _anyway_.\n\nWhat \"read-tree -m\" gives us, while being very efficient,\nelegant and effective in \"merge small and merge often\" use\npattern we recommend, may not be so useful to implement such an\nintelligent merge logic, and instead we would do better if we\ndid it the hard way by inspecting individual commits. I do not\nhave problem with that approach. It would be a much longer-term\nproject, though.\n\nSo, yes I ended up arguing that the intelligent merge logic\ncould and probably needs to look at the trees involved ;-).\n\n\nAmong the three-way cases, the only case I think that may make a\npractical difference is the case #5ALT, which deals with \"a file\nadded identically in both branches\" case. This is what happens\nwhen a widely accepted patch has been applied independently to\nboth trees recently (eh, \"since they forked\"). New files tend\nto get updated more often, and allowing the file to be locally\nmodified, instead of failing the merge in read-tree phase, would\nhelp the workflow. If the file were modified in the user's\nrepository, and checked in, then the current 3-way merge code\ncannot help the user that much, because we would be in !O && A\n&& B && A!=B situation. I have a suspicion that we could\nprobably help this case by looking at not just merge base but\nthe edge commits as well.\n\nI consider #14ALT an improvement, but at the same time I doubt\nthat particular one would make much practical difference. It is\nmore or less \"while we are at it\" kind of change. All others,\nincluding the \"remove\" cases (I botched -u but as you point out\nit is correctable), do not contribute to loosening the index\nrequirements, but I suspect they might help me later unify\ntwo-way fast forward and three-way merge. Yes, I am still\nlooking at \"read-tree -m H I-mixed-with-H M\" that emulates\n\"read-tree H M\".\n\n\n[Footnotes]\n\n*1* Remember merge-trees Perl script, which I did before you\ninvented the multi-stage read-tree? Boy it feels like it was so\ndistant past...\n\n*2* A casual reader may notice that we are arguing about renames\nafter both of us publicly stated that \"renames do not matter\".\nHere is a clarification. We both consider \"recording renames at\ncommit time\" does not matter, but we do take \"tracking and\nhandling the renames\" seriously. There is a difference.\n\n*3* Oops. There is not --find-copies-harder yet ;-).\n\n*4* This would be further helped if we had a --show-rename-only\ndiffcore filter. The operation is similar to the pickaxe, but\nit would prune changesets down only to renames and copies. I\nactually wrote and threw away such a filter back when I was\ntrying to find good test cases in linux-2.6 repository.\n\n*5* And the development line leading to A or B may not even be\nlinear, in which case it may be easier to first decompose the\nchain between O and A into individual epochs. Jon Seymour's\n\"rev-list --merge-order O A\" would be very handy for this.\n\n"},{"id":"4806","messageId":"7vekbbb2me.fsf_-_@assigned-by-dhcp.cox.net","threadId":"865","inReplyTo":"7vaclzclqd.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] Add git-diff-stages command.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-06-09T22:13:13Z","receivedAt":"2005-06-09T22:13:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The diff-* brothers acquired a sibling, git-diff-stages. With\nan unmerged index file, you specify two stage numbers and it\nshows the differences between them.\n\nSigned-off-by: Junio C Hamano \n---\n\n*** ... I think we also can write a simple program that reads an\n*** unmerged index file and do the equivalent of these two\n*** diff-tree commands.\n***\n*** Only lightly tested.\n\n Makefile | 3 +-\n diff-stages.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 114 insertions(+), 1 deletions(-)\n\ndiff --git a/Makefile b/Makefile\n--- a/Makefile\n+++ b/Makefile\n@@ -33,7 +33,7 @@ PROG= git-update-cache git-diff-files \n \tgit-http-pull git-ssh-push git-ssh-pull git-rev-list git-mktag \\\n \tgit-diff-helper git-tar-tree git-local-pull git-write-blob \\\n \tgit-get-tar-commit-id git-mkdelta git-apply git-stripspace \\\n-\tgit-cvs2git\n+\tgit-cvs2git git-diff-stages\n \n all: $(PROG)\n \n@@ -117,6 +117,7 @@ git-write-blob: write-blob.c\n git-mkdelta: mkdelta.c\n git-stripspace: stripspace.c\n git-cvs2git: cvs2git.c\n+git-diff-stages: diff-stages.c\n \n git-http-pull: LIBS += -lcurl\n git-rev-list: LIBS += -lssl\ndiff --git a/diff-stages.c b/diff-stages.c\nnew file mode 100644\n--- /dev/null\n+++ b/diff-stages.c\n@@ -0,0 +1,112 @@\n+/*\n+ * Copyright (c) 2005 Junio C Hamano\n+ */\n+\n+#include \"cache.h\"\n+#include \"diff.h\"\n+\n+static int diff_output_format = DIFF_FORMAT_HUMAN;\n+static int detect_rename = 0;\n+static int diff_setup_opt = 0;\n+static int diff_score_opt = 0;\n+static const char *pickaxe = NULL;\n+static int pickaxe_opts = 0;\n+static int diff_break_opt = -1;\n+static const char *orderfile = NULL;\n+\n+static char *diff_stages_usage =\n+\"git-diff-stages [-p] [-r] [-z] [-M] [-C] [-R] [-S] [-O] [...]\";\n+\n+int main(int ac, const char **av)\n+{\n+\tint stage1, stage2, i;\n+\n+\tread_cache();\n+\twhile (1 < ac && av[1][0] == '-') {\n+\t\tconst char *arg = av[1];\n+\t\tif (!strcmp(arg, \"-r\"))\n+\t\t\t; /* as usual */\n+\t\telse if (!strcmp(arg, \"-p\"))\n+\t\t\tdiff_output_format = DIFF_FORMAT_PATCH;\n+\t\telse if (!strncmp(arg, \"-B\", 2)) {\n+\t\t\tif ((diff_break_opt = diff_scoreopt_parse(arg)) == -1)\n+\t\t\t\tusage(diff_stages_usage);\n+\t\t}\n+\t\telse if (!strncmp(arg, \"-M\", 2)) {\n+\t\t\tdetect_rename = DIFF_DETECT_RENAME;\n+\t\t\tif ((diff_score_opt = diff_scoreopt_parse(arg)) == -1)\n+\t\t\t\tusage(diff_stages_usage);\n+\t\t}\n+\t\telse if (!strncmp(arg, \"-C\", 2)) {\n+\t\t\tdetect_rename = DIFF_DETECT_COPY;\n+\t\t\tif ((diff_score_opt = diff_scoreopt_parse(arg)) == -1)\n+\t\t\t\tusage(diff_stages_usage);\n+\t\t}\n+\t\telse if (!strcmp(arg, \"-z\"))\n+\t\t\tdiff_output_format = DIFF_FORMAT_MACHINE;\n+\t\telse if (!strcmp(arg, \"-R\"))\n+\t\t\tdiff_setup_opt |= DIFF_SETUP_REVERSE;\n+\t\telse if (!strncmp(arg, \"-S\", 2))\n+\t\t\tpickaxe = arg + 2;\n+\t\telse if (!strncmp(arg, \"-O\", 2))\n+\t\t\torderfile = arg + 2;\n+\t\telse if (!strcmp(arg, \"--pickaxe-all\"))\n+\t\t\tpickaxe_opts = DIFF_PICKAXE_ALL;\n+\t\telse\n+\t\t\tusage(diff_stages_usage);\n+\t\tac--; av++;\n+\t}\n+\n+\tif (ac < 3 ||\n+\t sscanf(av[1], \"%d\", &stage1) != 1 ||\n+\t ! (0 <= stage1 && stage1 <= 3) ||\n+\t sscanf(av[2], \"%d\", &stage2) != 1 ||\n+\t ! (0 <= stage2 && stage2 <= 3))\n+\t\tusage(diff_stages_usage);\n+\n+\tav += 3; /* The rest from av[0] are for paths restriction. */\n+\tdiff_setup(diff_setup_opt);\n+\n+\ti = 0;\n+\twhile (i < active_nr) {\n+\t\tstruct cache_entry *ce, *stages[4] = { NULL, };\n+\t\tstruct cache_entry *one, *two;\n+\t\tconst char *name;\n+\t\tint len;\n+\t\tce = active_cache[i];\n+\t\tlen = ce_namelen(ce);\n+\t\tname = ce->name;\n+\t\tfor (;;) {\n+\t\t\tint stage = ce_stage(ce);\n+\t\t\tstages[stage] = ce;\n+\t\t\tif (active_nr <= ++i)\n+\t\t\t\tbreak;\n+\t\t\tce = active_cache[i];\n+\t\t\tif (ce_namelen(ce) != len ||\n+\t\t\t memcmp(name, ce->name, len))\n+\t\t\t\tbreak;\n+\t\t}\n+\t\tone = stages[stage1];\n+\t\ttwo = stages[stage2];\n+\t\tif (!one && !two)\n+\t\t\tcontinue;\n+\t\tif (!one)\n+\t\t\tdiff_addremove('+', ntohl(two->ce_mode),\n+\t\t\t\t two->sha1, name, NULL);\n+\t\telse if (!two)\n+\t\t\tdiff_addremove('-', ntohl(one->ce_mode),\n+\t\t\t\t one->sha1, name, NULL);\n+\t\telse if (memcmp(one->sha1, two->sha1, 20) ||\n+\t\t\t (one->ce_mode != two->ce_mode))\n+\t\t\t diff_change(ntohl(one->ce_mode), ntohl(two->ce_mode),\n+\t\t\t\t one->sha1, two->sha1, name, NULL);\n+\t}\n+\n+\tdiffcore_std(av,\n+\t\t detect_rename, diff_score_opt,\n+\t\t pickaxe, pickaxe_opts,\n+\t\t diff_break_opt,\n+\t\t orderfile);\n+\tdiff_flush(diff_output_format, 1);\n+\treturn 0;\n+}\n------------\n\n"},{"id":"4807","messageId":"Pine.LNX.4.58.0506091529230.2286@ppc970.osdl.org","threadId":"865","inReplyTo":"7vekbbb2me.fsf_-_@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Add git-diff-stages command.","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-06-09T22:30:16Z","receivedAt":"2005-06-09T22:30:16Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 9 Jun 2005, Junio C Hamano wrote:\n>\n> The diff-* brothers acquired a sibling, git-diff-stages. With\n> an unmerged index file, you specify two stage numbers and it\n> shows the differences between them.\n\nI hate how you do one big \"main()\" function that does it all.\n\nI'll apply the patch, but really, this is pretty ugly.\n\n\t\tLinus\n"},{"id":"4808","messageId":"7vvf4n9mjc.fsf_-_@assigned-by-dhcp.cox.net","threadId":"865","inReplyTo":"Pine.LNX.4.58.0506091152530.2286@ppc970.osdl.org","subject":"[PATCH] read-tree.c: rename local variables used in 3-way merge code.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-06-09T22:45:59Z","receivedAt":"2005-06-09T22:45:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I'd hate to do this, but every time I try to touch this code and\nvalidate what it does against the case matrix in t1000 test I\nget confused. The variable names are renamed to match the case\nmatrix. Now they are named as:\n\n i -- entry from the index file (formerly known as \"old\")\n o -- merge base (formerly known as \"a\")\n a -- our head (formerly known as \"b\")\n b -- merge head (formerly known as \"c\")\n\nSigned-off-by: Junio C Hamano \n---\n\n*** Re-submit. I've rebased the series from the last night.\n*** This is the first of them.\n\n read-tree.c | 40 ++++++++++++++++++++--------------------\n 1 files changed, 20 insertions(+), 20 deletions(-)\n\ndiff --git a/read-tree.c b/read-tree.c\n--- a/read-tree.c\n+++ b/read-tree.c\n@@ -40,9 +40,9 @@ static int same(struct cache_entry *a, s\n * This removes all trivial merges that don't change the tree\n * and collapses them to state 0.\n */\n-static struct cache_entry *merge_entries(struct cache_entry *a,\n-\t\t\t\t\t struct cache_entry *b,\n-\t\t\t\t\t struct cache_entry *c)\n+static struct cache_entry *merge_entries(struct cache_entry *o,\n+\t\t\t\t\t struct cache_entry *a,\n+\t\t\t\t\t struct cache_entry *b)\n {\n \t/*\n \t * Ok, all three entries describe the same\n@@ -58,16 +58,16 @@ static struct cache_entry *merge_entries\n \t * The \"all entries exactly the same\" case falls out as\n \t * a special case of any of the \"two same\" cases.\n \t *\n-\t * Here \"a\" is \"original\", and \"b\" and \"c\" are the two\n+\t * Here \"o\" is \"original\", and \"a\" and \"b\" are the two\n \t * trees we are merging.\n \t */\n-\tif (a && b && c) {\n-\t\tif (same(b,c))\n-\t\t\treturn c;\n+\tif (o && a && b) {\n \t\tif (same(a,b))\n-\t\t\treturn c;\n-\t\tif (same(a,c))\n \t\t\treturn b;\n+\t\tif (same(o,a))\n+\t\t\treturn b;\n+\t\tif (same(o,b))\n+\t\t\treturn a;\n \t}\n \treturn NULL;\n }\n@@ -126,29 +126,29 @@ static int merged_entry(struct cache_ent\n \n static int threeway_merge(struct cache_entry *stages[4], struct cache_entry **dst)\n {\n-\tstruct cache_entry *old = stages[0];\n-\tstruct cache_entry *a = stages[1], *b = stages[2], *c = stages[3];\n+\tstruct cache_entry *i = stages[0];\n+\tstruct cache_entry *o = stages[1], *a = stages[2], *b = stages[3];\n \tstruct cache_entry *merge;\n \tint count;\n \n \t/*\n-\t * If we have an entry in the index cache (\"old\"), then we want\n+\t * If we have an entry in the index cache (\"i\"), then we want\n \t * to make sure that it matches any entries in stage 2 (\"first\n-\t * branch\", aka \"b\").\n+\t * branch\", aka \"a\").\n \t */\n-\tif (old) {\n-\t\tif (!b || !same(old, b))\n+\tif (i) {\n+\t\tif (!a || !same(i, a))\n \t\t\treturn -1;\n \t}\n-\tmerge = merge_entries(a, b, c);\n+\tmerge = merge_entries(o, a, b);\n \tif (merge)\n-\t\treturn merged_entry(merge, old, dst);\n-\tif (old)\n-\t\tverify_uptodate(old);\n+\t\treturn merged_entry(merge, i, dst);\n+\tif (i)\n+\t\tverify_uptodate(i);\n \tcount = 0;\n+\tif (o) { *dst++ = o; count++; }\n \tif (a) { *dst++ = a; count++; }\n \tif (b) { *dst++ = b; count++; }\n-\tif (c) { *dst++ = c; count++; }\n \treturn count;\n }\n \n------------\n\n"},{"id":"4809","messageId":"7voeaf9mhd.fsf_-_@assigned-by-dhcp.cox.net","threadId":"865","inReplyTo":"Pine.LNX.4.58.0506091152530.2286@ppc970.osdl.org","subject":"[PATCH] Handle entry removals during merge correctly.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-06-09T22:47:10Z","receivedAt":"2005-06-09T22:47:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Linus Torvalds \n\nWe could handle delete the same way - to set the ce_mode to zero\nand add them to the \"dst\" array, and teach write-cache not to\nwrite them out. Then the same loop that goes around doing the\nCE_UPDATE thing could check for the delete case.\n\nSigned-off-by: Junio C Hamano \n---\n\n*** Linus, did I get the patch submission format right? This is\n*** essentially what you wrote (with one correction) but not\n*** really \"a forwarded e-mail\".\n\n read-cache.c | 10 ++++++++--\n read-tree.c | 30 ++++++++++++++++++++----------\n 2 files changed, 28 insertions(+), 12 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -440,11 +440,15 @@ int write_cache(int newfd, struct cache_\n {\n \tSHA_CTX c;\n \tstruct cache_header hdr;\n-\tint i;\n+\tint i, removed;\n+\n+\tfor (i = removed = 0; i < entries; i++)\n+\t\tif (!cache[i]->ce_mode)\n+\t\t\tremoved++;\n \n \thdr.hdr_signature = htonl(CACHE_SIGNATURE);\n \thdr.hdr_version = htonl(2);\n-\thdr.hdr_entries = htonl(entries);\n+\thdr.hdr_entries = htonl(entries - removed);\n \n \tSHA1_Init(&c);\n \tif (ce_write(&c, newfd, &hdr, sizeof(hdr)) < 0)\n@@ -452,6 +456,8 @@ int write_cache(int newfd, struct cache_\n \n \tfor (i = 0; i < entries; i++) {\n \t\tstruct cache_entry *ce = cache[i];\n+\t\tif (!ce->ce_mode)\n+\t\t\tcontinue;\n \t\tif (ce_write(&c, newfd, ce, ce_size(ce)) < 0)\n \t\t\treturn -1;\n \t}\ndiff --git a/read-tree.c b/read-tree.c\n--- a/read-tree.c\n+++ b/read-tree.c\n@@ -124,6 +124,15 @@ static int merged_entry(struct cache_ent\n \treturn 1;\n }\n \n+static int deleted_entry(struct cache_entry *ce, struct cache_entry *old, struct cache_entry **dst)\n+{\n+\tif (old)\n+\t\tverify_uptodate(old);\n+\tce->ce_mode = 0;\n+\t*dst++ = ce;\n+\treturn 1;\n+}\n+\n static int threeway_merge(struct cache_entry *stages[4], struct cache_entry **dst)\n {\n \tstruct cache_entry *i = stages[0];\n@@ -181,25 +190,21 @@ static int twoway_merge(struct cache_ent\n \t\t\t*dst++ = current;\n \t\t\treturn 1;\n \t\t}\n-\t\telse if (oldtree && !newtree && same(current, oldtree)) {\n+\t\telse if (oldtree && !newtree && same(current, oldtree))\n \t\t\t/* 10 or 11 */\n-\t\t\tverify_uptodate(current);\n-\t\t\treturn 0;\n-\t\t}\n+\t\t\treturn deleted_entry(oldtree, current, dst);\n \t\telse if (oldtree && newtree &&\n-\t\t\t same(current, oldtree) && !same(current, newtree)) {\n+\t\t\t same(current, oldtree) && !same(current, newtree))\n \t\t\t/* 20 or 21 */\n-\t\t\tverify_uptodate(current);\n-\t\t\treturn merged_entry(newtree, NULL, dst);\n-\t\t}\n+\t\t\treturn merged_entry(newtree, current, dst);\n \t\telse\n \t\t\t/* all other failures */\n \t\t\treturn -1;\n \t}\n \telse if (newtree)\n-\t\treturn merged_entry(newtree, NULL, dst);\n+\t\treturn merged_entry(newtree, current, dst);\n \telse\n-\t\treturn 0;\n+\t\treturn deleted_entry(oldtree, current, dst);\n }\n \n /*\n@@ -236,6 +241,11 @@ static void check_updates(struct cache_e\n \tunsigned short mask = htons(CE_UPDATE);\n \twhile (nr--) {\n \t\tstruct cache_entry *ce = *src++;\n+\t\tif (!ce->ce_mode) {\n+\t\t\tif (update)\n+\t\t\t\tunlink(ce->name);\n+\t\t\tcontinue;\n+\t\t}\n \t\tif (ce->ce_flags & mask) {\n \t\t\tce->ce_flags &= ~mask;\n \t\t\tif (update)\n------------\n\n"},{"id":"4810","messageId":"7vekbb9mfg.fsf_-_@assigned-by-dhcp.cox.net","threadId":"865","inReplyTo":"Pine.LNX.4.58.0506091152530.2286@ppc970.osdl.org","subject":"[PATCH] read-tree -m 3-way: loosen an index requirement that was too strict.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-06-09T22:48:19Z","receivedAt":"2005-06-09T22:48:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This patch teaches \"read-tree -m O A B\" that, when only \"the\nother tree\" changed a path, and if the work tree already has\nthat change, we are not in a situation that would clobber the\ncache and the working tree, and lets the merge succeed; this is\ncase #14ALT in t1000 test. It does not change the result of the\nmerge, but prevents it from failing when it should not.\n\nSigned-off-by: Junio C Hamano \n---\n\n*** Rebased one from the last night.\n\n read-tree.c | 16 ++++++++++++++++\n t/t1000-read-tree-m-3way.sh | 9 +++++++++\n 2 files changed, 25 insertions(+), 0 deletions(-)\n\ndiff --git a/read-tree.c b/read-tree.c\n--- a/read-tree.c\n+++ b/read-tree.c\n@@ -140,6 +140,22 @@ static int threeway_merge(struct cache_e\n \tstruct cache_entry *merge;\n \tint count;\n \n+\t/* The case #14ALT is special in that it allows \"i\" to match\n+\t * the \"merged branch\", aka \"b\" and even be dirty, as an\n+\t * alternative to the usual 'must match \"a\" and be up-to-date'\n+\t * rule.\n+\t */\n+\tif (o && a && b && same(o, a) && !same(o, b)) {\n+\t\tif (i) {\n+\t\t\tif (same(i, b))\n+\t\t\t\t; /* case #14ALT exception */\n+\t\t\telse if (same(i, a))\n+\t\t\t\tverify_uptodate(i);\n+\t\t\telse\n+\t\t\t\treturn -1;\n+\t\t}\n+\t}\n+\telse /* otherwise the original rule applies */\n \t/*\n \t * If we have an entry in the index cache (\"i\"), then we want\n \t * to make sure that it matches any entries in stage 2 (\"first\ndiff --git a/t/t1000-read-tree-m-3way.sh b/t/t1000-read-tree-m-3way.sh\n--- a/t/t1000-read-tree-m-3way.sh\n+++ b/t/t1000-read-tree-m-3way.sh\n@@ -455,6 +455,15 @@ test_expect_success \\\n git-read-tree -m $tree_O $tree_A $tree_B &&\n check_result\"\n \n+test_expect_success \\\n+ '14ALT - in O && A && B && O==A && O!=B case, matching B is also OK' \\\n+ \"rm -f .git/index NM &&\n+ cp .orig-B/NM NM &&\n+ git-update-cache --add NM &&\n+ echo extra >>NM &&\n+ git-read-tree -m $tree_O $tree_A $tree_B &&\n+ check_result\"\n+\n test_expect_failure \\\n '14 (fail) - must match and be up-to-date in O && A && B && O==A && O!=B case' \\\n \"rm -f .git/index NM &&\n------------\n\n"},{"id":"4811","messageId":"7v4qc79mds.fsf_-_@assigned-by-dhcp.cox.net","threadId":"865","inReplyTo":"Pine.LNX.4.58.0506091152530.2286@ppc970.osdl.org","subject":"[PATCH] read-tree -m 3-way: handle more trivial merges internally.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-06-09T22:49:19Z","receivedAt":"2005-06-09T22:49:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This patch teaches \"read-tree -m O A B\" that some more trivial\ncases can be handled internally. This allows us to loosen\notherwise too strict index requirements in case #5ALT, where\nboth branches create a new file identically. The previous\ncode required index to be up-to-date and aborted the merge when\nit is not, but there is no reason to require it to be up-to-date\nin this case; it only needs to match A.\n\nThe test vector has been updated to match the new behaviour.\n\nSigned-off-by: Junio C Hamano \n---\n\n*** This has the \"removal\" fixes.\n\n read-tree.c | 16 ++++++++++++++++\n t/t1000-read-tree-m-3way.sh | 27 +++++++++------------------\n 2 files changed, 25 insertions(+), 18 deletions(-)\n\ndiff --git a/read-tree.c b/read-tree.c\n--- a/read-tree.c\n+++ b/read-tree.c\n@@ -69,6 +69,12 @@ static struct cache_entry *merge_entries\n \t\tif (same(o,b))\n \t\t\treturn a;\n \t}\n+\t/* #5ALT */\n+\tif (!o && a && b && same(a,b)) {\n+\t\t/* Match what git-merge-one-file-script does */\n+\t\tprintf(\"Adding %s\\n\", a->name);\n+\t\treturn a;\n+\t}\n \treturn NULL;\n }\n \n@@ -170,6 +176,16 @@ static int threeway_merge(struct cache_e\n \t\treturn merged_entry(merge, i, dst);\n \tif (i)\n \t\tverify_uptodate(i);\n+\n+\t/* #6ALT, #8ALT, and #10ALT */\n+\tif ((o && !a && !b) ||\n+\t (o && !a && b && same(o, b)) ||\n+\t (o && a && !b && same(o, a))) {\n+\t\t/* Match what git-merge-one-file-script does */\n+\t\tprintf(\"Removing %s\\n\", o->name); \n+\t\treturn deleted_entry(o, i, dst);\n+\t}\n+\n \tcount = 0;\n \tif (o) { *dst++ = o; count++; }\n \tif (a) { *dst++ = a; count++; }\ndiff --git a/t/t1000-read-tree-m-3way.sh b/t/t1000-read-tree-m-3way.sh\n--- a/t/t1000-read-tree-m-3way.sh\n+++ b/t/t1000-read-tree-m-3way.sh\n@@ -75,21 +75,18 @@ In addition:\n . ../lib-read-tree-m-3way.sh\n \n ################################################################\n-# This is the \"no trivial merge unless all three exists\" table.\n+# Trivial \"majority when 3 stages exist\" merge plus #5ALT, #6ALT,\n+# #8ALT, #10ALT trivial merges.\n \n cat >expected <<\\EOF\n 100644 X 2\tAA\n 100644 X 3\tAA\n 100644 X 2\tAN\n-100644 X 1\tDD\n 100644 X 3\tDF\n 100644 X 2\tDF/DF\n 100644 X 1\tDM\n 100644 X 3\tDM\n-100644 X 1\tDN\n-100644 X 3\tDN\n-100644 X 2\tLL\n-100644 X 3\tLL\n+100644 X 0\tLL\n 100644 X 1\tMD\n 100644 X 2\tMD\n 100644 X 1\tMM\n@@ -97,8 +94,6 @@ cat >expected <<\\EOF\n 100644 X 3\tMM\n 100644 X 0\tMN\n 100644 X 3\tNA\n-100644 X 1\tND\n-100644 X 2\tND\n 100644 X 0\tNM\n 100644 X 0\tNN\n 100644 X 0\tSS\n@@ -108,11 +103,8 @@ cat >expected <<\\EOF\n 100644 X 2\tZ/AA\n 100644 X 3\tZ/AA\n 100644 X 2\tZ/AN\n-100644 X 1\tZ/DD\n 100644 X 1\tZ/DM\n 100644 X 3\tZ/DM\n-100644 X 1\tZ/DN\n-100644 X 3\tZ/DN\n 100644 X 1\tZ/MD\n 100644 X 2\tZ/MD\n 100644 X 1\tZ/MM\n@@ -120,8 +112,6 @@ cat >expected <<\\EOF\n 100644 X 3\tZ/MM\n 100644 X 0\tZ/MN\n 100644 X 3\tZ/NA\n-100644 X 1\tZ/ND\n-100644 X 2\tZ/ND\n 100644 X 0\tZ/NM\n 100644 X 0\tZ/NN\n EOF\n@@ -289,23 +279,24 @@ test_expect_failure \\\n git-read-tree -m $tree_O $tree_A $tree_B\"\n \n test_expect_success \\\n- '5 - must match and be up-to-date in !O && A && B && A==B case.' \\\n+ '5 - must match in !O && A && B && A==B case.' \\\n \"rm -f .git/index LL &&\n cp .orig-A/LL LL &&\n git-update-cache --add LL &&\n git-read-tree -m $tree_O $tree_A $tree_B &&\n check_result\"\n \n-test_expect_failure \\\n- '5 (fail) - must match and be up-to-date in !O && A && B && A==B case.' \\\n+test_expect_success \\\n+ '5 - must match in !O && A && B && A==B case.' \\\n \"rm -f .git/index LL &&\n cp .orig-A/LL LL &&\n git-update-cache --add LL &&\n echo extra >>LL &&\n- git-read-tree -m $tree_O $tree_A $tree_B\"\n+ git-read-tree -m $tree_O $tree_A $tree_B &&\n+ check_result\"\n \n test_expect_failure \\\n- '5 (fail) - must match and be up-to-date in !O && A && B && A==B case.' \\\n+ '5 (fail) - must match in !O && A && B && A==B case.' \\\n \"rm -f .git/index LL &&\n cp .orig-A/LL LL &&\n echo extra >>LL &&\n------------\n\n"},{"id":"4830","messageId":"7vekba7zli.fsf@assigned-by-dhcp.cox.net","threadId":"865","inReplyTo":"7vaclzclqd.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 3/3] read-tree -m 3-way: handle more trivial merges internally","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-06-10T19:59:05Z","receivedAt":"2005-06-10T19:59:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">>>>> \"JCH\" == Junio C Hamano writes:\n\nJCH> So, yes I ended up arguing that the intelligent merge logic\nJCH> could and probably needs to look at the trees involved ;-).\n\n\"Could look at, and probably be better off looking at,\" would\nhave been a better wording.\n\nLinus, please discard the patches from me that you have not\napplied about the \"loosening of too strict index requirements\"\n(yesterday and the day before). I think I am finally getting\nsomewhere but the solution, if it works, would be somewhat\ndifferent from what I have been sending you.\n\n\n\n\n"},{"id":"4846","messageId":"7vu0k56517.fsf_-_@assigned-by-dhcp.cox.net","threadId":"865","inReplyTo":"7vekbbb2me.fsf_-_@assigned-by-dhcp.cox.net","subject":"[PATCH] diff-stages: unuglify the too big main() function.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-06-11T01:44:36Z","receivedAt":"2005-06-11T01:44:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Split the core of the program, diff_stage, from one big \"main()\"\nfunction that does it all and leave only the parameter parsing,\nsetup and finalize part in the main().\n\nSigned-off-by: Junio C Hamano \n---\n\n diff-stages.c | 75 ++++++++++++++++++++++++++++++---------------------------\n 1 files changed, 40 insertions(+), 35 deletions(-)\n\ndiff --git a/diff-stages.c b/diff-stages.c\n--- a/diff-stages.c\n+++ b/diff-stages.c\n@@ -17,9 +17,47 @@ static const char *orderfile = NULL;\n static char *diff_stages_usage =\n \"git-diff-stages [-p] [-r] [-z] [-M] [-C] [-R] [-S] [-O] [...]\";\n \n+static void diff_stages(int stage1, int stage2)\n+{\n+\tint i = 0;\n+\twhile (i < active_nr) {\n+\t\tstruct cache_entry *ce, *stages[4] = { NULL, };\n+\t\tstruct cache_entry *one, *two;\n+\t\tconst char *name;\n+\t\tint len;\n+\t\tce = active_cache[i];\n+\t\tlen = ce_namelen(ce);\n+\t\tname = ce->name;\n+\t\tfor (;;) {\n+\t\t\tint stage = ce_stage(ce);\n+\t\t\tstages[stage] = ce;\n+\t\t\tif (active_nr <= ++i)\n+\t\t\t\tbreak;\n+\t\t\tce = active_cache[i];\n+\t\t\tif (ce_namelen(ce) != len ||\n+\t\t\t memcmp(name, ce->name, len))\n+\t\t\t\tbreak;\n+\t\t}\n+\t\tone = stages[stage1];\n+\t\ttwo = stages[stage2];\n+\t\tif (!one && !two)\n+\t\t\tcontinue;\n+\t\tif (!one)\n+\t\t\tdiff_addremove('+', ntohl(two->ce_mode),\n+\t\t\t\t two->sha1, name, NULL);\n+\t\telse if (!two)\n+\t\t\tdiff_addremove('-', ntohl(one->ce_mode),\n+\t\t\t\t one->sha1, name, NULL);\n+\t\telse if (memcmp(one->sha1, two->sha1, 20) ||\n+\t\t\t (one->ce_mode != two->ce_mode))\n+\t\t\t diff_change(ntohl(one->ce_mode), ntohl(two->ce_mode),\n+\t\t\t\t one->sha1, two->sha1, name, NULL);\n+\t}\n+}\n+\n int main(int ac, const char **av)\n {\n-\tint stage1, stage2, i;\n+\tint stage1, stage2;\n \n \tread_cache();\n \twhile (1 < ac && av[1][0] == '-') {\n@@ -67,40 +105,7 @@ int main(int ac, const char **av)\n \tav += 3; /* The rest from av[0] are for paths restriction. */\n \tdiff_setup(diff_setup_opt);\n \n-\ti = 0;\n-\twhile (i < active_nr) {\n-\t\tstruct cache_entry *ce, *stages[4] = { NULL, };\n-\t\tstruct cache_entry *one, *two;\n-\t\tconst char *name;\n-\t\tint len;\n-\t\tce = active_cache[i];\n-\t\tlen = ce_namelen(ce);\n-\t\tname = ce->name;\n-\t\tfor (;;) {\n-\t\t\tint stage = ce_stage(ce);\n-\t\t\tstages[stage] = ce;\n-\t\t\tif (active_nr <= ++i)\n-\t\t\t\tbreak;\n-\t\t\tce = active_cache[i];\n-\t\t\tif (ce_namelen(ce) != len ||\n-\t\t\t memcmp(name, ce->name, len))\n-\t\t\t\tbreak;\n-\t\t}\n-\t\tone = stages[stage1];\n-\t\ttwo = stages[stage2];\n-\t\tif (!one && !two)\n-\t\t\tcontinue;\n-\t\tif (!one)\n-\t\t\tdiff_addremove('+', ntohl(two->ce_mode),\n-\t\t\t\t two->sha1, name, NULL);\n-\t\telse if (!two)\n-\t\t\tdiff_addremove('-', ntohl(one->ce_mode),\n-\t\t\t\t one->sha1, name, NULL);\n-\t\telse if (memcmp(one->sha1, two->sha1, 20) ||\n-\t\t\t (one->ce_mode != two->ce_mode))\n-\t\t\t diff_change(ntohl(one->ce_mode), ntohl(two->ce_mode),\n-\t\t\t\t one->sha1, two->sha1, name, NULL);\n-\t}\n+\tdiff_stages(stage1, stage2);\n \n \tdiffcore_std(av,\n \t\t detect_rename, diff_score_opt,\n------------\n\n"},{"id":"5011","messageId":"E1DjQza-0001wP-00@gondolin.me.apana.org.au","threadId":"865","inReplyTo":"Pine.LNX.4.58.0506081629370.2286@ppc970.osdl.org","subject":"Re: Handling merge conflicts a bit more gracefully..","fromName":"Herbert Xu","fromEmail":"herbert@gondor.apana.org.au","sentAt":"2005-06-18T00:15:22Z","receivedAt":"2005-06-18T00:15:22Z","isPatch":false,"sender":{"key":"herbert@gondor.apana.org.au","avatar":null},"body":"Linus Torvalds wrote:\n> \n>> # Modified in both, but differently.\n>> + merge -p \"$src1\" \"$orig\" \"$src2\" > \"$4\"\n>> \n>> Again, make sure \"$4\" is not a directory before redirecting into\n>> it from merge, so that you can tell merge failures from it?\n> \n> Hmm.. What's the cleanest way to check for redirection errors, but still\n> be able to distinguish those cleanly from \"merge\" itself returning an\n> error?\n\nI don't know whether this is the cleanest, but this is one way:\n\nredir=failed\n{\n\tredir=ok\n\tmerge -p \"$src1\" \"$orig\" \"$src2\"\n} > \"$4\" || err=$?\n\nif [ $redir = failed ]; then\n\t...\nfi\n\nCheers,\n-- \nVisit Openswan at http://www.openswan.org/\nEmail: Herbert Xu ~{PmV>HI~} \nHome Page: http://gondor.apana.org.au/~herbert/\nPGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt\n"},{"id":"5012","messageId":"Pine.LNX.4.58.0506171725150.2268@ppc970.osdl.org","threadId":"865","inReplyTo":"E1DjQza-0001wP-00@gondolin.me.apana.org.au","subject":"Re: Handling merge conflicts a bit more gracefully..","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-06-18T00:26:02Z","receivedAt":"2005-06-18T00:26:02Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 18 Jun 2005, Herbert Xu wrote:\n> \n> I don't know whether this is the cleanest, but this is one way:\n\nOh, wow.\n\nOne thing I have to say is that I've learnt a lot more shell tricks. \n\nNow I'll just have to unlearn them, so that I won't have nightmares.\n\n\t\tLinus\n"}]}