From bf0ab10fa84df6c49450a06077d1c52756d88222 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 19 Feb 2011 05:20:51 -0500 Subject: merge: improve inexact rename limit warning The warning is generated deep in the diffcore code, which means that it will come first, followed possibly by a spew of conflicts, making it hard to see. Instead, let's have diffcore pass back the information about how big the rename limit would needed to have been, and then the caller can provide a more appropriate message (and at a more appropriate time). No refactoring of other non-merge callers is necessary, because nobody else was even using the warn_on_rename_limit feature. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/diff.h b/diff.h index 0083d92..f774c9a 100644 --- a/diff.h +++ b/diff.h @@ -110,7 +110,7 @@ struct diff_options { int pickaxe_opts; int rename_score; int rename_limit; - int warn_on_too_large_rename; + int needed_rename_limit; int dirstat_percent; int setup; int abbrev; diff --git a/diffcore-rename.c b/diffcore-rename.c index df41be5..1943c46 100644 --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -493,12 +493,13 @@ void diffcore_rename(struct diff_options *options) * 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)) { - if (options->warn_on_too_large_rename) - warning("too many files (created: %d deleted: %d), skipping inexact rename detection", num_create, num_src); + options->needed_rename_limit = + num_src > num_create ? num_src : num_create; goto cleanup; } diff --git a/merge-recursive.c b/merge-recursive.c index 16c2dbe..2ecd456 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -22,6 +22,11 @@ #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) { @@ -436,12 +441,13 @@ static struct string_list *get_renames(struct merge_options *o, o->diff_rename_limit >= 0 ? o->diff_rename_limit : 500; opts.rename_score = o->rename_score; - opts.warn_on_too_large_rename = 1; opts.output_format = DIFF_FORMAT_NO_OUTPUT; if (diff_setup_done(&opts) < 0) die("diff setup failed"); diff_tree_sha1(o_tree->object.sha1, tree->object.sha1, "", &opts); diffcore_std(&opts); + if (opts.needed_rename_limit > o->needed_rename_limit) + o->needed_rename_limit = opts.needed_rename_limit; for (i = 0; i < diff_queued_diff.nr; ++i) { struct string_list_item *item; struct rename *re; @@ -1666,6 +1672,8 @@ 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); return clean; } diff --git a/merge-recursive.h b/merge-recursive.h index c8135b0..f0e0566 100644 --- a/merge-recursive.h +++ b/merge-recursive.h @@ -20,6 +20,7 @@ struct merge_options { int diff_rename_limit; int merge_rename_limit; int rename_score; + int needed_rename_limit; int call_depth; struct strbuf obuf; struct string_list current_file_set; -- cgit v0.10.2-6-g49f6 From 92c57e5c1d29db96a927e2a713c5aa4ae99ce749 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 19 Feb 2011 05:21:28 -0500 Subject: bump rename limit defaults (again) We did this once before in 5070591 (bump rename limit defaults, 2008-04-30). Back then, we were shooting for about 1 second for a diff/log calculation, and 5 seconds for a merge. There are a few new things to consider, though: 1. Average processors are faster now. 2. We've seen on the mailing list some ugly merges where not using inexact rename detection leads to many more conflicts. Merges of this size take a long time anyway, so users are probably happy to spend a little bit of time computing the renames. Let's bump the diff/merge default limits from 200/500 to 400/1000. Those are 2 seconds and 10 seconds respectively on my modern hardware. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/diff.c b/diff.c index 5422c43..869cca7 100644 --- a/diff.c +++ b/diff.c @@ -23,7 +23,7 @@ #endif static int diff_detect_rename_default; -static int diff_rename_limit_default = 200; +static int diff_rename_limit_default = 400; static int diff_suppress_blank_empty; int diff_use_color_default = -1; static const char *diff_word_regex_cfg; diff --git a/merge-recursive.c b/merge-recursive.c index 2ecd456..089aa10 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -439,7 +439,7 @@ static struct string_list *get_renames(struct merge_options *o, opts.detect_rename = DIFF_DETECT_RENAME; opts.rename_limit = o->merge_rename_limit >= 0 ? o->merge_rename_limit : o->diff_rename_limit >= 0 ? o->diff_rename_limit : - 500; + 1000; opts.rename_score = o->rename_score; opts.output_format = DIFF_FORMAT_NO_OUTPUT; if (diff_setup_done(&opts) < 0) -- cgit v0.10.2-6-g49f6 From 485445e22ab30e40a94ef5b59e46616912d01720 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 19 Feb 2011 05:21:57 -0500 Subject: commit: stop setting rename limit For its post-commit summary, commit was explicitly setting the default rename limit to 100. Presumably when the code was added, it was necessary to do so. These days, however, it will fall back properly to the diff default, and that default has long since changed from 100. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/builtin/commit.c b/builtin/commit.c index 03cff5a..9f6b3cb 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1215,7 +1215,6 @@ static void print_summary(const char *prefix, const unsigned char *sha1) get_commit_format(format.buf, &rev); rev.always_show_header = 0; rev.diffopt.detect_rename = 1; - rev.diffopt.rename_limit = 100; rev.diffopt.break_opt = 0; diff_setup_done(&rev.diffopt); -- cgit v0.10.2-6-g49f6 From 3ac942d42ebb1fb48e70d3e5a714d06396e3e2c6 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 20 Feb 2011 04:51:16 -0500 Subject: add inexact rename detection progress infrastructure We might spend many seconds doing inexact rename detection with no output. It's nice to let the user know that something is actually happening. This patch adds the infrastructure, but no callers actually turn on progress reporting. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/diff.h b/diff.h index f774c9a..9585e41 100644 --- a/diff.h +++ b/diff.h @@ -111,6 +111,7 @@ struct diff_options { int rename_score; int rename_limit; int needed_rename_limit; + int show_rename_progress; int dirstat_percent; int setup; int abbrev; diff --git a/diffcore-rename.c b/diffcore-rename.c index 1943c46..cb57f51 100644 --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -5,6 +5,7 @@ #include "diff.h" #include "diffcore.h" #include "hash.h" +#include "progress.h" /* Table of rename/copy destinations */ @@ -424,6 +425,7 @@ void diffcore_rename(struct diff_options *options) struct diff_score *mx; int i, j, rename_count; int num_create, num_src, dst_cnt; + struct progress *progress = NULL; if (!minimum_score) minimum_score = DEFAULT_RENAME_SCORE; @@ -503,6 +505,12 @@ void diffcore_rename(struct diff_options *options) goto cleanup; } + if (options->show_rename_progress) { + progress = start_progress_delay( + "Performing inexact rename detection", + rename_dst_nr * rename_src_nr, 50, 1); + } + mx = xcalloc(num_create * NUM_CANDIDATE_PER_DST, sizeof(*mx)); for (dst_cnt = i = 0; i < rename_dst_nr; i++) { struct diff_filespec *two = rename_dst[i].two; @@ -532,7 +540,9 @@ void diffcore_rename(struct diff_options *options) diff_free_filespec_blob(two); } dst_cnt++; + display_progress(progress, (i+1)*rename_src_nr); } + stop_progress(&progress); /* cost matrix sorted by most to least similar pair */ qsort(mx, dst_cnt * NUM_CANDIDATE_PER_DST, sizeof(*mx), score_compare); -- cgit v0.10.2-6-g49f6 From 99bfc6691d79eaffaaf170a8b6584bcfe97863dc Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 20 Feb 2011 04:53:21 -0500 Subject: merge: enable progress reporting for rename detection The user can enable or disable it explicitly with the new --progress, but it defaults to checking isatty(2). This works only with merge-recursive and subtree. In theory we could pass a progress flag to other strategies, but none of them support progress at this point, so let's wait until they grow such a feature before worrying about propagating it. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt index e33e0f8..b613d4e 100644 --- a/Documentation/merge-options.txt +++ b/Documentation/merge-options.txt @@ -75,9 +75,17 @@ option can be used to override --squash. ifndef::git-pull[] -q:: --quiet:: - Operate quietly. + Operate quietly. Implies --no-progress. -v:: --verbose:: Be verbose. + +--progress:: +--no-progress:: + Turn progress on/off explicitly. If neither is specified, + progress is shown if standard error is connected to a terminal. + Note that not all merge strategies may support progress + reporting. + endif::git-pull[] diff --git a/builtin/merge.c b/builtin/merge.c index 42fff38..6ce8210 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -58,6 +58,7 @@ static int option_renormalize; static int verbosity; static int allow_rerere_auto; static int abort_current_merge; +static int show_progress = -1; static struct strategy all_strategy[] = { { "recursive", DEFAULT_TWOHEAD | NO_TRIVIAL }, @@ -200,6 +201,7 @@ static struct option builtin_merge_options[] = { OPT__VERBOSITY(&verbosity), OPT_BOOLEAN(0, "abort", &abort_current_merge, "abort the current in-progress merge"), + OPT_SET_INT(0, "progress", &show_progress, "force progress reporting", 1), OPT_END() }; @@ -659,6 +661,8 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common, o.subtree_shift = ""; o.renormalize = option_renormalize; + o.show_rename_progress = + show_progress == -1 ? isatty(2) : show_progress; for (x = 0; x < xopts_nr; x++) if (parse_merge_opt(&o, xopts[x])) @@ -944,6 +948,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, builtin_merge_options, builtin_merge_usage, 0); + if (verbosity < 0 && show_progress == -1) + show_progress = 0; + if (abort_current_merge) { int nargc = 2; const char *nargv[] = {"reset", "--merge", NULL}; diff --git a/merge-recursive.c b/merge-recursive.c index 089aa10..6c8f957 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -441,6 +441,7 @@ static struct string_list *get_renames(struct merge_options *o, o->diff_rename_limit >= 0 ? o->diff_rename_limit : 1000; opts.rename_score = o->rename_score; + opts.show_rename_progress = o->show_rename_progress; opts.output_format = DIFF_FORMAT_NO_OUTPUT; if (diff_setup_done(&opts) < 0) die("diff setup failed"); diff --git a/merge-recursive.h b/merge-recursive.h index f0e0566..59d1475 100644 --- a/merge-recursive.h +++ b/merge-recursive.h @@ -21,6 +21,7 @@ struct merge_options { int merge_rename_limit; int rename_score; int needed_rename_limit; + int show_rename_progress; int call_depth; struct strbuf obuf; struct string_list current_file_set; -- cgit v0.10.2-6-g49f6 From bebd2fd77d385f198017fe297a6c79e26b2bf61c Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 20 Feb 2011 04:56:56 -0500 Subject: pull: propagate --progress to merge Now that merge understands progress, we should pass it along. While we're at it, pass along --no-progress, too. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/git-pull.sh b/git-pull.sh index eb87f49..5e8215c 100755 --- a/git-pull.sh +++ b/git-pull.sh @@ -53,6 +53,8 @@ do verbosity="$verbosity -v" ;; --progress) progress=--progress ;; + --no-progress) + progress=--no-progress ;; -n|--no-stat|--no-summary) diffstat=--no-stat ;; --stat|--summary) @@ -293,8 +295,8 @@ true) ;; *) eval="git-merge $diffstat $no_commit $squash $no_ff $ff_only" - eval="$eval $log_arg $strategy_args $merge_args" - eval="$eval \"\$merge_name\" HEAD $merge_head $verbosity" + eval="$eval $log_arg $strategy_args $merge_args $verbosity $progress" + eval="$eval \"\$merge_name\" HEAD $merge_head" ;; esac eval "exec $eval" -- cgit v0.10.2-6-g49f6