merge-tree: add -X strategy option

From: winglovet <[email protected]>

Add merge strategy option to produce more customizable merge result such as automatically solve conflicts.

Signed-off-by winglovet <[email protected]>

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

builtin/merge-tree.c +24 -0

index 0de42aecf4b..2ec6ec0d39a 100644
@@ -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

From: winglovet <[email protected]>

Add merge strategy option to produce more customizable merge result such as automatically solve conflicts.

Signed-off-by winglovet <[email protected]>

---
merge-tree: add -X strategy option

Change-Id: I16be592262d13cebcff8726eb043f7ecdb313b76

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1565%2FWingT%2Fmerge_tree_allow_strategy_option-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1565/WingT/merge_tree_allow_strategy_option-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1565

Range-diff vs v1:

1: b843caed596 ! 1: 7d53d08381e merge-tree: add -X strategy option

     @@ builtin/merge-tree.c: int cmd_merge_tree(int argc, const char **argv, const char
        /* Handle --stdin */
        if (o.use_stdin) {
                struct strbuf buf = STRBUF_INIT;
     +
     + ## t/t4301-merge-tree-write-tree.sh ##
     +@@ t/t4301-merge-tree-write-tree.sh: test_expect_success setup '
     +  git branch side1 &&
     +  git branch side2 &&
     +  git branch side3 &&
     ++ git branch side4 &&
     +
     +  git checkout side1 &&
     +  test_write_lines 1 2 3 4 5 6 >numbers &&
     +@@ t/t4301-merge-tree-write-tree.sh: test_expect_success setup '
     +  test_tick &&
     +  git commit -m rename-numbers &&
     +
     ++ git checkout side4 &&
     ++ test_write_lines 0 1 2 3 4 5 >numbers &&
     ++ echo yo >greeting &&
     ++ git add numbers greeting &&
     ++ test_tick &&
     ++ git commit -m other-content-modifications &&
     ++
     +  git switch --orphan unrelated &&
     +  >something-else &&
     +  git add something-else &&
     +@@ t/t4301-merge-tree-write-tree.sh: test_expect_success 'Content merge and a few conflicts' '
     +  test_cmp expect actual
     + '
     +
     ++test_expect_success 'Auto resolve conflicts by "ours" stragety option' '
     ++ git checkout side1^0 &&
     ++
     ++ # make sure merge conflict exists
     ++ test_must_fail git merge side4 &&
     ++ git merge --abort &&
     ++
     ++ git merge -X ours side4 &&
     ++ git rev-parse HEAD^{tree} > expected &&
     ++
     ++    git merge-tree -X ours side1 side4 > actual &&
     ++
     ++ test_cmp expected actual
     ++'
     ++
     + test_expect_success 'Barf on misspelled option, with exit code other than 0 or 1' '
     +  # Mis-spell with single "s" instead of double "s"
     +  test_expect_code 129 git merge-tree --write-tree --mesages FOOBAR side1 side2 2>expect &&

 builtin/merge-tree.c             | 24 ++++++++++++++++++++++++
 t/t4301-merge-tree-write-tree.sh | 23 +++++++++++++++++++++++
 2 files changed, 47 insertions(+)

builtin/merge-tree.c +24 -0

index 0de42aecf4b..2ec6ec0d39a 100644
@@ -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;

t/t4301-merge-tree-write-tree.sh +23 -0

index 250f721795b..2718817628c 100755
@@ -22,6 +22,7 @@ test_expect_success setup '
        git branch side1 &&
        git branch side2 &&
        git branch side3 &&
+       git branch side4 &&
 
        git checkout side1 &&
        test_write_lines 1 2 3 4 5 6 >numbers &&

@@ -46,6 +47,13 @@ test_expect_success setup '
        test_tick &&
        git commit -m rename-numbers &&
 
+       git checkout side4 &&
+       test_write_lines 0 1 2 3 4 5 >numbers &&
+       echo yo >greeting &&
+       git add numbers greeting &&
+       test_tick &&
+       git commit -m other-content-modifications &&
+
        git switch --orphan unrelated &&
        >something-else &&
        git add something-else &&

@@ -97,6 +105,21 @@ test_expect_success 'Content merge and a few conflicts' '
        test_cmp expect actual
 '
 
+test_expect_success 'Auto resolve conflicts by "ours" stragety option' '
+       git checkout side1^0 &&
+
+       # make sure merge conflict exists
+       test_must_fail git merge side4 &&
+       git merge --abort &&
+
+       git merge -X ours side4 &&
+       git rev-parse HEAD^{tree} > expected &&
+
+    git merge-tree -X ours side1 side4 > actual &&
+
+       test_cmp expected actual
+'
+
 test_expect_success 'Barf on misspelled option, with exit code other than 0 or 1' '
        # Mis-spell with single "s" instead of double "s"
        test_expect_code 129 git merge-tree --write-tree --mesages FOOBAR side1 side2 2>expect &&

base-commit: ac83bc5054c2ac489166072334b4147ce6d0fccb --
gitgitgadget

From: winglovet <[email protected]>

Add merge strategy option to produce more customizable merge result such as automatically solve conflicts.

Signed-off-by winglovet <[email protected]>

---
merge-tree: add -X strategy option

Change-Id: I16be592262d13cebcff8726eb043f7ecdb313b76

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1565%2FWingT%2Fmerge_tree_allow_strategy_option-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1565/WingT/merge_tree_allow_strategy_option-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1565

Range-diff vs v1:

1: b843caed596 ! 1: 7d53d08381e merge-tree: add -X strategy option

     @@ builtin/merge-tree.c: int cmd_merge_tree(int argc, const char **argv, const char
        /* Handle --stdin */
        if (o.use_stdin) {
                struct strbuf buf = STRBUF_INIT;
     +
     + ## t/t4301-merge-tree-write-tree.sh ##
     +@@ t/t4301-merge-tree-write-tree.sh: test_expect_success setup '
     +  git branch side1 &&
     +  git branch side2 &&
     +  git branch side3 &&
     ++ git branch side4 &&
     +
     +  git checkout side1 &&
     +  test_write_lines 1 2 3 4 5 6 >numbers &&
     +@@ t/t4301-merge-tree-write-tree.sh: test_expect_success setup '
     +  test_tick &&
     +  git commit -m rename-numbers &&
     +
     ++ git checkout side4 &&
     ++ test_write_lines 0 1 2 3 4 5 >numbers &&
     ++ echo yo >greeting &&
     ++ git add numbers greeting &&
     ++ test_tick &&
     ++ git commit -m other-content-modifications &&
     ++
     +  git switch --orphan unrelated &&
     +  >something-else &&
     +  git add something-else &&
     +@@ t/t4301-merge-tree-write-tree.sh: test_expect_success 'Content merge and a few conflicts' '
     +  test_cmp expect actual
     + '
     +
     ++test_expect_success 'Auto resolve conflicts by "ours" stragety option' '
     ++ git checkout side1^0 &&
     ++
     ++ # make sure merge conflict exists
     ++ test_must_fail git merge side4 &&
     ++ git merge --abort &&
     ++
     ++ git merge -X ours side4 &&
     ++ git rev-parse HEAD^{tree} > expected &&
     ++
     ++    git merge-tree -X ours side1 side4 > actual &&
     ++
     ++ test_cmp expected actual
     ++'
     ++
     + test_expect_success 'Barf on misspelled option, with exit code other than 0 or 1' '
     +  # Mis-spell with single "s" instead of double "s"
     +  test_expect_code 129 git merge-tree --write-tree --mesages FOOBAR side1 side2 2>expect &&

 builtin/merge-tree.c             | 24 ++++++++++++++++++++++++
 t/t4301-merge-tree-write-tree.sh | 23 +++++++++++++++++++++++
 2 files changed, 47 insertions(+)

builtin/merge-tree.c +0 -0

index 0de42aecf4b..2ec6ec0d39a 100644

builtin/merge-tree.c +21 -0

 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;

t/t4301-merge-tree-write-tree.sh +8 -0

index 250f721795b..2718817628c 100755
@@ -22,6 +22,7 @@ test_expect_success setup '
        git branch side1 &&
        git branch side2 &&
        git branch side3 &&
+       git branch side4 &&
 
        git checkout side1 &&
        test_write_lines 1 2 3 4 5 6 >numbers &&

@@ -46,6 +47,13 @@ test_expect_success setup '
        test_tick &&
        git commit -m rename-numbers &&
 
+       git checkout side4 &&
+       test_write_lines 0 1 2 3 4 5 >numbers &&
+       echo yo >greeting &&
+       git add numbers greeting &&
+       test_tick &&
+       git commit -m other-content-modifications &&
+
        git switch --orphan unrelated &&
        >something-else &&
        git add something-else &&

From: Tang Yuyi <[email protected]>

Add merge strategy option to produce more customizable merge result such as automatically solve conflicts.

Signed-off-by Tang Yuyi <[email protected]>

---
merge-tree: add -X strategy option

Change-Id: I16be592262d13cebcff8726eb043f7ecdb313b76

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1565%2FWingT%2Fmerge_tree_allow_strategy_option-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1565/WingT/merge_tree_allow_strategy_option-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1565

Range-diff vs v2:

1: 7d53d08381e ! 1: d64a774fa7c merge-tree: add -X strategy option
@@
## Metadata ##
-Author: winglovet <[email protected]>
+Author: Tang Yuyi <[email protected]>

## Commit message ##
merge-tree: add -X strategy option
@@ Commit message
Add merge strategy option to produce more customizable merge result such as automatically solve conflicts.

- Signed-off-by: winglovet <[email protected]>
+ Signed-off-by: Tang Yuyi <[email protected]>

## builtin/merge-tree.c ##
@@
@@ t/t4301-merge-tree-write-tree.sh: test_expect_success 'Content merge and a few c
test_cmp expect actual
'

-+test_expect_success 'Auto resolve conflicts by "ours" stragety option' ' ++test_expect_success 'Auto resolve conflicts by "ours" strategy option' '
+ git checkout side1^0 &&
+
+ # make sure merge conflict exists

 builtin/merge-tree.c             | 24 ++++++++++++++++++++++++
 t/t4301-merge-tree-write-tree.sh | 23 +++++++++++++++++++++++
 2 files changed, 47 insertions(+)

builtin/merge-tree.c +24 -0

index 0de42aecf4b..2ec6ec0d39a 100644
@@ -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;

t/t4301-merge-tree-write-tree.sh +23 -0

index 250f721795b..4125bb101ec 100755
@@ -22,6 +22,7 @@ test_expect_success setup '
        git branch side1 &&
        git branch side2 &&
        git branch side3 &&
+       git branch side4 &&
 
        git checkout side1 &&
        test_write_lines 1 2 3 4 5 6 >numbers &&

@@ -46,6 +47,13 @@ test_expect_success setup '
        test_tick &&
        git commit -m rename-numbers &&
 
+       git checkout side4 &&
+       test_write_lines 0 1 2 3 4 5 >numbers &&
+       echo yo >greeting &&
+       git add numbers greeting &&
+       test_tick &&
+       git commit -m other-content-modifications &&
+
        git switch --orphan unrelated &&
        >something-else &&
        git add something-else &&

@@ -97,6 +105,21 @@ test_expect_success 'Content merge and a few conflicts' '
        test_cmp expect actual
 '
 
+test_expect_success 'Auto resolve conflicts by "ours" strategy option' '
+       git checkout side1^0 &&
+
+       # make sure merge conflict exists
+       test_must_fail git merge side4 &&
+       git merge --abort &&
+
+       git merge -X ours side4 &&
+       git rev-parse HEAD^{tree} > expected &&
+
+    git merge-tree -X ours side1 side4 > actual &&
+
+       test_cmp expected actual
+'
+
 test_expect_success 'Barf on misspelled option, with exit code other than 0 or 1' '
        # Mis-spell with single "s" instead of double "s"
        test_expect_code 129 git merge-tree --write-tree --mesages FOOBAR side1 side2 2>expect &&

base-commit: ac83bc5054c2ac489166072334b4147ce6d0fccb --
gitgitgadget

Thanks for your advice!
I've fixed those blocking issues.

However, regarding the global variable issue, I'm not familiar with C and git code and don't know how to solve this. I think perhaps we need something like closure to parse opt into a local variable?

Our usecase is to achieve something like 'range-diff', we first use merge-tree to merge new patchset's base commit with old patchset's source commit, then use the merge result to diff against new patchset's source commit. So we only need to make sure conflict's are handled automatically, leaving other diff features to the second step.

From: Tang Yuyi <[email protected]>

Add merge strategy option to produce more customizable merge result such as automatically solve conflicts.

Signed-off-by Tang Yuyi <[email protected]>

---
merge-tree: add -X strategy option

Change-Id: I16be592262d13cebcff8726eb043f7ecdb313b76

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1565%2FWingT%2Fmerge_tree_allow_strategy_option-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1565/WingT/merge_tree_allow_strategy_option-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1565

Range-diff vs v2:

1: 7d53d08381e ! 1: d64a774fa7c merge-tree: add -X strategy option
@@
## Metadata ##
-Author: winglovet <[email protected]>
+Author: Tang Yuyi <[email protected]>

## Commit message ##
merge-tree: add -X strategy option
@@ Commit message
Add merge strategy option to produce more customizable merge result such as automatically solve conflicts.

- Signed-off-by: winglovet <[email protected]>
+ Signed-off-by: Tang Yuyi <[email protected]>

## builtin/merge-tree.c ##
@@
@@ t/t4301-merge-tree-write-tree.sh: test_expect_success 'Content merge and a few c
test_cmp expect actual
'

-+test_expect_success 'Auto resolve conflicts by "ours" stragety option' ' ++test_expect_success 'Auto resolve conflicts by "ours" strategy option' '
+ git checkout side1^0 &&
+
+ # make sure merge conflict exists

 builtin/merge-tree.c             | 24 ++++++++++++++++++++++++
 t/t4301-merge-tree-write-tree.sh | 23 +++++++++++++++++++++++
 2 files changed, 47 insertions(+)

builtin/merge-tree.c +24 -0

index 0de42aecf4b..2ec6ec0d39a 100644
@@ -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;

t/t4301-merge-tree-write-tree.sh +9 -0

index 250f721795b..4125bb101ec 100755
@@ -22,6 +22,7 @@ test_expect_success setup '
        git branch side1 &&
        git branch side2 &&
        git branch side3 &&
+       git branch side4 &&
 
        git checkout side1 &&
        test_write_lines 1 2 3 4 5 6 >numbers &&

@@ -46,6 +47,13 @@ test_expect_success setup '
        test_tick &&
        git commit -m rename-numbers &&
 
+       git checkout side4 &&
+       test_write_lines 0 1 2 3 4 5 >numbers &&
+       echo yo >greeting &&
+       git add numbers greeting &&
+       test_tick &&
+       git commit -m other-content-modifications &&
+
        git switch --orphan unrelated &&
        >something-else &&
        git add something-else &&

@@ -97,6 +105,21 @@ test_expect_success 'Content merge and a few conflicts' '
        test_cmp expect actual
 '
 
+test_expect_success 'Auto resolve conflicts by "ours" strategy option' '

t/t4301-merge-tree-write-tree.sh +4 -0

+
+       test_cmp expected actual
+'
+
 test_expect_success 'Barf on misspelled option, with exit code other than 0 or 1' '
        # Mis-spell with single "s" instead of double "s"
        test_expect_code 129 git merge-tree --write-tree --mesages FOOBAR side1 side2 2>expect &&

base-commit: ac83bc5054c2ac489166072334b4147ce6d0fccb

From: Tang Yuyi <[email protected]>

Add merge strategy option to produce more customizable merge result such as automatically resolving conflicts.

Signed-off-by Tang Yuyi <[email protected]>

---
merge-tree: add -X strategy option

Change-Id: I16be592262d13cebcff8726eb043f7ecdb313b76

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1565%2FWingT%2Fmerge_tree_allow_strategy_option-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1565/WingT/merge_tree_allow_strategy_option-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1565

Range-diff vs v3:

1: d64a774fa7c ! 1: d2d8fcc2e9b merge-tree: add -X strategy option
@@ Commit message
merge-tree: add -X strategy option

Add merge strategy option to produce more customizable merge result such
- as automatically solve conflicts.
+ as automatically resolving conflicts.

Signed-off-by Tang Yuyi <[email protected]>

 builtin/merge-tree.c             | 24 ++++++++++++++++++++++++
 t/t4301-merge-tree-write-tree.sh | 23 +++++++++++++++++++++++
 2 files changed, 47 insertions(+)

builtin/merge-tree.c +24 -0

index 0de42aecf4b..2ec6ec0d39a 100644
@@ -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;

t/t4301-merge-tree-write-tree.sh +9 -0

index 250f721795b..4125bb101ec 100755
@@ -22,6 +22,7 @@ test_expect_success setup '
        git branch side1 &&
        git branch side2 &&
        git branch side3 &&
+       git branch side4 &&
 
        git checkout side1 &&
        test_write_lines 1 2 3 4 5 6 >numbers &&

@@ -46,6 +47,13 @@ test_expect_success setup '
        test_tick &&
        git commit -m rename-numbers &&
 
+       git checkout side4 &&
+       test_write_lines 0 1 2 3 4 5 >numbers &&
+       echo yo >greeting &&
+       git add numbers greeting &&
+       test_tick &&
+       git commit -m other-content-modifications &&
+
        git switch --orphan unrelated &&
        >something-else &&
        git add something-else &&

@@ -97,6 +105,21 @@ test_expect_success 'Content merge and a few conflicts' '
        test_cmp expect actual
 '
 
+test_expect_success 'Auto resolve conflicts by "ours" strategy option' '

From: Tang Yuyi <[email protected]>

Add merge strategy option to produce more customizable merge result such as automatically resolving conflicts.

Signed-off-by Tang Yuyi <[email protected]>

---
merge-tree: add -X strategy option

Change-Id: I16be592262d13cebcff8726eb043f7ecdb313b76

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1565%2FWingT%2Fmerge_tree_allow_strategy_option-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1565/WingT/merge_tree_allow_strategy_option-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1565

Range-diff vs v3:

1: d64a774fa7c ! 1: d2d8fcc2e9b merge-tree: add -X strategy option
@@ Commit message
merge-tree: add -X strategy option

Add merge strategy option to produce more customizable merge result such
- as automatically solve conflicts.
+ as automatically resolving conflicts.

Signed-off-by Tang Yuyi <[email protected]>

 builtin/merge-tree.c             | 24 ++++++++++++++++++++++++
 t/t4301-merge-tree-write-tree.sh | 23 +++++++++++++++++++++++
 2 files changed, 47 insertions(+)

builtin/merge-tree.c +24 -0

index 0de42aecf4b..2ec6ec0d39a 100644
@@ -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;

t/t4301-merge-tree-write-tree.sh +23 -0

index 250f721795b..4125bb101ec 100755
@@ -22,6 +22,7 @@ test_expect_success setup '
        git branch side1 &&
        git branch side2 &&
        git branch side3 &&
+       git branch side4 &&
 
        git checkout side1 &&
        test_write_lines 1 2 3 4 5 6 >numbers &&

@@ -46,6 +47,13 @@ test_expect_success setup '
        test_tick &&
        git commit -m rename-numbers &&
 
+       git checkout side4 &&
+       test_write_lines 0 1 2 3 4 5 >numbers &&
+       echo yo >greeting &&
+       git add numbers greeting &&
+       test_tick &&
+       git commit -m other-content-modifications &&
+
        git switch --orphan unrelated &&
        >something-else &&
        git add something-else &&

@@ -97,6 +105,21 @@ test_expect_success 'Content merge and a few conflicts' '
        test_cmp expect actual
 '
 
+test_expect_success 'Auto resolve conflicts by "ours" strategy option' '
+       git checkout side1^0 &&
+
+       # make sure merge conflict exists
+       test_must_fail git merge side4 &&
+       git merge --abort &&
+
+       git merge -X ours side4 &&
+       git rev-parse HEAD^{tree} > expected &&
+
+    git merge-tree -X ours side1 side4 > actual &&
+
+       test_cmp expected actual
+'
+
 test_expect_success 'Barf on misspelled option, with exit code other than 0 or 1' '
        # Mis-spell with single "s" instead of double "s"
        test_expect_code 129 git merge-tree --write-tree --mesages FOOBAR side1 side2 2>expect &&

base-commit: ac83bc5054c2ac489166072334b4147ce6d0fccb --
gitgitgadget

From: Tang Yuyi <[email protected]>

Add merge strategy option to produce more customizable merge result such as automatically resolving conflicts.

Signed-off-by Tang Yuyi <[email protected]>

---
merge-tree: add -X strategy option

Change-Id: I16be592262d13cebcff8726eb043f7ecdb313b76

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1565%2FWingT%2Fmerge_tree_allow_strategy_option-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1565/WingT/merge_tree_allow_strategy_option-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/1565

Range-diff vs v4:

1: d2d8fcc2e9b ! 1: 28d4282e0d8 merge-tree: add -X strategy option

     @@ Commit message

       ## builtin/merge-tree.c ##
      @@
     + #include "quote.h"
       #include "tree.h"
       #include "config.h"
     ++#include "strvec.h"

     -+static const char **xopts;
     -+static size_t xopts_nr, xopts_alloc;
       static int line_termination = '\n';

     - struct merge_list {
      @@ builtin/merge-tree.c: struct merge_tree_options {
        int show_messages;
        int name_only;
     @@ builtin/merge-tree.c: static int real_merge(struct merge_tree_options *o,

        opt.branch1 = branch1;
      @@ builtin/merge-tree.c: 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 };
     ++ struct strvec xopts = STRVEC_INIT;
     +  int expected_remaining_argc;
     +  int original_argc;
     +  const char *merge_base = NULL;
      @@ builtin/merge-tree.c: 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_STRVEC('X', "strategy-option", &xopts, N_("option=value"),
     ++                 N_("option for selected merge strategy")),
                OPT_END()
        };

     @@ builtin/merge-tree.c: int cmd_merge_tree(int argc, const char **argv, const char
        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]);
     ++ for (int x = 0; x < xopts.nr; x++)
     ++         if (parse_merge_opt(&o.merge_options, xopts.v[x]))
     ++                 die(_("unknown strategy option: -X%s"), xopts.v[x]);
      +
        /* Handle --stdin */
        if (o.use_stdin) {
     @@ t/t4301-merge-tree-write-tree.sh: test_expect_success 'Content merge and a few c
      + git merge --abort &&
      +
      + git merge -X ours side4 &&
     -+ git rev-parse HEAD^{tree} > expected &&
     ++ git rev-parse HEAD^{tree} >expected &&
      +
     -+    git merge-tree -X ours side1 side4 > actual &&
     ++ git merge-tree -X ours side1 side4 >actual &&
      +
      + test_cmp expected actual
      +'

 builtin/merge-tree.c             | 11 +++++++++++
 t/t4301-merge-tree-write-tree.sh | 23 +++++++++++++++++++++++
 2 files changed, 34 insertions(+)

builtin/merge-tree.c +11 -0

index 0de42aecf4b..97d0fe6c952 100644
@@ -18,6 +18,7 @@
 #include "quote.h"
 #include "tree.h"
 #include "config.h"
+#include "strvec.h"
 
 static int line_termination = '\n';
 

@@ -414,6 +415,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 +441,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;

@@ -513,6 +517,7 @@ static int real_merge(struct merge_tree_options *o,
 int cmd_merge_tree(int argc, const char **argv, const char *prefix)
 {
        struct merge_tree_options o = { .show_messages = -1 };
+       struct strvec xopts = STRVEC_INIT;
        int expected_remaining_argc;
        int original_argc;
        const char *merge_base = NULL;

@@ -548,6 +553,8 @@ 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_STRVEC('X', "strategy-option", &xopts, N_("option=value"),
+                       N_("option for selected merge strategy")),
                OPT_END()
        };
 

@@ -556,6 +563,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.v[x]))
+                       die(_("unknown strategy option: -X%s"), xopts.v[x]);
+
        /* Handle --stdin */
        if (o.use_stdin) {
                struct strbuf buf = STRBUF_INIT;

t/t4301-merge-tree-write-tree.sh +23 -0

index 250f721795b..b2c8a43fce3 100755
@@ -22,6 +22,7 @@ test_expect_success setup '
        git branch side1 &&
        git branch side2 &&
        git branch side3 &&
+       git branch side4 &&
 
        git checkout side1 &&
        test_write_lines 1 2 3 4 5 6 >numbers &&

@@ -46,6 +47,13 @@ test_expect_success setup '
        test_tick &&
        git commit -m rename-numbers &&
 
+       git checkout side4 &&
+       test_write_lines 0 1 2 3 4 5 >numbers &&
+       echo yo >greeting &&
+       git add numbers greeting &&
+       test_tick &&
+       git commit -m other-content-modifications &&
+
        git switch --orphan unrelated &&
        >something-else &&
        git add something-else &&

@@ -97,6 +105,21 @@ test_expect_success 'Content merge and a few conflicts' '
        test_cmp expect actual
 '
 
+test_expect_success 'Auto resolve conflicts by "ours" strategy option' '
+       git checkout side1^0 &&
+
+       # make sure merge conflict exists
+       test_must_fail git merge side4 &&
+       git merge --abort &&
+
+       git merge -X ours side4 &&
+       git rev-parse HEAD^{tree} >expected &&
+
+       git merge-tree -X ours side1 side4 >actual &&
+
+       test_cmp expected actual
+'
+
 test_expect_success 'Barf on misspelled option, with exit code other than 0 or 1' '
        # Mis-spell with single "s" instead of double "s"
        test_expect_code 129 git merge-tree --write-tree --mesages FOOBAR side1 side2 2>expect &&

base-commit: ac83bc5054c2ac489166072334b4147ce6d0fccb --
gitgitgadget

From: Tang Yuyi <[email protected]>

Add merge strategy option to produce more customizable merge result such as automatically resolving conflicts.

builtin/merge-tree.c +4 -0

+	for (int x = 0; x < xopts.nr; x++)
+		if (parse_merge_opt(&o.merge_options, xopts.v[x]))
+			die(_("unknown strategy option: -X%s"), xopts.v[x]);
+
  	/* Handle --stdin */
  	if (o.use_stdin) {
  		struct strbuf buf = STRBUF_INIT;

t/t4301-merge-tree-write-tree.sh +23 -0

index 250f721795b..b2c8a43fce3 100755
@@ -22,6 +22,7 @@ test_expect_success setup '
  	git branch side1 &&
  	git branch side2 &&
  	git branch side3 &&
+	git branch side4 &&
 
  	git checkout side1 &&
  	test_write_lines 1 2 3 4 5 6 >numbers &&

@@ -46,6 +47,13 @@ test_expect_success setup '
  	test_tick &&
  	git commit -m rename-numbers &&
 
+	git checkout side4 &&
+	test_write_lines 0 1 2 3 4 5 >numbers &&
+	echo yo >greeting &&
+	git add numbers greeting &&
+	test_tick &&
+	git commit -m other-content-modifications &&
+
  	git switch --orphan unrelated &&
  	>something-else &&
  	git add something-else &&

@@ -97,6 +105,21 @@ test_expect_success 'Content merge and a few conflicts' '
  	test_cmp expect actual
  '
 
+test_expect_success 'Auto resolve conflicts by "ours" strategy option' '
+	git checkout side1^0 &&
+
+	# make sure merge conflict exists
+	test_must_fail git merge side4 &&
+	git merge --abort &&
+
+	git merge -X ours side4 &&
+	git rev-parse HEAD^{tree} >expected &&
+
+	git merge-tree -X ours side1 side4 >actual &&
+
+	test_cmp expected actual
+'
+
  test_expect_success 'Barf on misspelled option, with exit code other than 0 or 1' '
  	# Mis-spell with single "s" instead of double "s"
  	test_expect_code 129 git merge-tree --write-tree --mesages FOOBAR side1 side2 2>expect &&

base-commit: ac83bc5054c2ac489166072334b4147ce6d0fccb

...
This is the right place to call parse_merge_opt() but I think we should first check that the user has requested a real merge rather than a trivial merge.

	if (xopts.nr && o.mode == MODE_TRIVIAL)
		die(_("--trivial-merge is incompatible with all other options"));

Otherwise if the user passes in invalid strategy option to a trivial merge they'll get an error about an invalid strategy option rather than being told --strategy-option is not supported with --trivial-merge.

Ideally there would be a preparatory patch that moves the switch statement that is below the "if(o.use_stdin)" block up to this point so we'd always have set o.mode before checking if it is a trivial merge. (I think you'd to change the code slightly when it is moved to add a check for o.use_stdin)

From: Tang Yuyi <[email protected]>

Add merge strategy option to produce more customizable merge result such as automatically resolving conflicts.

Signed-off-by Tang Yuyi <[email protected]>

---

static int real_merge(struct merge_tree_options *o,

builtin/merge-tree.c static int real_merge(struct merge_tree_options *o, · line 428

Quoted 6 lines

builtin/merge-tree.c +1 -1

@@ -423,7 +425,7 @@ static int real_merge(struct merge_tree_options *o,
  {
  	struct commit *parent1, *parent2;
  	struct commit_list *merge_bases = NULL;
-	struct merge_options opt;
+	struct merge_options opt = o->merge_options;

Copying struct merge_options by value here is unusual in our code base. I don't think it introduces a bug (there is no function to free the resources allocated in struct merge_options so we do not end up freeing them twice for example) but it would be clearer that it was safe if you did

	struct merge_options *opt = &o->merge_options;

and updated the code to reflect that opt is now a pointer or just replaced all uses of "opt" with "o->merge_options"

I agree that struct-copying is an unusual pattern, and we'd potentially run into problems with duplication. But I think it is even trickier than that here. We also go on to actually _modify_ opt in this function, assigning to various members (both directly, and I think the merge code itself will write to opt->priv).

So if we use a pointer (rather than struct assignment), those changes will persist in the merge_options struct that was passed in. Which is also weird.

Between the two, I think using a pointer is probably the least-weird. This real_merge() function is only called once, and is a static-local helper for cmd_merge_tree(). So the two functions work as a single unit, and munging "opt" is not a big deal.

Quoted 6 lines · Jeff King

I agree that struct-copying is an unusual pattern, and we'd potentially run into problems with duplication. But I think it is even trickier than that here. We also go on to actually _modify_ opt in this function, assigning to various members (both directly, and I think the merge code itself will write to opt->priv).

Quoted 8 lines

So if we use a pointer (rather than struct assignment), those changes will persist in the merge_options struct that was passed in. Which is also weird.

Between the two, I think using a pointer is probably the least-weird. This real_merge() function is only called once, and is a static-local helper for cmd_merge_tree(). So the two functions work as a single unit, and munging "opt" is not a big deal.

It is called once per --stdin input to perform many merges in a row. The most obvious "structure to pointer to structure" conversion below seems to break an assertion (which is not very surprising, as it happens inside that --stdin loop), so I am tempted to revert the whole thing for now.

Oops, I totally missed the loop around the call to real_merge(). So yeah, I think this is rather tricky.
...
I don't think there are any bugs with the state at the current tip of ty/merge-tree-strategy-options, but if we want to make it safer, I think we have two options:

- delay the conversion of the "xopts" list into merge_options until we initialize it in real_merge(). This avoids breaking abstraction boundaries, but does mean that the sanity-checking of the options happens a little later (but not much in practice).

- provide a copy_merge_options() function, which makes this kind of "set up a template and then copy it" pattern official. It can be a struct assignment for now, but it at least alerts anybody adding new options to the notion that a deep copy might be required.

Option 1 looks something like this (a lot of the hunks are just reverting the tip of that branch, so squashed in it would be even smaller):

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.

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

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

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

merge-recursive.c void init_merge_options(struct merge_options *opt, · line 3929

Quoted 4 lines

merge-recursive.c +4 -0

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