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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]