On 5/3/21 10:12 PM, Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@xxxxxxxxx>
>
> As documented in Documentation/technical/remembering-renames.txt, and as
> tested for in the two testcases in t6429 with "rename same file
> identically" in their description, there is one case where we need to
> have renames in one commit NOT be cached for the next commit in our
> rebase sequence -- namely, rename/rename(1to1) cases. Rather than
> specifically trying to uncache those and fix up dir_rename_counts() to
> match (which would also be valid but more work), we simply disable the
> optimization when this really rare type of rename occurs.
>
> Signed-off-by: Elijah Newren <newren@xxxxxxxxx>
> ---
> merge-ort.c | 30 +++++++++++++++++++++++++++++-
> 1 file changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/merge-ort.c b/merge-ort.c
> index 741970cd05e7..2fc98b803d1c 100644
> --- a/merge-ort.c
> +++ b/merge-ort.c
> @@ -2100,6 +2100,9 @@ static int process_renames(struct merge_options *opt,
> VERIFY_CI(side2);
>
> if (!strcmp(pathnames[1], pathnames[2])) {
> + struct rename_info *ri = &opt->priv->renames;
> + int j;
> +
> /* Both sides renamed the same way */
> assert(side1 == side2);
> memcpy(&side1->stages[0], &base->stages[0],
> @@ -2109,6 +2112,16 @@ static int process_renames(struct merge_options *opt,
> base->merged.is_null = 1;
> base->merged.clean = 1;
>
> + /*
> + * Disable remembering renames optimization;
> + * rename/rename(1to1) is incredibly rare, and
> + * just disabling the optimization is easier
> + * than purging cached_pairs,
> + * cached_target_names, and dir_rename_counts.
> + */
> + for (j = 0; j < 3; j++)
> + ri->merge_trees[j] = NULL;
> +
nit: it might be helpful to initialize ri closer to when it will
be used, just to have the value of 'ri' be easier to find when
looking at this loop.
> @@ -3896,7 +3909,22 @@ static void merge_check_renames_reusable(struct merge_options *opt,
>
> renames = &opti->renames;
> merge_trees = renames->merge_trees;
> - /* merge_trees[0..2] will only be NULL if opti is */
> +
> + /*
> + * Handle case where previous merge operation did not want cache to
> + * take effect, e.g. because rename/rename(1to1) makes it invalid.
> + */
> + if (!merge_trees[0]) {
> + assert(!merge_trees[0] && !merge_trees[1] && !merge_trees[2]);
> + renames->cached_pairs_valid_side = 0; /* neither side valid */
> + return;
> + }
> +
> + /*
> + * Handle other cases; note that merge_trees[0..2] will only
> + * be NULL if opti is, or if all three were manually set to
> + * NULL by e.g. rename/rename(1to1) handling.
> + */
> assert(merge_trees[0] && merge_trees[1] && merge_trees[2]);
Thanks,
-Stolee
