Re: [PATCH v2 5/7] merge-ort: defer recursing into directories when merge base is matched

[Elijah Newren <newren@gmail.com>]

On Thu, Jul 15, 2021 at 7:43 AM Derrick Stolee <stolee@gmail.com> wrote:
>
> On 7/13/2021 3:33 PM, Elijah Newren via GitGitGadget wrote:
> > From: Elijah Newren <newren@gmail.com>
> ...
> > +             /*
> > +              * Check for whether we can avoid recursing due to one side
> > +              * matching the merge base.  The side that does NOT match is
> > +              * the one that might have a rename destination we need.
> > +              */
> > +             assert(!side1_matches_mbase || !side2_matches_mbase);
> > +             side = side1_matches_mbase ? MERGE_SIDE2 :
> > +                     side2_matches_mbase ? MERGE_SIDE1 : MERGE_BASE;
> > +             if (filemask == 0 && (dirmask == 2 || dirmask == 4)) {
> > +                     /*
> > +                      * Also defer recursing into new directories; set up a
> > +                      * few variables to let us do so.
> > +                      */
> > +                     ci->match_mask = (7 - dirmask);
> > +                     side = dirmask / 2;
>
> Clever.
>
> > +             }
> > +             if (renames->dir_rename_mask != 0x07 &&
> > +                 (side != MERGE_BASE) &&
>
> nit: these parens are not necessary?

Indeed; I'll remove them.

> > +                 renames->trivial_merges_okay[side] &&
> > +                 !strset_contains(&renames->target_dirs[side], pi.string)) {
>
> Does this last condition mean "this directory is not already a parent of a
> chosen rename target"?

I'm not sure what you mean by "chosen" here.  If by "chosen" you mean
"cached" (which comes from ort-perf-batch-11's caching of upstream
renames from previously cherry-picked commits), then yes.

Perhaps it's worth noting that rename detection has not yet been run
for this merge by the time we hit this logic, so the only renames we
can know are the cached ones from a previous pick.

> > +                     strintmap_set(&renames->possible_trivial_merges[side],
> > +                                   pi.string, renames->dir_rename_mask);
> > +                     renames->dir_rename_mask = prev_dir_rename_mask;
> > +                     return mask;
> > +             }
> > +
> > +             /* We need to recurse */
> >               ci->match_mask &= filemask;
> Thanks,
> -Stolee