{"thread":{"id":"13269","subject":"warning: too many files, skipping inexact rename detection","startedAt":"2008-04-26T13:32:09Z","lastAt":"2008-05-12T11:15:48Z","messageCount":22,"participants":["Andrew Morton","Jeff King","Ramsay Jones","Junio C Hamano","Paul Mackerras"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"75219","messageId":"20080426063209.5615dd5e.akpm@linux-foundation.org","threadId":"13269","inReplyTo":null,"subject":"warning: too many files, skipping inexact rename detection","fromName":"Andrew Morton","fromEmail":"akpm@linux-foundation.org","sentAt":"2008-04-26T13:32:09Z","receivedAt":"2008-04-26T13:32:09Z","isPatch":false,"sender":{"key":"akpm@linux-foundation.org","avatar":null},"body":"I get the above message all the time when pulling all the git trees.\n\nI'm frightened!\n\ny:/usr/src/git26> git --version\ngit version 1.5.5.rc1\n"},{"id":"75221","messageId":"20080426135737.GA382@sigill.intra.peff.net","threadId":"13269","inReplyTo":"20080426063209.5615dd5e.akpm@linux-foundation.org","subject":"Re: warning: too many files, skipping inexact rename detection","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-04-26T13:57:37Z","receivedAt":"2008-04-26T13:57:37Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Apr 26, 2008 at 06:32:09AM -0700, Andrew Morton wrote:\n\n> I get the above message all the time when pulling all the git trees.\n> \n> I'm frightened!\n\nRename detection is O(n^2), so when it looks like it will take a really\nlong time, we skip it. This has been happening for a while, but 1.5.5\nonly recently started telling the user (based on some people wondering\nwhy renames weren't found during their enormous merges).\n\nThe default rename limit is 100, but you can bump it via the\ndiff.renamelimit config option. A few tests that I did imply that\n200-400 is reasonable for logging, and 800-1000 for a merge:\n\n  http://article.gmane.org/gmane.comp.version-control.git/73519\n\nAre you running into actual problems with rename detection, or is the\nmessage just too scary and confusing?\n\n-Peff\n"},{"id":"75222","messageId":"20080426070656.e7a01d91.akpm@linux-foundation.org","threadId":"13269","inReplyTo":"20080426135737.GA382@sigill.intra.peff.net","subject":"Re: warning: too many files, skipping inexact rename detection","fromName":"Andrew Morton","fromEmail":"akpm@linux-foundation.org","sentAt":"2008-04-26T14:06:56Z","receivedAt":"2008-04-26T14:06:56Z","isPatch":false,"sender":{"key":"akpm@linux-foundation.org","avatar":null},"body":"On Sat, 26 Apr 2008 09:57:37 -0400 Jeff King <peff@peff.net> wrote:\n\n> On Sat, Apr 26, 2008 at 06:32:09AM -0700, Andrew Morton wrote:\n> \n> > I get the above message all the time when pulling all the git trees.\n> > \n> > I'm frightened!\n> \n> Rename detection is O(n^2), so when it looks like it will take a really\n> long time, we skip it. This has been happening for a while, but 1.5.5\n> only recently started telling the user (based on some people wondering\n> why renames weren't found during their enormous merges).\n> \n> The default rename limit is 100, but you can bump it via the\n> diff.renamelimit config option.\n\n<wonders how to set that>\n\n> A few tests that I did imply that\n> 200-400 is reasonable for logging, and 800-1000 for a merge:\n> \n>   http://article.gmane.org/gmane.comp.version-control.git/73519\n> \n> Are you running into actual problems with rename detection, or is the\n> message just too scary and confusing?\n\nNo observed problems, just scared!\n\nI don't use rename detection anyway - I use git to extract plain old diffs\nonly.\n\nPerhaps the default should be bumped up a bit based on your measurements,\ndunno.\n"},{"id":"75225","messageId":"20080426145236.GA4367@sigill.intra.peff.net","threadId":"13269","inReplyTo":"20080426070656.e7a01d91.akpm@linux-foundation.org","subject":"Re: warning: too many files, skipping inexact rename detection","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-04-26T14:52:36Z","receivedAt":"2008-04-26T14:52:36Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Apr 26, 2008 at 07:06:56AM -0700, Andrew Morton wrote:\n\n> > The default rename limit is 100, but you can bump it via the\n> > diff.renamelimit config option.\n> \n> <wonders how to set that>\n\ngit config --global diff.renamelimit 200\n\n(or edit your ~/.gitconfig)\n\n> > Are you running into actual problems with rename detection, or is the\n> > message just too scary and confusing?\n> \n> No observed problems, just scared!\n> \n> I don't use rename detection anyway - I use git to extract plain old diffs\n> only.\n\nAh, OK. If you are just doing a fast-forward merge, there is no rename\ndetection going on as part of the merge. But the diffstat for a large is\nprobably enough to trigger this behavior. So perhaps we should only\nprint that message on merges.\n\n> Perhaps the default should be bumped up a bit based on your measurements,\n> dunno.\n\nProbably. I'll work up a patch for that, as well as suppressing the\nmessage on diffstat (where you really shouldn't care, and it serves only\nto scare users).\n\n-Peff\n"},{"id":"75695","messageId":"20080430172136.GA22601@sigill.intra.peff.net","threadId":"13269","inReplyTo":"20080426145236.GA4367@sigill.intra.peff.net","subject":"[PATCH 0/3] rename limit improvements","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-04-30T17:21:37Z","receivedAt":"2008-04-30T17:21:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Apr 26, 2008 at 10:52:36AM -0400, Jeff King wrote:\n\n> > Perhaps the default should be bumped up a bit based on your measurements,\n> > dunno.\n> \n> Probably. I'll work up a patch for that, as well as suppressing the\n> message on diffstat (where you really shouldn't care, and it serves only\n> to scare users).\n\nHere's a patch series trying to improve rename limits, based on my\ndiscussion with Andrew and from some previous comments about the\nsettings[1].\n\nPatch 1 allows separate renamelimit values for diff vs merge. Patch 2\nbumps up the default values based on some measurements I did in\nFebruary. Patch 3 turns off the mostly cluttering warning message except\nfor merges.\n\n[1]: http://permalink.gmane.org/gmane.comp.version-control.git/73470\n\n-Peff\n"},{"id":"75697","messageId":"20080430172354.GA23747@sigill.intra.peff.net","threadId":"13269","inReplyTo":"20080430172136.GA22601@sigill.intra.peff.net","subject":"[PATCH 1/3] add merge.renamelimit config option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-04-30T17:23:55Z","receivedAt":"2008-04-30T17:23:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The point of rename limiting is to bound the amount of time\nwe spend figuring out inexact renames. Currently we use a\nsingle value, diff.renamelimit, for all situations. However,\nit is probably the case that a user is willing to spend more\ntime finding renames during a merge than they are while\nlooking at git-log.\n\nThis patch provides a way of setting those values separately\n(though for backwards compatibility, merge still falls back\non the diff renamelimit).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n Documentation/merge-config.txt |    5 +++\n builtin-merge-recursive.c      |   13 +++++--\n t/t6032-merge-large-rename.sh  |   73 ++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 88 insertions(+), 3 deletions(-)\n create mode 100755 t/t6032-merge-large-rename.sh\n\ndiff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt\nindex 9719311..48ce747 100644\n--- a/Documentation/merge-config.txt\n+++ b/Documentation/merge-config.txt\n@@ -6,6 +6,11 @@ merge.log::\n \tWhether to include summaries of merged commits in newly created\n \tmerge commit messages. False by default.\n \n+merge.renameLimit::\n+\tThe number of files to consider when performing rename detection\n+\tduring a merge; if not specified, defaults to the value of\n+\tdiff.renameLimit.\n+\n merge.tool::\n \tControls which merge resolution program is used by\n \tlinkgit:git-mergetool[1].  Valid built-in values are: \"kdiff3\",\ndiff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c\nindex 910c0d2..1293e3d 100644\n--- a/builtin-merge-recursive.c\n+++ b/builtin-merge-recursive.c\n@@ -92,7 +92,8 @@ static struct path_list current_directory_set = {NULL, 0, 0, 1};\n \n static int call_depth = 0;\n static int verbosity = 2;\n-static int rename_limit = -1;\n+static int diff_rename_limit = -1;\n+static int merge_rename_limit = -1;\n static int buffer_output = 1;\n static struct strbuf obuf = STRBUF_INIT;\n \n@@ -361,7 +362,9 @@ static struct path_list *get_renames(struct tree *tree,\n \tdiff_setup(&opts);\n \tDIFF_OPT_SET(&opts, RECURSIVE);\n \topts.detect_rename = DIFF_DETECT_RENAME;\n-\topts.rename_limit = rename_limit;\n+\topts.rename_limit = merge_rename_limit >= 0 ? merge_rename_limit :\n+\t\t\t    diff_rename_limit >= 0 ? diff_rename_limit :\n+\t\t\t    100;\n \topts.output_format = DIFF_FORMAT_NO_OUTPUT;\n \tif (diff_setup_done(&opts) < 0)\n \t\tdie(\"diff setup failed\");\n@@ -1343,7 +1346,11 @@ static int merge_config(const char *var, const char *value)\n \t\treturn 0;\n \t}\n \tif (!strcasecmp(var, \"diff.renamelimit\")) {\n-\t\trename_limit = git_config_int(var, value);\n+\t\tdiff_rename_limit = git_config_int(var, value);\n+\t\treturn 0;\n+\t}\n+\tif (!strcasecmp(var, \"merge.renamelimit\")) {\n+\t\tmerge_rename_limit = git_config_int(var, value);\n \t\treturn 0;\n \t}\n \treturn git_default_config(var, value);\ndiff --git a/t/t6032-merge-large-rename.sh b/t/t6032-merge-large-rename.sh\nnew file mode 100755\nindex 0000000..eac5eba\n--- /dev/null\n+++ b/t/t6032-merge-large-rename.sh\n@@ -0,0 +1,73 @@\n+#!/bin/sh\n+\n+test_description='merging with large rename matrix'\n+. ./test-lib.sh\n+\n+count() {\n+\ti=1\n+\twhile test $i -le $1; do\n+\t\techo $i\n+\t\ti=$(($i + 1))\n+\tdone\n+}\n+\n+test_expect_success 'setup (initial)' '\n+\ttouch file &&\n+\tgit add . &&\n+\tgit commit -m initial &&\n+\tgit tag initial\n+'\n+\n+make_text() {\n+\techo $1: $2\n+\tfor i in `count 20`; do\n+\t\techo $1: $i\n+\tdone\n+\techo $1: $3\n+}\n+\n+test_rename() {\n+\ttest_expect_success \"rename ($1, $2)\" '\n+\tn='$1'\n+\texpect='$2'\n+\tgit checkout -f master &&\n+\tgit branch -D test$n || true &&\n+\tgit reset --hard initial &&\n+\tfor i in $(count $n); do\n+\t\tmake_text $i initial initial >$i\n+\tdone &&\n+\tgit add . &&\n+\tgit commit -m add=$n &&\n+\tfor i in $(count $n); do\n+\t\tmake_text $i changed initial >$i\n+\tdone &&\n+\tgit commit -a -m change=$n &&\n+\tgit checkout -b test$n HEAD^ &&\n+\tfor i in $(count $n); do\n+\t\tgit rm $i\n+\t\tmake_text $i initial changed >$i.moved\n+\tdone &&\n+\tgit add . &&\n+\tgit commit -m change+rename=$n &&\n+\tcase \"$expect\" in\n+\t\tok) git merge master ;;\n+\t\t *) test_must_fail git merge master ;;\n+\tesac\n+\t'\n+}\n+\n+test_rename 5 ok\n+\n+test_expect_success 'set diff.renamelimit to 4' '\n+\tgit config diff.renamelimit 4\n+'\n+test_rename 4 ok\n+test_rename 5 fail\n+\n+test_expect_success 'set merge.renamelimit to 5' '\n+\tgit config merge.renamelimit 5\n+'\n+test_rename 5 ok\n+test_rename 6 fail\n+\n+test_done\n-- \n1.5.5.1.177.g8182d.dirty\n"},{"id":"75698","messageId":"20080430172443.GB23747@sigill.intra.peff.net","threadId":"13269","inReplyTo":"20080430172136.GA22601@sigill.intra.peff.net","subject":"[PATCH 2/3] bump rename limit defaults","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-04-30T17:24:43Z","receivedAt":"2008-04-30T17:24:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The current rename limit default of 100 was arbitrarily\nchosen. Testing[1] has shown that on modern hardware, a\nlimit of 200 adds about a second of computation time, and a\nlimit of 500 adds about 5 seconds of computation time.\n\nThis patch bumps the default limit to 200 for viewing diffs,\nand to 500 for performing a merge. The limit for generating\ngit-status templates is set independently; we bump it up to\n200 here, as well, to match the diff limit.\n\n[1]: See <20080211113516.GB6344@coredump.intra.peff.net>\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin-merge-recursive.c |    2 +-\n diff.c                    |    2 +-\n wt-status.c               |    2 +-\n 3 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c\nindex 1293e3d..3902e91 100644\n--- a/builtin-merge-recursive.c\n+++ b/builtin-merge-recursive.c\n@@ -364,7 +364,7 @@ static struct path_list *get_renames(struct tree *tree,\n \topts.detect_rename = DIFF_DETECT_RENAME;\n \topts.rename_limit = merge_rename_limit >= 0 ? merge_rename_limit :\n \t\t\t    diff_rename_limit >= 0 ? diff_rename_limit :\n-\t\t\t    100;\n+\t\t\t    500;\n \topts.output_format = DIFF_FORMAT_NO_OUTPUT;\n \tif (diff_setup_done(&opts) < 0)\n \t\tdie(\"diff setup failed\");\ndiff --git a/diff.c b/diff.c\nindex 3632b55..f735519 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -19,7 +19,7 @@\n #endif\n \n static int diff_detect_rename_default;\n-static int diff_rename_limit_default = 100;\n+static int diff_rename_limit_default = 200;\n int diff_use_color_default = -1;\n static const char *external_diff_cmd_cfg;\n int diff_auto_refresh_index = 1;\ndiff --git a/wt-status.c b/wt-status.c\nindex 532b4ea..a44c543 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -206,7 +206,7 @@ static void wt_status_print_updated(struct wt_status *s)\n \trev.diffopt.format_callback = wt_status_print_updated_cb;\n \trev.diffopt.format_callback_data = s;\n \trev.diffopt.detect_rename = 1;\n-\trev.diffopt.rename_limit = 100;\n+\trev.diffopt.rename_limit = 200;\n \trev.diffopt.break_opt = 0;\n \trun_diff_index(&rev, 1);\n }\n-- \n1.5.5.1.177.g8182d.dirty\n"},{"id":"75699","messageId":"20080430172553.GC23747@sigill.intra.peff.net","threadId":"13269","inReplyTo":"20080430172136.GA22601@sigill.intra.peff.net","subject":"[PATCH 3/3] diff: make \"too many files\" rename warning optional","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-04-30T17:25:53Z","receivedAt":"2008-04-30T17:25:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"In many cases, the warning ends up as clutter, because the\ndiff is being done \"behind the scenes\" from the user (e.g.,\nwhen generating a commit diffstat), and whether we show\nrenames or not is not particularly interesting to the user.\n\nHowever, in the case of a merge (which is what motivated the\nwarning in the first place), it is a useful hint as to why a\nmerge with renames might have failed.\n\nThis patch makes the warning optional based on the code\ncalling into diffcore. We default to not showing the\nwarning, but turn it on for merges.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThis neglects the case where the user specifically does a diff asking\nfor renames, but we turn it off. Maybe when \"-M\" is specified on the\ncommandline to git-diff, we should set this option as well.\n\n builtin-merge-recursive.c |    1 +\n diff.h                    |    1 +\n diffcore-rename.c         |    3 ++-\n 3 files changed, 4 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c\nindex 3902e91..46e636f 100644\n--- a/builtin-merge-recursive.c\n+++ b/builtin-merge-recursive.c\n@@ -365,6 +365,7 @@ static struct path_list *get_renames(struct tree *tree,\n \topts.rename_limit = merge_rename_limit >= 0 ? merge_rename_limit :\n \t\t\t    diff_rename_limit >= 0 ? diff_rename_limit :\n \t\t\t    500;\n+\topts.warn_on_too_large_rename = 1;\n \topts.output_format = DIFF_FORMAT_NO_OUTPUT;\n \tif (diff_setup_done(&opts) < 0)\n \t\tdie(\"diff setup failed\");\ndiff --git a/diff.h b/diff.h\nindex f2c7739..8931116 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -83,6 +83,7 @@ struct diff_options {\n \tint pickaxe_opts;\n \tint rename_score;\n \tint rename_limit;\n+\tint warn_on_too_large_rename;\n \tint dirstat_percent;\n \tint setup;\n \tint abbrev;\ndiff --git a/diffcore-rename.c b/diffcore-rename.c\nindex 1369a5e..1b2ebb4 100644\n--- a/diffcore-rename.c\n+++ b/diffcore-rename.c\n@@ -492,7 +492,8 @@ void diffcore_rename(struct diff_options *options)\n \t\trename_limit = 32767;\n \tif ((num_create > rename_limit && num_src > rename_limit) ||\n \t    (num_create * num_src > rename_limit * rename_limit)) {\n-\t\twarning(\"too many files, skipping inexact rename detection\");\n+\t\tif (options->warn_on_too_large_rename)\n+\t\t\twarning(\"too many files, skipping inexact rename detection\");\n \t\tgoto cleanup;\n \t}\n \n-- \n1.5.5.1.177.g8182d.dirty\n"},{"id":"75705","messageId":"20080430181809.GA28902@sigill.intra.peff.net","threadId":"13269","inReplyTo":"20080430172354.GA23747@sigill.intra.peff.net","subject":"Re: [PATCH 1/3] add merge.renamelimit config option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-04-30T18:18:09Z","receivedAt":"2008-04-30T18:18:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Apr 30, 2008 at 01:23:55PM -0400, Jeff King wrote:\n\n>  Documentation/merge-config.txt |    5 +++\n\nAside: this is based on next due to the reorganization of the merge\noptions into merge-config.txt.\n\n-Peff\n"},{"id":"75949","messageId":"481CA227.1000801@ramsay1.demon.co.uk","threadId":"13269","inReplyTo":"20080430172553.GC23747@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] diff: make \"too many files\" rename warning optional","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2008-05-03T17:34:31Z","receivedAt":"2008-05-03T17:34:31Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Jeff King wrote:\n> In many cases, the warning ends up as clutter, because the\n> diff is being done \"behind the scenes\" from the user (e.g.,\n> when generating a commit diffstat), and whether we show\n> renames or not is not particularly interesting to the user.\n> \n\n[snip]\n\n> diff --git a/diffcore-rename.c b/diffcore-rename.c\n> index 1369a5e..1b2ebb4 100644\n> --- a/diffcore-rename.c\n> +++ b/diffcore-rename.c\n> @@ -492,7 +492,8 @@ void diffcore_rename(struct diff_options *options)\n>  \t\trename_limit = 32767;\n>  \tif ((num_create > rename_limit && num_src > rename_limit) ||\n>  \t    (num_create * num_src > rename_limit * rename_limit)) {\n> -\t\twarning(\"too many files, skipping inexact rename detection\");\n> +\t\tif (options->warn_on_too_large_rename)\n> +\t\t\twarning(\"too many files, skipping inexact rename detection\");\n>  \t\tgoto cleanup;\n>  \t}\n>  \n\nThis will also fix the problem I had with gitk on cygwin; namely \"gitk --all &\" on my\n\"git\" repo pops up an error dialog (see below), after which the diff display disappears.\n\nThe error dialog shows:\n--- >8 ---\nwarning: too many files, skipping inexact rename detection\nwarning: too many files, skipping inexact rename detection\n    while executing\n\"close $bdf\"\n    (procedure \"getblobdiffline\" line 89)\n    invoked from within\n\"getblobdiffline file102daa00 1d6aeb410dc19893adbc0209bcf859f35ff1c7d6\"\n    (\"eval\" body line 1)\n    invoked from within\n\"eval $script\"\n    (procedure \"dorunq\" line 9)\n    invoked from within\n\"dorunq\"\n    (\"after\" script)\n--- 8< ---\n\nThe git command issued by gitk appears to be:\n   git diff-tree -r -p -C --no-commit-id -U3 1d6aeb410dc19893adbc0209bcf859f35ff1c7d6\n\nHowever, if I type the above into bash (and include --no-pager), then the error\nmessage does not appear! Ho hum. Also, the same repo on Linux does not exhibit this\nproblem at all.\n\nIn any event, if the above warning() call is commented out, then gitk is quite happy\non cygwin as well.\n\nAll the best,\n\nRamsay Jones\n"},{"id":"75958","messageId":"7viqxvta9h.fsf@gitster.siamese.dyndns.org","threadId":"13269","inReplyTo":"20080430172354.GA23747@sigill.intra.peff.net","subject":"Re: [PATCH 1/3] add merge.renamelimit config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-03T20:15:06Z","receivedAt":"2008-05-03T20:15:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> diff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c\n> index 910c0d2..1293e3d 100644\n> --- a/builtin-merge-recursive.c\n> +++ b/builtin-merge-recursive.c\n> @@ -361,7 +362,9 @@ static struct path_list *get_renames(struct tree *tree,\n>  \tdiff_setup(&opts);\n>  \tDIFF_OPT_SET(&opts, RECURSIVE);\n>  \topts.detect_rename = DIFF_DETECT_RENAME;\n> -\topts.rename_limit = rename_limit;\n> +\topts.rename_limit = merge_rename_limit >= 0 ? merge_rename_limit :\n> +\t\t\t    diff_rename_limit >= 0 ? diff_rename_limit :\n> +\t\t\t    100;\n>  \topts.output_format = DIFF_FORMAT_NO_OUTPUT;\n>  \tif (diff_setup_done(&opts) < 0)\n>  \t\tdie(\"diff setup failed\");\n\nMakes one wonder where the magic 100 comes from.  Wouldn't this\n\n\topts.rename_limit = (merge_rename_limit >= 0)\n        \t\t  ? merge_rename_limit\n                          : diff_rename_limit;\n\nbe easier to maintain, with the same semantics?\n"},{"id":"75964","messageId":"7vy76rrkry.fsf@gitster.siamese.dyndns.org","threadId":"13269","inReplyTo":"20080430172553.GC23747@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] diff: make \"too many files\" rename warning optional","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-04T00:10:57Z","receivedAt":"2008-05-04T00:10:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> In many cases, the warning ends up as clutter, because the\n> diff is being done \"behind the scenes\" from the user (e.g.,\n> when generating a commit diffstat), and whether we show\n> renames or not is not particularly interesting to the user.\n>\n> However, in the case of a merge (which is what motivated the\n> warning in the first place), it is a useful hint as to why a\n> merge with renames might have failed.\n>\n> This patch makes the warning optional based on the code\n> calling into diffcore. We default to not showing the\n> warning, but turn it on for merges.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> This neglects the case where the user specifically does a diff asking\n> for renames, but we turn it off. Maybe when \"-M\" is specified on the\n> commandline to git-diff, we should set this option as well.\n\nThat sounds sensible.  Like this?\n\ndiff --git a/diff.c b/diff.c\nindex f735519..e8a9286 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2443,6 +2443,7 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t\tif ((options->rename_score = diff_scoreopt_parse(arg)) == -1)\n \t\t\treturn -1;\n \t\toptions->detect_rename = DIFF_DETECT_RENAME;\n+\t\toptions->warn_on_too_large_rename = 1;\n \t}\n \telse if (!prefixcmp(arg, \"-C\")) {\n \t\tif (options->detect_rename == DIFF_DETECT_COPY)\n@@ -2450,6 +2451,7 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t\tif ((options->rename_score = diff_scoreopt_parse(arg)) == -1)\n \t\t\treturn -1;\n \t\toptions->detect_rename = DIFF_DETECT_COPY;\n+\t\toptions->warn_on_too_large_rename = 1;\n \t}\n \telse if (!strcmp(arg, \"--no-renames\"))\n \t\toptions->detect_rename = 0;\n"},{"id":"76035","messageId":"20080504192015.GA13029@sigill.intra.peff.net","threadId":"13269","inReplyTo":"7vy76rrkry.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 3/3] diff: make \"too many files\" rename warning optional","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-05-04T19:20:15Z","receivedAt":"2008-05-04T19:20:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, May 03, 2008 at 05:10:57PM -0700, Junio C Hamano wrote:\n\n> > This neglects the case where the user specifically does a diff asking\n> > for renames, but we turn it off. Maybe when \"-M\" is specified on the\n> > commandline to git-diff, we should set this option as well.\n> \n> That sounds sensible.  Like this?\n\nI would have ack'd this, except it seems that the message produces some\nproblems with gitk, which explicitly calls diff-tree with -C (see the\nmessage elsewhere in this thread from Ramsay Jones).\n\n-Peff\n"},{"id":"76036","messageId":"20080504192332.GB13029@sigill.intra.peff.net","threadId":"13269","inReplyTo":"481CA227.1000801@ramsay1.demon.co.uk","subject":"Re: [PATCH 3/3] diff: make \"too many files\" rename warning optional","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-05-04T19:23:32Z","receivedAt":"2008-05-04T19:23:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, May 03, 2008 at 06:34:31PM +0100, Ramsay Jones wrote:\n\n> This will also fix the problem I had with gitk on cygwin; namely \"gitk\n> --all &\" on my \"git\" repo pops up an error dialog (see below), after\n> which the diff display disappears.\n> \n> The error dialog shows:\n> --- >8 ---\n> warning: too many files, skipping inexact rename detection\n> warning: too many files, skipping inexact rename detection\n>     while executing\n> \"close $bdf\"\n>     (procedure \"getblobdiffline\" line 89)\n>     invoked from within\n> \"getblobdiffline file102daa00 1d6aeb410dc19893adbc0209bcf859f35ff1c7d6\"\n>     (\"eval\" body line 1)\n>     invoked from within\n> \"eval $script\"\n>     (procedure \"dorunq\" line 9)\n>     invoked from within\n> \"dorunq\"\n>     (\"after\" script)\n> --- 8< ---\n> \n> The git command issued by gitk appears to be:\n>    git diff-tree -r -p -C --no-commit-id -U3 1d6aeb410dc19893adbc0209bcf859f35ff1c7d6\n\nHrm. Is gitk on cygwin somehow squishing stderr and stdout together? Or\ndoes gitk in general look at what happens on stderr?\n\nBecause while I am happy that removing this message fixes your problem,\nit is a little disconcerting to think that we can break gitk just by\nissuing a warning diagnostic on stderr.\n\n-Peff\n"},{"id":"76037","messageId":"20080504193106.GC13029@sigill.intra.peff.net","threadId":"13269","inReplyTo":"7viqxvta9h.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/3] add merge.renamelimit config option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-05-04T19:31:06Z","receivedAt":"2008-05-04T19:31:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, May 03, 2008 at 01:15:06PM -0700, Junio C Hamano wrote:\n\n> >  \topts.detect_rename = DIFF_DETECT_RENAME;\n> > -\topts.rename_limit = rename_limit;\n> > +\topts.rename_limit = merge_rename_limit >= 0 ? merge_rename_limit :\n> > +\t\t\t    diff_rename_limit >= 0 ? diff_rename_limit :\n> > +\t\t\t    100;\n> >  \topts.output_format = DIFF_FORMAT_NO_OUTPUT;\n> >  \tif (diff_setup_done(&opts) < 0)\n> >  \t\tdie(\"diff setup failed\");\n> \n> Makes one wonder where the magic 100 comes from.  Wouldn't this\n> \n> \topts.rename_limit = (merge_rename_limit >= 0)\n>         \t\t  ? merge_rename_limit\n>                           : diff_rename_limit;\n> \n> be easier to maintain, with the same semantics?\n\nYes, though I was being a little sneaky in perparation for 2/3, which\nchanges the defaults differently for diff versus merge. I could maybe\nhave put that particular change into 2/3, but it looks like you have\napplied this to next as-is already.\n\n-Peff\n"},{"id":"76054","messageId":"18462.18066.769759.585596@cargo.ozlabs.ibm.com","threadId":"13269","inReplyTo":"20080504192332.GB13029@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] diff: make \"too many files\" rename warning optional","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2008-05-04T23:28:18Z","receivedAt":"2008-05-04T23:28:18Z","isPatch":true,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Jeff King writes:\n\n> Hrm. Is gitk on cygwin somehow squishing stderr and stdout together? Or\n> does gitk in general look at what happens on stderr?\n> \n> Because while I am happy that removing this message fixes your problem,\n> it is a little disconcerting to think that we can break gitk just by\n> issuing a warning diagnostic on stderr.\n\nIt's a more general Tcl thing - if you are reading from a process, and\nthe process writes to stderr, and the script hasn't explicitly\nredirected stderr, the Tcl infrastructure assumes that the process is\nsignalling an error, even if the exit status is 0.  Gitk does redirect\nstderr (to stdout) when it does a git reset, but not for other\ncommands.\n\nAt the moment I don't think there is a good way in Tcl to get hold of\nthe stderr output if a subcommand returns a non-zero exit status, but\nignore it if the exit status is 0, other than by redirecting stderr to\na temporary file, which has its own problems.  Tcl can bundle stderr\nin with stdout, or ignore it, or take it as an error indication, or\nsend it to a file.\n\nSo if git commands can avoid writing non-error messages to stderr,\nthat will make my life easier...\n\nPaul.\n"},{"id":"76126","messageId":"20080505135954.GA17334@sigill.intra.peff.net","threadId":"13269","inReplyTo":"18462.18066.769759.585596@cargo.ozlabs.ibm.com","subject":"Re: [PATCH 3/3] diff: make \"too many files\" rename warning optional","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-05-05T13:59:55Z","receivedAt":"2008-05-05T13:59:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, May 05, 2008 at 09:28:18AM +1000, Paul Mackerras wrote:\n\n> At the moment I don't think there is a good way in Tcl to get hold of\n> the stderr output if a subcommand returns a non-zero exit status, but\n> ignore it if the exit status is 0, other than by redirecting stderr to\n> a temporary file, which has its own problems.  Tcl can bundle stderr\n> in with stdout, or ignore it, or take it as an error indication, or\n> send it to a file.\n> \n> So if git commands can avoid writing non-error messages to stderr,\n> that will make my life easier...\n\nIn that case, Junio, perhaps we should restrict this particular warning\njust to merge.\n\nHowever, there a number of other warnings that can get printed on\nstderr, many of them related to reflogs. So I suspect with some reflog\nconditions (like a reflog that only goes back 1 day) you could end up\nwith a tcl error for \"gitk HEAD@{2.days.ago}\".\n\n-Peff\n"},{"id":"76143","messageId":"20080505170208.GC17863@sigill.intra.peff.net","threadId":"13269","inReplyTo":"20080505135954.GA17334@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] diff: make \"too many files\" rename warning optional","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-05-05T17:02:09Z","receivedAt":"2008-05-05T17:02:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, May 05, 2008 at 09:59:55AM -0400, Jeff King wrote:\n\n> However, there a number of other warnings that can get printed on\n> stderr, many of them related to reflogs. So I suspect with some reflog\n> conditions (like a reflog that only goes back 1 day) you could end up\n> with a tcl error for \"gitk HEAD@{2.days.ago}\".\n\nIndeed, I tested this:\n\n  $ git clone git://git.kernel.org/pub/scm/git/git.git\n  $ cd git\n  $ sleep 10\n  $ gitk HEAD@{10.seconds.ago} ;# works fine\n  $ gitk HEAD@{10.minutes.ago} ;# barfs due to warning\n\n-Peff\n"},{"id":"76151","messageId":"7vd4o0o7dw.fsf@gitster.siamese.dyndns.org","threadId":"13269","inReplyTo":"20080505135954.GA17334@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] diff: make \"too many files\" rename warning optional","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-05T19:52:59Z","receivedAt":"2008-05-05T19:52:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Mon, May 05, 2008 at 09:28:18AM +1000, Paul Mackerras wrote:\n>\n>> At the moment I don't think there is a good way in Tcl to get hold of\n>> the stderr output if a subcommand returns a non-zero exit status, but\n>> ignore it if the exit status is 0, other than by redirecting stderr to\n>> a temporary file, which has its own problems.  Tcl can bundle stderr\n>> in with stdout, or ignore it, or take it as an error indication, or\n>> send it to a file.\n>> \n>> So if git commands can avoid writing non-error messages to stderr,\n>> that will make my life easier...\n>\n> In that case, Junio, perhaps we should restrict this particular warning\n> just to merge.\n\nI am not sure if we really want to work around Tcl's braindamage like\nthat.\n\nThere is no stdwarn or stdinfo stream and I think it is a bug on the\nreceiving end to assume that anything that comes to stderr is an error.\n"},{"id":"76227","messageId":"4820DBB3.8020009@ramsay1.demon.co.uk","threadId":"13269","inReplyTo":"20080504192332.GB13029@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] diff: make \"too many files\" rename warning optional","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2008-05-06T22:29:07Z","receivedAt":"2008-05-06T22:29:07Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Jeff King wrote:\n> On Sat, May 03, 2008 at 06:34:31PM +0100, Ramsay Jones wrote:\n>>\n>> The git command issued by gitk appears to be:\n>>    git diff-tree -r -p -C --no-commit-id -U3 1d6aeb410dc19893adbc0209bcf859f35ff1c7d6\n> \n> Hrm. Is gitk on cygwin somehow squishing stderr and stdout together? Or\n> does gitk in general look at what happens on stderr?\n\nmaybe. After further testing, the above \"git diff-tree...\" does indeed issue the\nwarning on stderr; on both cygwin and Linux.\n\n[I forgot to type ./git-diff-tree as I had not installed the new git yet, since\nit does not pass \"make test\".  That is a different problem for another day...]\n\nIt appears that gitk on Linux is quite happy with that message on stderr, but on\ncygwin it chokes.\n\n> \n> Because while I am happy that removing this message fixes your problem,\n> it is a little disconcerting to think that we can break gitk just by\n> issuing a warning diagnostic on stderr.\n\nIndeed.\n\nAlso note that the warning is issued twice, since gitk issues that same\ncommand twice; viz:\n\n    $ GIT_TRACE=/home/ramsay/git-trace gitk --all & # exit asap\n    $ cat /home/ramsay/git-trace\n    trace: built-in: git 'config' '--get' 'i18n.commitencoding'\n    trace: built-in: git 'rev-parse' '--git-dir'\n    trace: built-in: git 'rev-parse' '--no-revs' '--no-flags' '--all'\n    trace: built-in: git 'rev-parse' '--is-inside-work-tree'\n    trace: built-in: git 'show-ref' '-d'\n    trace: built-in: git 'symbolic-ref' 'HEAD'\n    trace: built-in: git 'log' '--no-color' '-z' '--pretty=raw' '--topo-order' '--parents' '--boundary' '--all' '--'\n    trace: built-in: git 'diff-index' '--cached' 'HEAD'\n    trace: built-in: git 'rev-parse' '--git-dir'\n    trace: built-in: git 'diff-tree' '-r' '--no-commit-id' '1d6aeb410dc19893adbc0209bcf859f35ff1c7d6'\n    trace: built-in: git 'diff-files'\n    trace: built-in: git 'diff-tree' '-r' '-p' '-C' '--no-commit-id' '-U3' '1d6aeb410dc19893adbc0209bcf859f35ff1c7d6'\n    trace: built-in: git 'diff-tree' '-r' '-p' '-C' '--no-commit-id' '-U3' '1d6aeb410dc19893adbc0209bcf859f35ff1c7d6'\n    $\n\nNOTE: I get exactly the same trace on Linux and cygwin.\n\nAs a quick-fix, I added a \"-l300\" parameter to the above git command in ~/bin/gitk.\n[diff.renamelimit only affects git-diff]\n\nAll the Best,\n\nRamsay Jones\n"},{"id":"76228","messageId":"4820DCBA.2050406@ramsay1.demon.co.uk","threadId":"13269","inReplyTo":"18462.18066.769759.585596@cargo.ozlabs.ibm.com","subject":"Re: [PATCH 3/3] diff: make \"too many files\" rename warning optional","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2008-05-06T22:33:30Z","receivedAt":"2008-05-06T22:33:30Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Paul Mackerras wrote:\n> Jeff King writes:\n> \n>> Hrm. Is gitk on cygwin somehow squishing stderr and stdout together? Or\n>> does gitk in general look at what happens on stderr?\n>>\n>> Because while I am happy that removing this message fixes your problem,\n>> it is a little disconcerting to think that we can break gitk just by\n>> issuing a warning diagnostic on stderr.\n> \n> It's a more general Tcl thing - if you are reading from a process, and\n> the process writes to stderr, and the script hasn't explicitly\n> redirected stderr, the Tcl infrastructure assumes that the process is\n> signalling an error, even if the exit status is 0.  Gitk does redirect\n> stderr (to stdout) when it does a git reset, but not for other\n> commands.\n> \n\nAh, OK. That explains it.\n\nActually, I seem to have a vague recollection of having had this\ndiscussion before...\n\nAll the Best,\n\nRamsay Jones\n"},{"id":"76696","messageId":"18472.9956.700793.959475@cargo.ozlabs.ibm.com","threadId":"13269","inReplyTo":"7vd4o0o7dw.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 3/3] diff: make \"too many files\" rename warning optional","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2008-05-12T11:15:48Z","receivedAt":"2008-05-12T11:15:48Z","isPatch":true,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Junio C Hamano writes:\n\n> > In that case, Junio, perhaps we should restrict this particular warning\n> > just to merge.\n> \n> I am not sure if we really want to work around Tcl's braindamage like\n> that.\n\nIf this is the warning about too many files to do inexact rename\ndetection, I find that one annoying because I don't care about that\n(it's just for the diffstat when doing a pull AFAIK) and I don't know\nhow to turn it off.\n\n> There is no stdwarn or stdinfo stream and I think it is a bug on the\n> receiving end to assume that anything that comes to stderr is an error.\n\nThere are apparently some programs on some platforms for which this\nbehaviour is necessary...\n\nPaul.\n"}]}