Re: [PATCH v2 0/25] prune-safety

[Jeff King <peff@peff.net>]

On Thu, Oct 16, 2014 at 05:21:12PM -0400, Jeff King wrote:

> Hrm. I cannot reproduce the clone failure...

Oh, because I have bitmaps turned on, and we do not run the list-objects
code at all in that case.

The bug unsurprisingly bisects to "traverse_commit_list: support
pending blobs/trees with paths". The problem is that I didn't notice
that handle_commit actually rewrites the "object" pointer when
unwrapping the tags, and that commit reuses the original pointer from
the entry. So we end up putting two copies of the tag object into the
pending list, rather than the tag and the blob.

The fix is:

diff --git a/revision.c b/revision.c
index 9a0f99a..8030fc8 100644
--- a/revision.c
+++ b/revision.c
@@ -231,12 +231,6 @@ static void add_pending_object_with_mode(struct rev_info *revs,
 	add_pending_object_with_path(revs, obj, name, mode, NULL);
 }
 
-static void add_pending_object_from_entry(struct rev_info *revs,
-					  struct object_array_entry *e)
-{
-	add_pending_object_with_path(revs, e->item, e->name, e->mode, e->path);
-}
-
 void add_pending_object(struct rev_info *revs,
 			struct object *obj, const char *name)
 {
@@ -283,6 +277,8 @@ static struct commit *handle_commit(struct rev_info *revs,
 {
 	struct object *object = entry->item;
 	const char *name = entry->name;
+	const char *path = entry->path;
+	unsigned int mode = entry->mode;
 	unsigned long flags = object->flags;
 
 	/*
@@ -301,6 +297,14 @@ static struct commit *handle_commit(struct rev_info *revs,
 			die("bad object %s", sha1_to_hex(tag->tagged->sha1));
 		}
 		object->flags |= flags;
+		/*
+		 * We'll handle the tagged object by looping or dropping
+		 * through to the non-tag handlers below. Do not
+		 * propagate data from the tag's pending entry.
+		 */
+		name = NULL;
+		path = NULL;
+		mode = 0;
 	}
 
 	/*
@@ -332,7 +336,7 @@ static struct commit *handle_commit(struct rev_info *revs,
 			mark_tree_contents_uninteresting(tree);
 			return NULL;
 		}
-		add_pending_object_from_entry(revs, entry);
+		add_pending_object_with_path(revs, object, name, mode, path);
 		return NULL;
 	}
 
@@ -344,7 +348,7 @@ static struct commit *handle_commit(struct rev_info *revs,
 			return NULL;
 		if (flags & UNINTERESTING)
 			return NULL;
-		add_pending_object_from_entry(revs, entry);
+		add_pending_object_with_path(revs, object, name, mode, path);
 		return NULL;
 	}
 	die("%s is unknown object", name);

We should probably add a test for cloning a tag of an otherwise
unreferenced object, too.

-Peff