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

[Junio C Hamano <junkio@cox.net>]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

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

I think the conditional "output" is to mimic the first case in
git-merge-one-file; there we conditionally give that message
only when ours had that path.  If we lost the path while they
have it the same way as the common ancestor, then we do not have
the path to begin with when we start the merge.  It is not
correct to say "Removing" in such a case.

So the output() call being tied to if (a_sha) _is_ correct in
your code.

What we would want to prevent is to remove the path from the
working tree when we did not have the path at the beginning of
the merge and the merge result says we do not want that path.
In such a case, the file in the working tree is an untracked
file that is not touched by the merge.

E.g gitweb/gitweb.cgi is not tracked in the current "master",
but used to be around v1.4.0 time.  If you try to merge a
branch forked from v1.4.0 because you are interested in a work
on other part of the system (i.e. the branch did not touch
gitweb/ at all), we want to successfully merge that branch into
our "master" even after "make" created gitweb/gitweb.cgi.

Such a merge would start with your HEAD and index missing
gitweb/gitweb.cgi but the path still in your working tree.  The
common ancestor and their tree has the path tracked, so you
would end up with identical stage #1 and #3 with missing stage
#2.

The merge machinery should say the merge result does not have
the path, so it should remove it from the index.  However, it
should _not_ touch the untracked (from the beginning of the time
the merge started) working tree file.  So remove_file() call you
touch in your patch needs to be told not to update working
directory in such a case.

Under "aggressive" rule, threeway_merge() is requested to make
the merge policy decision, so it should also loosen this check
itself.  The change by commit 0b35995 needs to be updated with
this patch:

diff --git a/unpack-trees.c b/unpack-trees.c
index b1d78b8..7cfd628 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -642,7 +642,7 @@ int threeway_merge(struct cache_entry **
 		    (remote_deleted && head && head_match)) {
 			if (index)
 				return deleted_entry(index, index, o);
-			else if (path)
+			else if (path && !head_deleted)
 				verify_absent(path, "removed", o);
 			return 0;
 		}