git-diff-tree -B broken

git-diff-tree -B broken

[Petr Baudis]

  Hi,

  no pun intended in the $SUBJ, but git-diff-tree -B seems to be giving
me bogus output

	$ git-diff-tree -r -p -B -M 916d8240bfc8ef5b80780b1a32ebb83ed055a178 \
		3c9324ea16c345fc0bc0f11041905ba714797e4a cg-mkpatch
	diff --git a/cg-mkpatch b/cg-mkpatch
	dissimilarity index 57%
	index 74fe917..c932bc9 100755
	--- cg-mkpatch
	+++ cg-mkpatch
	@@ -0,0 +0,0 @@

and nothing else. I'm too tired to debug or bisect this (xdiff
introduction is the obvious suspect, I guess), just didn't want this
to slip into the looming 1.3.0 so reporting early.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Right now I am having amnesia and deja-vu at the same time.  I think
I have forgotten this before.

Re: git-diff-tree -B broken

[Junio C Hamano]

Petr Baudis <pasky@ucw.cz> writes:

> 	$ git-diff-tree -r -p -B -M 916d8240bfc8ef5b80780b1a32ebb83ed055a178 \
> 		3c9324ea16c345fc0bc0f11041905ba714797e4a cg-mkpatch
> 	diff --git a/cg-mkpatch b/cg-mkpatch
> 	dissimilarity index 57%
> 	index 74fe917..c932bc9 100755
> 	--- cg-mkpatch
> 	+++ cg-mkpatch
> 	@@ -0,0 +0,0 @@
>
> and nothing else. I'm too tired to debug or bisect this (xdiff
> introduction is the obvious suspect, I guess), just didn't want this
> to slip into the looming 1.3.0 so reporting early.

Thanks.

There are two issues.  One of them is addressed with the
attached patch, which is just an artifact of the xdiff
introduction.

Another issue is a bit bigger.

The changes to the sample cg-mkpatch is estimated to be big
enough to get broken, just in case if there is another file
whose postimage that is more similar than the postimage of
cg-mkpatch itself, so that it can be matched up with the other
file, but it is _not_ dissimilar enough that diffcore_break()
code marks it to be merged back together if pieces of the broken
pair do not get matched up with other files.  And in this
particular case, since you are limiting the search space to a
single file, there is no other files to match, so the broken
pair should survive the rename detection match-up, and they
should be shown as a normal diff without dissimilarity index at
all.

The funny thing is, the broken pair are matched up with
themselves, with similarity index of 57% (they originally are
from the same filepair, so that is natural).  So in this case,
rename code _should_ take notice.  I'll work on a fix.

-- >8 --
diff --git a/diff.c b/diff.c
index ce98a90..e887410 100644
--- a/diff.c
+++ b/diff.c
@@ -145,8 +145,9 @@ static void emit_rewrite_diff(const char
 			      struct diff_filespec *one, 
 			      struct diff_filespec *two)
 {
-	/* Use temp[i].name as input, name_a and name_b as labels */
 	int lc_a, lc_b;
+	diff_populate_filespec(one, 0);
+	diff_populate_filespec(two, 0);
 	lc_a = count_lines(one->data, one->size);
 	lc_b = count_lines(two->data, two->size);
 	printf("--- %s\n+++ %s\n@@ -", name_a, name_b);

[PATCH] diffcore-rename: fix merging back a broken pair.

[Junio C Hamano]

When a broken pair is matched up by rename detector to be merged
back, we do not want to say it is "dissimilar" with the
similarity index.  The output should just say they were changed,
taking the break score left by the earlier diffcore-break run if
any.

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

---

 diffcore-rename.c |   14 ++++++++++----
 1 files changed, 10 insertions(+), 4 deletions(-)

c17de73d793ff20fce6bcc8427e5f10ab8a2a7c5
diff --git a/diffcore-rename.c b/diffcore-rename.c
index e992698..d57e865 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -54,12 +54,14 @@ static struct diff_rename_dst *locate_re
 /* Table of rename/copy src files */
 static struct diff_rename_src {
 	struct diff_filespec *one;
+	unsigned short score; /* to remember the break score */
 	unsigned src_path_left : 1;
 } *rename_src;
 static int rename_src_nr, rename_src_alloc;
 
 static struct diff_rename_src *register_rename_src(struct diff_filespec *one,
-						   int src_path_left)
+						   int src_path_left,
+						   unsigned short score)
 {
 	int first, last;
 
@@ -89,6 +91,7 @@ static struct diff_rename_src *register_
 		memmove(rename_src + first + 1, rename_src + first,
 			(rename_src_nr - first - 1) * sizeof(*rename_src));
 	rename_src[first].one = one;
+	rename_src[first].score = score;
 	rename_src[first].src_path_left = src_path_left;
 	return &(rename_src[first]);
 }
@@ -198,7 +201,10 @@ static void record_rename_pair(int dst_i
 	fill_filespec(two, dst->sha1, dst->mode);
 
 	dp = diff_queue(NULL, one, two);
-	dp->score = score;
+	if (!strcmp(src->path, dst->path))
+		dp->score = rename_src[src_index].score;
+	else
+		dp->score = score;
 	dp->source_stays = rename_src[src_index].src_path_left;
 	rename_dst[dst_index].pair = dp;
 }
@@ -256,10 +262,10 @@ void diffcore_rename(struct diff_options
 			 * that means the source actually stays.
 			 */
 			int stays = (p->broken_pair && !p->score);
-			register_rename_src(p->one, stays);
+			register_rename_src(p->one, stays, p->score);
 		}
 		else if (detect_rename == DIFF_DETECT_COPY)
-			register_rename_src(p->one, 1);
+			register_rename_src(p->one, 1, p->score);
 	}
 	if (rename_dst_nr == 0 || rename_src_nr == 0 ||
 	    (0 < rename_limit && rename_limit < rename_dst_nr))
-- 
1.2.6.gad0b