Use diff3 instead of merge in merge-recursive.

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.

---
 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

index 2ba43ae..9e3f9d7 100644
@@ -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, 1);
+			close(fd);
 
-			code = run_command_v(10, argv);
+			code = run_command_v(12, argv);
 
 			free(la);
 			free(lb);
 			free(lo);
 			if (code && code < -256) {
-				die("Failed to execute 'merge'. merge(1) is used as the "
-				    "file-level merge tool. Is 'merge' in your path?");
+				die("Failed to execute 'diff3'. diff3(1) is used as the "
+				    "file-level merge tool. Is 'diff3' in your path?");
 			}
-			fd = open(src1, O_RDONLY);
+			fd = open(tmppath, O_RDONLY);
 			if (fd < 0 || fstat(fd, &st) < 0 ||
 					index_fd(result.sha, fd, &st, 1,
 						"blob"))

@@ -693,6 +704,7 @@ static struct merge_file_info merge_file
 			unlink(orig);
 			unlink(src1);
 			unlink(src2);
+			unlink(tmppath);
 
 			result.clean = WEXITSTATUS(code) == 0;
 		} else {

1.4.3.rc2

-- 
Uwe Zeisberger

http://www.google.com/search?q=1+newton+in+kg*m+%2F+s%5E2

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 removed when the call to diff3 fails.

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

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 the "combined approach" (see above).

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?

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

Doesn't xdiff library git uses have diff3/merge equivalent? Couldn't we use that instead (on less dependency, better performance)?

Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Hi,

We have only half of libxdiff, lacking the patch functionality. And AFAIUI the minimal implementation of diff3 in xdiff just uses a diff of the first pair on the third file (or some other order). So, it is not really a diff3/merge: it would fail in fairly trivial cases.

Ciao,
Dscho

P.S.: Davide, correct me if I'm wrong.

Nope. It has something it calls "merge", but it's really just "apply the diff from the common base to the other end".

IOW, if "a" is your common ancestor, and "b1" and "b2" are the branches, it's literally

diff a b1 | patch b2

and not actually a real 3-way merge.

As to why git uses "merge" - I have this strong memory of having seen machines that had one but not the other, and that, along with the fact that I've used "merge" personally, is why we call "merge" rather than diff3.

In Linux systems, "merge" usually comes with the RCS package, and "diff3" is usually from "diffutils". It may be that "diff3" is more common. I'm not sure what the history is, and what the situation would tend to be like on other systems..

Linus

AFAICT, "merge" actually execs "diff3", so if a system has only one, either it's diff3, it's a "merge" from somewhere other than current rcs, or it doesn't actually work.

-Daniel
*This .sig left intentionally blank*