Re: [PATCH 2/4] Remove silent clamp of renameLimit

> In commit 0024a5492 (Fix the rename detection limit checking; 2007-09-14),
> the renameLimit was clamped to 32767.  This appears to have been to simply
> avoid integer overflow in the following computation:
>
>    num_create * num_src <= rename_limit * rename_limit
>
> although it also could be viewed as a hardcoded bound on the amount of CPU
> time we're willing to allow users to tell git to spend on handling
> renames.  An upper bound may make sense, particularly as the computation
> is O(rename_limit^2), but only if the bound is documented and communicated
> to the user -- neither of which were true.
>
> In fact, the silent clamping of the renameLimit to a smaller value and
> lack of reporting of the needed renameLimit when it was too large made it
> appear to the user as though they had used a high enough value; however,
> git would proceed to mess up the merge or cherry-pick badly based on the
> lack of rename detection.  Some hardy folks, despite the lack of feedback
> on the correct limit to choose, were desperate enough to repeatedly retry
> their cherry-picks with increasingly larger renameLimit values (going
> orders of magnitude beyond the built-in limit of 32767), but were
> consistently met with the same failure.
>
> Although large limits can make things slow, we have users who would be
> ecstatic to have a small five file change be correctly cherry picked even
> if they have to manually specify a large limit and it took git ten minutes
> to compute it.
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  diff.c            |  2 +-
>  diffcore-rename.c | 11 ++++-------
>  2 files changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index 6fd288420b..c6597e3231 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -5524,7 +5524,7 @@ void diff_warn_rename_limit(const char *varname, int needed, int degraded_cc)
>                 warning(_(rename_limit_warning));
>         else
>                 return;
> -       if (0 < needed && needed < 32767)
> +       if (0 < needed)
>                 warning(_(rename_limit_advice), varname, needed);
>  }
>
> diff --git a/diffcore-rename.c b/diffcore-rename.c
> index 0d8c3d2ee4..7f9a463f5a 100644
> --- a/diffcore-rename.c
> +++ b/diffcore-rename.c
> @@ -391,14 +391,10 @@ static int too_many_rename_candidates(int num_create,
>          * growing larger than a "rename_limit" square matrix, ie:
>          *
>          *    num_create * num_src > rename_limit * rename_limit
> -        *
> -        * but handles the potential overflow case specially (and we
> -        * assume at least 32-bit integers)
>          */
> -       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))
> +           ((double)num_create * (double)num_src
> +            <= (double)rename_limit * (double)rename_limit))
>                 return 0;

>
>         options->needed_rename_limit =
> @@ -415,7 +411,8 @@ static int too_many_rename_candidates(int num_create,
>                 num_src++;
>         }
>         if ((num_create <= rename_limit || num_src <= rename_limit) &&
> -           (num_create * num_src <= rename_limit * rename_limit))
> +           ((double)num_create * (double)num_src
> +            <= (double)rename_limit * (double)rename_limit))
>                 return 2;
>         return 1;
>  }
> --
> 2.15.0.5.g9567be9905
>