Re: [PATCH 04/11] merge-ort: have ll_merge() calls use the attr_index for renormalization

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Mar 8, 2021, 12:49 UTC
Message-ID: <87a6rdkdxq.fsf@evledraar.gmail.com>
In-Reply-To: <cb035ac5fe4ab18b697eff42afedcab62880ceec.1614905738.git.gitgitgadget@gmail.com>

On Fri, Mar 05 2021, Elijah Newren via GitGitGadget wrote:

Show 22 quoted lines

> From: Elijah Newren <newren@gmail.com>
>
> ll_merge() needs an index when renormalization is requested.  Give it
> the special one we created exactly for that purpose.  This fixes t6418.4
> and t6418.5 under GIT_TEST_MERGE_ALGORITHM=ort.
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  merge-ort.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/merge-ort.c b/merge-ort.c
> index 028d1adcd2c9..87c553c0882c 100644
> --- a/merge-ort.c
> +++ b/merge-ort.c
> @@ -360,7 +360,7 @@ static void clear_or_reinit_internal_opts(struct merge_options_internal *opti,
>  	string_list_clear(&opti->paths_to_free, 0);
>  	opti->paths_to_free.strdup_strings = 0;
>  
> -	if (opti->attr_index.cache_nr)
> +	if (opti->attr_index.cache_nr) /* true iff opt->renormalize */
>  		discard_index(&opti->attr_index);

Perhaps instead of a comment, in that "if":

    assert(opt->renormalize);

Show 14 quoted lines

>  	/* Free memory used by various renames maps */
> @@ -988,7 +988,6 @@ static int merge_submodule(struct merge_options *opt,
>  	return 0;
>  }
>  
> -MAYBE_UNUSED
>  static void initialize_attr_index(struct merge_options *opt)
>  {
>  	/*
> @@ -1063,6 +1062,8 @@ static int merge_3way(struct merge_options *opt,
>  	char *base, *name1, *name2;
>  	int merge_status;
>  
> +	initialize_attr_index(opt);

Subjective, but I think it's more readable to move the "initialized" check in initialize_attr_index() here, so:

    if (!attr_index->initialized)
        initialize_attr_index(opt);

Saves the reader a trip to the function to see that it doesn't do anything except exit early on that flag.

Show 12 quoted lines

>  	ll_opts.renormalize = opt->renormalize;
>  	ll_opts.extra_marker_size = extra_marker_size;
>  	ll_opts.xdl_opts = opt->xdl_opts;
> @@ -1101,7 +1102,7 @@ static int merge_3way(struct merge_options *opt,
>  
>  	merge_status = ll_merge(result_buf, path, &orig, base,
>  				&src1, name1, &src2, name2,
> -				opt->repo->index, &ll_opts);
> +				&opt->priv->attr_index, &ll_opts);
>  
>  	free(base);
>  	free(name1);