From: Dan McGee <dpmcgee@gmail.com>
To: git@vger.kernel.org
Subject: Fwd: [PATCH 1/5] diff_tree_sha1: skip diff_tree if old == new
Date: Sat, 2 Apr 2011 13:38:21 -0500 [thread overview]
Message-ID: <BANLkTi=hJm4ax__5DDCvK9VdLcNxVO2bVA@mail.gmail.com> (raw)
In-Reply-To: <AANLkTinPSqDPdGi5nA3sH1D2wMSW1SQc+5gRqdLy++y0@mail.gmail.com>
Forgot to forward this to the list as well, I apologize.
On Fri, Apr 1, 2011 at 5:28 PM, Junio C Hamano <gitster@pobox.com> wrote:
> 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?
Damn, I forgot to update the message- see below.
>> 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
next prev parent reply other threads:[~2011-04-02 18:38 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-03-31 1:37 [PATCH 1/5] diff_tree_sha1: skip diff_tree if old == new Dan McGee
2011-03-31 1:37 ` [PATCH 2/5] tree-walk: drop unused parameter from match_dir_prefix Dan McGee
2011-08-30 18:55 ` Dan McGee
2011-03-31 1:37 ` [PATCH 3/5] tree-walk: micro-optimization in tree_entry_interesting Dan McGee
2011-04-03 4:01 ` Nguyen Thai Ngoc Duy
2011-04-03 18:55 ` Junio C Hamano
2011-04-05 0:22 ` Dan McGee
[not found] ` <CAEik5nOKrpFycZYVnSu4_5LYWxn0JS_hVXyiQH-80Bu-C4k8VQ@mail.gmail.com>
2011-08-30 19:51 ` Dan McGee
2011-08-30 20:40 ` Junio C Hamano
2011-09-09 2:02 ` [PATCH 1/2] tree-walk: drop unused parameter from match_dir_prefix Dan McGee
2011-09-09 2:02 ` [PATCH 2/2] tree-walk: micro-optimization in tree_entry_interesting Dan McGee
2011-04-04 14:46 ` [PATCH] tree_entry_interesting: inline strncmp() Nguyễn Thái Ngọc Duy
2011-03-31 1:38 ` [PATCH 4/5] tree-walk: unroll get_mode since loop boundaries are well-known Dan McGee
2011-04-02 9:28 ` Nguyen Thai Ngoc Duy
2011-04-02 17:28 ` Dan McGee
2011-04-03 4:07 ` Nguyen Thai Ngoc Duy
2011-04-04 10:29 ` Erik Faye-Lund
2011-04-04 12:30 ` Andreas Ericsson
2011-04-04 16:55 ` Junio C Hamano
2011-04-05 5:33 ` Dan McGee
2011-04-05 23:55 ` Antriksh Pany
2011-04-06 20:45 ` Dan McGee
2011-03-31 1:38 ` [PATCH 5/5] tree-walk: match_entry microoptimization Dan McGee
2011-04-02 9:06 ` Nguyen Thai Ngoc Duy
2011-04-02 17:54 ` Dan McGee
2011-03-31 12:58 ` [PATCH 1/5] diff_tree_sha1: skip diff_tree if old == new Nguyen Thai Ngoc Duy
2011-03-31 13:56 ` Dan McGee
2011-04-01 22:28 ` Junio C Hamano
[not found] ` <AANLkTinPSqDPdGi5nA3sH1D2wMSW1SQc+5gRqdLy++y0@mail.gmail.com>
2011-04-02 18:38 ` Dan McGee [this message]
2011-05-03 7:34 ` Nguyen Thai Ngoc Duy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='BANLkTi=hJm4ax__5DDCvK9VdLcNxVO2bVA@mail.gmail.com' \
--to=dpmcgee@gmail.com \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).