{"thread":{"id":"5947","subject":"[PATCH] Use diff3 instead of merge in merge-recursive.","startedAt":"2006-10-18T08:59:49Z","lastAt":"2006-10-21T00:06:44Z","messageCount":8,"participants":["Uwe Zeisberger","Jakub Narebski","Johannes Schindelin","Linus Torvalds","Daniel Barkalow"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"29088","messageId":"20061018085949.GA26501@cepheus.pub","threadId":"5947","inReplyTo":null,"subject":"[PATCH] Use diff3 instead of merge in merge-recursive.","fromName":"Uwe Zeisberger","fromEmail":"zeisberg@informatik.uni-freiburg.de","sentAt":"2006-10-18T08:59:49Z","receivedAt":"2006-10-18T08:59:49Z","isPatch":true,"sender":{"key":"u.kleine-koenig@pengutronix.de","avatar":"https://gravatar.com/avatar/354b5e3ceb2806a2f1e1e382ac29ddbdad18288654da62b61eb13583a857eee7?d=mp&s=160"},"body":"If no error occurs, merge (from rcs 5.7) is nothing but:\n\n\tdiff3 -E -am -L label1 -L label2 -L label3 file1 file2 file3 > tmpfile\n\tcat tmpfile > file1\n\nUsing diff3 directly saves one fork per conflicting file.\n\nSigned-off-by: Uwe Zeisberger <zeisberg@informatik.uni-freiburg.de>\n---\n merge-recursive.c |   34 +++++++++++++++++++++++-----------\n 1 files changed, 23 insertions(+), 11 deletions(-)\n\nIt passes `make test` when NO_SVN_TESTS is defined.  (I'll write a\nseparate mail about that.)\n\nI didn't made any timing tests or further tests for correctness, but I\nhope Johannes still has the framework from the time when he converted\nthe Python script to C?  \n\n@Johannes: If so, could you test this patch?\n\nBest regards\nUwe\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 2ba43ae..9e3f9d7 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -657,8 +657,11 @@ static struct merge_file_info merge_file\n \t\t\tchar orig[PATH_MAX];\n \t\t\tchar src1[PATH_MAX];\n \t\t\tchar src2[PATH_MAX];\n+\t\t\tchar tmppath[PATH_MAX];\n+\n \t\t\tconst char *argv[] = {\n-\t\t\t\t\"merge\", \"-L\", NULL, \"-L\", NULL, \"-L\", NULL,\n+\t\t\t\t\"diff3\", \"-E\", \"-am\",\n+\t\t\t\t\"-L\", NULL, \"-L\", NULL, \"-L\", NULL,\n \t\t\t\tNULL, NULL, NULL,\n \t\t\t\tNULL\n \t\t\t};\n@@ -668,23 +671,31 @@ static struct merge_file_info merge_file\n \t\t\tgit_unpack_file(a->sha1, src1);\n \t\t\tgit_unpack_file(b->sha1, src2);\n \n-\t\t\targv[2] = la = xstrdup(mkpath(\"%s/%s\", branch1, a->path));\n-\t\t\targv[6] = lb = xstrdup(mkpath(\"%s/%s\", branch2, b->path));\n-\t\t\targv[4] = lo = xstrdup(mkpath(\"orig/%s\", o->path));\n-\t\t\targv[7] = src1;\n-\t\t\targv[8] = orig;\n-\t\t\targv[9] = src2,\n+\t\t\targv[4] = la = xstrdup(mkpath(\"%s/%s\", branch1, a->path));\n+\t\t\targv[8] = lb = xstrdup(mkpath(\"%s/%s\", branch2, b->path));\n+\t\t\targv[6] = lo = xstrdup(mkpath(\"orig/%s\", o->path));\n+\t\t\targv[9] = src1;\n+\t\t\targv[10] = orig;\n+\t\t\targv[11] = src2;\n+\n+\t\t\tfd = git_mkstemp(tmppath, sizeof(tmppath),\n+\t\t\t\t\t\".merge_file_XXXXXX\");\n+\t\t\tif (fd < 0)\n+\t\t\t\tdie(\"unable to create temp-file\");\n+\n+\t\t\tdup2(fd, 1);\n+\t\t\tclose(fd);\n \n-\t\t\tcode = run_command_v(10, argv);\n+\t\t\tcode = run_command_v(12, argv);\n \n \t\t\tfree(la);\n \t\t\tfree(lb);\n \t\t\tfree(lo);\n \t\t\tif (code && code < -256) {\n-\t\t\t\tdie(\"Failed to execute 'merge'. merge(1) is used as the \"\n-\t\t\t\t    \"file-level merge tool. Is 'merge' in your path?\");\n+\t\t\t\tdie(\"Failed to execute 'diff3'. diff3(1) is used as the \"\n+\t\t\t\t    \"file-level merge tool. Is 'diff3' in your path?\");\n \t\t\t}\n-\t\t\tfd = open(src1, O_RDONLY);\n+\t\t\tfd = open(tmppath, O_RDONLY);\n \t\t\tif (fd < 0 || fstat(fd, &st) < 0 ||\n \t\t\t\t\tindex_fd(result.sha, fd, &st, 1,\n \t\t\t\t\t\t\"blob\"))\n@@ -693,6 +704,7 @@ static struct merge_file_info merge_file\n \t\t\tunlink(orig);\n \t\t\tunlink(src1);\n \t\t\tunlink(src2);\n+\t\t\tunlink(tmppath);\n \n \t\t\tresult.clean = WEXITSTATUS(code) == 0;\n \t\t} else {\n-- \n1.4.3.rc2\n\n-- \nUwe Zeisberger\n\nhttp://www.google.com/search?q=1+newton+in+kg*m+%2F+s%5E2\n"},{"id":"29096","messageId":"eh4sfr$3ej$1@sea.gmane.org","threadId":"5947","inReplyTo":"20061018085949.GA26501@cepheus.pub","subject":"Re: [PATCH] Use diff3 instead of merge in merge-recursive.","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-10-18T09:35:28Z","receivedAt":"2006-10-18T09:35:28Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Uwe Zeisberger wrote:\n\n> If no error occurs, merge (from rcs 5.7) is nothing but:\n> \n>         diff3 -E -am -L label1 -L label2 -L label3 file1 file2 file3 > tmpfile\n>         cat tmpfile > file1\n> \n> Using diff3 directly saves one fork per conflicting file.\n\nDoesn't xdiff library git uses have diff3/merge equivalent?\nCouldn't we use that instead (on less dependency, better performance)?\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"29097","messageId":"Pine.LNX.4.63.0610181135120.14200@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"5947","inReplyTo":"20061018085949.GA26501@cepheus.pub","subject":"Re: [PATCH] Use diff3 instead of merge in merge-recursive.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-10-18T09:38:15Z","receivedAt":"2006-10-18T09:38:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Uwe,\n\nOn Wed, 18 Oct 2006, Uwe Zeisberger wrote:\n\n> If no error occurs, merge (from rcs 5.7) is nothing but:\n> \n> \tdiff3 -E -am -L label1 -L label2 -L label3 file1 file2 file3 > tmpfile\n> \tcat tmpfile > file1\n\nInteresting. I wonder if we could streamline the code such that index_fd \nis called directly on the output of diff3? Of course, the result has to be \nremoved when the call to diff3 fails.\n\n> I didn't made any timing tests or further tests for correctness, but I\n> hope Johannes still has the framework from the time when he converted\n> the Python script to C?  \n> \n> @Johannes: If so, could you test this patch?\n\nI have to dig a little where I have it, but I think I can give it a try in \na few hours (imagine this lyrics to the melody of the day job blues).\n\nCiao,\nDscho\n"},{"id":"29100","messageId":"Pine.LNX.4.63.0610181202260.14200@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"5947","inReplyTo":"eh4sfr$3ej$1@sea.gmane.org","subject":"Re: [PATCH] Use diff3 instead of merge in merge-recursive.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-10-18T10:04:22Z","receivedAt":"2006-10-18T10:04:22Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 18 Oct 2006, Jakub Narebski wrote:\n\n> Uwe Zeisberger wrote:\n> \n> > If no error occurs, merge (from rcs 5.7) is nothing but:\n> > \n> >         diff3 -E -am -L label1 -L label2 -L label3 file1 file2 file3 > tmpfile\n> >         cat tmpfile > file1\n> > \n> > Using diff3 directly saves one fork per conflicting file.\n> \n> Doesn't xdiff library git uses have diff3/merge equivalent?\n> Couldn't we use that instead (on less dependency, better performance)?\n\nWe have only half of libxdiff, lacking the patch functionality. And AFAIUI \nthe minimal implementation of diff3 in xdiff just uses a diff of the first \npair on the third file (or some other order). So, it is not really a \ndiff3/merge: it would fail in fairly trivial cases.\n\nCiao,\nDscho\n\nP.S.: Davide, correct me if I'm wrong."},{"id":"29124","messageId":"Pine.LNX.4.64.0610180835000.3962@g5.osdl.org","threadId":"5947","inReplyTo":"eh4sfr$3ej$1@sea.gmane.org","subject":"Re: [PATCH] Use diff3 instead of merge in merge-recursive.","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-10-18T15:53:53Z","receivedAt":"2006-10-18T15:53:53Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 18 Oct 2006, Jakub Narebski wrote:\n> \n> Doesn't xdiff library git uses have diff3/merge equivalent?\n\nNope. It has something it calls \"merge\", but it's really just \"apply the \ndiff from the common base to the other end\".\n\nIOW, if \"a\" is your common ancestor, and \"b1\" and \"b2\" are the branches, \nit's literally\n\n\tdiff a b1 | patch b2\n\nand not actually a real 3-way merge.\n\nAs to why git uses \"merge\" - I have this strong memory of having seen \nmachines that had one but not the other, and that, along with the fact \nthat I've used \"merge\" personally, is why we call \"merge\" rather than \ndiff3.\n\nIn Linux systems, \"merge\" usually comes with the RCS package, and \"diff3\" \nis usually from \"diffutils\". It may be that \"diff3\" is more common. I'm \nnot sure what the history is, and what the situation would tend to be like \non other systems..\n\n\t\tLinus\n"},{"id":"29210","messageId":"Pine.LNX.4.64.0610190229590.9789@iabervon.org","threadId":"5947","inReplyTo":"Pine.LNX.4.64.0610180835000.3962@g5.osdl.org","subject":"Re: [PATCH] Use diff3 instead of merge in merge-recursive.","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2006-10-19T06:31:25Z","receivedAt":"2006-10-19T06:31:25Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 18 Oct 2006, Linus Torvalds wrote:\n\n> As to why git uses \"merge\" - I have this strong memory of having seen \n> machines that had one but not the other, and that, along with the fact \n> that I've used \"merge\" personally, is why we call \"merge\" rather than \n> diff3.\n> \n> In Linux systems, \"merge\" usually comes with the RCS package, and \"diff3\" \n> is usually from \"diffutils\". It may be that \"diff3\" is more common. I'm \n> not sure what the history is, and what the situation would tend to be like \n> on other systems..\n\nAFAICT, \"merge\" actually execs \"diff3\", so if a system has only one, \neither it's diff3, it's a \"merge\" from somewhere other than current rcs, \nor it doesn't actually work.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"29424","messageId":"20061020211121.GB3024@cepheus.pub","threadId":"5947","inReplyTo":"Pine.LNX.4.63.0610181135120.14200@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Use diff3 instead of merge in merge-recursive.","fromName":"Uwe Zeisberger","fromEmail":"zeisberg@informatik.uni-freiburg.de","sentAt":"2006-10-20T21:11:21Z","receivedAt":"2006-10-20T21:11:21Z","isPatch":true,"sender":{"key":"u.kleine-koenig@pengutronix.de","avatar":"https://gravatar.com/avatar/354b5e3ceb2806a2f1e1e382ac29ddbdad18288654da62b61eb13583a857eee7?d=mp&s=160"},"body":"Johannes Schindelin wrote:\n> Hi Uwe,\n> \n> On Wed, 18 Oct 2006, Uwe Zeisberger wrote:\n> \n> > If no error occurs, merge (from rcs 5.7) is nothing but:\n> > \n> > \tdiff3 -E -am -L label1 -L label2 -L label3 file1 file2 file3 > tmpfile\n> > \tcat tmpfile > file1\n> \n> Interesting.\n\n> I wonder if we could streamline the code such that index_fd \n> is called directly on the output of diff3? Of course, the result has to be \n> removed when the call to diff3 fails.\nI thought about that, too.  But my primary intention was to get rid of\n'merge', because the Solaris boxes I use from time to time lack merge,\nbut have (GNU) diff3[1].  I already had a mental note to look into that.\n\nIf Linus is right that there are systems that have merge but lack diff3,\nthen a combined approach is maybe the best?  That is, try diff3 and if\nthat is missing, try merge.  (Or the other way round if you prefer.)\n\nOK, I looked a bit deeper into rcs, and it seems to handle the BSD diff3\ncase.  So Linus might be right.\n\nBTW, merge -p sends the merged result to stdout instead of overwriting\nthe first file given.  That is\n\n\tmerge -p -L label1 -L label2 -L label3 file1 file2 file3\n\nand (GNU)\n\n\tdiff3 -E -am -L label1 -L label2 -L label3 file1 file2 file3\n\nare exactly equivalent.\nSo if that option of merge is old enough, these are the candidates for\nthe \"combined approach\" (see above).\n\n> > I didn't made any timing tests or further tests for correctness, but I\n> > hope Johannes still has the framework from the time when he converted\n> > the Python script to C?  \n> > \n> > @Johannes: If so, could you test this patch?\n> \n> I have to dig a little where I have it, but I think I can give it a try in \n> a few hours (imagine this lyrics to the melody of the day job blues).\nSeems to be a long blues because you didn't sent any results. :-(\n\nBest regards\nUwe\n\n[1] They also have a version of diff3 (I guess from BSD) that is not\nsuited to be used for merging, at least rcs' merge cannot use it.\n\n-- \nUwe Zeisberger\n\nIf a lawyer and an IRS agent were both drowning, and you could only save\none of them, would you go to lunch or read the paper?\n"},{"id":"29444","messageId":"Pine.LNX.4.63.0610210203390.14200@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"5947","inReplyTo":"20061020211121.GB3024@cepheus.pub","subject":"Re: [PATCH] Use diff3 instead of merge in merge-recursive.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-10-21T00:06:44Z","receivedAt":"2006-10-21T00:06:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 20 Oct 2006, Uwe Zeisberger wrote:\n\n> Johannes Schindelin wrote:\n>\n> > I have to dig a little where I have it, but I think I can give it a try in \n> > a few hours (imagine this lyrics to the melody of the day job blues).\n>\n> Seems to be a long blues because you didn't sent any results. :-(\n\nYes. It was a long blues, and now instead of going to sleep I tested it. \nIn the long run, it was negligible, with a high local variability (which \nstems from the fact that I had to run this test while the machine was \nunder high load, which it will be for another 48 hours minimum).\n\nIt makes sense that performance-wise, it will not make a great difference. \nAfter all, the expensive operation is not the file-writing, but the \nmerging pass.\n\nCiao,\nDscho\n"}]}