Re: bug in read-tree -m on A -> A/A

From: Junio C Hamano <junkio@cox.net>
Date: Mar 16, 2007, 05:01 UTC
Message-ID: <7v4polzsrn.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <20070316041932.GA31606@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> ....  The following test appears to
> trigger the same failure,...

You have file "A" on one branch, and file "A/A" on another branch. You are on the latter branch and switching to the former one.

The following patch illustrates where you need to implement an alternate, loosened check, but should not be applied to your tree as-is. If you have local modification to path "A/A", this will lose it.

-- >8 --

diff --git a/unpack-trees.c b/unpack-trees.c
index 2e2232c..345b2ee 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -479,9 +479,27 @@ static void verify_absent(const char *path, const char *action,
 
 	if (o->index_only || o->reset || !o->update)
 		return;
-	if (!lstat(path, &st) && !(o->dir && excluded(o->dir, path)))
+
+	if (!lstat(path, &st)) {
+		if (o->dir && excluded(o->dir, path))
+			/*
+			 * path is explicitly excluded, so it is Ok to
+			 * overwrite it.
+			 */
+			return;
+		if (S_ISDIR(st.st_mode))
+			/*
+			 * We are checking out path "foo" and
+			 * found "foo/." in the working tree.
+			 * This is tricky -- if we have modified
+			 * files that are in "foo/" we would lose
+			 * it if we just uncoditinally return here.
+			 */
+			return;
+
 		die("Untracked working tree file '%s' "
 		    "would be %s by merge.", path, action);
+	}
 }
 
 static int merged_entry(struct cache_entry *merge, struct cache_entry *old,


-- 8< --

To solve this sanely I think you would need to merge bottom-up,
which is quite a large change to read-tree.  Currently the code
asks "is it Ok to extract A from the new tree?" and if we say
yes here it would remove all existing cache entries "A/*" and
replaces it with "A".  After that happens, you would not even
have a chance to see if A/A has local modifications, or if you
have a local file that is not even known to git, say A/C, that
will also be lost by this tree switching.