Merge branch 'jc/rename-degrade-cc-to-c'
authorJunio C Hamano <gitster@pobox.com>
Thu, 28 Apr 2011 21:11:43 +0000 (14:11 -0700)
committerJunio C Hamano <gitster@pobox.com>
Thu, 28 Apr 2011 21:11:43 +0000 (14:11 -0700)
* jc/rename-degrade-cc-to-c:
diffcore-rename: fall back to -C when -C -C busts the rename limit
diffcore-rename: record filepair for rename src
diffcore-rename: refactor "too many candidates" logic
builtin/diff.c: remove duplicated call to diff_result_code()

builtin/diff-tree.c
builtin/diff.c
builtin/log.c
diff.c
diff.h
diffcore-rename.c
merge-recursive.c
t/t4001-diff-rename.sh
index 0d2a3e9fa2023cc673f1f6d9e17d3deeca1c7e9d..be6417d166abf428d379a70b9d67f894da4641d1 100644 (file)
@@ -163,6 +163,9 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
        }
 
        if (read_stdin) {
+               int saved_nrl = 0;
+               int saved_dcctc = 0;
+
                if (opt->diffopt.detect_rename)
                        opt->diffopt.setup |= (DIFF_SETUP_USE_SIZE_CACHE |
                                               DIFF_SETUP_USE_CACHE);
@@ -173,9 +176,16 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
                                fputs(line, stdout);
                                fflush(stdout);
                        }
-                       else
+                       else {
                                diff_tree_stdin(line);
+                               if (saved_nrl < opt->diffopt.needed_rename_limit)
+                                       saved_nrl = opt->diffopt.needed_rename_limit;
+                               if (opt->diffopt.degraded_cc_to_c)
+                                       saved_dcctc = 1;
+                       }
                }
+               opt->diffopt.degraded_cc_to_c = saved_dcctc;
+               opt->diffopt.needed_rename_limit = saved_nrl;
        }
 
        return diff_result_code(&opt->diffopt, 0);
index 717fa1a3414e18cbdd4c6eb1e0785761687688d7..14bd14fce0fa6b9c9b047142d23d4a2237b57a36 100644 (file)
@@ -202,7 +202,6 @@ static void refresh_index_quietly(void)
 
 static int builtin_diff_files(struct rev_info *revs, int argc, const char **argv)
 {
-       int result;
        unsigned int options = 0;
 
        while (1 < argc && argv[1][0] == '-') {
@@ -236,8 +235,7 @@ static int builtin_diff_files(struct rev_info *revs, int argc, const char **argv
                perror("read_cache_preload");
                return -1;
        }
-       result = run_diff_files(revs, options);
-       return diff_result_code(&revs->diffopt, result);
+       return run_diff_files(revs, options);
 }
 
 int cmd_diff(int argc, const char **argv, const char *prefix)
index c6dce9b8950410b08e13c11aa940404a7fcd2b78..55abe07610bce0a959b1878be128e09906f8fea4 100644 (file)
@@ -256,6 +256,8 @@ static void finish_early_output(struct rev_info *rev)
 static int cmd_log_walk(struct rev_info *rev)
 {
        struct commit *commit;
+       int saved_nrl = 0;
+       int saved_dcctc = 0;
 
        if (rev->early_output)
                setup_early_output(rev);
@@ -286,7 +288,14 @@ static int cmd_log_walk(struct rev_info *rev)
                }
                free_commit_list(commit->parents);
                commit->parents = NULL;
+               if (saved_nrl < rev->diffopt.needed_rename_limit)
+                       saved_nrl = rev->diffopt.needed_rename_limit;
+               if (rev->diffopt.degraded_cc_to_c)
+                       saved_dcctc = 1;
        }
+       rev->diffopt.degraded_cc_to_c = saved_dcctc;
+       rev->diffopt.needed_rename_limit = saved_nrl;
+
        if (rev->diffopt.output_format & DIFF_FORMAT_CHECKDIFF &&
            DIFF_OPT_TST(&rev->diffopt, CHECK_FAILED)) {
                return 02;
diff --git a/diff.c b/diff.c
index 5376d01e1b26f0297e20f14bd8adcfefd374ab08..56758c7dca3e7529cbb050afd792425a45d05517 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -3987,6 +3987,28 @@ static int is_summary_empty(const struct diff_queue_struct *q)
        return 1;
 }
 
+static const char rename_limit_warning[] =
+"inexact rename detection was skipped due to too many files.";
+
+static const char degrade_cc_to_c_warning[] =
+"only found copies from modified paths due to too many files.";
+
+static const char rename_limit_advice[] =
+"you may want to set your %s variable to at least "
+"%d and retry the command.";
+
+void diff_warn_rename_limit(const char *varname, int needed, int degraded_cc)
+{
+       if (degraded_cc)
+               warning(degrade_cc_to_c_warning);
+       else if (needed)
+               warning(rename_limit_warning);
+       else
+               return;
+       if (0 < needed && needed < 32767)
+               warning(rename_limit_advice, varname, needed);
+}
+
 void diff_flush(struct diff_options *options)
 {
        struct diff_queue_struct *q = &diff_queued_diff;
@@ -4268,6 +4290,10 @@ void diffcore_std(struct diff_options *options)
 int diff_result_code(struct diff_options *opt, int status)
 {
        int result = 0;
+
+       diff_warn_rename_limit("diff.renamelimit",
+                              opt->needed_rename_limit,
+                              opt->degraded_cc_to_c);
        if (!DIFF_OPT_TST(opt, EXIT_WITH_STATUS) &&
            !(opt->output_format & DIFF_FORMAT_CHECKDIFF))
                return status;
diff --git a/diff.h b/diff.h
index 007a0554d4b252e83e98f5578758d51ec0c6e120..b8dde8a750b17730f2e65da395700e5bd3ee2210 100644 (file)
--- a/diff.h
+++ b/diff.h
@@ -111,6 +111,7 @@ struct diff_options {
        int rename_score;
        int rename_limit;
        int needed_rename_limit;
+       int degraded_cc_to_c;
        int show_rename_progress;
        int dirstat_percent;
        int setup;
@@ -273,6 +274,7 @@ extern void diffcore_fix_diff_index(struct diff_options *);
 
 extern int diff_queue_is_empty(void);
 extern void diff_flush(struct diff_options*);
+extern void diff_warn_rename_limit(const char *varname, int needed, int degraded_cc);
 
 /* diff-raw status letters */
 #define DIFF_STATUS_ADDED              'A'
index d40e40a3ac9c919475d6157464fa906e37b0dd80..f62587e523e1b38c2fd440780d2635677e734c4d 100644 (file)
@@ -55,22 +55,23 @@ static struct diff_rename_dst *locate_rename_dst(struct diff_filespec *two,
 
 /* Table of rename/copy src files */
 static struct diff_rename_src {
-       struct diff_filespec *one;
+       struct diff_filepair *p;
        unsigned short score; /* to remember the break score */
 } *rename_src;
 static int rename_src_nr, rename_src_alloc;
 
-static struct diff_rename_src *register_rename_src(struct diff_filespec *one,
-                                                  unsigned short score)
+static struct diff_rename_src *register_rename_src(struct diff_filepair *p)
 {
        int first, last;
+       struct diff_filespec *one = p->one;
+       unsigned short score = p->score;
 
        first = 0;
        last = rename_src_nr;
        while (last > first) {
                int next = (last + first) >> 1;
                struct diff_rename_src *src = &(rename_src[next]);
-               int cmp = strcmp(one->path, src->one->path);
+               int cmp = strcmp(one->path, src->p->one->path);
                if (!cmp)
                        return src;
                if (cmp < 0) {
@@ -90,7 +91,7 @@ static struct diff_rename_src *register_rename_src(struct diff_filespec *one,
        if (first < rename_src_nr)
                memmove(rename_src + first + 1, rename_src + first,
                        (rename_src_nr - first - 1) * sizeof(*rename_src));
-       rename_src[first].one = one;
+       rename_src[first].p = p;
        rename_src[first].score = score;
        return &(rename_src[first]);
 }
@@ -205,7 +206,7 @@ static void record_rename_pair(int dst_index, int src_index, int score)
        if (rename_dst[dst_index].pair)
                die("internal error: dst already matched.");
 
-       src = rename_src[src_index].one;
+       src = rename_src[src_index].p->one;
        src->rename_used++;
        src->count++;
 
@@ -389,7 +390,7 @@ static int find_exact_renames(struct diff_options *options)
 
        init_hash(&file_table);
        for (i = 0; i < rename_src_nr; i++)
-               insert_file_table(&file_table, -1, i, rename_src[i].one);
+               insert_file_table(&file_table, -1, i, rename_src[i].p->one);
 
        for (i = 0; i < rename_dst_nr; i++)
                insert_file_table(&file_table, 1, i, rename_dst[i].two);
@@ -419,6 +420,55 @@ static void record_if_better(struct diff_score m[], struct diff_score *o)
                m[worst] = *o;
 }
 
+/*
+ * Returns:
+ * 0 if we are under the limit;
+ * 1 if we need to disable inexact rename detection;
+ * 2 if we would be under the limit if we were given -C instead of -C -C.
+ */
+static int too_many_rename_candidates(int num_create,
+                                     struct diff_options *options)
+{
+       int rename_limit = options->rename_limit;
+       int num_src = rename_src_nr;
+       int i;
+
+       options->needed_rename_limit = 0;
+
+       /*
+        * This basically does a test for the rename matrix not
+        * growing larger than a "rename_limit" square matrix, ie:
+        *
+        *    num_create * num_src > rename_limit * rename_limit
+        *
+        * but handles the potential overflow case specially (and we
+        * assume at least 32-bit integers)
+        */
+       if (rename_limit <= 0 || rename_limit > 32767)
+               rename_limit = 32767;
+       if ((num_create <= rename_limit || num_src <= rename_limit) &&
+           (num_create * num_src <= rename_limit * rename_limit))
+               return 0;
+
+       options->needed_rename_limit =
+               num_src > num_create ? num_src : num_create;
+
+       /* Are we running under -C -C? */
+       if (!DIFF_OPT_TST(options, FIND_COPIES_HARDER))
+               return 1;
+
+       /* Would we bust the limit if we were running under -C? */
+       for (num_src = i = 0; i < rename_src_nr; i++) {
+               if (diff_unmodified_pair(rename_src[i].p))
+                       continue;
+               num_src++;
+       }
+       if ((num_create <= rename_limit || num_src <= rename_limit) &&
+           (num_create * num_src <= rename_limit * rename_limit))
+               return 2;
+       return 1;
+}
+
 static int find_renames(struct diff_score *mx, int dst_cnt, int minimum_score, int copies)
 {
        int count = 0, i;
@@ -432,7 +482,7 @@ static int find_renames(struct diff_score *mx, int dst_cnt, int minimum_score, i
                dst = &rename_dst[mx[i].dst];
                if (dst->pair)
                        continue; /* already done, either exact or fuzzy. */
-               if (!copies && rename_src[mx[i].src].one->rename_used)
+               if (!copies && rename_src[mx[i].src].p->one->rename_used)
                        continue;
                record_rename_pair(mx[i].dst, mx[i].src, mx[i].score);
                count++;
@@ -444,11 +494,10 @@ void diffcore_rename(struct diff_options *options)
 {
        int detect_rename = options->detect_rename;
        int minimum_score = options->rename_score;
-       int rename_limit = options->rename_limit;
        struct diff_queue_struct *q = &diff_queued_diff;
        struct diff_queue_struct outq;
        struct diff_score *mx;
-       int i, j, rename_count;
+       int i, j, rename_count, skip_unmodified = 0;
        int num_create, num_src, dst_cnt;
        struct progress *progress = NULL;
 
@@ -476,7 +525,7 @@ void diffcore_rename(struct diff_options *options)
                         */
                        if (p->broken_pair && !p->score)
                                p->one->rename_used++;
-                       register_rename_src(p->one, p->score);
+                       register_rename_src(p);
                }
                else if (detect_rename == DIFF_DETECT_COPY) {
                        /*
@@ -484,7 +533,7 @@ void diffcore_rename(struct diff_options *options)
                         * one, to indicate ourselves as a user.
                         */
                        p->one->rename_used++;
-                       register_rename_src(p->one, p->score);
+                       register_rename_src(p);
                }
        }
        if (rename_dst_nr == 0 || rename_src_nr == 0)
@@ -511,23 +560,15 @@ void diffcore_rename(struct diff_options *options)
        if (!num_create)
                goto cleanup;
 
-       /*
-        * This basically does a test for the rename matrix not
-        * growing larger than a "rename_limit" square matrix, ie:
-        *
-        *    num_create * num_src > rename_limit * rename_limit
-        *
-        * but handles the potential overflow case specially (and we
-        * assume at least 32-bit integers)
-        */
-       options->needed_rename_limit = 0;
-       if (rename_limit <= 0 || rename_limit > 32767)
-               rename_limit = 32767;
-       if ((num_create > rename_limit && num_src > rename_limit) ||
-           (num_create * num_src > rename_limit * rename_limit)) {
-               options->needed_rename_limit =
-                       num_src > num_create ? num_src : num_create;
+       switch (too_many_rename_candidates(num_create, options)) {
+       case 1:
                goto cleanup;
+       case 2:
+               options->degraded_cc_to_c = 1;
+               skip_unmodified = 1;
+               break;
+       default:
+               break;
        }
 
        if (options->show_rename_progress) {
@@ -549,8 +590,13 @@ void diffcore_rename(struct diff_options *options)
                        m[j].dst = -1;
 
                for (j = 0; j < rename_src_nr; j++) {
-                       struct diff_filespec *one = rename_src[j].one;
+                       struct diff_filespec *one = rename_src[j].p->one;
                        struct diff_score this_src;
+
+                       if (skip_unmodified &&
+                           diff_unmodified_pair(rename_src[j].p))
+                               continue;
+
                        this_src.score = estimate_similarity(one, two,
                                                             minimum_score);
                        this_src.name_score = basename_same(one, two);
index 7c126735535f390b54c02920f1df58acaf34532e..fb3c874ff47c949dc8df89edb320accf90bc8ac9 100644 (file)
 #include "dir.h"
 #include "submodule.h"
 
-static const char rename_limit_advice[] =
-"inexact rename detection was skipped because there were too many\n"
-"  files. You may want to set your merge.renamelimit variable to at least\n"
-"  %d and retry this merge.";
-
 static struct tree *shift_tree_object(struct tree *one, struct tree *two,
                                      const char *subtree_shift)
 {
@@ -1664,8 +1659,9 @@ int merge_recursive(struct merge_options *o,
                commit_list_insert(h2, &(*result)->parents->next);
        }
        flush_output(o);
-       if (o->needed_rename_limit)
-               warning(rename_limit_advice, o->needed_rename_limit);
+       if (show(o, 2))
+               diff_warn_rename_limit("merge.renamelimit",
+                                      o->needed_rename_limit, 0);
        return clean;
 }
 
index cad85450b78891a98a0b60af1422c62c7288265c..3dadf9b31636b862b2ab7377ce7100ebebd397e2 100755 (executable)
@@ -77,4 +77,29 @@ test_expect_success C_LOCALE_OUTPUT  'favour same basenames even with minor diff
        git show HEAD:path1 | sed "s/15/16/" > subdir/path1 &&
        git status | grep "renamed: .*path1 -> subdir/path1"'
 
+test_expect_success 'setup for many rename source candidates' '
+       git reset --hard &&
+       for i in 0 1 2 3 4 5 6 7 8 9;
+       do
+               for j in 0 1 2 3 4 5 6 7 8 9;
+               do
+                       echo "$i$j" >"path$i$j"
+               done
+       done &&
+       git add "path??" &&
+       test_tick &&
+       git commit -m "hundred" &&
+       (cat path1; echo new) >new-path &&
+       echo old >>path1 &&
+       git add new-path path1 &&
+       git diff -l 4 -C -C --cached --name-status >actual 2>actual.err &&
+       sed -e "s/^\([CM]\)[0-9]*       /\1     /" actual >actual.munged &&
+       cat >expect <<-EOF &&
+       C       path1   new-path
+       M       path1
+       EOF
+       test_cmp expect actual.munged &&
+       grep warning actual.err
+'
+
 test_done