Re: "fatal: Untracked working tree file 'so-and-so' would be overwritten by merge"

From: Junio C Hamano <junkio@cox.net>
Date: Oct 9, 2006, 04:48 UTC
Message-ID: <7v7izaf62c.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <Pine.LNX.4.64.0610081657400.3952@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

Show 11 quoted lines

> Hmm. I'm getting this message annoyingly often, simply because a few files 
> that used to be tracked are now generated, and so they exist in my tree 
> but are no longer tracked.
>
> However, they may be tracked in an older tree that I pull, because in that 
> older tree they _do_ exist, and we get the
>
> 	fatal: Untracked working tree file 'so-and-so' would be overwritten by merge.
>
> which is actually incorrect, because the merge result will not even 
> _contain_ that untracked file any more.

Show 9 quoted lines

> So the message is misleading - we should only consider this a fatal thing 
> if we actually do generate that file as part of a git-read-tree, but if a 
> merge won't touch a file, it shouldn't be "overwritten".
>
> It's true that if the _other_ end actually removed a file that we used to 
> have (ie the file _disappears_ as part of the merge), then we should 
> verify that that file matched what we're going to remove, but if the old 
> index didn't contain the file at all, and the new index won't contain it 
> either, it really should be a no-op.

True.

I think it is verify_absent() on l.665 in threeway_merge().

	if (index) {
		verify_uptodate(index, o);
	}
	else if (path)
		verify_absent(path, "overwritten", o);

	o->nontrivial_merge = 1;

We say "we know this path is involved in the non-trivial merge; if the current index has it, it had better be up-to-date" (the first "if"). I think that up to that check is fine.

However, we say that otherwise, the path should not exist in the working tree; this should not be done unconditionally. As you say, the check should depend on the merge result.

But that is a bit tricky. This is not on the aggressive path, and the merge result is decided by the policy implemented by the caller of read-tree. So in that sense we should not be doing the working tree check ourselves either. We just should leave that to the caller.

Hence, I think removing the above "else if" part altogether is the right thing to do here.

---

diff --git a/unpack-trees.c b/unpack-trees.c
index 3ac0289..b1d78b8 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -661,8 +661,6 @@ int threeway_merge(struct cache_entry **
 	if (index) {
 		verify_uptodate(index, o);
 	}
-	else if (path)
-		verify_absent(path, "overwritten", o);
 
 	o->nontrivial_merge = 1;