Re: [PATCH 1/5] diff_tree_sha1: skip diff_tree if old == new

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 1, 2011, 22:28 UTC
Message-ID: <7vfwq1ehcq.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1301535481-1085-1-git-send-email-dpmcgee@gmail.com>

Dan McGee <dpmcgee@gmail.com> writes:

> This was seen to happen in some invocations of git-log with a filtered
> path. Only do it if we are not recursively descending, as otherwise we
> mess with copy and rename detection in full tree moves.

There is no code that corresponds to your "Only do it..." description in your patch, though. The existing code already takes care of that part with or without your patch, no?

Show 11 quoted lines

> diff --git a/tree-diff.c b/tree-diff.c
> index 76f83fc..ab90f1a 100644
> --- a/tree-diff.c
> +++ b/tree-diff.c
> @@ -286,6 +286,9 @@ int diff_tree_sha1(const unsigned char *old, const unsigned char *new, const cha
>  	unsigned long size1, size2;
>  	int retval;
>  
> +	if (!DIFF_OPT_TST(opt, FIND_COPIES_HARDER) && !hashcmp(old, new))
> +		return 0;
> +

I am very curious why this patch makes a difference; doesn't an existing test in compare_tree_entry() oalready cull extra recursion? There is:

	if (!DIFF_OPT_TST(opt, FIND_COPIES_HARDER) && !hashcmp(sha1, sha2) &&
		mode1 == mode2)
		return 0;

before a recursive call to diff_tree_sha1() to dig deeper.