Re: [PATCH] Use diff3 instead of merge in merge-recursive.

Previous thread: t9100-git-svn-basic.sh fails by Uwe Zeisberger on Wednesday, October 18, 2006 - 1:59 am. (3 messages)

Next thread: Re: [PATCH] Use diff3 instead of merge in merge-recursive. by Jakub Narebski on Wednesday, October 18, 2006 - 2:35 am. (4 messages)
From: Uwe Zeisberger
Date: Wednesday, October 18, 2006 - 1:59 am

If no error occurs, merge (from rcs 5.7) is nothing but:

	diff3 -E -am -L label1 -L label2 -L label3 file1 file2 file3 > tmpfile
	cat tmpfile > file1

Using diff3 directly saves one fork per conflicting file.

Signed-off-by: Uwe Zeisberger <zeisberg@informatik.uni-freiburg.de>
---
 merge-recursive.c |   34 +++++++++++++++++++++++-----------
 1 files changed, 23 insertions(+), 11 deletions(-)

It passes `make test` when NO_SVN_TESTS is defined.  (I'll write a
separate mail about that.)

I didn't made any timing tests or further tests for correctness, but I
hope Johannes still has the framework from the time when he converted
the Python script to C?  

@Johannes: If so, could you test this patch?

Best regards
Uwe

diff --git a/merge-recursive.c b/merge-recursive.c
index 2ba43ae..9e3f9d7 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -657,8 +657,11 @@ static struct merge_file_info merge_file
 			char orig[PATH_MAX];
 			char src1[PATH_MAX];
 			char src2[PATH_MAX];
+			char tmppath[PATH_MAX];
+
 			const char *argv[] = {
-				"merge", "-L", NULL, "-L", NULL, "-L", NULL,
+				"diff3", "-E", "-am",
+				"-L", NULL, "-L", NULL, "-L", NULL,
 				NULL, NULL, NULL,
 				NULL
 			};
@@ -668,23 +671,31 @@ static struct merge_file_info merge_file
 			git_unpack_file(a->sha1, src1);
 			git_unpack_file(b->sha1, src2);
 
-			argv[2] = la = xstrdup(mkpath("%s/%s", branch1, a->path));
-			argv[6] = lb = xstrdup(mkpath("%s/%s", branch2, b->path));
-			argv[4] = lo = xstrdup(mkpath("orig/%s", o->path));
-			argv[7] = src1;
-			argv[8] = orig;
-			argv[9] = src2,
+			argv[4] = la = xstrdup(mkpath("%s/%s", branch1, a->path));
+			argv[8] = lb = xstrdup(mkpath("%s/%s", branch2, b->path));
+			argv[6] = lo = xstrdup(mkpath("orig/%s", o->path));
+			argv[9] = src1;
+			argv[10] = orig;
+			argv[11] = src2;
+
+			fd = git_mkstemp(tmppath, sizeof(tmppath),
+					".merge_file_XXXXXX");
+			if (fd < 0)
+				die("unable to create temp-file");
+
+			dup2(fd, ...
From: Johannes Schindelin
Date: Wednesday, October 18, 2006 - 2:38 am

Hi Uwe,


Interesting. I wonder if we could streamline the code such that index_fd 
is called directly on the output of diff3? Of course, the result has to be 

I have to dig a little where I have it, but I think I can give it a try in 
a few hours (imagine this lyrics to the melody of the day job blues).

Ciao,
Dscho

-

From: Uwe Zeisberger
Date: Friday, October 20, 2006 - 2:11 pm

I thought about that, too.  But my primary intention was to get rid of
'merge', because the Solaris boxes I use from time to time lack merge,
but have (GNU) diff3[1].  I already had a mental note to look into that.

If Linus is right that there are systems that have merge but lack diff3,
then a combined approach is maybe the best?  That is, try diff3 and if
that is missing, try merge.  (Or the other way round if you prefer.)

OK, I looked a bit deeper into rcs, and it seems to handle the BSD diff3
case.  So Linus might be right.

BTW, merge -p sends the merged result to stdout instead of overwriting
the first file given.  That is

	merge -p -L label1 -L label2 -L label3 file1 file2 file3

and (GNU)

	diff3 -E -am -L label1 -L label2 -L label3 file1 file2 file3

are exactly equivalent.
So if that option of merge is old enough, these are the candidates for
Seems to be a long blues because you didn't sent any results. :-(

Best regards
Uwe

[1] They also have a version of diff3 (I guess from BSD) that is not
suited to be used for merging, at least rcs' merge cannot use it.

-- 
Uwe Zeisberger

If a lawyer and an IRS agent were both drowning, and you could only save
one of them, would you go to lunch or read the paper?
-

From: Johannes Schindelin
Date: Friday, October 20, 2006 - 5:06 pm

Hi,


Yes. It was a long blues, and now instead of going to sleep I tested it. 
In the long run, it was negligible, with a high local variability (which 
stems from the fact that I had to run this test while the machine was 
under high load, which it will be for another 48 hours minimum).

It makes sense that performance-wise, it will not make a great difference. 
After all, the expensive operation is not the file-writing, but the 
merging pass.

Ciao,
Dscho

-

Previous thread: t9100-git-svn-basic.sh fails by Uwe Zeisberger on Wednesday, October 18, 2006 - 1:59 am. (3 messages)

Next thread: Re: [PATCH] Use diff3 instead of merge in merge-recursive. by Jakub Narebski on Wednesday, October 18, 2006 - 2:35 am. (4 messages)