Re: "git diff-index" broken in 'next'

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Mar 11, 2008, 03:20 UTC
Message-ID: <alpine.LFD.1.00.0803101958350.5896@woody.linux-foundation.org>
In-Reply-To: <7vk5kagexu.fsf_-_@gitster.siamese.dyndns.org>

On Mon, 10 Mar 2008, Junio C Hamano wrote:

> 
> I think it is somewhere around ll. 280 in unpack_callback() that causes
> this, but I don't have time to dig this further for now.

Yes indeed. I was confused. I even knew I was confused about the whole difference between "skip_unmerged" and not skipping unmerged entries in the tree unpacking, but nothing seemed to care, so I was lazy.

Now that I remember what the heck that oneway_diff thing wanted, it all makes sense to me again.

This should fix it.

That oneway_diff() thing is magic, and used to have a "count_skip()" function to skip over unmerged entries. In my confusion, I had instead skipped them in the generic unpack-trees.c code, and thus removed the skipping from oneway_diff().

That wasn't correct - we should pass the unmerged entries down and let the callback handle it.

[ Side note: nobody else than the diff code should ever have them in the 
  source index anyway - that would be a bug, since we cannot merge with a 
  source index that has unmerged entries ]

Sorry about that. Holler if you notice anythign else.

		Linus

---
 diff-lib.c     |   18 ++++++++++++++++++
 unpack-trees.c |    2 --
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/diff-lib.c b/diff-lib.c
index 9520773..52dbac3 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -641,6 +641,21 @@ static void do_oneway_diff(struct unpack_trees_options *o,
 	show_modified(revs, tree, idx, 1, cached, match_missing);
 }
 
+static inline void skip_same_name(struct cache_entry *ce, struct unpack_trees_options *o)
+{
+	int len = ce_namelen(ce);
+	const struct index_state *index = o->src_index;
+
+	while (o->pos < index->cache_nr) {
+		struct cache_entry *next = index->cache[o->pos];
+		if (len != ce_namelen(next))
+			break;
+		if (memcmp(ce->name, next->name, len))
+			break;
+		o->pos++;
+	}
+}
+
 /*
  * The unpack_trees() interface is designed for merging, so
  * the different source entries are designed primarily for
@@ -662,6 +677,9 @@ static int oneway_diff(struct cache_entry **src, struct unpack_trees_options *o)
 	struct cache_entry *tree = src[1];
 	struct rev_info *revs = o->unpack_data;
 
+	if (idx && ce_stage(idx))
+		skip_same_name(idx, o);
+
 	/*
 	 * Unpack-trees generates a DF/conflict entry if
 	 * there was a directory in the index and a tree
diff --git a/unpack-trees.c b/unpack-trees.c
index da68557..7a30361 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -116,7 +116,6 @@ static int unpack_index_entry(struct cache_entry *ce, struct unpack_trees_option
 			add_entry(o, ce, 0, 0);
 			return 0;
 		}
-		return 0;
 	}
 	return call_unpack_fn(src, o);
 }
@@ -287,7 +286,6 @@ static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str
 						add_entry(o, ce, 0, 0);
 						return mask;
 					}
-					continue;
 				}
 				src[0] = ce;
 			}