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

From: Junio C Hamano <[email-masked]>
Date: Sun, 06 Aug 2023 19:10:57 -0700

"Izzy via GitGitGadget" <[email-masked]> writes:

> From: winglovet <[email-masked]>
>
> Add merge strategy option to produce more customizable merge result such
> as automatically solve conflicts.
>
> Signed-off-by: winglovet <[email-masked]>

cf. Documentation/SubmittingPatches::[real-name], a part of the DCO
section of the document.

>  builtin/merge-tree.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)

A new feature should be protected by new tests to make sure it will
not be broken accidentally by others.  Probably a couple of new
tests to both t4300 and t4301?

$ git shortlog -sn --no-merges --since=6.months builtin/merge-tree.c t/t430[01]*.sh

tells me that Elijah and Ævar had been active, but by looking at the
output of

$ git log --author=Ævar --since=6.months builtin/merge-tree.c t/t430[01]*.sh

shows that the contribution by the latter was solely about code
clean-up with Coccinelle and not about code features and correctness,
so for a new-feature change like this, I'd ask comments by Elijah if
I were writing this patch.

Thanks.

> diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
> index 0de42aecf4b..2ec6ec0d39a 100644
> --- a/builtin/merge-tree.c
> +++ b/builtin/merge-tree.c
> @@ -19,6 +19,8 @@
>  #include "tree.h"
>  #include "config.h"
>  
> +static const char **xopts;
> +static size_t xopts_nr, xopts_alloc;
>  static int line_termination = '\n';
>  
>  struct merge_list {
> @@ -414,6 +416,7 @@ struct merge_tree_options {
>  	int show_messages;
>  	int name_only;
>  	int use_stdin;
> +	struct merge_options merge_options;
>  };
>  
>  static int real_merge(struct merge_tree_options *o,
> @@ -439,6 +442,8 @@ static int real_merge(struct merge_tree_options *o,
>  
>  	init_merge_options(&opt, the_repository);
>  
> +	opt.recursive_variant = o->merge_options.recursive_variant;
> +
>  	opt.show_rename_progress = 0;
>  
>  	opt.branch1 = branch1;
> @@ -510,6 +515,17 @@ static int real_merge(struct merge_tree_options *o,
>  	return !result.clean; /* result.clean < 0 handled above */
>  }
>  
> +static int option_parse_x(const struct option *opt,
> +			  const char *arg, int unset)
> +{
> +	if (unset)
> +		return 0;
> +
> +	ALLOC_GROW(xopts, xopts_nr + 1, xopts_alloc);
> +	xopts[xopts_nr++] = xstrdup(arg);
> +	return 0;
> +}
> +
>  int cmd_merge_tree(int argc, const char **argv, const char *prefix)
>  {
>  	struct merge_tree_options o = { .show_messages = -1 };
> @@ -548,6 +564,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)
>  			   &merge_base,
>  			   N_("commit"),
>  			   N_("specify a merge-base for the merge")),
> +		OPT_CALLBACK('X', "strategy-option", &xopts,
> +			N_("option=value"),
> +			N_("option for selected merge strategy"),
> +			option_parse_x),
>  		OPT_END()
>  	};
>  
> @@ -556,6 +576,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)
>  	argc = parse_options(argc, argv, prefix, mt_options,
>  			     merge_tree_usage, PARSE_OPT_STOP_AT_NON_OPTION);
>  
> +	for (int x = 0; x < xopts_nr; x++)
> +		if (parse_merge_opt(&o.merge_options, xopts[x]))
> +			die(_("unknown strategy option: -X%s"), xopts[x]);
> +
>  	/* Handle --stdin */
>  	if (o.use_stdin) {
>  		struct strbuf buf = STRBUF_INIT;
>
> base-commit: ac83bc5054c2ac489166072334b4147ce6d0fccb