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

Forgot to forward this to the list as well, I apologize.

On Fri, Apr 1, 2011 at 5:28 PM, Junio C Hamano [off-list ref] wrote:

Dan McGee [off-list ref] writes:

quoted

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?

Damn, I forgot to update the message- see below.

quoted

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:

This was originally testing RECURSIVE; however I discovered that was
not the culprit to my failed tests.

t9300-fastimport.sh was failing on "copy then modify subdirectory" due
to the full info not being loaded for the before sha1 in that test-
instead of showing the fcf778cda ... C100 part (this is just the first
line of expected, all were the same), it was 000000 ... A. once I
added the above fallthrough to not shortcut if this option was
enabled, things worked fine and all tests passed.

       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.

I'm not totally sure why this check wasn't working, but without the
above exception my patch definitely broke tests.

-Dan