Re: [RFC] Bump {diff,merge}.renameLimit ?

[Jeff King <peff@peff.net>]

On Mon, Jul 12, 2021 at 06:48:50PM +0200, Ævar Arnfjörð Bjarmason wrote:

> > So I've read and re-read your response multiple times, but I am still
> > not sure what you're advocating for.  I think you're either advocating
> > for rename detection to be turned off by default, or for a new
> > "unlimited" mode to be introduced and be the default (maybe even
> > redefining what the value of "0" means in order to implement this),
> > but I can't tell which.  Could you clarify?
> 
> I'm advocating for an "unlimited" default as long as we have
> progress/advice or whatever other output would direct users for whom
> it's very slow to tweaking the setting (or not).
> 
> Anyway, yes some may disagree with this stance. I'm not saying it's
> demonstrably obvious that we should have "unlimited".
> 
> I do think that it's much better if git behaves that way, i.e. that we
> don't have arbitrary limits that completely change behavior once they're
> tripped.
> 
> Better to spend more CPU, and if it's too slow for someone they can
> tweak the limits.

This seems like a reasonable view to me for merges. As noted, we already
show progress for rename detection there, so at least the use knows that
we're working.

It might be nice to provide similar advice to what we show now when the
limit kicks in.  I.e., now we say "we turned off rename detection
because this merge is huge; you can bump this config to enable it". But
we may want to say "woah, this rename detection is taking a long time;
you can skip it by tweaking this config". Probably with a much higher
limit than the progress meter kicking in (I'm thinking something like
30+ seconds).

For diffs we don't ever show progress. There are two things that make
that a bit hairy:

  1. Most diff operations use the pager, and we send stderr there. That
     gets weird with our "\r" overwriting.

  2. Traversing via git-log, etc, will show a series of diffs, which
     means an unbounded number of progress meters. And potentially we're
     doing work that doesn't match what the user is looking at in the
     pager (e.g., they are looking at commit X, but we are computing
     X~30 while the pager has buffered all points in between; showing a
     progress meter for X~30 is distracting at that point).

I posted a series back in 2011 to try to deal with these. It's in the
sub-thread at:

  https://lore.kernel.org/git/20110324174556.GA30661@sigill.intra.peff.net/

It's also been part of my "rebase forward and merge into my custom
build" strategy for the past 10+ years (even though I had totally
forgotten it existed). So you can get working current patches by
fetching:

  https://github/com/peff/git jk/rename-progress

They do work, and running:

  git -c diff.renamelimit=65535 diff v2.6.12 --raw

in linux.git is very satisfying. I think one sticking point is that
sending separate lines to stderr (outside of the pager) only looks good
if your pager is careful not to take over the terminal until there's
something to show. "less" does this, and I'd expect most traditional
pagers to do so. But there may be those that don't, so we'd probably
need another config option to give an escape hatch.

-Peff