Re: Broken adding of cache entries

From: Junio C Hamano <junkio@cox.net>
Date: May 8, 2005, 22:42 UTC
Message-ID: <7voebl2vcb.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <7v3bsx4aum.fsf@assigned-by-dhcp.cox.net>

>>>>> "JCH" == Junio C Hamano <junkio@cox.net> writes:

JCH> Ok, having thought about it a bit more, I think we can yank it JCH> out. I'd rather keep ourselves cautious, though; there may be JCH> some other ways we haven't thought of to create such nonsense, JCH> and it would not hurt to be cautious before writing a tree out.

Here is a revert you can apply after merging with the tip of my tree if you wanted to. As you pointed out correctly, three-way read-tree would not produce the situation as I feared, and the attempt to postprocess the result by git-merge-one-file-script git-merge-cache calls would be caught with the changes we made to git-update-cache, so there is no need for the extra check by write-tree in that workflow example. Sorry for being ultra dense.

The only reason why I am holding off from pushing this out is because I still cannot convince myself that we eradicated _all_ the possibile paths to create such nonsense in the index, and I feel safer to keep the check this patch is reverting, at least for now.

---
Author: Junio C Hamano <junkio@cox.net>
Date:   Sun May 8 15:34:17 2005 -0700
    
Revert the extra-careful check in write-tree.c

I am not particularly fond of reverting this, but with the git-update-cache changes it is unlikely that we will have file vs directory conflicts in the index anymore, so there may be no point in being extra careful when writing the tree out.

Signed-off-by: Junio C Hamano <junkio@cox.net>

--- a/write-tree.c
+++ b/write-tree.c
@@ -84,7 +84,7 @@ static int write_tree(struct cache_entry
 
 int main(int argc, char **argv)
 {
-	int i, funny;
+	int i, unmerged;
 	int entries = read_cache();
 	unsigned char sha1[20];
 
@@ -92,45 +92,18 @@ int main(int argc, char **argv)
 		die("write-tree: no cache contents to write");
 
 	/* Verify that the tree is merged */
-	funny = 0;
+	unmerged = 0;
 	for (i = 0; i < entries; i++) {
 		struct cache_entry *ce = active_cache[i];
 		if (ntohs(ce->ce_flags) & ~CE_NAMEMASK) {
-			if (10 < ++funny) {
+			if (++unmerged > 10) {
 				fprintf(stderr, "...\n");
 				break;
 			}
 			fprintf(stderr, "%s: unmerged (%s)\n", ce->name, sha1_to_hex(ce->sha1));
 		}
 	}
-	if (funny)
-		die("write-tree: not able to write tree");
-
-	/* Also verify that the cache does not have path and path/file
-	 * at the same time.  At this point we know the cache has only
-	 * stage 0 entries.
-	 */
-	funny = 0;
-	for (i = 0; i < entries - 1; i++) {
-		/* path/file always comes after path because of the way
-		 * the cache is sorted.  Also path can appear only once,
-		 * which means conflicting one would immediately follow.
-		 */
-		const char *this_name = active_cache[i]->name;
-		const char *next_name = active_cache[i+1]->name;
-		int this_len = strlen(this_name);
-		if (this_len < strlen(next_name) &&
-		    strncmp(this_name, next_name, this_len) == 0 &&
-		    next_name[this_len] == '/') {
-			if (10 < ++funny) {
-				fprintf(stderr, "...\n");
-				break;
-			}
-			fprintf(stderr, "You have both %s and %s\n",
-				this_name, next_name);
-		}
-	}
-	if (funny)
+	if (unmerged)
 		die("write-tree: not able to write tree");
 
 	/* Ok, write it out */