I get the above message all the time when pulling all the git trees.
I'm frightened!
y:/usr/src/git26> git --version git version 1.5.5.rc1
threads / discuss / 13269
warning: too many files, skipping inexact rename detection
Subject: warning: too many files, skipping inexact rename detection
Jeff KingRe: warning: too many files, skipping inexact rename detection
On Sat, Apr 26, 2008 at 06:32:09AM -0700, Andrew Morton wrote:
> I get the above message all the time when pulling all the git trees. > > I'm frightened!
Rename detection is O(n^2), so when it looks like it will take a really long time, we skip it. This has been happening for a while, but 1.5.5 only recently started telling the user (based on some people wondering why renames weren't found during their enormous merges).
The default rename limit is 100, but you can bump it via the diff.renamelimit config option. A few tests that I did imply that 200-400 is reasonable for logging, and 800-1000 for a merge:
http://article.gmane.org/gmane.comp.version-control.git/73519
Are you running into actual problems with rename detection, or is the message just too scary and confusing?
-Peff
Re: warning: too many files, skipping inexact rename detection
On Sat, 26 Apr 2008 09:57:37 -0400 Jeff King <peff@peff.net> wrote:
Show 13 quoted lines
> On Sat, Apr 26, 2008 at 06:32:09AM -0700, Andrew Morton wrote: > > > I get the above message all the time when pulling all the git trees. > > > > I'm frightened! > > Rename detection is O(n^2), so when it looks like it will take a really > long time, we skip it. This has been happening for a while, but 1.5.5 > only recently started telling the user (based on some people wondering > why renames weren't found during their enormous merges). > > The default rename limit is 100, but you can bump it via the > diff.renamelimit config option.
<wonders how to set that>
Show 7 quoted lines
> A few tests that I did imply that > 200-400 is reasonable for logging, and 800-1000 for a merge: > > http://article.gmane.org/gmane.comp.version-control.git/73519 > > Are you running into actual problems with rename detection, or is the > message just too scary and confusing?
No observed problems, just scared!
I don't use rename detection anyway - I use git to extract plain old diffs only.
Perhaps the default should be bumped up a bit based on your measurements, dunno.
Jeff KingRe: warning: too many files, skipping inexact rename detection
On Sat, Apr 26, 2008 at 07:06:56AM -0700, Andrew Morton wrote:
> > The default rename limit is 100, but you can bump it via the > > diff.renamelimit config option. > > <wonders how to set that>
git config --global diff.renamelimit 200
(or edit your ~/.gitconfig)
Show 7 quoted lines
> > Are you running into actual problems with rename detection, or is the > > message just too scary and confusing? > > No observed problems, just scared! > > I don't use rename detection anyway - I use git to extract plain old diffs > only.
Ah, OK. If you are just doing a fast-forward merge, there is no rename detection going on as part of the merge. But the diffstat for a large is probably enough to trigger this behavior. So perhaps we should only print that message on merges.
> Perhaps the default should be bumped up a bit based on your measurements, > dunno.
Probably. I'll work up a patch for that, as well as suppressing the message on diffstat (where you really shouldn't care, and it serves only to scare users).
-Peff
Jeff King[PATCH 0/3] rename limit improvements
On Sat, Apr 26, 2008 at 10:52:36AM -0400, Jeff King wrote:
Show 6 quoted lines
> > Perhaps the default should be bumped up a bit based on your measurements, > > dunno. > > Probably. I'll work up a patch for that, as well as suppressing the > message on diffstat (where you really shouldn't care, and it serves only > to scare users).
Here's a patch series trying to improve rename limits, based on my discussion with Andrew and from some previous comments about the settings[1].
Patch 1 allows separate renamelimit values for diff vs merge. Patch 2 bumps up the default values based on some measurements I did in February. Patch 3 turns off the mostly cluttering warning message except for merges.
[1]: http://permalink.gmane.org/gmane.comp.version-control.git/73470
-Peff
Jeff King[PATCH 1/3] add merge.renamelimit config option
The point of rename limiting is to bound the amount of time we spend figuring out inexact renames. Currently we use a single value, diff.renamelimit, for all situations. However, it is probably the case that a user is willing to spend more time finding renames during a merge than they are while looking at git-log.
This patch provides a way of setting those values separately (though for backwards compatibility, merge still falls back on the diff renamelimit).
Signed-off-by: Jeff King <peff@peff.net> --- Documentation/merge-config.txt | 5 +++ builtin-merge-recursive.c | 13 +++++-- t/t6032-merge-large-rename.sh | 73 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 3 deletions(-) create mode 100755 t/t6032-merge-large-rename.sh
diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt index 9719311..48ce747 100644 --- a/Documentation/merge-config.txt +++ b/Documentation/merge-config.txt @@ -6,6 +6,11 @@ merge.log:: Whether to include summaries of merged commits in newly created merge commit messages. False by default. +merge.renameLimit:: + The number of files to consider when performing rename detection + during a merge; if not specified, defaults to the value of + diff.renameLimit. + merge.tool:: Controls which merge resolution program is used by linkgit:git-mergetool[1]. Valid built-in values are: "kdiff3", diff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c index 910c0d2..1293e3d 100644 --- a/builtin-merge-recursive.c +++ b/builtin-merge-recursive.c @@ -92,7 +92,8 @@ static struct path_list current_directory_set = {NULL, 0, 0, 1}; static int call_depth = 0; static int verbosity = 2; -static int rename_limit = -1; +static int diff_rename_limit = -1; +static int merge_rename_limit = -1; static int buffer_output = 1; static struct strbuf obuf = STRBUF_INIT; @@ -361,7 +362,9 @@ static struct path_list *get_renames(struct tree *tree, diff_setup(&opts); DIFF_OPT_SET(&opts, RECURSIVE); opts.detect_rename = DIFF_DETECT_RENAME; - opts.rename_limit = rename_limit; + opts.rename_limit = merge_rename_limit >= 0 ? merge_rename_limit : + diff_rename_limit >= 0 ? diff_rename_limit : + 100; opts.output_format = DIFF_FORMAT_NO_OUTPUT; if (diff_setup_done(&opts) < 0) die("diff setup failed"); @@ -1343,7 +1346,11 @@ static int merge_config(const char *var, const char *value) return 0; } if (!strcasecmp(var, "diff.renamelimit")) { - rename_limit = git_config_int(var, value); + diff_rename_limit = git_config_int(var, value); + return 0; + } + if (!strcasecmp(var, "merge.renamelimit")) { + merge_rename_limit = git_config_int(var, value); return 0; } return git_default_config(var, value); diff --git a/t/t6032-merge-large-rename.sh b/t/t6032-merge-large-rename.sh new file mode 100755 index 0000000..eac5eba --- /dev/null +++ b/t/t6032-merge-large-rename.sh @@ -0,0 +1,73 @@ +#!/bin/sh + +test_description='merging with large rename matrix' +. ./test-lib.sh + +count() { + i=1 + while test $i -le $1; do + echo $i + i=$(($i + 1)) + done +} + +test_expect_success 'setup (initial)' ' + touch file && + git add . && + git commit -m initial && + git tag initial +' + +make_text() { + echo $1: $2 + for i in `count 20`; do + echo $1: $i + done + echo $1: $3 +} + +test_rename() { + test_expect_success "rename ($1, $2)" ' + n='$1' + expect='$2' + git checkout -f master && + git branch -D test$n || true && + git reset --hard initial && + for i in $(count $n); do + make_text $i initial initial >$i + done && + git add . && + git commit -m add=$n && + for i in $(count $n); do + make_text $i changed initial >$i + done && + git commit -a -m change=$n && + git checkout -b test$n HEAD^ && + for i in $(count $n); do + git rm $i + make_text $i initial changed >$i.moved + done && + git add . && + git commit -m change+rename=$n && + case "$expect" in + ok) git merge master ;; + *) test_must_fail git merge master ;; + esac + ' +} + +test_rename 5 ok + +test_expect_success 'set diff.renamelimit to 4' ' + git config diff.renamelimit 4 +' +test_rename 4 ok +test_rename 5 fail + +test_expect_success 'set merge.renamelimit to 5' ' + git config merge.renamelimit 5 +' +test_rename 5 ok +test_rename 6 fail + +test_done
-- 1.5.5.1.177.g8182d.dirty
Jeff KingRe: [PATCH 1/3] add merge.renamelimit config option
On Wed, Apr 30, 2008 at 01:23:55PM -0400, Jeff King wrote:
> Documentation/merge-config.txt | 5 +++
Aside: this is based on next due to the reorganization of the merge options into merge-config.txt.
-Peff
Junio C HamanoRe: [PATCH 1/3] add merge.renamelimit config option
Jeff King <peff@peff.net> writes:
Show 15 quoted lines
> diff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c
> index 910c0d2..1293e3d 100644
> --- a/builtin-merge-recursive.c
> +++ b/builtin-merge-recursive.c
> @@ -361,7 +362,9 @@ static struct path_list *get_renames(struct tree *tree,
> diff_setup(&opts);
> DIFF_OPT_SET(&opts, RECURSIVE);
> opts.detect_rename = DIFF_DETECT_RENAME;
> - opts.rename_limit = rename_limit;
> + opts.rename_limit = merge_rename_limit >= 0 ? merge_rename_limit :
> + diff_rename_limit >= 0 ? diff_rename_limit :
> + 100;
> opts.output_format = DIFF_FORMAT_NO_OUTPUT;
> if (diff_setup_done(&opts) < 0)
> die("diff setup failed");Makes one wonder where the magic 100 comes from. Wouldn't this
opts.rename_limit = (merge_rename_limit >= 0)
? merge_rename_limit
: diff_rename_limit;be easier to maintain, with the same semantics?
Jeff KingRe: [PATCH 1/3] add merge.renamelimit config option
On Sat, May 03, 2008 at 01:15:06PM -0700, Junio C Hamano wrote:
Show 16 quoted lines
> > opts.detect_rename = DIFF_DETECT_RENAME;
> > - opts.rename_limit = rename_limit;
> > + opts.rename_limit = merge_rename_limit >= 0 ? merge_rename_limit :
> > + diff_rename_limit >= 0 ? diff_rename_limit :
> > + 100;
> > opts.output_format = DIFF_FORMAT_NO_OUTPUT;
> > if (diff_setup_done(&opts) < 0)
> > die("diff setup failed");
>
> Makes one wonder where the magic 100 comes from. Wouldn't this
>
> opts.rename_limit = (merge_rename_limit >= 0)
> ? merge_rename_limit
> : diff_rename_limit;
>
> be easier to maintain, with the same semantics?Yes, though I was being a little sneaky in perparation for 2/3, which changes the defaults differently for diff versus merge. I could maybe have put that particular change into 2/3, but it looks like you have applied this to next as-is already.
-Peff
Jeff King[PATCH 2/3] bump rename limit defaults
The current rename limit default of 100 was arbitrarily chosen. Testing[1] has shown that on modern hardware, a limit of 200 adds about a second of computation time, and a limit of 500 adds about 5 seconds of computation time.
This patch bumps the default limit to 200 for viewing diffs, and to 500 for performing a merge. The limit for generating git-status templates is set independently; we bump it up to 200 here, as well, to match the diff limit.
[1]: See <20080211113516.GB6344@coredump.intra.peff.net>
Signed-off-by: Jeff King <peff@peff.net> --- builtin-merge-recursive.c | 2 +- diff.c | 2 +- wt-status.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c index 1293e3d..3902e91 100644 --- a/builtin-merge-recursive.c +++ b/builtin-merge-recursive.c @@ -364,7 +364,7 @@ static struct path_list *get_renames(struct tree *tree, opts.detect_rename = DIFF_DETECT_RENAME; opts.rename_limit = merge_rename_limit >= 0 ? merge_rename_limit : diff_rename_limit >= 0 ? diff_rename_limit : - 100; + 500; opts.output_format = DIFF_FORMAT_NO_OUTPUT; if (diff_setup_done(&opts) < 0) die("diff setup failed"); diff --git a/diff.c b/diff.c index 3632b55..f735519 100644 --- a/diff.c +++ b/diff.c @@ -19,7 +19,7 @@ #endif static int diff_detect_rename_default; -static int diff_rename_limit_default = 100; +static int diff_rename_limit_default = 200; int diff_use_color_default = -1; static const char *external_diff_cmd_cfg; int diff_auto_refresh_index = 1; diff --git a/wt-status.c b/wt-status.c index 532b4ea..a44c543 100644 --- a/wt-status.c +++ b/wt-status.c @@ -206,7 +206,7 @@ static void wt_status_print_updated(struct wt_status *s) rev.diffopt.format_callback = wt_status_print_updated_cb; rev.diffopt.format_callback_data = s; rev.diffopt.detect_rename = 1; - rev.diffopt.rename_limit = 100; + rev.diffopt.rename_limit = 200; rev.diffopt.break_opt = 0; run_diff_index(&rev, 1); }
-- 1.5.5.1.177.g8182d.dirty
Jeff King[PATCH 3/3] diff: make "too many files" rename warning optional
In many cases, the warning ends up as clutter, because the diff is being done "behind the scenes" from the user (e.g., when generating a commit diffstat), and whether we show renames or not is not particularly interesting to the user.
However, in the case of a merge (which is what motivated the warning in the first place), it is a useful hint as to why a merge with renames might have failed.
This patch makes the warning optional based on the code calling into diffcore. We default to not showing the warning, but turn it on for merges.
Signed-off-by: Jeff King <peff@peff.net> --- This neglects the case where the user specifically does a diff asking for renames, but we turn it off. Maybe when "-M" is specified on the commandline to git-diff, we should set this option as well.
builtin-merge-recursive.c | 1 + diff.h | 1 + diffcore-rename.c | 3 ++- 3 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c index 3902e91..46e636f 100644 --- a/builtin-merge-recursive.c +++ b/builtin-merge-recursive.c @@ -365,6 +365,7 @@ static struct path_list *get_renames(struct tree *tree, opts.rename_limit = merge_rename_limit >= 0 ? merge_rename_limit : diff_rename_limit >= 0 ? diff_rename_limit : 500; + 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 --git a/diff.h b/diff.h index f2c7739..8931116 100644 --- a/diff.h +++ b/diff.h @@ -83,6 +83,7 @@ struct diff_options { int pickaxe_opts; int rename_score; int rename_limit; + int warn_on_too_large_rename; int dirstat_percent; int setup; int abbrev; diff --git a/diffcore-rename.c b/diffcore-rename.c index 1369a5e..1b2ebb4 100644 --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -492,7 +492,8 @@ void diffcore_rename(struct diff_options *options) rename_limit = 32767; if ((num_create > rename_limit && num_src > rename_limit) || (num_create * num_src > rename_limit * rename_limit)) { - warning("too many files, skipping inexact rename detection"); + if (options->warn_on_too_large_rename) + warning("too many files, skipping inexact rename detection"); goto cleanup; }
-- 1.5.5.1.177.g8182d.dirty
Ramsay JonesRe: [PATCH 3/3] diff: make "too many files" rename warning optional
Jeff King wrote:
Show 5 quoted lines
> In many cases, the warning ends up as clutter, because the > diff is being done "behind the scenes" from the user (e.g., > when generating a commit diffstat), and whether we show > renames or not is not particularly interesting to the user. >
[snip]
Show 14 quoted lines
> diff --git a/diffcore-rename.c b/diffcore-rename.c
> index 1369a5e..1b2ebb4 100644
> --- a/diffcore-rename.c
> +++ b/diffcore-rename.c
> @@ -492,7 +492,8 @@ void diffcore_rename(struct diff_options *options)
> rename_limit = 32767;
> if ((num_create > rename_limit && num_src > rename_limit) ||
> (num_create * num_src > rename_limit * rename_limit)) {
> - warning("too many files, skipping inexact rename detection");
> + if (options->warn_on_too_large_rename)
> + warning("too many files, skipping inexact rename detection");
> goto cleanup;
> }
> This will also fix the problem I had with gitk on cygwin; namely "gitk --all &" on my "git" repo pops up an error dialog (see below), after which the diff display disappears.
The error dialog shows:
--- >8 ---
warning: too many files, skipping inexact rename detection
warning: too many files, skipping inexact rename detection
while executing
"close $bdf"
(procedure "getblobdiffline" line 89)
invoked from within
"getblobdiffline file102daa00 1d6aeb410dc19893adbc0209bcf859f35ff1c7d6"
("eval" body line 1)
invoked from within
"eval $script"
(procedure "dorunq" line 9)
invoked from within
"dorunq"
("after" script)
--- 8< ---The git command issued by gitk appears to be: git diff-tree -r -p -C --no-commit-id -U3 1d6aeb410dc19893adbc0209bcf859f35ff1c7d6
However, if I type the above into bash (and include --no-pager), then the error message does not appear! Ho hum. Also, the same repo on Linux does not exhibit this problem at all.
In any event, if the above warning() call is commented out, then gitk is quite happy on cygwin as well.
All the best,
Ramsay Jones
Jeff KingRe: [PATCH 3/3] diff: make "too many files" rename warning optional
On Sat, May 03, 2008 at 06:34:31PM +0100, Ramsay Jones wrote:
Show 24 quoted lines
> This will also fix the problem I had with gitk on cygwin; namely "gitk
> --all &" on my "git" repo pops up an error dialog (see below), after
> which the diff display disappears.
>
> The error dialog shows:
> --- >8 ---
> warning: too many files, skipping inexact rename detection
> warning: too many files, skipping inexact rename detection
> while executing
> "close $bdf"
> (procedure "getblobdiffline" line 89)
> invoked from within
> "getblobdiffline file102daa00 1d6aeb410dc19893adbc0209bcf859f35ff1c7d6"
> ("eval" body line 1)
> invoked from within
> "eval $script"
> (procedure "dorunq" line 9)
> invoked from within
> "dorunq"
> ("after" script)
> --- 8< ---
>
> The git command issued by gitk appears to be:
> git diff-tree -r -p -C --no-commit-id -U3 1d6aeb410dc19893adbc0209bcf859f35ff1c7d6Hrm. Is gitk on cygwin somehow squishing stderr and stdout together? Or does gitk in general look at what happens on stderr?
Because while I am happy that removing this message fixes your problem, it is a little disconcerting to think that we can break gitk just by issuing a warning diagnostic on stderr.
-Peff
Paul MackerrasRe: [PATCH 3/3] diff: make "too many files" rename warning optional
Jeff King writes:
Show 6 quoted lines
> Hrm. Is gitk on cygwin somehow squishing stderr and stdout together? Or > does gitk in general look at what happens on stderr? > > Because while I am happy that removing this message fixes your problem, > it is a little disconcerting to think that we can break gitk just by > issuing a warning diagnostic on stderr.
It's a more general Tcl thing - if you are reading from a process, and the process writes to stderr, and the script hasn't explicitly redirected stderr, the Tcl infrastructure assumes that the process is signalling an error, even if the exit status is 0. Gitk does redirect stderr (to stdout) when it does a git reset, but not for other commands.
At the moment I don't think there is a good way in Tcl to get hold of the stderr output if a subcommand returns a non-zero exit status, but ignore it if the exit status is 0, other than by redirecting stderr to a temporary file, which has its own problems. Tcl can bundle stderr in with stdout, or ignore it, or take it as an error indication, or send it to a file.
So if git commands can avoid writing non-error messages to stderr, that will make my life easier...
Paul.
Jeff KingRe: [PATCH 3/3] diff: make "too many files" rename warning optional
On Mon, May 05, 2008 at 09:28:18AM +1000, Paul Mackerras wrote:
Show 9 quoted lines
> At the moment I don't think there is a good way in Tcl to get hold of > the stderr output if a subcommand returns a non-zero exit status, but > ignore it if the exit status is 0, other than by redirecting stderr to > a temporary file, which has its own problems. Tcl can bundle stderr > in with stdout, or ignore it, or take it as an error indication, or > send it to a file. > > So if git commands can avoid writing non-error messages to stderr, > that will make my life easier...
In that case, Junio, perhaps we should restrict this particular warning just to merge.
However, there a number of other warnings that can get printed on stderr, many of them related to reflogs. So I suspect with some reflog conditions (like a reflog that only goes back 1 day) you could end up with a tcl error for "gitk HEAD@{2.days.ago}".
-Peff
Jeff KingRe: [PATCH 3/3] diff: make "too many files" rename warning optional
On Mon, May 05, 2008 at 09:59:55AM -0400, Jeff King wrote:
> However, there a number of other warnings that can get printed on
> stderr, many of them related to reflogs. So I suspect with some reflog
> conditions (like a reflog that only goes back 1 day) you could end up
> with a tcl error for "gitk HEAD@{2.days.ago}".Indeed, I tested this:
$ git clone git://git.kernel.org/pub/scm/git/git.git
$ cd git
$ sleep 10
$ gitk HEAD@{10.seconds.ago} ;# works fine
$ gitk HEAD@{10.minutes.ago} ;# barfs due to warning-Peff
Junio C HamanoRe: [PATCH 3/3] diff: make "too many files" rename warning optional
Jeff King <peff@peff.net> writes:
Show 14 quoted lines
> On Mon, May 05, 2008 at 09:28:18AM +1000, Paul Mackerras wrote: > >> At the moment I don't think there is a good way in Tcl to get hold of >> the stderr output if a subcommand returns a non-zero exit status, but >> ignore it if the exit status is 0, other than by redirecting stderr to >> a temporary file, which has its own problems. Tcl can bundle stderr >> in with stdout, or ignore it, or take it as an error indication, or >> send it to a file. >> >> So if git commands can avoid writing non-error messages to stderr, >> that will make my life easier... > > In that case, Junio, perhaps we should restrict this particular warning > just to merge.
I am not sure if we really want to work around Tcl's braindamage like that.
There is no stdwarn or stdinfo stream and I think it is a bug on the receiving end to assume that anything that comes to stderr is an error.
Paul MackerrasRe: [PATCH 3/3] diff: make "too many files" rename warning optional
Junio C Hamano writes:
Show 5 quoted lines
> > In that case, Junio, perhaps we should restrict this particular warning > > just to merge. > > I am not sure if we really want to work around Tcl's braindamage like > that.
If this is the warning about too many files to do inexact rename detection, I find that one annoying because I don't care about that (it's just for the diffstat when doing a pull AFAIK) and I don't know how to turn it off.
> There is no stdwarn or stdinfo stream and I think it is a bug on the > receiving end to assume that anything that comes to stderr is an error.
There are apparently some programs on some platforms for which this behaviour is necessary...
Paul.
Ramsay JonesRe: [PATCH 3/3] diff: make "too many files" rename warning optional
Paul Mackerras wrote:
Show 16 quoted lines
> Jeff King writes: > >> Hrm. Is gitk on cygwin somehow squishing stderr and stdout together? Or >> does gitk in general look at what happens on stderr? >> >> Because while I am happy that removing this message fixes your problem, >> it is a little disconcerting to think that we can break gitk just by >> issuing a warning diagnostic on stderr. > > It's a more general Tcl thing - if you are reading from a process, and > the process writes to stderr, and the script hasn't explicitly > redirected stderr, the Tcl infrastructure assumes that the process is > signalling an error, even if the exit status is 0. Gitk does redirect > stderr (to stdout) when it does a git reset, but not for other > commands. >
Ah, OK. That explains it.
Actually, I seem to have a vague recollection of having had this discussion before...
All the Best,
Ramsay Jones
Ramsay JonesRe: [PATCH 3/3] diff: make "too many files" rename warning optional
Jeff King wrote:
Show 7 quoted lines
> On Sat, May 03, 2008 at 06:34:31PM +0100, Ramsay Jones wrote: >> >> The git command issued by gitk appears to be: >> git diff-tree -r -p -C --no-commit-id -U3 1d6aeb410dc19893adbc0209bcf859f35ff1c7d6 > > Hrm. Is gitk on cygwin somehow squishing stderr and stdout together? Or > does gitk in general look at what happens on stderr?
maybe. After further testing, the above "git diff-tree..." does indeed issue the warning on stderr; on both cygwin and Linux.
[I forgot to type ./git-diff-tree as I had not installed the new git yet, since it does not pass "make test". That is a different problem for another day...]
It appears that gitk on Linux is quite happy with that message on stderr, but on cygwin it chokes.
> > Because while I am happy that removing this message fixes your problem, > it is a little disconcerting to think that we can break gitk just by > issuing a warning diagnostic on stderr.
Indeed.
Also note that the warning is issued twice, since gitk issues that same command twice; viz:
$ GIT_TRACE=/home/ramsay/git-trace gitk --all & # exit asap
$ cat /home/ramsay/git-trace
trace: built-in: git 'config' '--get' 'i18n.commitencoding'
trace: built-in: git 'rev-parse' '--git-dir'
trace: built-in: git 'rev-parse' '--no-revs' '--no-flags' '--all'
trace: built-in: git 'rev-parse' '--is-inside-work-tree'
trace: built-in: git 'show-ref' '-d'
trace: built-in: git 'symbolic-ref' 'HEAD'
trace: built-in: git 'log' '--no-color' '-z' '--pretty=raw' '--topo-order' '--parents' '--boundary' '--all' '--'
trace: built-in: git 'diff-index' '--cached' 'HEAD'
trace: built-in: git 'rev-parse' '--git-dir'
trace: built-in: git 'diff-tree' '-r' '--no-commit-id' '1d6aeb410dc19893adbc0209bcf859f35ff1c7d6'
trace: built-in: git 'diff-files'
trace: built-in: git 'diff-tree' '-r' '-p' '-C' '--no-commit-id' '-U3' '1d6aeb410dc19893adbc0209bcf859f35ff1c7d6'
trace: built-in: git 'diff-tree' '-r' '-p' '-C' '--no-commit-id' '-U3' '1d6aeb410dc19893adbc0209bcf859f35ff1c7d6'
$NOTE: I get exactly the same trace on Linux and cygwin.
As a quick-fix, I added a "-l300" parameter to the above git command in ~/bin/gitk. [diff.renamelimit only affects git-diff]
All the Best,
Ramsay Jones
Junio C HamanoRe: [PATCH 3/3] diff: make "too many files" rename warning optional
Jeff King <peff@peff.net> writes:
Show 18 quoted lines
> In many cases, the warning ends up as clutter, because the > diff is being done "behind the scenes" from the user (e.g., > when generating a commit diffstat), and whether we show > renames or not is not particularly interesting to the user. > > However, in the case of a merge (which is what motivated the > warning in the first place), it is a useful hint as to why a > merge with renames might have failed. > > This patch makes the warning optional based on the code > calling into diffcore. We default to not showing the > warning, but turn it on for merges. > > Signed-off-by: Jeff King <peff@peff.net> > --- > This neglects the case where the user specifically does a diff asking > for renames, but we turn it off. Maybe when "-M" is specified on the > commandline to git-diff, we should set this option as well.
That sounds sensible. Like this?
diff --git a/diff.c b/diff.c index f735519..e8a9286 100644 --- a/diff.c +++ b/diff.c @@ -2443,6 +2443,7 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac) if ((options->rename_score = diff_scoreopt_parse(arg)) == -1) return -1; options->detect_rename = DIFF_DETECT_RENAME; + options->warn_on_too_large_rename = 1; } else if (!prefixcmp(arg, "-C")) { if (options->detect_rename == DIFF_DETECT_COPY) @@ -2450,6 +2451,7 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac) if ((options->rename_score = diff_scoreopt_parse(arg)) == -1) return -1; options->detect_rename = DIFF_DETECT_COPY; + options->warn_on_too_large_rename = 1; } else if (!strcmp(arg, "--no-renames")) options->detect_rename = 0;
Jeff KingRe: [PATCH 3/3] diff: make "too many files" rename warning optional
On Sat, May 03, 2008 at 05:10:57PM -0700, Junio C Hamano wrote:
Show 5 quoted lines
> > This neglects the case where the user specifically does a diff asking > > for renames, but we turn it off. Maybe when "-M" is specified on the > > commandline to git-diff, we should set this option as well. > > That sounds sensible. Like this?
I would have ack'd this, except it seems that the message produces some problems with gitk, which explicitly calls diff-tree with -C (see the message elsewhere in this thread from Ramsay Jones).
-Peff