From bf0ab10fa84df6c49450a06077d1c52756d88222 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 19 Feb 2011 05:20:51 -0500 Subject: [PATCH 1/6] 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.h | 2 +- diffcore-rename.c | 5 +++-- merge-recursive.c | 10 +++++++++- merge-recursive.h | 1 + 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/diff.h b/diff.h index 0083d92438..f774c9ab51 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 df41be56de..1943c46b94 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 16c2dbeab9..2ecd456c28 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 c8135b0ec7..f0e056652f 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; From 92c57e5c1d29db96a927e2a713c5aa4ae99ce749 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 19 Feb 2011 05:21:28 -0500 Subject: [PATCH 2/6] 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.c | 2 +- merge-recursive.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/diff.c b/diff.c index 5422c43882..869cca7536 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 2ecd456c28..089aa10cc1 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) From 485445e22ab30e40a94ef5b59e46616912d01720 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 19 Feb 2011 05:21:57 -0500 Subject: [PATCH 3/6] 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 --- builtin/commit.c | 1 - 1 file changed, 1 deletion(-) diff --git a/builtin/commit.c b/builtin/commit.c index 03cff5af63..9f6b3cb82a 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); From 3ac942d42ebb1fb48e70d3e5a714d06396e3e2c6 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 20 Feb 2011 04:51:16 -0500 Subject: [PATCH 4/6] 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.h | 1 + diffcore-rename.c | 10 ++++++++++ 2 files changed, 11 insertions(+) diff --git a/diff.h b/diff.h index f774c9ab51..9585e41aea 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 1943c46b94..cb57f51272 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); From 99bfc6691d79eaffaaf170a8b6584bcfe97863dc Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 20 Feb 2011 04:53:21 -0500 Subject: [PATCH 5/6] 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 --- Documentation/merge-options.txt | 10 +++++++++- builtin/merge.c | 7 +++++++ merge-recursive.c | 1 + merge-recursive.h | 1 + 4 files changed, 18 insertions(+), 1 deletion(-) diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt index e33e0f8e11..b613d4ed08 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 42fff387e6..6ce8210501 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 089aa10cc1..6c8f957712 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 f0e056652f..59d1475be9 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; From bebd2fd77d385f198017fe297a6c79e26b2bf61c Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 20 Feb 2011 04:56:56 -0500 Subject: [PATCH 6/6] 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 --- git-pull.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/git-pull.sh b/git-pull.sh index eb87f49062..5e8215ca74 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"