Re: [PATCH] threeway_merge: if file will not be touched, leave it alone

Hi,

On Sun, 22 Oct 2006, Junio C Hamano wrote:

Complaining when no_anc_exists means that threeway_merge() is deciding 
that the merge result should have the path in this case.

Two points:

- you are correct for at least the case of choosing the merge strategy 
"theirs". (Which does not exist yet.)

- in merge-recursive.c:process_entry() (which is called on _all_ unmerged 
entries after threeway merge), "Case A" reads "deleted in one branch". 
Reading the code again, I believe there is a bug, which should be fixed by

diff --git a/merge-recursive.c b/merge-recursive.c
index 2ba43ae..9f6538a 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c

@@ -1005,9 +1005,10 @@ static int process_entry(const char *pat
 		    (!a_sha && sha_eq(b_sha, o_sha))) {
 			/* Deleted in both or deleted in one and
 			 * unchanged in the other */
-			if (a_sha)
+			if (!a_sha) {
 				output("Removing %s", path);
-			remove_file(1, path);
+				remove_file(1, path);
+			}
 		} else {
 			/* Deleted in one and changed in the other */
 			clean_merge = 0;

Note that not only it groups the call to output() and remove_file(), which 
matches the expectation, but also changes the condition to "!a_sha", 
meaning that the file is deleted in branch "a", but existed in the merge 
base, where it is identical to what is in branch "b".

Of course, this assumes that even in the recursive case, branch "a" is to 
be preferred over branch "b". (If I still remember correctly, then branch 
"a" is either the current head, or the temporary recursive merge, so this 
would make sense to me.)

So, after applying this patchlet, merge-recursive (more precisely: the 
function process_entry()) should behave correctly with the change to 
unpack-trees.c you have in pu, i.e. the change that drops that 
verify_absent() call to the floor.

However, I could use some additional optical lobes here.

Ciao,
Dscho

P.S.: Maybe I was wrong on my earlier assessment, that merge-recursive 
does not optimize the "subtrees have identical SHA1s" case. This should be 
handled pretty well by the call to unpack_trees() with threeway merge.