Re: [RFC] diff-cache buglet
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:41:54
quoted
quoted
quoted
quoted
"LT" == Linus Torvalds [off-list ref] writes:
LT> On Tue, 26 Apr 2005, Junio C Hamano wrote:
quoted
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 <redacted> --- 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;