From 9ca5df90615aa3c6b60e1bc8f03db6cae98e816c Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Thu, 6 May 2010 21:52:27 -0700 Subject: [PATCH 1/4] Add a macro DIFF_QUEUE_CLEAR. Refactor the diff_queue_struct code, this macro help to reset the structure. Signed-off-by: Bo Yang Signed-off-by: Junio C Hamano --- diff.c | 13 +++++-------- diffcore-break.c | 6 ++---- diffcore-pickaxe.c | 3 +-- diffcore-rename.c | 3 +-- diffcore.h | 5 +++++ 5 files changed, 14 insertions(+), 16 deletions(-) diff --git a/diff.c b/diff.c index e40c1271da..4a350e365e 100644 --- a/diff.c +++ b/diff.c @@ -2540,6 +2540,7 @@ static void run_checkdiff(struct diff_filepair *p, struct diff_options *o) void diff_setup(struct diff_options *options) { memset(options, 0, sizeof(*options)); + memset(&diff_queued_diff, 0, sizeof(diff_queued_diff)); options->file = stdout; @@ -3457,8 +3458,7 @@ int diff_flush_patch_id(struct diff_options *options, unsigned char *sha1) diff_free_filepair(q->queue[i]); free(q->queue); - q->queue = NULL; - q->nr = q->alloc = 0; + DIFF_QUEUE_CLEAR(q); return result; } @@ -3586,8 +3586,7 @@ void diff_flush(struct diff_options *options) diff_free_filepair(q->queue[i]); free_queue: free(q->queue); - q->queue = NULL; - q->nr = q->alloc = 0; + DIFF_QUEUE_CLEAR(q); if (options->close_file) fclose(options->file); @@ -3609,8 +3608,7 @@ static void diffcore_apply_filter(const char *filter) int i; struct diff_queue_struct *q = &diff_queued_diff; struct diff_queue_struct outq; - outq.queue = NULL; - outq.nr = outq.alloc = 0; + DIFF_QUEUE_CLEAR(&outq); if (!filter) return; @@ -3678,8 +3676,7 @@ static void diffcore_skip_stat_unmatch(struct diff_options *diffopt) int i; struct diff_queue_struct *q = &diff_queued_diff; struct diff_queue_struct outq; - outq.queue = NULL; - outq.nr = outq.alloc = 0; + DIFF_QUEUE_CLEAR(&outq); for (i = 0; i < q->nr; i++) { struct diff_filepair *p = q->queue[i]; diff --git a/diffcore-break.c b/diffcore-break.c index 3a7b60a037..44f8678d22 100644 --- a/diffcore-break.c +++ b/diffcore-break.c @@ -162,8 +162,7 @@ void diffcore_break(int break_score) if (!merge_score) merge_score = DEFAULT_MERGE_SCORE; - outq.nr = outq.alloc = 0; - outq.queue = NULL; + DIFF_QUEUE_CLEAR(&outq); for (i = 0; i < q->nr; i++) { struct diff_filepair *p = q->queue[i]; @@ -256,8 +255,7 @@ void diffcore_merge_broken(void) struct diff_queue_struct outq; int i, j; - outq.nr = outq.alloc = 0; - outq.queue = NULL; + DIFF_QUEUE_CLEAR(&outq); for (i = 0; i < q->nr; i++) { struct diff_filepair *p = q->queue[i]; diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c index d0ef839700..929de15aa9 100644 --- a/diffcore-pickaxe.c +++ b/diffcore-pickaxe.c @@ -55,8 +55,7 @@ void diffcore_pickaxe(const char *needle, int opts) int i, has_changes; regex_t regex, *regexp = NULL; struct diff_queue_struct outq; - outq.queue = NULL; - outq.nr = outq.alloc = 0; + DIFF_QUEUE_CLEAR(&outq); if (opts & DIFF_PICKAXE_REGEX) { int err; diff --git a/diffcore-rename.c b/diffcore-rename.c index d6fd3cacd6..df41be56de 100644 --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -569,8 +569,7 @@ void diffcore_rename(struct diff_options *options) /* At this point, we have found some renames and copies and they * are recorded in rename_dst. The original list is still in *q. */ - outq.queue = NULL; - outq.nr = outq.alloc = 0; + DIFF_QUEUE_CLEAR(&outq); for (i = 0; i < q->nr; i++) { struct diff_filepair *p = q->queue[i]; struct diff_filepair *pair_to_free = NULL; diff --git a/diffcore.h b/diffcore.h index fcd00bf27a..5d05deaf68 100644 --- a/diffcore.h +++ b/diffcore.h @@ -92,6 +92,11 @@ struct diff_queue_struct { int alloc; int nr; }; +#define DIFF_QUEUE_CLEAR(q) \ + do { \ + (q)->queue = NULL; \ + (q)->nr = (q)->alloc = 0; \ + } while(0); extern struct diff_queue_struct diff_queued_diff; extern struct diff_filepair *diff_queue(struct diff_queue_struct *, From 1da6175d438a9849db07a68326ee05f291510074 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Thu, 6 May 2010 21:52:28 -0700 Subject: [PATCH 2/4] Make diffcore_std only can run once before a diff_flush When file renames/copies detection is turned on, the second diffcore_std will degrade a 'C' pair to a 'R' pair. And this may happen when we run 'git log --follow' with hard copies finding. That is, the try_to_follow_renames() will run diffcore_std to find the copies, and then 'git log' will issue another diffcore_std, which will reduce 'src->rename_used' and recognize this copy as a rename. This is not what we want. So, I think we really don't need to run diffcore_std more than one time. Signed-off-by: Bo Yang Signed-off-by: Junio C Hamano --- diff.c | 8 ++++++++ diffcore.h | 2 ++ 2 files changed, 10 insertions(+) diff --git a/diff.c b/diff.c index 4a350e365e..f0985bc76a 100644 --- a/diff.c +++ b/diff.c @@ -3737,6 +3737,12 @@ void diffcore_fix_diff_index(struct diff_options *options) void diffcore_std(struct diff_options *options) { + /* We never run this function more than one time, because the + * rename/copy detection logic can only run once. + */ + if (diff_queued_diff.run) + return; + if (options->skip_stat_unmatch) diffcore_skip_stat_unmatch(options); if (options->break_opt != -1) @@ -3756,6 +3762,8 @@ void diffcore_std(struct diff_options *options) DIFF_OPT_SET(options, HAS_CHANGES); else DIFF_OPT_CLR(options, HAS_CHANGES); + + diff_queued_diff.run = 1; } int diff_result_code(struct diff_options *opt, int status) diff --git a/diffcore.h b/diffcore.h index 5d05deaf68..491bea0b44 100644 --- a/diffcore.h +++ b/diffcore.h @@ -91,11 +91,13 @@ struct diff_queue_struct { struct diff_filepair **queue; int alloc; int nr; + int run; }; #define DIFF_QUEUE_CLEAR(q) \ do { \ (q)->queue = NULL; \ (q)->nr = (q)->alloc = 0; \ + (q)->run = 0; \ } while(0); extern struct diff_queue_struct diff_queued_diff; From 0cdca133ec8aa10fe4c806f6695ccbf32394aa91 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Thu, 6 May 2010 21:52:29 -0700 Subject: [PATCH 3/4] Make git log --follow find copies among unmodified files. 'git log --follow ' don't track copies from unmodified files, and this patch fix it. Signed-off-by: Bo Yang Signed-off-by: Junio C Hamano --- Documentation/git-log.txt | 2 +- t/t4205-log-follow-harder-copies.sh | 56 +++++++++++++++++++++++++++++ tree-diff.c | 2 +- 3 files changed, 58 insertions(+), 2 deletions(-) create mode 100755 t/t4205-log-follow-harder-copies.sh diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt index fb184ba186..0727818bb1 100644 --- a/Documentation/git-log.txt +++ b/Documentation/git-log.txt @@ -56,7 +56,7 @@ include::diff-options.txt[] commits, and doesn't limit diff for those commits. --follow:: - Continue listing the history of a file beyond renames. + Continue listing the history of a file beyond renames/copies. --log-size:: Before the log message print out its size in bytes. Intended diff --git a/t/t4205-log-follow-harder-copies.sh b/t/t4205-log-follow-harder-copies.sh new file mode 100755 index 0000000000..ad29e65fcb --- /dev/null +++ b/t/t4205-log-follow-harder-copies.sh @@ -0,0 +1,56 @@ +#!/bin/sh +# +# Copyright (c) 2010 Bo Yang +# + +test_description='Test --follow should always find copies hard in git log. + +' +. ./test-lib.sh +. "$TEST_DIRECTORY"/diff-lib.sh + +echo >path0 'Line 1 +Line 2 +Line 3 +' + +test_expect_success \ + 'add a file path0 and commit.' \ + 'git add path0 && + git commit -m "Add path0"' + +echo >path0 'New line 1 +New line 2 +New line 3 +' +test_expect_success \ + 'Change path0.' \ + 'git add path0 && + git commit -m "Change path0"' + +cat path1 +test_expect_success \ + 'copy path0 to path1.' \ + 'git add path1 && + git commit -m "Copy path1 from path0"' + +test_expect_success \ + 'find the copy path0 -> path1 harder' \ + 'git log --follow --name-status --pretty="format:%s" path1 > current' + +cat >expected <<\EOF +Copy path1 from path0 +C100 path0 path1 + +Change path0 +M path0 + +Add path0 +A path0 +EOF + +test_expect_success \ + 'validate the output.' \ + 'compare_diff_patch current expected' + +test_done diff --git a/tree-diff.c b/tree-diff.c index fe9f52c479..1fb3e94614 100644 --- a/tree-diff.c +++ b/tree-diff.c @@ -346,7 +346,7 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co diff_setup(&diff_opts); DIFF_OPT_SET(&diff_opts, RECURSIVE); - diff_opts.detect_rename = DIFF_DETECT_RENAME; + DIFF_OPT_SET(&diff_opts, FIND_COPIES_HARDER); diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT; diff_opts.single_follow = opt->paths[0]; diff_opts.break_opt = opt->break_opt; From 9feeaa2bf39aed4a0148296f504f88b09b8901d4 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 10 May 2010 05:47:39 -0400 Subject: [PATCH 4/4] tests: rename duplicate t4205 Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- ...-follow-harder-copies.sh => t4206-log-follow-harder-copies.sh} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename t/{t4205-log-follow-harder-copies.sh => t4206-log-follow-harder-copies.sh} (100%) diff --git a/t/t4205-log-follow-harder-copies.sh b/t/t4206-log-follow-harder-copies.sh similarity index 100% rename from t/t4205-log-follow-harder-copies.sh rename to t/t4206-log-follow-harder-copies.sh