Re: [PATCH] Also use unpack_trees() in do_diff_cache()

!MAILaRCHIVE_VOTE_RePLACE
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
To: Johannes Schindelin <Johannes.Schindelin@...>
Cc: Steffen Prohaska <prohaska@...>, Junio C Hamano <gitster@...>, Git Mailing List <git@...>
Date: Sunday, January 20, 2008 - 4:32 pm

On Sun, 20 Jan 2008, Johannes Schindelin wrote:

Sadly, this doesn't work yet.

diiff_cache() does magic things with different stages. 

The new unpack_trees() model *should* make it possible to do an even 
better job at this (we should really approximate the diff_files() 
behaviour), but oneway_diff() currently doesn't do it right.

Try

	git diff-index [-p] [--cached] -m HEAD

in a tree with unmerged entries (or just "git diff HEAD", which does the 
same thing).

The following patch gets us closer, but really only for the "--cached" 
case (for the non-cached case, it should use the working tree entry ratehr 
than generate a unmerged entry), and I also suspect there's a case it gets 
wrong for the case of the unmerged path not existign in the tree at all 
(the "if (tree)" entry basically ends up being a stand-in for the "is this 
the first index entry for this path we see" case).

The really nice thing would be to do what "diff_files()" does for this 
all, something that the old model couldn't do at all (that's why it 
currently prints out that "* Unmerged path xyzzy" thing - the *correct* 
thing to do would be to generate the same combined diff that diff_files() 
does).

So some more work is needed - either to get back the old (arguably 
quite broken) behaviour, or to go the whole way and do it right.

Anyway, my point with this all is that even without any of this set of 
fixes, git doesn't do this optimally. Try this with the current stable git 
master:

	git diff --cached HEAD

with a unmerged file, and compare it to the much more useful "git diff" 
output. But right now, this patch series makes this case behave even 
worse, I think.

		Linus

---
 diff-lib.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/diff-lib.c b/diff-lib.c
index 23d0fa6..3ac4d1f 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -685,6 +685,12 @@ static int oneway_diff(struct cache_entry **src,
 	if (tree == o->df_conflict_entry)
 		tree = NULL;
 
+	if (ce_stage(idx)) {
+		if (tree)
+			diff_unmerge(&revs->diffopt, idx->name, idx->ce_mode, idx->sha1);
+		return 1;
+	}
+
 	/*
 	 * Something added to the tree?
 	 */
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
Updated in-memory index cleanup, Linus Torvalds, (Fri Jan 18, 11:25 pm)
[PATCH] Avoid running lstat(2) on the same cache entry., Junio C Hamano, (Sat Jan 19, 3:45 am)
Re: [PATCH] Avoid running lstat(2) on the same cache entry., Johannes Schindelin, (Sat Jan 19, 10:42 pm)
Re: [PATCH] Avoid running lstat(2) on the same cache entry., Johannes Schindelin, (Sat Jan 19, 9:48 pm)
Re: [PATCH] Avoid running lstat(2) on the same cache entry., Steffen Prohaska, (Sun Jan 20, 11:10 am)
Re: [PATCH] Avoid running lstat(2) on the same cache entry., Steffen Prohaska, (Sun Jan 20, 5:40 pm)
Re: [PATCH] Avoid running lstat(2) on the same cache entry., Johannes Schindelin, (Sun Jan 20, 11:18 am)
[PATCH] Try to resurrect the handling for 'diff-index -m', Johannes Schindelin, (Sun Jan 20, 11:21 am)
[PATCH] Also use unpack_trees() in do_diff_cache(), Johannes Schindelin, (Sun Jan 20, 11:19 am)
Re: [PATCH] Also use unpack_trees() in do_diff_cache(), Linus Torvalds, (Sun Jan 20, 4:32 pm)
Re: [PATCH] Also use unpack_trees() in do_diff_cache(), Linus Torvalds, (Sun Jan 20, 5:53 pm)
Re: [PATCH] Also use unpack_trees() in do_diff_cache(), Johannes Schindelin, (Sun Jan 20, 7:34 pm)
Re: [PATCH] Also use unpack_trees() in do_diff_cache(), Linus Torvalds, (Sun Jan 20, 7:58 pm)
Re: [PATCH] Also use unpack_trees() in do_diff_cache(), Johannes Schindelin, (Sun Jan 20, 8:19 pm)
Re: [PATCH] Avoid running lstat(2) on the same cache entry., Steffen Prohaska, (Sun Jan 20, 6:33 am)
Re: [PATCH] Avoid running lstat(2) on the same cache entry., Johannes Schindelin, (Sun Jan 20, 10:15 am)
Re: [PATCH] Avoid running lstat(2) on the same cache entry., Linus Torvalds, (Sat Jan 19, 10:02 pm)
[PATCH] index: be careful when handling long names, Junio C Hamano, (Sat Jan 19, 3:42 am)