Re: [PATCH 6/7] merge-ort: add die-not-implemented stub handle_content_merge() function

[Elijah Newren <newren@gmail.com>]

On Thu, Dec 3, 2020 at 10:40 AM Derrick Stolee <stolee@gmail.com> wrote:
>
> On 12/3/2020 10:59 AM, Elijah Newren via GitGitGadget wrote:
> > From: Elijah Newren <newren@gmail.com>
> >
> > This simplistic and weird-looking patch is here to facilitate future
> > patch submissions.  Adding this stub allows rename detection code to
> > reference it in one patch series, while a separate patch series can
> > define the implementation, and then both series can merge cleanly and
> > work nicely together at that point.
> >
> > Signed-off-by: Elijah Newren <newren@gmail.com>
> > ---
> >  merge-ort.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/merge-ort.c b/merge-ort.c
> > index e653ba35ea..e7220cbbb4 100644
> > --- a/merge-ort.c
> > +++ b/merge-ort.c
> > @@ -523,6 +523,18 @@ static int collect_merge_info(struct merge_options *opt,
> >
> >  /*** Function Grouping: functions related to threeway content merges ***/
> >
> > +static int handle_content_merge(struct merge_options *opt,
> > +                             const char *path,
> > +                             const struct version_info *o,
> > +                             const struct version_info *a,
> > +                             const struct version_info *b,
> > +                             const char *pathnames[3],
> > +                             const int extra_marker_size,
> > +                             struct version_info *result)
> > +{
> > +     die("Not yet implemented");
> > +}
> > +
> >  /*** Function Grouping: functions related to detect_and_process_renames(), ***
> >   *** which are split into directory and regular rename detection sections. ***/
> >
> > @@ -919,6 +931,8 @@ static void process_entry(struct merge_options *opt,
> >               ci->merged.clean = 0;
> >               ci->merged.result.mode = ci->stages[1].mode;
> >               oidcpy(&ci->merged.result.oid, &ci->stages[1].oid);
> > +             /* When we fix above, we'll call handle_content_merge() */
> > +             (void)handle_content_merge;
>
> I'm not exactly sure what the value is of this line. Is it just to
> make sure we have a reference to the 'static' method without actually
> calling it anywhere?

Yes; without the reference the compiler fails with an unused function
error message.  I know it's not used yet, but I really need it there,
so I have to fake the compiler out with a lame expression (take the
address of the function, cast to void, and discard the result since I
don't assign it anywhere or anything).

> "weird-looking patch" indeed! I'm more confused than anything.

In general, rename detection occurs before process_entry() and thus
process_entry() can handle the content merging.  However, some unusual
rename conflicts require multiple content merges (and possibly result
in nested conflict markers) and so the rename code needs to be able to
call handle_content_merge() for the first of those.

I really wanted to split apart the series for rename detection (12
patches), and the one for more conflict handling (10 patches),
especially since the latter series includes 6 patches for building up
handle_content_merge() (3 for regular file content merging and another
3 for submodule "merging").  The two series are nearly orthogonal, but
I had to somehow allow the rename side to call handle_content_merge()
without having both series try to introduce the same function.  Hence
this patch.