Re: [RFC] diff-cache buglet

[Junio C Hamano <junkio@cox.net>]

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> On Tue, 26 Apr 2005, Junio C Hamano wrote:
>> 
>> We should just fix "remove-merge-entries" and call that
>> unconditionally before the read-tree is called.  Once it is
>> fixed, we need to think about how to show this stage
>> information but that should be a separate discussion.

LT> I just thought that _if_ you wanted the unmerged parts to show up, then 
LT> the "1//filename.c" thing might be acceptable. Personally, I just think 
LT> diff-cache is pretty nonsensical with unmerged files,

I agree to what you said here.  We are not interested in the
unmerged files in the original GIT_INDEX_FILE at all.

However, remember that "unmerged" entries diff_cache() function
sees are from the tree you are comparing against, either your
GIT_INDEX_FILE (with --cached) or your working tree (without).

Currently I suspect the behaviour of diff-cache without --cached
flag may be broken.  Don't we need to check cached_only before
or inside of the first two if() statements in diff_cache()?  For
a path that appears in the tree you are comparing against (i.e.
stage 1 entries):

 - if GIT_INDEX_FILE does not have it but the working tree does,
   it would still say "deleted".

 - if GIT_INDEX_FILE does have it, the comparison goes against
   that entry, not against the working tree.

Similarly for entries that are not in the stage 1, the code ends
up comparing only the dircache entries and never goes to the
filesystem.

Here is a proposed fix.  When running without --cached,
diff_cache function really goes to the filesystem if the stage 0
entry in GIT_INDEX_FILE does not match what is in the working
tree for these cases.

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

diff-cache.c |   40 ++++++++++++++++++++++++++++++++++++----
1 files changed, 36 insertions(+), 4 deletions(-)

./jit-snap -v 3:6
# - 04/26 12:25 Fix agreed with Linus.
# + 04/26 13:26 Proposed fix.
--- k/diff-cache.c
+++ l/diff-cache.c
@@ -10,6 +10,19 @@ static void show_file(const char *prefix
 	       sha1_to_hex(ce->sha1), ce->name, line_termination);
 }
 
+/* A file *may* have been added to the working tree */
+static void show_possible_local_add(struct cache_entry *new)
+{
+	static unsigned char no_sha1[20];
+	struct stat st;
+	if (stat(new->name, &st) < 0)
+		return; /* no, working tree does not have one. */
+	if (cache_match_stat(new, &st))
+		return show_file("+", new);
+
+	printf("+%o\t%s\t%s\t%s%c", st.st_mode, "blob",
+	       sha1_to_hex(no_sha1), new->name, line_termination);
+}
 static int show_modified(struct cache_entry *old, struct cache_entry *new)
 {
 	unsigned int mode = ntohl(new->ce_mode), oldmode;
@@ -29,6 +42,8 @@ static int show_modified(struct cache_en
 			mode = st.st_mode;
 			sha1 = no_sha1;
 		}
+		else if (old == new)
+			return 0;
 	}
 
 	oldmode = ntohl(old->ce_mode);
@@ -48,16 +63,33 @@ static int diff_cache(struct cache_entry
 	while (entries) {
 		struct cache_entry *ce = *ac;
 
-		/* No matching 0-stage (current) entry? Show it as deleted */
+		/* No matching 0-stage (current) entry?
+		 * Show it as deleted.
+		 */
 		if (ce_stage(ce)) {
-			show_file("-", ce);
+			/* ... well, not so fast.  We may have it in the
+			 * working tree and operating without --cache.
+			 */
+			if (cached_only)
+				show_file("-", ce);
+			else
+				/* this is sneaky but it works.  trust me. */
+				show_modified(ce, ce);
 			ac++;
 			entries--;
 			continue;
 		}
-		/* No matching 1-stage (tree) entry? Show the current one as added */
+		/* No matching 1-stage (tree) entry?
+		 * Show the current one as added.
+		 */
 		if (entries == 1 || !same_name(ce, ac[1])) {
-			show_file("+", ce);
+			/* ... again, we may not have that in the
+			 * working tree and operating without --cache.
+			 */
+			if (cached_only)
+				show_file("+", ce);
+			else
+				show_possible_local_add(ce);
 			ac++;
 			entries--;
 			continue;