{"thread":{"id":"23727","subject":"[PATCH 0/3 v4] Make git log --follow find copies among unmodified files.","startedAt":"2010-05-07T04:52:26Z","lastAt":"2010-08-02T15:40:51Z","messageCount":7,"participants":["Bo Yang","Sven Verdoolaege","Junio C Hamano"],"isPatch":true,"patchVersion":4,"patchTotal":3},"messages":[{"id":"141110","messageId":"1273207949-18500-1-git-send-email-struggleyb.nku@gmail.com","threadId":"23727","inReplyTo":null,"subject":"[PATCH 0/3 v4] Make git log --follow find copies among unmodified files.","fromName":"Bo Yang","fromEmail":"struggleyb.nku@gmail.com","sentAt":"2010-05-07T04:52:26Z","receivedAt":"2010-05-07T04:52:26Z","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 (3):\n  Add a macro DIFF_QUEUE_CLEAR.\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                          |    7 ++++\n t/t4205-log-follow-harder-copies.sh |   56 +++++++++++++++++++++++++++++++++++\n tree-diff.c                         |    2 +-\n 8 files changed, 82 insertions(+), 18 deletions(-)\n create mode 100755 t/t4205-log-follow-harder-copies.sh\n"},{"id":"141112","messageId":"1273207949-18500-2-git-send-email-struggleyb.nku@gmail.com","threadId":"23727","inReplyTo":"1273207949-18500-1-git-send-email-struggleyb.nku@gmail.com","subject":"[PATCH 1/3 v4] Add a macro DIFF_QUEUE_CLEAR.","fromName":"Bo Yang","fromEmail":"struggleyb.nku@gmail.com","sentAt":"2010-05-07T04:52:27Z","receivedAt":"2010-05-07T04:52:27Z","isPatch":true,"sender":{"key":"struggleyb.nku@gmail.com","avatar":"https://avatars.githubusercontent.com/u/233030?v=4"},"body":"Refactor the diff_queue_struct code, this macro help\nto reset the structure.\n\nSigned-off-by: Bo Yang <struggleyb.nku@gmail.com>\n---\n diff.c             |   13 +++++--------\n diffcore-break.c   |    6 ++----\n diffcore-pickaxe.c |    3 +--\n diffcore-rename.c  |    3 +--\n diffcore.h         |    5 +++++\n 5 files changed, 14 insertions(+), 16 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex e40c127..4a350e3 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2540,6 +2540,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@@ -3457,8 +3458,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@@ -3586,8 +3586,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@@ -3609,8 +3608,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@@ -3678,8 +3676,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];\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..5d05dea 100644\n--- a/diffcore.h\n+++ b/diffcore.h\n@@ -92,6 +92,11 @@ struct diff_queue_struct {\n \tint alloc;\n \tint nr;\n };\n+#define DIFF_QUEUE_CLEAR(q) \\\n+\tdo { \\\n+\t\t(q)->queue = NULL; \\\n+\t\t(q)->nr = (q)->alloc = 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.6.0.4\n"},{"id":"141111","messageId":"1273207949-18500-3-git-send-email-struggleyb.nku@gmail.com","threadId":"23727","inReplyTo":"1273207949-18500-2-git-send-email-struggleyb.nku@gmail.com","subject":"[PATCH 2/3 v4] Make diffcore_std only can run once before a diff_flush","fromName":"Bo Yang","fromEmail":"struggleyb.nku@gmail.com","sentAt":"2010-05-07T04:52:28Z","receivedAt":"2010-05-07T04:52:28Z","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     |    8 ++++++++\n diffcore.h |    2 ++\n 2 files changed, 10 insertions(+), 0 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 4a350e3..f0985bc 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3737,6 +3737,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@@ -3756,6 +3762,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.h b/diffcore.h\nindex 5d05dea..491bea0 100644\n--- a/diffcore.h\n+++ b/diffcore.h\n@@ -91,11 +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 = 0; \\\n+\t\t(q)->run = 0; \\\n \t} while(0);\n \n extern struct diff_queue_struct diff_queued_diff;\n-- \n1.6.0.4\n"},{"id":"141113","messageId":"1273207949-18500-4-git-send-email-struggleyb.nku@gmail.com","threadId":"23727","inReplyTo":"1273207949-18500-3-git-send-email-struggleyb.nku@gmail.com","subject":"[PATCH 3/3 v4] Make git log --follow find copies among unmodified files.","fromName":"Bo Yang","fromEmail":"struggleyb.nku@gmail.com","sentAt":"2010-05-07T04:52:29Z","receivedAt":"2010-05-07T04:52:29Z","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.6.0.4\n"},{"id":"146941","messageId":"20100802124729.GK12084MdfPADPa@purples","threadId":"23727","inReplyTo":"1273207949-18500-2-git-send-email-struggleyb.nku@gmail.com","subject":"Re: [PATCH 1/3 v4] Add a macro DIFF_QUEUE_CLEAR.","fromName":"Sven Verdoolaege","fromEmail":"skimo@kotnet.org","sentAt":"2010-08-02T12:47:29Z","receivedAt":"2010-08-02T12:47:29Z","isPatch":true,"sender":{"key":"skimo@kotnet.org","avatar":null},"body":"On Thu, May 06, 2010 at 09:52:27PM -0700, Bo Yang wrote:\n> Refactor the diff_queue_struct code, this macro help\n> to reset the structure.\n> \n[..]\n> \n> diff --git a/diff.c b/diff.c\n> index e40c127..4a350e3 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -2540,6 +2540,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\nWhat's this line for?  It doesn't seem to be explained by the commit\nmessage and it breaks \"git diff-files -p --submodule\".\nWithout this line, I get the following output in one of my projects:\n\n    Submodule barvinok contains untracked content\n    Submodule barvinok contains modified content\n    Submodule barvinok e129555..833e4a6:\n      > iscc: use simplified CLooG interface\n    Submodule cloog contains untracked content\n    Submodule cloog f083938..4684a24:\n      > partial doc\n      > cloog_names_read_strings: do not generate names if they cannot be read\n      > cloog_program_read: separate reading from input from construction of CloogProgram\n    Submodule cloog-polylib contains untracked content\n    Submodule cloog-polylib contains modified content\n    Submodule isl contains untracked content\n    Submodule isl 892fb27..5292e00:\n      > isl_transitive_closure.c: anonymize input map during incremental computation\n      > isl_transitive_closure.c: keep track of domains for Floyd-Warshall\n      > isl_dim_drop: always remove tuple name, even if number of dims to drop is zero\n      > isl_dim_set_tuple_name: allow explicit removal of tuple name\n    Submodule isl-polylib contains untracked content\n    Submodule isl-polylib 531cb00..e9e2edf:\n      > stop using isl_basic_map internals\n    Submodule polylib contains untracked content\n\nWith the line, I only get\n\n    Submodule barvinok contains untracked content\n    Submodule barvinok contains modified content\n    Submodule barvinok e129555..833e4a6:\n      > iscc: use simplified CLooG interface\n\nskimo\n"},{"id":"146947","messageId":"7vvd7tnihx.fsf@alter.siamese.dyndns.org","threadId":"23727","inReplyTo":"20100802124729.GK12084MdfPADPa@purples","subject":"Re: [PATCH 1/3 v4] Add a macro DIFF_QUEUE_CLEAR.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-02T15:26:18Z","receivedAt":"2010-08-02T15:26:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Verdoolaege <skimo@kotnet.org> writes:\n\n> On Thu, May 06, 2010 at 09:52:27PM -0700, Bo Yang wrote:\n>> Refactor the diff_queue_struct code, this macro help\n>> to reset the structure.\n>> \n> [..]\n>> \n>> diff --git a/diff.c b/diff.c\n>> index e40c127..4a350e3 100644\n>> --- a/diff.c\n>> +++ b/diff.c\n>> @@ -2540,6 +2540,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>\n> What's this line for?\n\nI don't think this change is warranted.  The macro was supposed to reduce\nthe repetition of assignment to q->queue, q->nr and q->alloc, and nothing\nelse.\n\nAlso the commit messages in this series are unreadable---I should have\nbeen a bit more careful.\n\nSorry and thanks for noticing.\n"},{"id":"146948","messageId":"AANLkTinZGWmA=9UZ6AiojtT861uM_Q4FXVK5DRxFyx4g@mail.gmail.com","threadId":"23727","inReplyTo":"20100802124729.GK12084MdfPADPa@purples","subject":"Re: [PATCH 1/3 v4] Add a macro DIFF_QUEUE_CLEAR.","fromName":"Bo Yang","fromEmail":"struggleyb.nku@gmail.com","sentAt":"2010-08-02T15:40:51Z","receivedAt":"2010-08-02T15:40:51Z","isPatch":true,"sender":{"key":"struggleyb.nku@gmail.com","avatar":"https://avatars.githubusercontent.com/u/233030?v=4"},"body":"Hi Sven,\nOn Mon, Aug 2, 2010 at 8:47 PM, Sven Verdoolaege <skimo@kotnet.org> wrote:\n> On Thu, May 06, 2010 at 09:52:27PM -0700, Bo Yang wrote:\n>> Refactor the diff_queue_struct code, this macro help\n>> to reset the structure.\n>>\n> [..]\n>>\n>> diff --git a/diff.c b/diff.c\n>> index e40c127..4a350e3 100644\n>> --- a/diff.c\n>> +++ b/diff.c\n>> @@ -2540,6 +2540,7 @@ static void run_checkdiff(struct diff_filepair *p, struct diff_options *o)\n>>  void diff_setup(struct diff_options *options)\n>>  {\n>>       memset(options, 0, sizeof(*options));\n>> +     memset(&diff_queued_diff, 0, sizeof(diff_queued_diff));\n>>\n\nSorry about the broken code and the bad commit message...\n\nThis line is used to clear the global queue structure and make it\nusable in next round of diff. I am wondering how the submodule part\nuse the diff API to cause such an issue. :)\n\n-- \nRegards!\nBo\n----------------------------\nMy blog: http://blog.morebits.org\nWhy Git: http://www.whygitisbetterthanx.com/\n"}]}