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

On Mon, Mar 8, 2021 at 4:49 AM Ævar Arnfjörð Bjarmason [off-list ref] wrote:

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

quoted

From: Elijah Newren <redacted>

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 <redacted>
---
 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.

quoted

      /* 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.

quoted

      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);