{"thread":{"id":"23564","subject":"[PATCH 1/2 v3] Make diffcore_std only can run once before a diff_flush.","startedAt":"2010-04-22T14:05:40Z","lastAt":"2010-04-28T03:37:00Z","messageCount":6,"participants":["Bo Yang","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":2},"messages":[{"id":"140130","messageId":"1271945142-27015-1-git-send-email-struggleyb.nku@gmail.com","threadId":"23564","inReplyTo":null,"subject":"[PATCH 0/2 v3] Make git log --follow find copies among unmodified files.","fromName":"Bo Yang","fromEmail":"struggleyb.nku@gmail.com","sentAt":"2010-04-22T14:05:40Z","receivedAt":"2010-04-22T14:05:40Z","isPatch":true,"sender":{"key":"struggleyb.nku@gmail.com","avatar":"https://avatars.githubusercontent.com/u/233030?v=4"},"body":"I have tried to make --follow to support finding copies among unmodified files. And the first patch is to fix a bug introduced by '--follow' and 'git log' combination.\nWe use the code:\n\n    else if (--p->one->rename_used > 0)\n        p->status = DIFF_STATUS_COPIED;\n\nto detect copies and renames. So, if diffcore_std run more than one time, p->one->rename_used will be reduced to a 'R' from 'C'. And this patch will fix this by allowing diffcore_std can only run once before a diff_flush, which seems rationale for our code.\n\nBo Yang (2):\n  Make diffcore_std only can run once before a diff_flush.\n  Make git log --follow find copies among unmodified files.\n\n Documentation/git-log.txt           |    2 +-\n diff.c                              |   21 ++++++++-----\n diffcore-break.c                    |    6 +--\n diffcore-pickaxe.c                  |    3 +-\n diffcore-rename.c                   |    3 +-\n diffcore.h                          |    6 ++++\n t/t4205-log-follow-harder-copies.sh |   56 +++++++++++++++++++++++++++++++++++\n tree-diff.c                         |    2 +-\n 8 files changed, 81 insertions(+), 18 deletions(-)\n create mode 100755 t/t4205-log-follow-harder-copies.sh\n"},{"id":"140129","messageId":"1271945142-27015-2-git-send-email-struggleyb.nku@gmail.com","threadId":"23564","inReplyTo":"1271945142-27015-1-git-send-email-struggleyb.nku@gmail.com","subject":"[PATCH 1/2 v3] Make diffcore_std only can run once before a diff_flush.","fromName":"Bo Yang","fromEmail":"struggleyb.nku@gmail.com","sentAt":"2010-04-22T14:05:41Z","receivedAt":"2010-04-22T14:05:41Z","isPatch":true,"sender":{"key":"struggleyb.nku@gmail.com","avatar":"https://avatars.githubusercontent.com/u/233030?v=4"},"body":"When file renames/copies detection is turned on, the\nsecond diffcore_std will degrade a 'C' pair to a 'R' pair.\n\nAnd this may happen when we run 'git log --follow' with\nhard copies finding. That is, the try_to_follow_renames()\nwill run diffcore_std to find the copies, and then\n'git log' will issue another diffcore_std, which will reduce\n'src->rename_used' and recognize this copy as a rename.\nThis is not what we want.\n\nSo, I think we really don't need to run diffcore_std more\nthan one time.\n\nSigned-off-by: Bo Yang <struggleyb.nku@gmail.com>\n---\n diff.c             |   21 +++++++++++++--------\n diffcore-break.c   |    6 ++----\n diffcore-pickaxe.c |    3 +--\n diffcore-rename.c  |    3 +--\n diffcore.h         |    6 ++++++\n 5 files changed, 23 insertions(+), 16 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex d0ecbc3..d32fc68 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2544,6 +2544,7 @@ static void run_checkdiff(struct diff_filepair *p, struct diff_options *o)\n void diff_setup(struct diff_options *options)\n {\n \tmemset(options, 0, sizeof(*options));\n+\tmemset(&diff_queued_diff, 0, sizeof(diff_queued_diff));\n \n \toptions->file = stdout;\n \n@@ -3462,8 +3463,7 @@ int diff_flush_patch_id(struct diff_options *options, unsigned char *sha1)\n \t\tdiff_free_filepair(q->queue[i]);\n \n \tfree(q->queue);\n-\tq->queue = NULL;\n-\tq->nr = q->alloc = 0;\n+\tDIFF_QUEUE_CLEAR(q);\n \n \treturn result;\n }\n@@ -3591,8 +3591,7 @@ void diff_flush(struct diff_options *options)\n \t\tdiff_free_filepair(q->queue[i]);\n free_queue:\n \tfree(q->queue);\n-\tq->queue = NULL;\n-\tq->nr = q->alloc = 0;\n+\tDIFF_QUEUE_CLEAR(q);\n \tif (options->close_file)\n \t\tfclose(options->file);\n \n@@ -3614,8 +3613,7 @@ static void diffcore_apply_filter(const char *filter)\n \tint i;\n \tstruct diff_queue_struct *q = &diff_queued_diff;\n \tstruct diff_queue_struct outq;\n-\toutq.queue = NULL;\n-\toutq.nr = outq.alloc = 0;\n+\tDIFF_QUEUE_CLEAR(&outq);\n \n \tif (!filter)\n \t\treturn;\n@@ -3683,8 +3681,7 @@ static void diffcore_skip_stat_unmatch(struct diff_options *diffopt)\n \tint i;\n \tstruct diff_queue_struct *q = &diff_queued_diff;\n \tstruct diff_queue_struct outq;\n-\toutq.queue = NULL;\n-\toutq.nr = outq.alloc = 0;\n+\tDIFF_QUEUE_CLEAR(&outq);\n \n \tfor (i = 0; i < q->nr; i++) {\n \t\tstruct diff_filepair *p = q->queue[i];\n@@ -3745,6 +3742,12 @@ void diffcore_fix_diff_index(struct diff_options *options)\n \n void diffcore_std(struct diff_options *options)\n {\n+\t/* We never run this function more than one time, because the\n+\t * rename/copy detection logic can only run once.\n+\t */\n+\tif (diff_queued_diff.run)\n+\t\treturn;\n+\n \tif (options->skip_stat_unmatch)\n \t\tdiffcore_skip_stat_unmatch(options);\n \tif (options->break_opt != -1)\n@@ -3764,6 +3767,8 @@ void diffcore_std(struct diff_options *options)\n \t\tDIFF_OPT_SET(options, HAS_CHANGES);\n \telse\n \t\tDIFF_OPT_CLR(options, HAS_CHANGES);\n+\n+\tdiff_queued_diff.run = 1;\n }\n \n int diff_result_code(struct diff_options *opt, int status)\ndiff --git a/diffcore-break.c b/diffcore-break.c\nindex 3a7b60a..44f8678 100644\n--- a/diffcore-break.c\n+++ b/diffcore-break.c\n@@ -162,8 +162,7 @@ void diffcore_break(int break_score)\n \tif (!merge_score)\n \t\tmerge_score = DEFAULT_MERGE_SCORE;\n \n-\toutq.nr = outq.alloc = 0;\n-\toutq.queue = NULL;\n+\tDIFF_QUEUE_CLEAR(&outq);\n \n \tfor (i = 0; i < q->nr; i++) {\n \t\tstruct diff_filepair *p = q->queue[i];\n@@ -256,8 +255,7 @@ void diffcore_merge_broken(void)\n \tstruct diff_queue_struct outq;\n \tint i, j;\n \n-\toutq.nr = outq.alloc = 0;\n-\toutq.queue = NULL;\n+\tDIFF_QUEUE_CLEAR(&outq);\n \n \tfor (i = 0; i < q->nr; i++) {\n \t\tstruct diff_filepair *p = q->queue[i];\ndiff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c\nindex d0ef839..929de15 100644\n--- a/diffcore-pickaxe.c\n+++ b/diffcore-pickaxe.c\n@@ -55,8 +55,7 @@ void diffcore_pickaxe(const char *needle, int opts)\n \tint i, has_changes;\n \tregex_t regex, *regexp = NULL;\n \tstruct diff_queue_struct outq;\n-\toutq.queue = NULL;\n-\toutq.nr = outq.alloc = 0;\n+\tDIFF_QUEUE_CLEAR(&outq);\n \n \tif (opts & DIFF_PICKAXE_REGEX) {\n \t\tint err;\ndiff --git a/diffcore-rename.c b/diffcore-rename.c\nindex d6fd3ca..df41be5 100644\n--- a/diffcore-rename.c\n+++ b/diffcore-rename.c\n@@ -569,8 +569,7 @@ void diffcore_rename(struct diff_options *options)\n \t/* At this point, we have found some renames and copies and they\n \t * are recorded in rename_dst.  The original list is still in *q.\n \t */\n-\toutq.queue = NULL;\n-\toutq.nr = outq.alloc = 0;\n+\tDIFF_QUEUE_CLEAR(&outq);\n \tfor (i = 0; i < q->nr; i++) {\n \t\tstruct diff_filepair *p = q->queue[i];\n \t\tstruct diff_filepair *pair_to_free = NULL;\ndiff --git a/diffcore.h b/diffcore.h\nindex fcd00bf..e77db31 100644\n--- a/diffcore.h\n+++ b/diffcore.h\n@@ -91,7 +91,13 @@ struct diff_queue_struct {\n \tstruct diff_filepair **queue;\n \tint alloc;\n \tint nr;\n+\tint run;\n };\n+#define DIFF_QUEUE_CLEAR(q) \\\n+\tdo { \\\n+\t\t(q)->queue = NULL; \\\n+\t\t(q)->nr = (q)->alloc = (q)->run = 0; \\\n+\t} while(0);\n \n extern struct diff_queue_struct diff_queued_diff;\n extern struct diff_filepair *diff_queue(struct diff_queue_struct *,\n-- \n1.7.0.2.273.gc2413.dirty\n"},{"id":"140131","messageId":"1271945142-27015-3-git-send-email-struggleyb.nku@gmail.com","threadId":"23564","inReplyTo":"1271945142-27015-1-git-send-email-struggleyb.nku@gmail.com","subject":"[PATCH 2/2 v3] Make git log --follow find copies among unmodified files.","fromName":"Bo Yang","fromEmail":"struggleyb.nku@gmail.com","sentAt":"2010-04-22T14:05:42Z","receivedAt":"2010-04-22T14:05:42Z","isPatch":true,"sender":{"key":"struggleyb.nku@gmail.com","avatar":"https://avatars.githubusercontent.com/u/233030?v=4"},"body":"'git log --follow <path>' don't track copies from unmodified\nfiles, and this patch fix it.\n\nSigned-off-by: Bo Yang <struggleyb.nku@gmail.com>\n---\n Documentation/git-log.txt           |    2 +-\n t/t4205-log-follow-harder-copies.sh |   56 +++++++++++++++++++++++++++++++++++\n tree-diff.c                         |    2 +-\n 3 files changed, 58 insertions(+), 2 deletions(-)\n create mode 100755 t/t4205-log-follow-harder-copies.sh\n\ndiff --git a/Documentation/git-log.txt b/Documentation/git-log.txt\nindex fb184ba..0727818 100644\n--- a/Documentation/git-log.txt\n+++ b/Documentation/git-log.txt\n@@ -56,7 +56,7 @@ include::diff-options.txt[]\n \tcommits, and doesn't limit diff for those commits.\n \n --follow::\n-\tContinue listing the history of a file beyond renames.\n+\tContinue listing the history of a file beyond renames/copies.\n \n --log-size::\n \tBefore the log message print out its size in bytes. Intended\ndiff --git a/t/t4205-log-follow-harder-copies.sh b/t/t4205-log-follow-harder-copies.sh\nnew file mode 100755\nindex 0000000..ad29e65\n--- /dev/null\n+++ b/t/t4205-log-follow-harder-copies.sh\n@@ -0,0 +1,56 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2010 Bo Yang\n+#\n+\n+test_description='Test --follow should always find copies hard in git log.\n+\n+'\n+. ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/diff-lib.sh\n+\n+echo >path0 'Line 1\n+Line 2\n+Line 3\n+'\n+\n+test_expect_success \\\n+    'add a file path0 and commit.' \\\n+    'git add path0 &&\n+     git commit -m \"Add path0\"'\n+\n+echo >path0 'New line 1\n+New line 2\n+New line 3\n+'\n+test_expect_success \\\n+    'Change path0.' \\\n+    'git add path0 &&\n+     git commit -m \"Change path0\"'\n+\n+cat <path0 >path1\n+test_expect_success \\\n+    'copy path0 to path1.' \\\n+    'git add path1 &&\n+     git commit -m \"Copy path1 from path0\"'\n+\n+test_expect_success \\\n+    'find the copy path0 -> path1 harder' \\\n+    'git log --follow --name-status --pretty=\"format:%s\"  path1 > current'\n+\n+cat >expected <<\\EOF\n+Copy path1 from path0\n+C100\tpath0\tpath1\n+\n+Change path0\n+M\tpath0\n+\n+Add path0\n+A\tpath0\n+EOF\n+\n+test_expect_success \\\n+    'validate the output.' \\\n+    'compare_diff_patch current expected'\n+\n+test_done\ndiff --git a/tree-diff.c b/tree-diff.c\nindex fe9f52c..1fb3e94 100644\n--- a/tree-diff.c\n+++ b/tree-diff.c\n@@ -346,7 +346,7 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co\n \n \tdiff_setup(&diff_opts);\n \tDIFF_OPT_SET(&diff_opts, RECURSIVE);\n-\tdiff_opts.detect_rename = DIFF_DETECT_RENAME;\n+\tDIFF_OPT_SET(&diff_opts, FIND_COPIES_HARDER);\n \tdiff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;\n \tdiff_opts.single_follow = opt->paths[0];\n \tdiff_opts.break_opt = opt->break_opt;\n-- \n1.7.0.2.273.gc2413.dirty\n"},{"id":"140158","messageId":"7v1ve743vb.fsf@alter.siamese.dyndns.org","threadId":"23564","inReplyTo":"1271945142-27015-2-git-send-email-struggleyb.nku@gmail.com","subject":"Re: [PATCH 1/2 v3] Make diffcore_std only can run once before a diff_flush.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-04-22T20:41:12Z","receivedAt":"2010-04-22T20:41:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Bo Yang <struggleyb.nku@gmail.com> writes:\n\n> So, I think we really don't need to run diffcore_std more\n> than one time.\n\nIt actually is stronger than that; we should never run it more than once,\nand it would be a bug if we did so.  Which codepath tries to call *_std()\ntwice?\n\nThe standard calling sequence is:\n\n - start from an empty queue.\n\n - use diff_change() and diff_addremove() to populate the queue.\n\n - call diffcore_std(). if you need to use a non-standard chain of\n   diffcore transformations, you _could_ call the diffcore_* routines that\n   diffcore_std() calls, if you choose to, but as you found out, some of\n   them are not idempotent operations, and shouldn't be called twice.\n\n - and finally call diffcore_flush().\n\n> @@ -3745,6 +3742,12 @@ void diffcore_fix_diff_index(struct diff_options *options)\n>  \n>  void diffcore_std(struct diff_options *options)\n>  {\n> +\t/* We never run this function more than one time, because the\n> +\t * rename/copy detection logic can only run once.\n> +\t */\n> +\tif (diff_queued_diff.run)\n> +\t\treturn;\n\nShouldn't this be a BUG() instead?\n\nThe trivial rewrite to use this macro is a good idea, but it probably\nshould be a separate patch.\n\n> +#define DIFF_QUEUE_CLEAR(q) \\\n> +\tdo { \\\n> +\t\t(q)->queue = NULL; \\\n> +\t\t(q)->nr = (q)->alloc = (q)->run = 0; \\\n> +\t} while(0);\n"},{"id":"140183","messageId":"n2m41f08ee11004222055i498174dfi47a5991a08114cd2@mail.gmail.com","threadId":"23564","inReplyTo":"7v1ve743vb.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2 v3] Make diffcore_std only can run once before a diff_flush.","fromName":"Bo Yang","fromEmail":"struggleyb.nku@gmail.com","sentAt":"2010-04-23T03:55:44Z","receivedAt":"2010-04-23T03:55:44Z","isPatch":true,"sender":{"key":"struggleyb.nku@gmail.com","avatar":"https://avatars.githubusercontent.com/u/233030?v=4"},"body":"On Fri, Apr 23, 2010 at 4:41 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> It actually is stronger than that; we should never run it more than once,\n> and it would be a bug if we did so.  Which codepath tries to call *_std()\n> twice?\n\nIn command 'git log --follow ...'\nlog_tree_diff call diff_tree_sha1 and then diff_tree_diff_flush, when\n'--follow' is given, the former function will call\ntry_to_follow_renames, which will call diffcore_std to detect rename.\nAnd then, diff_tree_diff_flush call 'diffcore_std' again\nunconditional. (and I have try to find a condition to make the call,\nbut I fail, so I figure out this patch.)\n\nBreakpoint 1, diffcore_std (options=0xbf9cc044) at diff.c:3748\n3748\t\tif (diff_queued_diff.run)\n(gdb) bt\n#0  diffcore_std (options=0xbf9cc044) at diff.c:3748\n#1  0x08124206 in try_to_follow_renames (t1=0xbf9cc130, t2=0xbf9cc11c,\nbase=0x81571c9 \"\", opt=0xbf9cc468) at tree-diff.c:358\n#2  0x08124480 in diff_tree_sha1 (old=0x9c51d8c\n\"$\\033\\222T���\\a\\035\\200T����\\210;8\\235i\", new=0x9c51d2c\n\"\\201�\\017<�\\v��n]\\226{�+�\\001\\003\\232\\232\\230\",\n    base=0x81571c9 \"\", opt=0xbf9cc468) at tree-diff.c:418\n#3  0x080e660e in log_tree_diff (opt=0xbf9cc220, commit=0x9c51d28,\nlog=0xbf9cc1ac) at log-tree.c:536\n#4  0x080e668f in log_tree_commit (opt=0xbf9cc220, commit=0x9c51d28)\nat log-tree.c:560\n#5  0x0807faa1 in cmd_log_walk (rev=0xbf9cc220) at builtin/log.c:237\n#6  0x080806e2 in cmd_log (argc=5, argv=0xbf9cc788, prefix=0x0) at\nbuiltin/log.c:481\n#7  0x0804b8eb in run_builtin (p=0x8161524, argc=5, argv=0xbf9cc788)\nat git.c:260\n#8  0x0804ba51 in handle_internal_command (argc=5, argv=0xbf9cc788) at git.c:416\n#9  0x0804bb2c in run_argv (argcp=0xbf9cc700, argv=0xbf9cc704) at git.c:458\n#10 0x0804bcbe in main (argc=5, argv=0xbf9cc788) at git.c:529\n(gdb) c\nContinuing.\n\nBreakpoint 1, diffcore_std (options=0xbf9cc468) at diff.c:3748\n3748\t\tif (diff_queued_diff.run)\n(gdb) bt\n#0  diffcore_std (options=0xbf9cc468) at diff.c:3748\n#1  0x080e6356 in log_tree_diff_flush (opt=0xbf9cc220) at log-tree.c:449\n#2  0x080e6619 in log_tree_diff (opt=0xbf9cc220, commit=0x9c51d28,\nlog=0xbf9cc1ac) at log-tree.c:537\n#3  0x080e668f in log_tree_commit (opt=0xbf9cc220, commit=0x9c51d28)\nat log-tree.c:560\n#4  0x0807faa1 in cmd_log_walk (rev=0xbf9cc220) at builtin/log.c:237\n#5  0x080806e2 in cmd_log (argc=5, argv=0xbf9cc788, prefix=0x0) at\nbuiltin/log.c:481\n#6  0x0804b8eb in run_builtin (p=0x8161524, argc=5, argv=0xbf9cc788)\nat git.c:260\n#7  0x0804ba51 in handle_internal_command (argc=5, argv=0xbf9cc788) at git.c:416\n#8  0x0804bb2c in run_argv (argcp=0xbf9cc700, argv=0xbf9cc704) at git.c:458\n#9  0x0804bcbe in main (argc=5, argv=0xbf9cc788) at git.c:529\n(gdb)\n\n> The standard calling sequence is:\n>\n>  - start from an empty queue.\n>\n>  - use diff_change() and diff_addremove() to populate the queue.\n>\n>  - call diffcore_std(). if you need to use a non-standard chain of\n>   diffcore transformations, you _could_ call the diffcore_* routines that\n>   diffcore_std() calls, if you choose to, but as you found out, some of\n>   them are not idempotent operations, and shouldn't be called twice.\n>\n>  - and finally call diffcore_flush().\n>\n>> @@ -3745,6 +3742,12 @@ void diffcore_fix_diff_index(struct diff_options *options)\n>>\n>>  void diffcore_std(struct diff_options *options)\n>>  {\n>> +     /* We never run this function more than one time, because the\n>> +      * rename/copy detection logic can only run once.\n>> +      */\n>> +     if (diff_queued_diff.run)\n>> +             return;\n>\n> Shouldn't this be a BUG() instead?\n\nAnyone may call diff_tree_sha1 and then diffcore_std, and\ndiff_tree_sha1 may call another diffcore_std if '--follow' given. If\nthis is a BUG, the calling pattern, diff_tree_sha1 -> diffcore_std\nshould all disappear from our code. And this involved much code\nrefactor. And I suggest my way that we avoid the duplicate call\nactively in diffcore_std.\n\n> The trivial rewrite to use this macro is a good idea, but it probably\n> should be a separate patch.\n>\n>> +#define DIFF_QUEUE_CLEAR(q) \\\n>> +     do { \\\n>> +             (q)->queue = NULL; \\\n>> +             (q)->nr = (q)->alloc = (q)->run = 0; \\\n>> +     } while(0);\n>\n\nYou mean split this commit into two?\n\nRegards!\nBo\n-- \nMy blog: http://blog.morebits.org\n"},{"id":"140564","messageId":"o2r41f08ee11004272037k5be968adh1d7b98768bf4e398@mail.gmail.com","threadId":"23564","inReplyTo":"n2m41f08ee11004222055i498174dfi47a5991a08114cd2@mail.gmail.com","subject":"Re: [PATCH 1/2 v3] Make diffcore_std only can run once before a diff_flush.","fromName":"Bo Yang","fromEmail":"struggleyb.nku@gmail.com","sentAt":"2010-04-28T03:37:00Z","receivedAt":"2010-04-28T03:37:00Z","isPatch":true,"sender":{"key":"struggleyb.nku@gmail.com","avatar":"https://avatars.githubusercontent.com/u/233030?v=4"},"body":"Hi Junio,\n\n   I have not receive any comments on this thread from you, but I\nthink it worth some words. I want to make these series patches landed\nand could you please give some more advice on this?\n\nRegards!\nBo\n\nOn Fri, Apr 23, 2010 at 11:55 AM, Bo Yang <struggleyb.nku@gmail.com> wrote:\n> On Fri, Apr 23, 2010 at 4:41 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>> It actually is stronger than that; we should never run it more than once,\n>> and it would be a bug if we did so.  Which codepath tries to call *_std()\n>> twice?\n>\n> In command 'git log --follow ...'\n> log_tree_diff call diff_tree_sha1 and then diff_tree_diff_flush, when\n> '--follow' is given, the former function will call\n> try_to_follow_renames, which will call diffcore_std to detect rename.\n> And then, diff_tree_diff_flush call 'diffcore_std' again\n> unconditional. (and I have try to find a condition to make the call,\n> but I fail, so I figure out this patch.)\n>\n> Breakpoint 1, diffcore_std (options=0xbf9cc044) at diff.c:3748\n> 3748            if (diff_queued_diff.run)\n> (gdb) bt\n> #0  diffcore_std (options=0xbf9cc044) at diff.c:3748\n> #1  0x08124206 in try_to_follow_renames (t1=0xbf9cc130, t2=0xbf9cc11c,\n> base=0x81571c9 \"\", opt=0xbf9cc468) at tree-diff.c:358\n> #2  0x08124480 in diff_tree_sha1 (old=0x9c51d8c\n> \"$\\033\\222T���\\a\\035\\200T����\\210;8\\235i\", new=0x9c51d2c\n> \"\\201�\\017<�\\v��n]\\226{�+�\\001\\003\\232\\232\\230\",\n>    base=0x81571c9 \"\", opt=0xbf9cc468) at tree-diff.c:418\n> #3  0x080e660e in log_tree_diff (opt=0xbf9cc220, commit=0x9c51d28,\n> log=0xbf9cc1ac) at log-tree.c:536\n> #4  0x080e668f in log_tree_commit (opt=0xbf9cc220, commit=0x9c51d28)\n> at log-tree.c:560\n> #5  0x0807faa1 in cmd_log_walk (rev=0xbf9cc220) at builtin/log.c:237\n> #6  0x080806e2 in cmd_log (argc=5, argv=0xbf9cc788, prefix=0x0) at\n> builtin/log.c:481\n> #7  0x0804b8eb in run_builtin (p=0x8161524, argc=5, argv=0xbf9cc788)\n> at git.c:260\n> #8  0x0804ba51 in handle_internal_command (argc=5, argv=0xbf9cc788) at git.c:416\n> #9  0x0804bb2c in run_argv (argcp=0xbf9cc700, argv=0xbf9cc704) at git.c:458\n> #10 0x0804bcbe in main (argc=5, argv=0xbf9cc788) at git.c:529\n> (gdb) c\n> Continuing.\n>\n> Breakpoint 1, diffcore_std (options=0xbf9cc468) at diff.c:3748\n> 3748            if (diff_queued_diff.run)\n> (gdb) bt\n> #0  diffcore_std (options=0xbf9cc468) at diff.c:3748\n> #1  0x080e6356 in log_tree_diff_flush (opt=0xbf9cc220) at log-tree.c:449\n> #2  0x080e6619 in log_tree_diff (opt=0xbf9cc220, commit=0x9c51d28,\n> log=0xbf9cc1ac) at log-tree.c:537\n> #3  0x080e668f in log_tree_commit (opt=0xbf9cc220, commit=0x9c51d28)\n> at log-tree.c:560\n> #4  0x0807faa1 in cmd_log_walk (rev=0xbf9cc220) at builtin/log.c:237\n> #5  0x080806e2 in cmd_log (argc=5, argv=0xbf9cc788, prefix=0x0) at\n> builtin/log.c:481\n> #6  0x0804b8eb in run_builtin (p=0x8161524, argc=5, argv=0xbf9cc788)\n> at git.c:260\n> #7  0x0804ba51 in handle_internal_command (argc=5, argv=0xbf9cc788) at git.c:416\n> #8  0x0804bb2c in run_argv (argcp=0xbf9cc700, argv=0xbf9cc704) at git.c:458\n> #9  0x0804bcbe in main (argc=5, argv=0xbf9cc788) at git.c:529\n> (gdb)\n>\n>> The standard calling sequence is:\n>>\n>>  - start from an empty queue.\n>>\n>>  - use diff_change() and diff_addremove() to populate the queue.\n>>\n>>  - call diffcore_std(). if you need to use a non-standard chain of\n>>   diffcore transformations, you _could_ call the diffcore_* routines that\n>>   diffcore_std() calls, if you choose to, but as you found out, some of\n>>   them are not idempotent operations, and shouldn't be called twice.\n>>\n>>  - and finally call diffcore_flush().\n>>\n>>> @@ -3745,6 +3742,12 @@ void diffcore_fix_diff_index(struct diff_options *options)\n>>>\n>>>  void diffcore_std(struct diff_options *options)\n>>>  {\n>>> +     /* We never run this function more than one time, because the\n>>> +      * rename/copy detection logic can only run once.\n>>> +      */\n>>> +     if (diff_queued_diff.run)\n>>> +             return;\n>>\n>> Shouldn't this be a BUG() instead?\n>\n> Anyone may call diff_tree_sha1 and then diffcore_std, and\n> diff_tree_sha1 may call another diffcore_std if '--follow' given. If\n> this is a BUG, the calling pattern, diff_tree_sha1 -> diffcore_std\n> should all disappear from our code. And this involved much code\n> refactor. And I suggest my way that we avoid the duplicate call\n> actively in diffcore_std.\n>\n>> The trivial rewrite to use this macro is a good idea, but it probably\n>> should be a separate patch.\n>>\n>>> +#define DIFF_QUEUE_CLEAR(q) \\\n>>> +     do { \\\n>>> +             (q)->queue = NULL; \\\n>>> +             (q)->nr = (q)->alloc = (q)->run = 0; \\\n>>> +     } while(0);\n>>\n>\n> You mean split this commit into two?\n>\n> Regards!\n> Bo\n> --\n> My blog: http://blog.morebits.org\n>\n"}]}