On Mon, Mar 8, 2021 at 4:49 AM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote:
>
>
> On Fri, Mar 05 2021, Elijah Newren via GitGitGadget wrote:
>
> > From: Elijah Newren <newren@xxxxxxxxx>
> >
> > 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@xxxxxxxxx>
> > ---
> > 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);
I would, but opt isn't defined here, and passing it in merely for an
assertion seems overboard.
> > /* 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.
Makes sense; I'll fix this up.
>
> > 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);
>
