Junio C Hamano <junkio@xxxxxxx> wrote:
> Shawn Pearce <spearce@xxxxxxxxxxx> writes:
>
> > But looking at is_exact_match I'm now wondering why we bother
> > to open the working tree file at all here.
>
> You are right. When we _know_ both SHA-1 can be trusted, we
> should be able to do just this, regardless of contents_too
> (whose only purpose is to delay comparing a cache dirty working
> tree file with something else):
>
> diff --git a/diffcore-rename.c b/diffcore-rename.c
> index 57a74b6..677db85 100644
> --- a/diffcore-rename.c
> +++ b/diffcore-rename.c
> @@ -100,9 +100,8 @@ static int is_exact_match(struct diff_filespec *src,
> struct diff_filespec *dst,
> int contents_too)
> {
> - if (src->sha1_valid && dst->sha1_valid &&
> - !hashcmp(src->sha1, dst->sha1))
> - return 1;
> + if (src->sha1_valid && dst->sha1_valid)
> + return !hashcmp(src->sha1, dst->sha1);
> if (!contents_too)
> return 0;
> if (diff_populate_filespec(src, 1) || diff_populate_filespec(dst, 1))
>
OK, I've stared at this problem for hours. The above patch breaks
a number of test cases (such as ./t4005-diff-rename-2.sh) as it
causes the rename/copy detection to come out differently.
It turns out the difference is not filling in the size field of
struct diff_filespec. By bypassing that step in is_exact_match
during the second pass we are altering the outcome.
--
Shawn.
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html