From: Junio C Hamano Date: Tue, 24 May 2005 01:14:03 +0000 (-0700) Subject: [PATCH] Fix diff-pruning logic which was running prune too early. X-Git-Tag: v0.99~479 X-Git-Url: https://git.verplant.org/?a=commitdiff_plain;h=bceafe752c03f4b13b9b1671a55d9e2acd26432d;p=git.git [PATCH] Fix diff-pruning logic which was running prune too early. For later stages to reorder patches, pruning logic and rename detection logic should not decide which delete to discard (because another entry said it will take over the file as a rename) until the very end. Also fix some tests that were assuming the earlier "last one is rename or keep everything else is copy" semantics of diff-raw format, which no longer is true. Signed-off-by: Junio C Hamano Signed-off-by: Linus Torvalds --- diff --git a/diff-cache.c b/diff-cache.c index 10d5c955..579a673f 100644 --- a/diff-cache.c +++ b/diff-cache.c @@ -229,7 +229,6 @@ int main(int argc, const char **argv) ret = diff_cache(active_cache, active_nr); if (detect_rename) diffcore_rename(detect_rename, diff_score_opt); - diffcore_prune(); if (pickaxe) diffcore_pickaxe(pickaxe); if (2 <= argc) diff --git a/diff-files.c b/diff-files.c index 17c328a8..d7cfad79 100644 --- a/diff-files.c +++ b/diff-files.c @@ -115,7 +115,6 @@ int main(int argc, const char **argv) } if (detect_rename) diffcore_rename(detect_rename, diff_score_opt); - diffcore_prune(); if (pickaxe) diffcore_pickaxe(pickaxe); if (1 < argc) diff --git a/diff-tree.c b/diff-tree.c index 31f80190..a7e7345b 100644 --- a/diff-tree.c +++ b/diff-tree.c @@ -266,10 +266,8 @@ static int call_diff_flush(void) { if (detect_rename) diffcore_rename(detect_rename, diff_score_opt); - diffcore_prune(); if (pickaxe) diffcore_pickaxe(pickaxe); - if (diff_queue_is_empty()) { diff_flush(DIFF_FORMAT_NO_OUTPUT, 0); return 0; diff --git a/diff.c b/diff.c index 5a2aee7a..05e52779 100644 --- a/diff.c +++ b/diff.c @@ -12,9 +12,6 @@ static const char *diff_opts = "-pu"; static unsigned char null_sha1[20] = { 0, }; static int reverse_diff; -static int generate_patch; -static int line_termination = '\n'; -static int inter_name_termination = '\t'; static const char *external_diff(void) { @@ -502,7 +499,9 @@ struct diff_filepair *diff_queue(struct diff_queue_struct *queue, return dp; } -static void diff_flush_raw(struct diff_filepair *p) +static void diff_flush_raw(struct diff_filepair *p, + int line_termination, + int inter_name_termination) { int two_paths; char status[10]; @@ -566,10 +565,6 @@ static void diff_flush_patch(struct diff_filepair *p) const char *name, *other; char msg_[PATH_MAX*2+200], *msg; - /* diffcore_prune() keeps "stay" entries for diff-raw - * copy/rename detection, but when we are generating - * patches we do not need them. - */ if (diff_unmodified_pair(p)) return; @@ -585,7 +580,7 @@ static void diff_flush_patch(struct diff_filepair *p) "similarity index %d%%\n" "copy from %s\n" "copy to %s\n", - (int)(0.5 + p->score * 100/MAX_SCORE), + (int)(0.5 + p->score * 100.0/MAX_SCORE), p->one->path, p->two->path); msg = msg_; break; @@ -594,7 +589,7 @@ static void diff_flush_patch(struct diff_filepair *p) "similarity index %d%%\n" "rename old %s\n" "rename new %s\n", - (int)(0.5 + p->score * 100/MAX_SCORE), + (int)(0.5 + p->score * 100.0/MAX_SCORE), p->one->path, p->two->path); msg = msg_; break; @@ -630,105 +625,82 @@ int diff_needs_to_stay(struct diff_queue_struct *q, int i, return 0; } -static int diff_used_as_source(struct diff_queue_struct *q, int lim, - struct diff_filespec *it) +int diff_queue_is_empty(void) { - int i; - for (i = 0; i < lim; i++) { - struct diff_filepair *p = q->queue[i++]; - if (!strcmp(p->one->path, it->path)) - return 1; - } - return 0; + struct diff_queue_struct *q = &diff_queued_diff; + return q->nr == 0; } -void diffcore_prune(void) +static void diff_resolve_rename_copy(void) { - /* - * Although rename/copy detection wants to have "no-change" - * entries fed into them, the downstream do not need to see - * them, unless we had rename/copy for the same path earlier. - * This function removes such entries. - * - * The applications that use rename/copy should: - * - * (1) feed change and "no-change" entries via diff_queue(). - * (2) call diffcore_rename, and any other future diffcore_xxx - * that would benefit by still having "no-change" entries. - * (3) call diffcore_prune - * (4) call other diffcore_xxx that do not need to see - * "no-change" entries. - * (5) call diff_flush(). - */ - struct diff_queue_struct *q = &diff_queued_diff; - struct diff_queue_struct outq; int i; - - outq.queue = NULL; - outq.nr = outq.alloc = 0; - + struct diff_queue_struct *q = &diff_queued_diff; for (i = 0; i < q->nr; i++) { struct diff_filepair *p = q->queue[i]; - if (!diff_unmodified_pair(p) || - diff_used_as_source(q, i, p->one)) - diff_q(&outq, p); - else - free(p); + p->status = 0; + if (DIFF_PAIR_UNMERGED(p)) + p->status = 'U'; + else if (!DIFF_FILE_VALID((p)->one)) + p->status = 'N'; + else if (!DIFF_FILE_VALID((p)->two)) { + /* maybe earlier one said 'R', meaning + * it will take it, in which case we do + * not need to keep 'D'. + */ + int j; + for (j = 0; j < i; j++) { + struct diff_filepair *pp = q->queue[j]; + if (pp->status == 'R' && + !strcmp(pp->one->path, p->one->path)) + break; + } + if (j < i) + continue; + p->status = 'D'; + } + else if (strcmp(p->one->path, p->two->path)) { + /* This is rename or copy. Which one is it? */ + if (diff_needs_to_stay(q, i+1, p->one)) + p->status = 'C'; + else + p->status = 'R'; + } + else if (memcmp(p->one->sha1, p->two->sha1, 20)) + p->status = 'M'; + else { + /* we do not need this one */ + p->status = 0; + } } - free(q->queue); - *q = outq; - return; -} - -int diff_queue_is_empty(void) -{ - struct diff_queue_struct *q = &diff_queued_diff; - return q->nr == 0; } void diff_flush(int diff_output_style, int resolve_rename_copy) { struct diff_queue_struct *q = &diff_queued_diff; int i; + int line_termination = '\n'; + int inter_name_termination = '\t'; - generate_patch = 0; - switch (diff_output_style) { - case DIFF_FORMAT_HUMAN: - line_termination = '\n'; - inter_name_termination = '\t'; - break; - case DIFF_FORMAT_MACHINE: + if (diff_output_style == DIFF_FORMAT_MACHINE) line_termination = inter_name_termination = 0; - break; - case DIFF_FORMAT_PATCH: - generate_patch = 1; - break; - } + if (resolve_rename_copy) + diff_resolve_rename_copy(); + for (i = 0; i < q->nr; i++) { struct diff_filepair *p = q->queue[i]; - if (resolve_rename_copy) { - if (DIFF_PAIR_UNMERGED(p)) - p->status = 'U'; - else if (!DIFF_FILE_VALID((p)->one)) - p->status = 'N'; - else if (!DIFF_FILE_VALID((p)->two)) - p->status = 'D'; - else if (strcmp(p->one->path, p->two->path)) { - /* This is rename or copy. Which one is it? */ - if (diff_needs_to_stay(q, i+1, p->one)) - p->status = 'C'; - else - p->status = 'R'; - } - else - p->status = 'M'; - } - if (generate_patch) + if (p->status == 0) + continue; + switch (diff_output_style) { + case DIFF_FORMAT_PATCH: diff_flush_patch(p); - else - diff_flush_raw(p); + break; + case DIFF_FORMAT_HUMAN: + case DIFF_FORMAT_MACHINE: + diff_flush_raw(p, line_termination, + inter_name_termination); + break; + } } - for (i = 0; i < q->nr; i++) { struct diff_filepair *p = q->queue[i]; diff_free_filespec_data(p->one); @@ -755,9 +727,9 @@ void diff_addremove(int addremove, unsigned mode, * with something like '=' or '*' (I haven't decided * which but should not make any difference). * Feeding the same new and old to diff_change() - * also has the same effect. diffcore_prune() should - * be used to filter uninteresting ones out before the - * final output happens. + * also has the same effect. + * Before the final output happens, they are pruned after + * merged into rename/copy pairs as appropriate. */ if (reverse_diff) addremove = (addremove == '+' ? '-' : diff --git a/diff.h b/diff.h index 6aa41286..0d1f9ae3 100644 --- a/diff.h +++ b/diff.h @@ -39,8 +39,6 @@ extern void diff_setup(int reverse); extern void diffcore_rename(int rename_copy, int minimum_score); -extern void diffcore_prune(void); - extern void diffcore_pickaxe(const char *needle); extern void diffcore_pathspec(const char **pathspec); diff --git a/diffcore-rename.c b/diffcore-rename.c index e8de4935..f40ab78b 100644 --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -133,10 +133,7 @@ static void record_rename_pair(struct diff_queue_struct *outq, * The downstream diffcore transformers are free to reorder * the entries as long as they keep file pairs that has the * same p->one->path in earlier rename_rank to appear before - * later ones. This ordering is used by the diff_flush() - * logic to tell renames from copies, and also used by the - * diffcore_prune() logic to omit unnecessary - * "no-modification" entries. + * later ones. * * To the final output routine, and in the diff-raw format * output, a rename/copy that is based on a path that has a @@ -271,14 +268,8 @@ void diffcore_rename(int detect_rename, int minimum_score) /* We really want to cull the candidates list early * with cheap tests in order to avoid doing deltas. - * - * With the current callers, we should not have already - * matched entries at this point, but it is nonetheless - * checked for sanity. */ for (i = 0; i < created.nr; i++) { - if (created.s[i]->xfrm_flags & RENAME_DST_MATCHED) - continue; /* we have matched exactly already */ for (h = 0; h < sizeof(srcs)/sizeof(srcs[0]); h++) { struct diff_rename_pool *p = srcs[h]; for (j = 0; j < p->nr; j++) { @@ -386,25 +377,13 @@ void diffcore_rename(int detect_rename, int minimum_score) } else if (!DIFF_FILE_VALID(p->two)) { /* deleted */ - if (p->one->xfrm_flags & RENAME_SRC_GONE) - ; /* rename/copy deleted it already */ - else - diff_queue(q, p->one, p->two); + diff_queue(q, p->one, p->two); } else if (strcmp(p->one->path, p->two->path)) { /* rename or copy */ struct diff_filepair *dp = diff_queue(q, p->one, p->two); dp->score = p->score; - - /* if we have a later entry that is a rename/copy - * that depends on p->one, then we copy here. - * otherwise we rename it. - */ - if (!diff_needs_to_stay(&outq, i+1, p->one)) - /* this is the last one, so mark it as gone. - */ - p->one->xfrm_flags |= RENAME_SRC_GONE; } else /* otherwise it is a modified (or "stay") entry */ diff --git a/diffcore.h b/diffcore.h index c709cddc..1ffc78d0 100644 --- a/diffcore.h +++ b/diffcore.h @@ -12,8 +12,6 @@ #define DEFAULT_MINIMUM_SCORE 5000 #define RENAME_DST_MATCHED 01 -#define RENAME_SRC_GONE 02 -#define RENAME_SCORE_SHIFT 8 struct diff_filespec { unsigned char sha1[20]; diff --git a/t/t4005-diff-rename-2.sh b/t/t4005-diff-rename-2.sh index 2009aec8..24d5365a 100644 --- a/t/t4005-diff-rename-2.sh +++ b/t/t4005-diff-rename-2.sh @@ -147,14 +147,13 @@ test_expect_success \ ################################################################ # tree has COPYING and rezrov. work tree has the same COPYING and -# copy-edited COPYING.1, and unchanged rezrov. We should see -# unmodified COPYING in the output, so that downstream diff-helper can -# notice. We should not say anything about rezrov. +# copy-edited COPYING.1, and unchanged rezrov. We should not say +# anything about rezrov nor COPYING, since the revised again diff-raw +# nows how to say Copy. git-diff-cache -C $tree >current cat >expected <<\EOF :100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0603b3238a076dc6c8022aedc6648fa523a17178 C1234 COPYING COPYING.1 -:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 6ff87c4664981e4397625791c8ea3bbb5f2279a3 M COPYING EOF test_expect_success \