On Fri, Mar 07, 2025 at 03:48:41PM +0000, Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@xxxxxxxxx>
>
> When merge-ort was written, I did not at first allow rename detection to
> be disabled, because I suspected that most folks disabling rename
> detection were doing so solely for performance reasons. Since I put a
> lot of working into providing dramatic speedups for rename detection
> performance as used by the merge machinery, I wanted to know if there
> were still real world repositories where rename detection was
> problematic from a performance perspective. We have had years now to
> collect such information, and while we never received one, waiting
> longer with the option disabled seems unlikely to help surface such
> issues at this point. Also, there has been at least one request to
> allow rename detection to be disabled for behavioral rather than
> performance reasons, so let's start heeding the config and command line
> settings.
It might be nice to provide a link to that request for more context.
> diff --git a/Documentation/merge-strategies.adoc b/Documentation/merge-strategies.adoc
> index 93822ebc4e8..59f5ae36ccb 100644
> --- a/Documentation/merge-strategies.adoc
> +++ b/Documentation/merge-strategies.adoc
> @@ -82,6 +82,11 @@ find-renames[=<n>];;
> rename-threshold=<n>;;
> Deprecated synonym for `find-renames=<n>`.
>
> +no-renames;;
> + Turn off rename detection. This overrides the `merge.renames`
> + configuration variable.
> + See also linkgit:git-diff[1] `--no-renames`.
> +
> subtree[=<path>];;
> This option is a more advanced form of 'subtree' strategy, where
> the strategy makes a guess on how two trees must be shifted to
> @@ -107,7 +112,7 @@ For a path that is a submodule, the same caution as 'ort' applies to this
> strategy.
> +
> The 'recursive' strategy takes the same options as 'ort'. However,
> -there are three additional options that 'ort' ignores (not documented
> +there are two additional options that 'ort' ignores (not documented
> above) that are potentially useful with the 'recursive' strategy:
>
> patience;;
> @@ -121,11 +126,6 @@ diff-algorithm=[patience|minimal|histogram|myers];;
> specifically uses `diff-algorithm=histogram`, while `recursive`
> defaults to the `diff.algorithm` config setting.
>
> -no-renames;;
> - Turn off rename detection. This overrides the `merge.renames`
> - configuration variable.
> - See also linkgit:git-diff[1] `--no-renames`.
> -
> resolve::
> This can only resolve two heads (i.e. the current branch
> and another branch you pulled from) using a 3-way merge
Makes sense.
> diff --git a/merge-ort.c b/merge-ort.c
> index b4ff24403a1..a6960b6a1b4 100644
> --- a/merge-ort.c
> +++ b/merge-ort.c
> @@ -3448,6 +3448,11 @@ static int detect_and_process_renames(struct merge_options *opt)
>
> if (!possible_renames(renames))
> goto cleanup;
> + if (opt->detect_renames == 0) {
> + renames->redo_after_renames = 0;
> + renames->cached_pairs_valid_side = 0;
> + goto cleanup;
> + }
>
> trace2_region_enter("merge", "regular renames", opt->repo);
> detection_run |= detect_regular_renames(opt, MERGE_SIDE1);
Do we want to add a test that demonstrates that the option works as
expected?
Patrick
