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

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 11, 2023, 22:19 UTC
Message-ID: <xmqqedi0kdjj.fsf@gitster.g>
In-Reply-To: <20231011214340.GA518221@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 5 quoted lines

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

I was alluding more about teaching "merge-tree" various optional behaviour merge_options represents. In today's patch it may be -X<options>, who knows what tomorrow's patch wants to bring "merge-tree" to feature-parity with "merge". And the first approach would mean we would add xopts today to the struct, but we will be required passing more details as we add other things.

Show 14 quoted lines

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

Yes, once we start leaking, somebody hopefully notice the lack of a call to this on the original/template copy and add one. Until then...