Re: [RFC] rename detection: allow more renames

From: Jeff King <peff@peff.net>
Date: Nov 24, 2015, 23:33 UTC
Message-ID: <20151124233328.GA13872@sigill.intra.peff.net>
In-Reply-To: <20151113163506.GD16219@inner.h.apk.li>

On Fri, Nov 13, 2015 at 05:35:06PM +0100, Andreas Krey wrote:

Show 11 quoted lines

> The code talks about limiting the size
> of the rename matrix, but as far as I
> can see, the matrix itself never exists
> as such, and the only thing that could
> actually overflow is the computation
> for the progress indication. This
> can be fixed by reporting just the
> destinations checked instead of the
> combinations, since we only update
> the progress once per destination
> anyway.

I didn't dig in the archive, but I think we discussed the "just show progress for destinations" before. The problem you run into is that the items aren't a good indication of the amount of work. You really are doing n*m work, so if you just count "m", it can be very misleading if "n" is high (and vice versa).

Might it make more sense just to move to a larger integer size? Or perhaps to allow a higher limit for each side as long as the product of the sides does not overflow?

Show 16 quoted lines

> And we would also like to see progress when doing
> a cherry pick - in our case this takes a few minutes:
> 
>  
> diff --git a/sequencer.c b/sequencer.c
> index 3c060e0..8fce028 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -282,6 +282,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next,
>  
>  	for (xopt = opts->xopts; xopt != opts->xopts + opts->xopts_nr; xopt++)
>  		parse_merge_opt(&o, *xopt);
> +	o.show_rename_progress = isatty(2);
>  
>  	clean = merge_trees(&o,
>  			    head_tree,

I think that's sensible, for the same reasons that "merge" shows progress. I suspect the patch you show above catches "git revert", too, but if not, it should probably get the same treatment.

-Peff