Re: [PATCH v6] merge-tree: add -X strategy option

[Jeff King <peff@peff.net>]

On Wed, Oct 11, 2023 at 12:38:03PM -0700, Junio C Hamano wrote:

> If we have no plan and intention to extend "merge-tree" even more in
> the future, then option 1 would be the approach with least patch
> noise, and as your "something like this" shows, it is a nice and
> clean solution.  I very much like it.
> 
> But as the renovated "merge-tree" is a relatively young thing in our
> toolbox, I suspect that more and more work may want to go into it.
> And the other "official copy_merge_options()" approach would be a
> more healthy solution in the longer run, I would think.  If we were
> to go that route, we should also give an interface to free the
> resources held by the copy.

I am happy with either, as they both resolve the "merge-tree knows
intimate details about merge_options" issue. The patch I showed would
require manually passing more details down to real_merge(), which is I
guess what you are getting at with the "more work may want to go into
it".

> It is not that much code on top of the commit that is already queued
> in 'next', I suspect.  Perhaps something like this?

This looks OK, though...

> +void clear_merge_options(struct merge_options *opt UNUSED)
> +{
> +	; /* no-op as our copy is shallow right now */
> +}

Clearing is generally not just about copies, but any use of the struct.
so this invites the question of whether the original non-copy struct
should have a call to clear_merge_options() in cmd_merge_tree(). And
ditto for every other user.

I do not mind adding such calls, but of course we know that they are
currently noops (and we don't have any particular plan to change that).

-Peff