{"thread":{"id":"60069","subject":"[PATCH] merge-tree: add -X strategy option","startedAt":"2023-08-05T14:24:49Z","lastAt":"2023-10-11T22:19:37Z","messageCount":27,"participants":["Izzy via GitGitGadget","Junio C Hamano","唐宇奕","Elijah Newren","Jeff King","Phillip Wood"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"480186","messageId":"pull.1565.git.1691245481977.gitgitgadget@gmail.com","threadId":"60069","inReplyTo":null,"subject":"[PATCH] merge-tree: add -X strategy option","fromName":"Izzy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-08-05T14:24:41Z","receivedAt":"2023-08-05T14:24:49Z","isPatch":true,"sender":{"key":"name:Izzy","avatar":null},"body":"From: winglovet \n\nAdd merge strategy option to produce more customizable merge result such\nas automatically solve conflicts.\n\nSigned-off-by: winglovet \n---\n merge-tree: add -X strategy option\n \n Change-Id: I16be592262d13cebcff8726eb043f7ecdb313b76\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1565%2FWingT%2Fmerge_tree_allow_strategy_option-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1565/WingT/merge_tree_allow_strategy_option-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1565\n\n builtin/merge-tree.c | 24 ++++++++++++++++++++++++\n 1 file changed, 24 insertions(+)\n\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex 0de42aecf4b..2ec6ec0d39a 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -19,6 +19,8 @@\n #include \"tree.h\"\n #include \"config.h\"\n \n+static const char **xopts;\n+static size_t xopts_nr, xopts_alloc;\n static int line_termination = '\\n';\n \n struct merge_list {\n@@ -414,6 +416,7 @@ struct merge_tree_options {\n \tint show_messages;\n \tint name_only;\n \tint use_stdin;\n+\tstruct merge_options merge_options;\n };\n \n static int real_merge(struct merge_tree_options *o,\n@@ -439,6 +442,8 @@ static int real_merge(struct merge_tree_options *o,\n \n \tinit_merge_options(&opt, the_repository);\n \n+\topt.recursive_variant = o->merge_options.recursive_variant;\n+\n \topt.show_rename_progress = 0;\n \n \topt.branch1 = branch1;\n@@ -510,6 +515,17 @@ static int real_merge(struct merge_tree_options *o,\n \treturn !result.clean; /* result.clean < 0 handled above */\n }\n \n+static int option_parse_x(const struct option *opt,\n+\t\t\t const char *arg, int unset)\n+{\n+\tif (unset)\n+\t\treturn 0;\n+\n+\tALLOC_GROW(xopts, xopts_nr + 1, xopts_alloc);\n+\txopts[xopts_nr++] = xstrdup(arg);\n+\treturn 0;\n+}\n+\n int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n {\n \tstruct merge_tree_options o = { .show_messages = -1 };\n@@ -548,6 +564,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\t\t &merge_base,\n \t\t\t N_(\"commit\"),\n \t\t\t N_(\"specify a merge-base for the merge\")),\n+\t\tOPT_CALLBACK('X', \"strategy-option\", &xopts,\n+\t\t\tN_(\"option=value\"),\n+\t\t\tN_(\"option for selected merge strategy\"),\n+\t\t\toption_parse_x),\n \t\tOPT_END()\n \t};\n \n@@ -556,6 +576,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, mt_options,\n \t\t\t merge_tree_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n \n+\tfor (int x = 0; x < xopts_nr; x++)\n+\t\tif (parse_merge_opt(&o.merge_options, xopts[x]))\n+\t\t\tdie(_(\"unknown strategy option: -X%s\"), xopts[x]);\n+\n \t/* Handle --stdin */\n \tif (o.use_stdin) {\n \t\tstruct strbuf buf = STRBUF_INIT;\n\nbase-commit: ac83bc5054c2ac489166072334b4147ce6d0fccb\n-- \ngitgitgadget\n"},{"id":"480204","messageId":"xmqqfs4vioum.fsf@gitster.g","threadId":"60069","inReplyTo":"pull.1565.git.1691245481977.gitgitgadget@gmail.com","subject":"Re: [PATCH] merge-tree: add -X strategy option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-07T02:10:57Z","receivedAt":"2023-08-07T02:11:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Izzy via GitGitGadget\" writes:\n\n> From: winglovet \n>\n> Add merge strategy option to produce more customizable merge result such\n> as automatically solve conflicts.\n>\n> Signed-off-by: winglovet \n\ncf. Documentation/SubmittingPatches::[real-name], a part of the DCO\nsection of the document.\n\n> builtin/merge-tree.c | 24 ++++++++++++++++++++++++\n> 1 file changed, 24 insertions(+)\n\nA new feature should be protected by new tests to make sure it will\nnot be broken accidentally by others. Probably a couple of new\ntests to both t4300 and t4301?\n\n$ git shortlog -sn --no-merges --since=6.months builtin/merge-tree.c t/t430[01]*.sh\n\ntells me that Elijah and Ævar had been active, but by looking at the\noutput of\n\n$ git log --author=Ævar --since=6.months builtin/merge-tree.c t/t430[01]*.sh\n\nshows that the contribution by the latter was solely about code\nclean-up with Coccinelle and not about code features and correctness,\nso for a new-feature change like this, I'd ask comments by Elijah if\nI were writing this patch.\n\nThanks.\n\n> diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\n> index 0de42aecf4b..2ec6ec0d39a 100644\n> --- a/builtin/merge-tree.c\n> +++ b/builtin/merge-tree.c\n> @@ -19,6 +19,8 @@\n> #include \"tree.h\"\n> #include \"config.h\"\n> \n> +static const char **xopts;\n> +static size_t xopts_nr, xopts_alloc;\n> static int line_termination = '\\n';\n> \n> struct merge_list {\n> @@ -414,6 +416,7 @@ struct merge_tree_options {\n> \tint show_messages;\n> \tint name_only;\n> \tint use_stdin;\n> +\tstruct merge_options merge_options;\n> };\n> \n> static int real_merge(struct merge_tree_options *o,\n> @@ -439,6 +442,8 @@ static int real_merge(struct merge_tree_options *o,\n> \n> \tinit_merge_options(&opt, the_repository);\n> \n> +\topt.recursive_variant = o->merge_options.recursive_variant;\n> +\n> \topt.show_rename_progress = 0;\n> \n> \topt.branch1 = branch1;\n> @@ -510,6 +515,17 @@ static int real_merge(struct merge_tree_options *o,\n> \treturn !result.clean; /* result.clean < 0 handled above */\n> }\n> \n> +static int option_parse_x(const struct option *opt,\n> +\t\t\t const char *arg, int unset)\n> +{\n> +\tif (unset)\n> +\t\treturn 0;\n> +\n> +\tALLOC_GROW(xopts, xopts_nr + 1, xopts_alloc);\n> +\txopts[xopts_nr++] = xstrdup(arg);\n> +\treturn 0;\n> +}\n> +\n> int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> {\n> \tstruct merge_tree_options o = { .show_messages = -1 };\n> @@ -548,6 +564,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> \t\t\t &merge_base,\n> \t\t\t N_(\"commit\"),\n> \t\t\t N_(\"specify a merge-base for the merge\")),\n> +\t\tOPT_CALLBACK('X', \"strategy-option\", &xopts,\n> +\t\t\tN_(\"option=value\"),\n> +\t\t\tN_(\"option for selected merge strategy\"),\n> +\t\t\toption_parse_x),\n> \t\tOPT_END()\n> \t};\n> \n> @@ -556,6 +576,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> \targc = parse_options(argc, argv, prefix, mt_options,\n> \t\t\t merge_tree_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n> \n> +\tfor (int x = 0; x < xopts_nr; x++)\n> +\t\tif (parse_merge_opt(&o.merge_options, xopts[x]))\n> +\t\t\tdie(_(\"unknown strategy option: -X%s\"), xopts[x]);\n> +\n> \t/* Handle --stdin */\n> \tif (o.use_stdin) {\n> \t\tstruct strbuf buf = STRBUF_INIT;\n>\n> base-commit: ac83bc5054c2ac489166072334b4147ce6d0fccb\n"},{"id":"480625","messageId":"pull.1565.v2.git.1691818386345.gitgitgadget@gmail.com","threadId":"60069","inReplyTo":"pull.1565.git.1691245481977.gitgitgadget@gmail.com","subject":"[PATCH v2] merge-tree: add -X strategy option","fromName":"Izzy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-08-12T05:33:06Z","receivedAt":"2023-08-12T05:33:12Z","isPatch":true,"sender":{"key":"name:Izzy","avatar":null},"body":"From: winglovet \n\nAdd merge strategy option to produce more customizable merge result such\nas automatically solve conflicts.\n\nSigned-off-by: winglovet \n---\n merge-tree: add -X strategy option\n \n Change-Id: I16be592262d13cebcff8726eb043f7ecdb313b76\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1565%2FWingT%2Fmerge_tree_allow_strategy_option-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1565/WingT/merge_tree_allow_strategy_option-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1565\n\nRange-diff vs v1:\n\n 1: b843caed596 ! 1: 7d53d08381e merge-tree: add -X strategy option\n @@ builtin/merge-tree.c: int cmd_merge_tree(int argc, const char **argv, const char\n \t/* Handle --stdin */\n \tif (o.use_stdin) {\n \t\tstruct strbuf buf = STRBUF_INIT;\n +\n + ## t/t4301-merge-tree-write-tree.sh ##\n +@@ t/t4301-merge-tree-write-tree.sh: test_expect_success setup '\n + \tgit branch side1 &&\n + \tgit branch side2 &&\n + \tgit branch side3 &&\n ++\tgit branch side4 &&\n + \n + \tgit checkout side1 &&\n + \ttest_write_lines 1 2 3 4 5 6 >numbers &&\n +@@ t/t4301-merge-tree-write-tree.sh: test_expect_success setup '\n + \ttest_tick &&\n + \tgit commit -m rename-numbers &&\n + \n ++\tgit checkout side4 &&\n ++\ttest_write_lines 0 1 2 3 4 5 >numbers &&\n ++\techo yo >greeting &&\n ++\tgit add numbers greeting &&\n ++\ttest_tick &&\n ++\tgit commit -m other-content-modifications &&\n ++\n + \tgit switch --orphan unrelated &&\n + \t>something-else &&\n + \tgit add something-else &&\n +@@ t/t4301-merge-tree-write-tree.sh: test_expect_success 'Content merge and a few conflicts' '\n + \ttest_cmp expect actual\n + '\n + \n ++test_expect_success 'Auto resolve conflicts by \"ours\" stragety option' '\n ++\tgit checkout side1^0 &&\n ++\n ++\t# make sure merge conflict exists\n ++\ttest_must_fail git merge side4 &&\n ++\tgit merge --abort &&\n ++\n ++\tgit merge -X ours side4 &&\n ++\tgit rev-parse HEAD^{tree} > expected &&\n ++\n ++ git merge-tree -X ours side1 side4 > actual &&\n ++\n ++\ttest_cmp expected actual\n ++'\n ++\n + test_expect_success 'Barf on misspelled option, with exit code other than 0 or 1' '\n + \t# Mis-spell with single \"s\" instead of double \"s\"\n + \ttest_expect_code 129 git merge-tree --write-tree --mesages FOOBAR side1 side2 2>expect &&\n\n\n builtin/merge-tree.c | 24 ++++++++++++++++++++++++\n t/t4301-merge-tree-write-tree.sh | 23 +++++++++++++++++++++++\n 2 files changed, 47 insertions(+)\n\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex 0de42aecf4b..2ec6ec0d39a 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -19,6 +19,8 @@\n #include \"tree.h\"\n #include \"config.h\"\n \n+static const char **xopts;\n+static size_t xopts_nr, xopts_alloc;\n static int line_termination = '\\n';\n \n struct merge_list {\n@@ -414,6 +416,7 @@ struct merge_tree_options {\n \tint show_messages;\n \tint name_only;\n \tint use_stdin;\n+\tstruct merge_options merge_options;\n };\n \n static int real_merge(struct merge_tree_options *o,\n@@ -439,6 +442,8 @@ static int real_merge(struct merge_tree_options *o,\n \n \tinit_merge_options(&opt, the_repository);\n \n+\topt.recursive_variant = o->merge_options.recursive_variant;\n+\n \topt.show_rename_progress = 0;\n \n \topt.branch1 = branch1;\n@@ -510,6 +515,17 @@ static int real_merge(struct merge_tree_options *o,\n \treturn !result.clean; /* result.clean < 0 handled above */\n }\n \n+static int option_parse_x(const struct option *opt,\n+\t\t\t const char *arg, int unset)\n+{\n+\tif (unset)\n+\t\treturn 0;\n+\n+\tALLOC_GROW(xopts, xopts_nr + 1, xopts_alloc);\n+\txopts[xopts_nr++] = xstrdup(arg);\n+\treturn 0;\n+}\n+\n int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n {\n \tstruct merge_tree_options o = { .show_messages = -1 };\n@@ -548,6 +564,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\t\t &merge_base,\n \t\t\t N_(\"commit\"),\n \t\t\t N_(\"specify a merge-base for the merge\")),\n+\t\tOPT_CALLBACK('X', \"strategy-option\", &xopts,\n+\t\t\tN_(\"option=value\"),\n+\t\t\tN_(\"option for selected merge strategy\"),\n+\t\t\toption_parse_x),\n \t\tOPT_END()\n \t};\n \n@@ -556,6 +576,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, mt_options,\n \t\t\t merge_tree_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n \n+\tfor (int x = 0; x < xopts_nr; x++)\n+\t\tif (parse_merge_opt(&o.merge_options, xopts[x]))\n+\t\t\tdie(_(\"unknown strategy option: -X%s\"), xopts[x]);\n+\n \t/* Handle --stdin */\n \tif (o.use_stdin) {\n \t\tstruct strbuf buf = STRBUF_INIT;\ndiff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\nindex 250f721795b..2718817628c 100755\n--- a/t/t4301-merge-tree-write-tree.sh\n+++ b/t/t4301-merge-tree-write-tree.sh\n@@ -22,6 +22,7 @@ test_expect_success setup '\n \tgit branch side1 &&\n \tgit branch side2 &&\n \tgit branch side3 &&\n+\tgit branch side4 &&\n \n \tgit checkout side1 &&\n \ttest_write_lines 1 2 3 4 5 6 >numbers &&\n@@ -46,6 +47,13 @@ test_expect_success setup '\n \ttest_tick &&\n \tgit commit -m rename-numbers &&\n \n+\tgit checkout side4 &&\n+\ttest_write_lines 0 1 2 3 4 5 >numbers &&\n+\techo yo >greeting &&\n+\tgit add numbers greeting &&\n+\ttest_tick &&\n+\tgit commit -m other-content-modifications &&\n+\n \tgit switch --orphan unrelated &&\n \t>something-else &&\n \tgit add something-else &&\n@@ -97,6 +105,21 @@ test_expect_success 'Content merge and a few conflicts' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'Auto resolve conflicts by \"ours\" stragety option' '\n+\tgit checkout side1^0 &&\n+\n+\t# make sure merge conflict exists\n+\ttest_must_fail git merge side4 &&\n+\tgit merge --abort &&\n+\n+\tgit merge -X ours side4 &&\n+\tgit rev-parse HEAD^{tree} > expected &&\n+\n+ git merge-tree -X ours side1 side4 > actual &&\n+\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'Barf on misspelled option, with exit code other than 0 or 1' '\n \t# Mis-spell with single \"s\" instead of double \"s\"\n \ttest_expect_code 129 git merge-tree --write-tree --mesages FOOBAR side1 side2 2>expect &&\n\nbase-commit: ac83bc5054c2ac489166072334b4147ce6d0fccb\n-- \ngitgitgadget\n"},{"id":"480627","messageId":"CAFWsj_X=_PVyLcRgy77PL+kisjFqRSTCojB61B5cSK=6YROajg@mail.gmail.com","threadId":"60069","inReplyTo":"pull.1565.v2.git.1691818386345.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] merge-tree: add -X strategy option","fromName":"唐宇奕","fromEmail":"winglovet@gmail.com","sentAt":"2023-08-12T05:41:17Z","receivedAt":"2023-08-12T05:41:31Z","isPatch":true,"sender":{"key":"winglovet@gmail.com","avatar":"https://avatars.githubusercontent.com/u/16458679?v=4"},"body":"Thanks for your advice, I have uploaded a new patch including tests.\n\nOn Sat, Aug 12, 2023 at 1:33 PM Izzy via GitGitGadget\n wrote:\n>\n> From: winglovet \n>\n> Add merge strategy option to produce more customizable merge result such\n> as automatically solve conflicts.\n>\n> Signed-off-by: winglovet \n> ---\n> merge-tree: add -X strategy option\n>\n> Change-Id: I16be592262d13cebcff8726eb043f7ecdb313b76\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1565%2FWingT%2Fmerge_tree_allow_strategy_option-v2\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1565/WingT/merge_tree_allow_strategy_option-v2\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1565\n>\n> Range-diff vs v1:\n>\n> 1: b843caed596 ! 1: 7d53d08381e merge-tree: add -X strategy option\n> @@ builtin/merge-tree.c: int cmd_merge_tree(int argc, const char **argv, const char\n> /* Handle --stdin */\n> if (o.use_stdin) {\n> struct strbuf buf = STRBUF_INIT;\n> +\n> + ## t/t4301-merge-tree-write-tree.sh ##\n> +@@ t/t4301-merge-tree-write-tree.sh: test_expect_success setup '\n> + git branch side1 &&\n> + git branch side2 &&\n> + git branch side3 &&\n> ++ git branch side4 &&\n> +\n> + git checkout side1 &&\n> + test_write_lines 1 2 3 4 5 6 >numbers &&\n> +@@ t/t4301-merge-tree-write-tree.sh: test_expect_success setup '\n> + test_tick &&\n> + git commit -m rename-numbers &&\n> +\n> ++ git checkout side4 &&\n> ++ test_write_lines 0 1 2 3 4 5 >numbers &&\n> ++ echo yo >greeting &&\n> ++ git add numbers greeting &&\n> ++ test_tick &&\n> ++ git commit -m other-content-modifications &&\n> ++\n> + git switch --orphan unrelated &&\n> + >something-else &&\n> + git add something-else &&\n> +@@ t/t4301-merge-tree-write-tree.sh: test_expect_success 'Content merge and a few conflicts' '\n> + test_cmp expect actual\n> + '\n> +\n> ++test_expect_success 'Auto resolve conflicts by \"ours\" stragety option' '\n> ++ git checkout side1^0 &&\n> ++\n> ++ # make sure merge conflict exists\n> ++ test_must_fail git merge side4 &&\n> ++ git merge --abort &&\n> ++\n> ++ git merge -X ours side4 &&\n> ++ git rev-parse HEAD^{tree} > expected &&\n> ++\n> ++ git merge-tree -X ours side1 side4 > actual &&\n> ++\n> ++ test_cmp expected actual\n> ++'\n> ++\n> + test_expect_success 'Barf on misspelled option, with exit code other than 0 or 1' '\n> + # Mis-spell with single \"s\" instead of double \"s\"\n> + test_expect_code 129 git merge-tree --write-tree --mesages FOOBAR side1 side2 2>expect &&\n>\n>\n> builtin/merge-tree.c | 24 ++++++++++++++++++++++++\n> t/t4301-merge-tree-write-tree.sh | 23 +++++++++++++++++++++++\n> 2 files changed, 47 insertions(+)\n>\n> diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\n> index 0de42aecf4b..2ec6ec0d39a 100644\n> --- a/builtin/merge-tree.c\n> +++ b/builtin/merge-tree.c\n> @@ -19,6 +19,8 @@\n> #include \"tree.h\"\n> #include \"config.h\"\n>\n> +static const char **xopts;\n> +static size_t xopts_nr, xopts_alloc;\n> static int line_termination = '\\n';\n>\n> struct merge_list {\n> @@ -414,6 +416,7 @@ struct merge_tree_options {\n> int show_messages;\n> int name_only;\n> int use_stdin;\n> + struct merge_options merge_options;\n> };\n>\n> static int real_merge(struct merge_tree_options *o,\n> @@ -439,6 +442,8 @@ static int real_merge(struct merge_tree_options *o,\n>\n> init_merge_options(&opt, the_repository);\n>\n> + opt.recursive_variant = o->merge_options.recursive_variant;\n> +\n> opt.show_rename_progress = 0;\n>\n> opt.branch1 = branch1;\n> @@ -510,6 +515,17 @@ static int real_merge(struct merge_tree_options *o,\n> return !result.clean; /* result.clean < 0 handled above */\n> }\n>\n> +static int option_parse_x(const struct option *opt,\n> + const char *arg, int unset)\n> +{\n> + if (unset)\n> + return 0;\n> +\n> + ALLOC_GROW(xopts, xopts_nr + 1, xopts_alloc);\n> + xopts[xopts_nr++] = xstrdup(arg);\n> + return 0;\n> +}\n> +\n> int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> {\n> struct merge_tree_options o = { .show_messages = -1 };\n> @@ -548,6 +564,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> &merge_base,\n> N_(\"commit\"),\n> N_(\"specify a merge-base for the merge\")),\n> + OPT_CALLBACK('X', \"strategy-option\", &xopts,\n> + N_(\"option=value\"),\n> + N_(\"option for selected merge strategy\"),\n> + option_parse_x),\n> OPT_END()\n> };\n>\n> @@ -556,6 +576,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> argc = parse_options(argc, argv, prefix, mt_options,\n> merge_tree_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n>\n> + for (int x = 0; x < xopts_nr; x++)\n> + if (parse_merge_opt(&o.merge_options, xopts[x]))\n> + die(_(\"unknown strategy option: -X%s\"), xopts[x]);\n> +\n> /* Handle --stdin */\n> if (o.use_stdin) {\n> struct strbuf buf = STRBUF_INIT;\n> diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\n> index 250f721795b..2718817628c 100755\n> --- a/t/t4301-merge-tree-write-tree.sh\n> +++ b/t/t4301-merge-tree-write-tree.sh\n> @@ -22,6 +22,7 @@ test_expect_success setup '\n> git branch side1 &&\n> git branch side2 &&\n> git branch side3 &&\n> + git branch side4 &&\n>\n> git checkout side1 &&\n> test_write_lines 1 2 3 4 5 6 >numbers &&\n> @@ -46,6 +47,13 @@ test_expect_success setup '\n> test_tick &&\n> git commit -m rename-numbers &&\n>\n> + git checkout side4 &&\n> + test_write_lines 0 1 2 3 4 5 >numbers &&\n> + echo yo >greeting &&\n> + git add numbers greeting &&\n> + test_tick &&\n> + git commit -m other-content-modifications &&\n> +\n> git switch --orphan unrelated &&\n> >something-else &&\n> git add something-else &&\n> @@ -97,6 +105,21 @@ test_expect_success 'Content merge and a few conflicts' '\n> test_cmp expect actual\n> '\n>\n> +test_expect_success 'Auto resolve conflicts by \"ours\" stragety option' '\n> + git checkout side1^0 &&\n> +\n> + # make sure merge conflict exists\n> + test_must_fail git merge side4 &&\n> + git merge --abort &&\n> +\n> + git merge -X ours side4 &&\n> + git rev-parse HEAD^{tree} > expected &&\n> +\n> + git merge-tree -X ours side1 side4 > actual &&\n> +\n> + test_cmp expected actual\n> +'\n> +\n> test_expect_success 'Barf on misspelled option, with exit code other than 0 or 1' '\n> # Mis-spell with single \"s\" instead of double \"s\"\n> test_expect_code 129 git merge-tree --write-tree --mesages FOOBAR side1 side2 2>expect &&\n>\n> base-commit: ac83bc5054c2ac489166072334b4147ce6d0fccb\n> --\n> gitgitgadget\n"},{"id":"481343","messageId":"CAFWsj_XCbAy4yuHxsV3DgqMWzN8oE_Jfkp1mxxZggGtbEEfnsw@mail.gmail.com","threadId":"60069","inReplyTo":"CAFWsj_X=_PVyLcRgy77PL+kisjFqRSTCojB61B5cSK=6YROajg@mail.gmail.com","subject":"Re: [PATCH v2] merge-tree: add -X strategy option","fromName":"唐宇奕","fromEmail":"winglovet@gmail.com","sentAt":"2023-09-03T01:31:30Z","receivedAt":"2023-09-03T01:31:45Z","isPatch":true,"sender":{"key":"winglovet@gmail.com","avatar":"https://avatars.githubusercontent.com/u/16458679?v=4"},"body":"Hello, is there any problem with this patch?\n"},{"id":"481767","messageId":"CABPp-BG+MxZukNagJD90GgTOh1Te8Dk3pk86DOhSpxasAJViFQ@mail.gmail.com","threadId":"60069","inReplyTo":"pull.1565.v2.git.1691818386345.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] merge-tree: add -X strategy option","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2023-09-12T15:03:11Z","receivedAt":"2023-09-12T15:03:30Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nSorry for the delay in responding; I've been busy with non-git things...\n\nOn Sat, Aug 12, 2023 at 12:19 AM Izzy via GitGitGadget\n wrote:\n>\n> From: winglovet \n>\n> Add merge strategy option to produce more customizable merge result such\n> as automatically solve conflicts.\n>\n> Signed-off-by: winglovet \n\nJunio already flagged this line in your v1; you either need to correct\nit or respond to his comments about it.\n\n> ---\n> merge-tree: add -X strategy option\n>\n> Change-Id: I16be592262d13cebcff8726eb043f7ecdb313b76\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1565%2FWingT%2Fmerge_tree_allow_strategy_option-v2\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1565/WingT/merge_tree_allow_strategy_option-v2\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1565\n>\n> Range-diff vs v1:\n>\n> 1: b843caed596 ! 1: 7d53d08381e merge-tree: add -X strategy option\n> @@ builtin/merge-tree.c: int cmd_merge_tree(int argc, const char **argv, const char\n> /* Handle --stdin */\n> if (o.use_stdin) {\n> struct strbuf buf = STRBUF_INIT;\n> +\n> + ## t/t4301-merge-tree-write-tree.sh ##\n> +@@ t/t4301-merge-tree-write-tree.sh: test_expect_success setup '\n> + git branch side1 &&\n> + git branch side2 &&\n> + git branch side3 &&\n> ++ git branch side4 &&\n> +\n> + git checkout side1 &&\n> + test_write_lines 1 2 3 4 5 6 >numbers &&\n> +@@ t/t4301-merge-tree-write-tree.sh: test_expect_success setup '\n> + test_tick &&\n> + git commit -m rename-numbers &&\n> +\n> ++ git checkout side4 &&\n> ++ test_write_lines 0 1 2 3 4 5 >numbers &&\n> ++ echo yo >greeting &&\n> ++ git add numbers greeting &&\n> ++ test_tick &&\n> ++ git commit -m other-content-modifications &&\n> ++\n> + git switch --orphan unrelated &&\n> + >something-else &&\n> + git add something-else &&\n> +@@ t/t4301-merge-tree-write-tree.sh: test_expect_success 'Content merge and a few conflicts' '\n> + test_cmp expect actual\n> + '\n> +\n> ++test_expect_success 'Auto resolve conflicts by \"ours\" stragety option' '\n> ++ git checkout side1^0 &&\n> ++\n> ++ # make sure merge conflict exists\n> ++ test_must_fail git merge side4 &&\n> ++ git merge --abort &&\n> ++\n> ++ git merge -X ours side4 &&\n> ++ git rev-parse HEAD^{tree} > expected &&\n> ++\n> ++ git merge-tree -X ours side1 side4 > actual &&\n> ++\n> ++ test_cmp expected actual\n> ++'\n> ++\n> + test_expect_success 'Barf on misspelled option, with exit code other than 0 or 1' '\n> + # Mis-spell with single \"s\" instead of double \"s\"\n> + test_expect_code 129 git merge-tree --write-tree --mesages FOOBAR side1 side2 2>expect &&\n>\n>\n> builtin/merge-tree.c | 24 ++++++++++++++++++++++++\n> t/t4301-merge-tree-write-tree.sh | 23 +++++++++++++++++++++++\n> 2 files changed, 47 insertions(+)\n>\n> diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\n> index 0de42aecf4b..2ec6ec0d39a 100644\n> --- a/builtin/merge-tree.c\n> +++ b/builtin/merge-tree.c\n> @@ -19,6 +19,8 @@\n> #include \"tree.h\"\n> #include \"config.h\"\n>\n> +static const char **xopts;\n> +static size_t xopts_nr, xopts_alloc;\n\nI know we already have lots of globals, and I'm partially to blame\nsince I also copied this style from elsewhere into this file, but it'd\nsure be nice to get rid of these needless globals rather than add\nmore. But, certainly not a blocker....\n\n> static int line_termination = '\\n';\n>\n> struct merge_list {\n> @@ -414,6 +416,7 @@ struct merge_tree_options {\n> int show_messages;\n> int name_only;\n> int use_stdin;\n> + struct merge_options merge_options;\n> };\n\nProbably the best we can do now, but feels a little icky to me that\nparse_merge_opt() works on all of struct merge_options rather than\nhaving a dedicated struct for the particular options it can set. Made\nsense back in the merge-recursive.[ch] days, but it provides too many\nextra unrelated and unnecessary fields now.\n\nThis isn't something you need to fix in this patch, though, just\nsomething else I'll keep in mind when I get back to working on\nremoving merge-recursive.[ch].\n\n> static int real_merge(struct merge_tree_options *o,\n> @@ -439,6 +442,8 @@ static int real_merge(struct merge_tree_options *o,\n>\n> init_merge_options(&opt, the_repository);\n>\n> + opt.recursive_variant = o->merge_options.recursive_variant;\n> +\n> opt.show_rename_progress = 0;\n>\n> opt.branch1 = branch1;\n> @@ -510,6 +515,17 @@ static int real_merge(struct merge_tree_options *o,\n> return !result.clean; /* result.clean < 0 handled above */\n> }\n>\n> +static int option_parse_x(const struct option *opt,\n> + const char *arg, int unset)\n> +{\n> + if (unset)\n> + return 0;\n> +\n> + ALLOC_GROW(xopts, xopts_nr + 1, xopts_alloc);\n> + xopts[xopts_nr++] = xstrdup(arg);\n> + return 0;\n> +}\n> +\n> int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> {\n> struct merge_tree_options o = { .show_messages = -1 };\n> @@ -548,6 +564,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> &merge_base,\n> N_(\"commit\"),\n> N_(\"specify a merge-base for the merge\")),\n> + OPT_CALLBACK('X', \"strategy-option\", &xopts,\n> + N_(\"option=value\"),\n> + N_(\"option for selected merge strategy\"),\n> + option_parse_x),\n> OPT_END()\n> };\n>\n> @@ -556,6 +576,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> argc = parse_options(argc, argv, prefix, mt_options,\n> merge_tree_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n>\n> + for (int x = 0; x < xopts_nr; x++)\n> + if (parse_merge_opt(&o.merge_options, xopts[x]))\n> + die(_(\"unknown strategy option: -X%s\"), xopts[x]);\n> +\n> /* Handle --stdin */\n> if (o.use_stdin) {\n> struct strbuf buf = STRBUF_INIT;\n> diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\n> index 250f721795b..2718817628c 100755\n> --- a/t/t4301-merge-tree-write-tree.sh\n> +++ b/t/t4301-merge-tree-write-tree.sh\n> @@ -22,6 +22,7 @@ test_expect_success setup '\n> git branch side1 &&\n> git branch side2 &&\n> git branch side3 &&\n> + git branch side4 &&\n>\n> git checkout side1 &&\n> test_write_lines 1 2 3 4 5 6 >numbers &&\n> @@ -46,6 +47,13 @@ test_expect_success setup '\n> test_tick &&\n> git commit -m rename-numbers &&\n>\n> + git checkout side4 &&\n> + test_write_lines 0 1 2 3 4 5 >numbers &&\n> + echo yo >greeting &&\n> + git add numbers greeting &&\n> + test_tick &&\n> + git commit -m other-content-modifications &&\n> +\n> git switch --orphan unrelated &&\n> >something-else &&\n> git add something-else &&\n> @@ -97,6 +105,21 @@ test_expect_success 'Content merge and a few conflicts' '\n> test_cmp expect actual\n> '\n>\n> +test_expect_success 'Auto resolve conflicts by \"ours\" stragety option' '\n\nstragety -> strategy\n\n> + git checkout side1^0 &&\n> +\n> + # make sure merge conflict exists\n> + test_must_fail git merge side4 &&\n> + git merge --abort &&\n> +\n> + git merge -X ours side4 &&\n> + git rev-parse HEAD^{tree} > expected &&\n\nStyle: '>filename', not '> filename'.\n\n> +\n> + git merge-tree -X ours side1 side4 > actual &&\n\nSame style issue with redirect.\n\nAlso, why is the leading spacing inconsistent with the surrounding lines?\n\n> +\n> + test_cmp expected actual\n> +'\n> +\n> test_expect_success 'Barf on misspelled option, with exit code other than 0 or 1' '\n> # Mis-spell with single \"s\" instead of double \"s\"\n> test_expect_code 129 git merge-tree --write-tree --mesages FOOBAR side1 side2 2>expect &&\n\nI personally don't see the value in passing `-X ours` or `-X theirs`\nto merges. I've never really been a fan of either. Granted, I\nunderstand the value in making merge-tree support the same things that\nexisting merges support, so I'm fine with supporting it, but I'd\nrather have a useful example for the testcase such as\n-Xignore-space-change or something (or maybe use it as an additional\nexample?) Not a blocker on its own, but just a personal preference.\n\nAnyway, the general direction of the patch seems reasonable. Not\neverything I commented on needs to be fixed, as I noted, but there are\na few small things to fix up.\n"},{"id":"481902","messageId":"pull.1565.v3.git.1694830462463.gitgitgadget@gmail.com","threadId":"60069","inReplyTo":"pull.1565.v2.git.1691818386345.gitgitgadget@gmail.com","subject":"[PATCH v3] merge-tree: add -X strategy option","fromName":"Izzy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-09-16T02:14:22Z","receivedAt":"2023-09-16T02:15:30Z","isPatch":true,"sender":{"key":"name:Izzy","avatar":null},"body":"From: Tang Yuyi \n\nAdd merge strategy option to produce more customizable merge result such\nas automatically solve conflicts.\n\nSigned-off-by: Tang Yuyi \n---\n merge-tree: add -X strategy option\n \n Change-Id: I16be592262d13cebcff8726eb043f7ecdb313b76\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1565%2FWingT%2Fmerge_tree_allow_strategy_option-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1565/WingT/merge_tree_allow_strategy_option-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/1565\n\nRange-diff vs v2:\n\n 1: 7d53d08381e ! 1: d64a774fa7c merge-tree: add -X strategy option\n @@\n ## Metadata ##\n -Author: winglovet \n +Author: Tang Yuyi \n \n ## Commit message ##\n merge-tree: add -X strategy option\n @@ Commit message\n Add merge strategy option to produce more customizable merge result such\n as automatically solve conflicts.\n \n - Signed-off-by: winglovet \n + Signed-off-by: Tang Yuyi \n \n ## builtin/merge-tree.c ##\n @@\n @@ t/t4301-merge-tree-write-tree.sh: test_expect_success 'Content merge and a few c\n \ttest_cmp expect actual\n '\n \n -+test_expect_success 'Auto resolve conflicts by \"ours\" stragety option' '\n ++test_expect_success 'Auto resolve conflicts by \"ours\" strategy option' '\n +\tgit checkout side1^0 &&\n +\n +\t# make sure merge conflict exists\n\n\n builtin/merge-tree.c | 24 ++++++++++++++++++++++++\n t/t4301-merge-tree-write-tree.sh | 23 +++++++++++++++++++++++\n 2 files changed, 47 insertions(+)\n\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex 0de42aecf4b..2ec6ec0d39a 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -19,6 +19,8 @@\n #include \"tree.h\"\n #include \"config.h\"\n \n+static const char **xopts;\n+static size_t xopts_nr, xopts_alloc;\n static int line_termination = '\\n';\n \n struct merge_list {\n@@ -414,6 +416,7 @@ struct merge_tree_options {\n \tint show_messages;\n \tint name_only;\n \tint use_stdin;\n+\tstruct merge_options merge_options;\n };\n \n static int real_merge(struct merge_tree_options *o,\n@@ -439,6 +442,8 @@ static int real_merge(struct merge_tree_options *o,\n \n \tinit_merge_options(&opt, the_repository);\n \n+\topt.recursive_variant = o->merge_options.recursive_variant;\n+\n \topt.show_rename_progress = 0;\n \n \topt.branch1 = branch1;\n@@ -510,6 +515,17 @@ static int real_merge(struct merge_tree_options *o,\n \treturn !result.clean; /* result.clean < 0 handled above */\n }\n \n+static int option_parse_x(const struct option *opt,\n+\t\t\t const char *arg, int unset)\n+{\n+\tif (unset)\n+\t\treturn 0;\n+\n+\tALLOC_GROW(xopts, xopts_nr + 1, xopts_alloc);\n+\txopts[xopts_nr++] = xstrdup(arg);\n+\treturn 0;\n+}\n+\n int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n {\n \tstruct merge_tree_options o = { .show_messages = -1 };\n@@ -548,6 +564,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\t\t &merge_base,\n \t\t\t N_(\"commit\"),\n \t\t\t N_(\"specify a merge-base for the merge\")),\n+\t\tOPT_CALLBACK('X', \"strategy-option\", &xopts,\n+\t\t\tN_(\"option=value\"),\n+\t\t\tN_(\"option for selected merge strategy\"),\n+\t\t\toption_parse_x),\n \t\tOPT_END()\n \t};\n \n@@ -556,6 +576,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, mt_options,\n \t\t\t merge_tree_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n \n+\tfor (int x = 0; x < xopts_nr; x++)\n+\t\tif (parse_merge_opt(&o.merge_options, xopts[x]))\n+\t\t\tdie(_(\"unknown strategy option: -X%s\"), xopts[x]);\n+\n \t/* Handle --stdin */\n \tif (o.use_stdin) {\n \t\tstruct strbuf buf = STRBUF_INIT;\ndiff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\nindex 250f721795b..4125bb101ec 100755\n--- a/t/t4301-merge-tree-write-tree.sh\n+++ b/t/t4301-merge-tree-write-tree.sh\n@@ -22,6 +22,7 @@ test_expect_success setup '\n \tgit branch side1 &&\n \tgit branch side2 &&\n \tgit branch side3 &&\n+\tgit branch side4 &&\n \n \tgit checkout side1 &&\n \ttest_write_lines 1 2 3 4 5 6 >numbers &&\n@@ -46,6 +47,13 @@ test_expect_success setup '\n \ttest_tick &&\n \tgit commit -m rename-numbers &&\n \n+\tgit checkout side4 &&\n+\ttest_write_lines 0 1 2 3 4 5 >numbers &&\n+\techo yo >greeting &&\n+\tgit add numbers greeting &&\n+\ttest_tick &&\n+\tgit commit -m other-content-modifications &&\n+\n \tgit switch --orphan unrelated &&\n \t>something-else &&\n \tgit add something-else &&\n@@ -97,6 +105,21 @@ test_expect_success 'Content merge and a few conflicts' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'Auto resolve conflicts by \"ours\" strategy option' '\n+\tgit checkout side1^0 &&\n+\n+\t# make sure merge conflict exists\n+\ttest_must_fail git merge side4 &&\n+\tgit merge --abort &&\n+\n+\tgit merge -X ours side4 &&\n+\tgit rev-parse HEAD^{tree} > expected &&\n+\n+ git merge-tree -X ours side1 side4 > actual &&\n+\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'Barf on misspelled option, with exit code other than 0 or 1' '\n \t# Mis-spell with single \"s\" instead of double \"s\"\n \ttest_expect_code 129 git merge-tree --write-tree --mesages FOOBAR side1 side2 2>expect &&\n\nbase-commit: ac83bc5054c2ac489166072334b4147ce6d0fccb\n-- \ngitgitgadget\n"},{"id":"481904","messageId":"CAFWsj_UeK-5uyCtBfLYeRZXRMdAD_eFassxEo6FvgeVzmwqLNQ@mail.gmail.com","threadId":"60069","inReplyTo":"pull.1565.v3.git.1694830462463.gitgitgadget@gmail.com","subject":"Re: [PATCH v3] merge-tree: add -X strategy option","fromName":"唐宇奕","fromEmail":"winglovet@gmail.com","sentAt":"2023-09-16T02:26:49Z","receivedAt":"2023-09-16T02:27:46Z","isPatch":true,"sender":{"key":"winglovet@gmail.com","avatar":"https://avatars.githubusercontent.com/u/16458679?v=4"},"body":"Thanks for your advice!\nI've fixed those blocking issues.\nHowever, regarding the global variable issue, I'm not familiar with C\nand git code and don't know how to solve this. I think perhaps we need\nsomething like closure to parse opt into a local variable?\nOur usecase is to achieve something like 'range-diff', we first use\nmerge-tree to merge new patchset's base commit with old patchset's\nsource commit, then use the merge result to diff against new\npatchset's source commit. So we only need to make sure conflict's are\nhandled automatically, leaving other diff features to the second step.\n\n\nOn Sat, Sep 16, 2023 at 10:14 AM Izzy via GitGitGadget\n wrote:\n>\n> From: Tang Yuyi \n>\n> Add merge strategy option to produce more customizable merge result such\n> as automatically solve conflicts.\n>\n> Signed-off-by: Tang Yuyi \n> ---\n> merge-tree: add -X strategy option\n>\n> Change-Id: I16be592262d13cebcff8726eb043f7ecdb313b76\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1565%2FWingT%2Fmerge_tree_allow_strategy_option-v3\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1565/WingT/merge_tree_allow_strategy_option-v3\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1565\n>\n> Range-diff vs v2:\n>\n> 1: 7d53d08381e ! 1: d64a774fa7c merge-tree: add -X strategy option\n> @@\n> ## Metadata ##\n> -Author: winglovet \n> +Author: Tang Yuyi \n>\n> ## Commit message ##\n> merge-tree: add -X strategy option\n> @@ Commit message\n> Add merge strategy option to produce more customizable merge result such\n> as automatically solve conflicts.\n>\n> - Signed-off-by: winglovet \n> + Signed-off-by: Tang Yuyi \n>\n> ## builtin/merge-tree.c ##\n> @@\n> @@ t/t4301-merge-tree-write-tree.sh: test_expect_success 'Content merge and a few c\n> test_cmp expect actual\n> '\n>\n> -+test_expect_success 'Auto resolve conflicts by \"ours\" stragety option' '\n> ++test_expect_success 'Auto resolve conflicts by \"ours\" strategy option' '\n> + git checkout side1^0 &&\n> +\n> + # make sure merge conflict exists\n>\n>\n> builtin/merge-tree.c | 24 ++++++++++++++++++++++++\n> t/t4301-merge-tree-write-tree.sh | 23 +++++++++++++++++++++++\n> 2 files changed, 47 insertions(+)\n>\n> diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\n> index 0de42aecf4b..2ec6ec0d39a 100644\n> --- a/builtin/merge-tree.c\n> +++ b/builtin/merge-tree.c\n> @@ -19,6 +19,8 @@\n> #include \"tree.h\"\n> #include \"config.h\"\n>\n> +static const char **xopts;\n> +static size_t xopts_nr, xopts_alloc;\n> static int line_termination = '\\n';\n>\n> struct merge_list {\n> @@ -414,6 +416,7 @@ struct merge_tree_options {\n> int show_messages;\n> int name_only;\n> int use_stdin;\n> + struct merge_options merge_options;\n> };\n>\n> static int real_merge(struct merge_tree_options *o,\n> @@ -439,6 +442,8 @@ static int real_merge(struct merge_tree_options *o,\n>\n> init_merge_options(&opt, the_repository);\n>\n> + opt.recursive_variant = o->merge_options.recursive_variant;\n> +\n> opt.show_rename_progress = 0;\n>\n> opt.branch1 = branch1;\n> @@ -510,6 +515,17 @@ static int real_merge(struct merge_tree_options *o,\n> return !result.clean; /* result.clean < 0 handled above */\n> }\n>\n> +static int option_parse_x(const struct option *opt,\n> + const char *arg, int unset)\n> +{\n> + if (unset)\n> + return 0;\n> +\n> + ALLOC_GROW(xopts, xopts_nr + 1, xopts_alloc);\n> + xopts[xopts_nr++] = xstrdup(arg);\n> + return 0;\n> +}\n> +\n> int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> {\n> struct merge_tree_options o = { .show_messages = -1 };\n> @@ -548,6 +564,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> &merge_base,\n> N_(\"commit\"),\n> N_(\"specify a merge-base for the merge\")),\n> + OPT_CALLBACK('X', \"strategy-option\", &xopts,\n> + N_(\"option=value\"),\n> + N_(\"option for selected merge strategy\"),\n> + option_parse_x),\n> OPT_END()\n> };\n>\n> @@ -556,6 +576,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> argc = parse_options(argc, argv, prefix, mt_options,\n> merge_tree_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n>\n> + for (int x = 0; x < xopts_nr; x++)\n> + if (parse_merge_opt(&o.merge_options, xopts[x]))\n> + die(_(\"unknown strategy option: -X%s\"), xopts[x]);\n> +\n> /* Handle --stdin */\n> if (o.use_stdin) {\n> struct strbuf buf = STRBUF_INIT;\n> diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\n> index 250f721795b..4125bb101ec 100755\n> --- a/t/t4301-merge-tree-write-tree.sh\n> +++ b/t/t4301-merge-tree-write-tree.sh\n> @@ -22,6 +22,7 @@ test_expect_success setup '\n> git branch side1 &&\n> git branch side2 &&\n> git branch side3 &&\n> + git branch side4 &&\n>\n> git checkout side1 &&\n> test_write_lines 1 2 3 4 5 6 >numbers &&\n> @@ -46,6 +47,13 @@ test_expect_success setup '\n> test_tick &&\n> git commit -m rename-numbers &&\n>\n> + git checkout side4 &&\n> + test_write_lines 0 1 2 3 4 5 >numbers &&\n> + echo yo >greeting &&\n> + git add numbers greeting &&\n> + test_tick &&\n> + git commit -m other-content-modifications &&\n> +\n> git switch --orphan unrelated &&\n> >something-else &&\n> git add something-else &&\n> @@ -97,6 +105,21 @@ test_expect_success 'Content merge and a few conflicts' '\n> test_cmp expect actual\n> '\n>\n> +test_expect_success 'Auto resolve conflicts by \"ours\" strategy option' '\n> + git checkout side1^0 &&\n> +\n> + # make sure merge conflict exists\n> + test_must_fail git merge side4 &&\n> + git merge --abort &&\n> +\n> + git merge -X ours side4 &&\n> + git rev-parse HEAD^{tree} > expected &&\n> +\n> + git merge-tree -X ours side1 side4 > actual &&\n> +\n> + test_cmp expected actual\n> +'\n> +\n> test_expect_success 'Barf on misspelled option, with exit code other than 0 or 1' '\n> # Mis-spell with single \"s\" instead of double \"s\"\n> test_expect_code 129 git merge-tree --write-tree --mesages FOOBAR side1 side2 2>expect &&\n>\n> base-commit: ac83bc5054c2ac489166072334b4147ce6d0fccb\n> --\n> gitgitgadget\n"},{"id":"481909","messageId":"CABPp-BGo2UUJKm39Ljs+-Usf1F02sTej3phr84i8wiUSf_FB=Q@mail.gmail.com","threadId":"60069","inReplyTo":"pull.1565.v3.git.1694830462463.gitgitgadget@gmail.com","subject":"Re: [PATCH v3] merge-tree: add -X strategy option","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2023-09-16T03:16:00Z","receivedAt":"2023-09-16T03:17:40Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nOn Fri, Sep 15, 2023 at 7:14 PM Izzy via GitGitGadget\n wrote:\n>\n> From: Tang Yuyi \n>\n> Add merge strategy option to produce more customizable merge result such\n> as automatically solve conflicts.\n\ns/solve/resolving/\n\nI think \"solve\" should be \"solving\" here, except that \"solve\" isn't\nreally the right word. It's not solving (figuring out) anything, it's\nusing a big hammer to make the problem just go away. So, I think\n\"resolving\" is a better choice.\n\n> Signed-off-by: Tang Yuyi \n> ---\n> merge-tree: add -X strategy option\n>\n> Change-Id: I16be592262d13cebcff8726eb043f7ecdb313b76\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1565%2FWingT%2Fmerge_tree_allow_strategy_option-v3\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1565/WingT/merge_tree_allow_strategy_option-v3\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1565\n>\n> Range-diff vs v2:\n>\n> 1: 7d53d08381e ! 1: d64a774fa7c merge-tree: add -X strategy option\n> @@\n> ## Metadata ##\n> -Author: winglovet \n> +Author: Tang Yuyi \n>\n> ## Commit message ##\n> merge-tree: add -X strategy option\n> @@ Commit message\n> Add merge strategy option to produce more customizable merge result such\n> as automatically solve conflicts.\n>\n> - Signed-off-by: winglovet \n> + Signed-off-by: Tang Yuyi \n>\n> ## builtin/merge-tree.c ##\n> @@\n> @@ t/t4301-merge-tree-write-tree.sh: test_expect_success 'Content merge and a few c\n> test_cmp expect actual\n> '\n>\n> -+test_expect_success 'Auto resolve conflicts by \"ours\" stragety option' '\n> ++test_expect_success 'Auto resolve conflicts by \"ours\" strategy option' '\n> + git checkout side1^0 &&\n> +\n> + # make sure merge conflict exists\n>\n>\n> builtin/merge-tree.c | 24 ++++++++++++++++++++++++\n> t/t4301-merge-tree-write-tree.sh | 23 +++++++++++++++++++++++\n> 2 files changed, 47 insertions(+)\n>\n> diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\n> index 0de42aecf4b..2ec6ec0d39a 100644\n> --- a/builtin/merge-tree.c\n> +++ b/builtin/merge-tree.c\n> @@ -19,6 +19,8 @@\n> #include \"tree.h\"\n> #include \"config.h\"\n>\n> +static const char **xopts;\n> +static size_t xopts_nr, xopts_alloc;\n> static int line_termination = '\\n';\n>\n> struct merge_list {\n> @@ -414,6 +416,7 @@ struct merge_tree_options {\n> int show_messages;\n> int name_only;\n> int use_stdin;\n> + struct merge_options merge_options;\n> };\n>\n> static int real_merge(struct merge_tree_options *o,\n> @@ -439,6 +442,8 @@ static int real_merge(struct merge_tree_options *o,\n>\n> init_merge_options(&opt, the_repository);\n>\n> + opt.recursive_variant = o->merge_options.recursive_variant;\n> +\n> opt.show_rename_progress = 0;\n>\n> opt.branch1 = branch1;\n> @@ -510,6 +515,17 @@ static int real_merge(struct merge_tree_options *o,\n> return !result.clean; /* result.clean < 0 handled above */\n> }\n>\n> +static int option_parse_x(const struct option *opt,\n> + const char *arg, int unset)\n> +{\n> + if (unset)\n> + return 0;\n> +\n> + ALLOC_GROW(xopts, xopts_nr + 1, xopts_alloc);\n> + xopts[xopts_nr++] = xstrdup(arg);\n> + return 0;\n> +}\n> +\n> int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> {\n> struct merge_tree_options o = { .show_messages = -1 };\n> @@ -548,6 +564,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> &merge_base,\n> N_(\"commit\"),\n> N_(\"specify a merge-base for the merge\")),\n> + OPT_CALLBACK('X', \"strategy-option\", &xopts,\n> + N_(\"option=value\"),\n> + N_(\"option for selected merge strategy\"),\n> + option_parse_x),\n> OPT_END()\n> };\n>\n> @@ -556,6 +576,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> argc = parse_options(argc, argv, prefix, mt_options,\n> merge_tree_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n>\n> + for (int x = 0; x < xopts_nr; x++)\n> + if (parse_merge_opt(&o.merge_options, xopts[x]))\n> + die(_(\"unknown strategy option: -X%s\"), xopts[x]);\n> +\n> /* Handle --stdin */\n> if (o.use_stdin) {\n> struct strbuf buf = STRBUF_INIT;\n> diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\n> index 250f721795b..4125bb101ec 100755\n> --- a/t/t4301-merge-tree-write-tree.sh\n> +++ b/t/t4301-merge-tree-write-tree.sh\n> @@ -22,6 +22,7 @@ test_expect_success setup '\n> git branch side1 &&\n> git branch side2 &&\n> git branch side3 &&\n> + git branch side4 &&\n>\n> git checkout side1 &&\n> test_write_lines 1 2 3 4 5 6 >numbers &&\n> @@ -46,6 +47,13 @@ test_expect_success setup '\n> test_tick &&\n> git commit -m rename-numbers &&\n>\n> + git checkout side4 &&\n> + test_write_lines 0 1 2 3 4 5 >numbers &&\n> + echo yo >greeting &&\n> + git add numbers greeting &&\n> + test_tick &&\n> + git commit -m other-content-modifications &&\n> +\n> git switch --orphan unrelated &&\n> >something-else &&\n> git add something-else &&\n> @@ -97,6 +105,21 @@ test_expect_success 'Content merge and a few conflicts' '\n> test_cmp expect actual\n> '\n>\n> +test_expect_success 'Auto resolve conflicts by \"ours\" strategy option' '\n> + git checkout side1^0 &&\n> +\n> + # make sure merge conflict exists\n> + test_must_fail git merge side4 &&\n> + git merge --abort &&\n> +\n> + git merge -X ours side4 &&\n> + git rev-parse HEAD^{tree} > expected &&\n> +\n> + git merge-tree -X ours side1 side4 > actual &&\n\nMultiple style problems still here from V2:\n * There should be no space between the redirection operator ('>')\nand the filename following it.\n * You have indented most lines with tabs, but the line just above\nthis one with 4 spaces.\n\n\n> +\n> + test_cmp expected actual\n> +'\n> +\n> test_expect_success 'Barf on misspelled option, with exit code other than 0 or 1' '\n> # Mis-spell with single \"s\" instead of double \"s\"\n> test_expect_code 129 git merge-tree --write-tree --mesages FOOBAR side1 side2 2>expect &&\n>\n> base-commit: ac83bc5054c2ac489166072334b4147ce6d0fccb\n"},{"id":"481910","messageId":"CABPp-BFDzn=3NzCKyL6aHK0ADbXXBoDhF_sGF8DRdj_VLKQ3SQ@mail.gmail.com","threadId":"60069","inReplyTo":"CAFWsj_UeK-5uyCtBfLYeRZXRMdAD_eFassxEo6FvgeVzmwqLNQ@mail.gmail.com","subject":"Re: [PATCH v3] merge-tree: add -X strategy option","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2023-09-16T03:21:00Z","receivedAt":"2023-09-16T03:23:32Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, Sep 15, 2023 at 7:27 PM 唐宇奕 wrote:\n>\n> Thanks for your advice!\n> I've fixed those blocking issues.\n\nThanks, you did fix most of them. I left a comment on a few things on V3.\n\n> However, regarding the global variable issue, I'm not familiar with C\n> and git code and don't know how to solve this. I think perhaps we need\n> something like closure to parse opt into a local variable?\n\nYou could merely make xopts, xopts_nr, and xopts_alloc local\nvariables, and pass them around as needed. Closures would make things\nlook cleaner perhaps, but certainly aren't necessarily.\n\nBut, I don't think the globals thing is a blocking issue, especially\nsince we already have a number of unnecessary globals in that file\nalready.\n\n> Our usecase is to achieve something like 'range-diff', we first use\n> merge-tree to merge new patchset's base commit with old patchset's\n> source commit, then use the merge result to diff against new\n> patchset's source commit. So we only need to make sure conflict's are\n> handled automatically, leaving other diff features to the second step.\n\nIf you're trying to do something like range-diff, then I don't see why\nthere's any point in creating a merge at all. I'm afraid I don't\nfollow your explanations about the steps you are taking and more\nimportantly why you are taking them, and how it helps you achieve your\ngoal of doing \"something like 'range-diff'\". Could you elaborate?\n"},{"id":"481911","messageId":"pull.1565.v4.git.1694836025469.gitgitgadget@gmail.com","threadId":"60069","inReplyTo":"pull.1565.v3.git.1694830462463.gitgitgadget@gmail.com","subject":"[PATCH v4] merge-tree: add -X strategy option","fromName":"Izzy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-09-16T03:47:05Z","receivedAt":"2023-09-16T03:47:53Z","isPatch":true,"sender":{"key":"name:Izzy","avatar":null},"body":"From: Tang Yuyi \n\nAdd merge strategy option to produce more customizable merge result such\nas automatically resolving conflicts.\n\nSigned-off-by: Tang Yuyi \n---\n merge-tree: add -X strategy option\n \n Change-Id: I16be592262d13cebcff8726eb043f7ecdb313b76\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1565%2FWingT%2Fmerge_tree_allow_strategy_option-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1565/WingT/merge_tree_allow_strategy_option-v4\nPull-Request: https://github.com/gitgitgadget/git/pull/1565\n\nRange-diff vs v3:\n\n 1: d64a774fa7c ! 1: d2d8fcc2e9b merge-tree: add -X strategy option\n @@ Commit message\n merge-tree: add -X strategy option\n \n Add merge strategy option to produce more customizable merge result such\n - as automatically solve conflicts.\n + as automatically resolving conflicts.\n \n Signed-off-by: Tang Yuyi \n \n\n\n builtin/merge-tree.c | 24 ++++++++++++++++++++++++\n t/t4301-merge-tree-write-tree.sh | 23 +++++++++++++++++++++++\n 2 files changed, 47 insertions(+)\n\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex 0de42aecf4b..2ec6ec0d39a 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -19,6 +19,8 @@\n #include \"tree.h\"\n #include \"config.h\"\n \n+static const char **xopts;\n+static size_t xopts_nr, xopts_alloc;\n static int line_termination = '\\n';\n \n struct merge_list {\n@@ -414,6 +416,7 @@ struct merge_tree_options {\n \tint show_messages;\n \tint name_only;\n \tint use_stdin;\n+\tstruct merge_options merge_options;\n };\n \n static int real_merge(struct merge_tree_options *o,\n@@ -439,6 +442,8 @@ static int real_merge(struct merge_tree_options *o,\n \n \tinit_merge_options(&opt, the_repository);\n \n+\topt.recursive_variant = o->merge_options.recursive_variant;\n+\n \topt.show_rename_progress = 0;\n \n \topt.branch1 = branch1;\n@@ -510,6 +515,17 @@ static int real_merge(struct merge_tree_options *o,\n \treturn !result.clean; /* result.clean < 0 handled above */\n }\n \n+static int option_parse_x(const struct option *opt,\n+\t\t\t const char *arg, int unset)\n+{\n+\tif (unset)\n+\t\treturn 0;\n+\n+\tALLOC_GROW(xopts, xopts_nr + 1, xopts_alloc);\n+\txopts[xopts_nr++] = xstrdup(arg);\n+\treturn 0;\n+}\n+\n int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n {\n \tstruct merge_tree_options o = { .show_messages = -1 };\n@@ -548,6 +564,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\t\t &merge_base,\n \t\t\t N_(\"commit\"),\n \t\t\t N_(\"specify a merge-base for the merge\")),\n+\t\tOPT_CALLBACK('X', \"strategy-option\", &xopts,\n+\t\t\tN_(\"option=value\"),\n+\t\t\tN_(\"option for selected merge strategy\"),\n+\t\t\toption_parse_x),\n \t\tOPT_END()\n \t};\n \n@@ -556,6 +576,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, mt_options,\n \t\t\t merge_tree_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n \n+\tfor (int x = 0; x < xopts_nr; x++)\n+\t\tif (parse_merge_opt(&o.merge_options, xopts[x]))\n+\t\t\tdie(_(\"unknown strategy option: -X%s\"), xopts[x]);\n+\n \t/* Handle --stdin */\n \tif (o.use_stdin) {\n \t\tstruct strbuf buf = STRBUF_INIT;\ndiff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\nindex 250f721795b..4125bb101ec 100755\n--- a/t/t4301-merge-tree-write-tree.sh\n+++ b/t/t4301-merge-tree-write-tree.sh\n@@ -22,6 +22,7 @@ test_expect_success setup '\n \tgit branch side1 &&\n \tgit branch side2 &&\n \tgit branch side3 &&\n+\tgit branch side4 &&\n \n \tgit checkout side1 &&\n \ttest_write_lines 1 2 3 4 5 6 >numbers &&\n@@ -46,6 +47,13 @@ test_expect_success setup '\n \ttest_tick &&\n \tgit commit -m rename-numbers &&\n \n+\tgit checkout side4 &&\n+\ttest_write_lines 0 1 2 3 4 5 >numbers &&\n+\techo yo >greeting &&\n+\tgit add numbers greeting &&\n+\ttest_tick &&\n+\tgit commit -m other-content-modifications &&\n+\n \tgit switch --orphan unrelated &&\n \t>something-else &&\n \tgit add something-else &&\n@@ -97,6 +105,21 @@ test_expect_success 'Content merge and a few conflicts' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'Auto resolve conflicts by \"ours\" strategy option' '\n+\tgit checkout side1^0 &&\n+\n+\t# make sure merge conflict exists\n+\ttest_must_fail git merge side4 &&\n+\tgit merge --abort &&\n+\n+\tgit merge -X ours side4 &&\n+\tgit rev-parse HEAD^{tree} > expected &&\n+\n+ git merge-tree -X ours side1 side4 > actual &&\n+\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'Barf on misspelled option, with exit code other than 0 or 1' '\n \t# Mis-spell with single \"s\" instead of double \"s\"\n \ttest_expect_code 129 git merge-tree --write-tree --mesages FOOBAR side1 side2 2>expect &&\n\nbase-commit: ac83bc5054c2ac489166072334b4147ce6d0fccb\n-- \ngitgitgadget\n"},{"id":"481912","messageId":"CABPp-BGLdKMLL96XdF2bEppa-jbWFbC4m0AtSHPb-okavj3e=w@mail.gmail.com","threadId":"60069","inReplyTo":"pull.1565.v4.git.1694836025469.gitgitgadget@gmail.com","subject":"Re: [PATCH v4] merge-tree: add -X strategy option","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2023-09-16T03:55:38Z","receivedAt":"2023-09-16T03:56:55Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nOn Fri, Sep 15, 2023 at 8:47 PM Izzy via GitGitGadget\n wrote:\n>\n> From: Tang Yuyi \n>\n> Add merge strategy option to produce more customizable merge result such\n> as automatically resolving conflicts.\n>\n> Signed-off-by: Tang Yuyi \n> ---\n> merge-tree: add -X strategy option\n>\n> Change-Id: I16be592262d13cebcff8726eb043f7ecdb313b76\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1565%2FWingT%2Fmerge_tree_allow_strategy_option-v4\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1565/WingT/merge_tree_allow_strategy_option-v4\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1565\n>\n> Range-diff vs v3:\n>\n> 1: d64a774fa7c ! 1: d2d8fcc2e9b merge-tree: add -X strategy option\n> @@ Commit message\n> merge-tree: add -X strategy option\n>\n> Add merge strategy option to produce more customizable merge result such\n> - as automatically solve conflicts.\n> + as automatically resolving conflicts.\n\nThanks for fixing this part of what I pointed out in the review.\n\n> Signed-off-by: Tang Yuyi \n>\n>\n>\n> builtin/merge-tree.c | 24 ++++++++++++++++++++++++\n> t/t4301-merge-tree-write-tree.sh | 23 +++++++++++++++++++++++\n> 2 files changed, 47 insertions(+)\n>\n> diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\n> index 0de42aecf4b..2ec6ec0d39a 100644\n> --- a/builtin/merge-tree.c\n> +++ b/builtin/merge-tree.c\n> @@ -19,6 +19,8 @@\n> #include \"tree.h\"\n> #include \"config.h\"\n>\n> +static const char **xopts;\n> +static size_t xopts_nr, xopts_alloc;\n> static int line_termination = '\\n';\n>\n> struct merge_list {\n> @@ -414,6 +416,7 @@ struct merge_tree_options {\n> int show_messages;\n> int name_only;\n> int use_stdin;\n> + struct merge_options merge_options;\n> };\n>\n> static int real_merge(struct merge_tree_options *o,\n> @@ -439,6 +442,8 @@ static int real_merge(struct merge_tree_options *o,\n>\n> init_merge_options(&opt, the_repository);\n>\n> + opt.recursive_variant = o->merge_options.recursive_variant;\n> +\n> opt.show_rename_progress = 0;\n>\n> opt.branch1 = branch1;\n> @@ -510,6 +515,17 @@ static int real_merge(struct merge_tree_options *o,\n> return !result.clean; /* result.clean < 0 handled above */\n> }\n>\n> +static int option_parse_x(const struct option *opt,\n> + const char *arg, int unset)\n> +{\n> + if (unset)\n> + return 0;\n> +\n> + ALLOC_GROW(xopts, xopts_nr + 1, xopts_alloc);\n> + xopts[xopts_nr++] = xstrdup(arg);\n> + return 0;\n> +}\n> +\n> int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> {\n> struct merge_tree_options o = { .show_messages = -1 };\n> @@ -548,6 +564,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> &merge_base,\n> N_(\"commit\"),\n> N_(\"specify a merge-base for the merge\")),\n> + OPT_CALLBACK('X', \"strategy-option\", &xopts,\n> + N_(\"option=value\"),\n> + N_(\"option for selected merge strategy\"),\n> + option_parse_x),\n> OPT_END()\n> };\n>\n> @@ -556,6 +576,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> argc = parse_options(argc, argv, prefix, mt_options,\n> merge_tree_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n>\n> + for (int x = 0; x < xopts_nr; x++)\n> + if (parse_merge_opt(&o.merge_options, xopts[x]))\n> + die(_(\"unknown strategy option: -X%s\"), xopts[x]);\n> +\n> /* Handle --stdin */\n> if (o.use_stdin) {\n> struct strbuf buf = STRBUF_INIT;\n> diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\n> index 250f721795b..4125bb101ec 100755\n> --- a/t/t4301-merge-tree-write-tree.sh\n> +++ b/t/t4301-merge-tree-write-tree.sh\n> @@ -22,6 +22,7 @@ test_expect_success setup '\n> git branch side1 &&\n> git branch side2 &&\n> git branch side3 &&\n> + git branch side4 &&\n>\n> git checkout side1 &&\n> test_write_lines 1 2 3 4 5 6 >numbers &&\n> @@ -46,6 +47,13 @@ test_expect_success setup '\n> test_tick &&\n> git commit -m rename-numbers &&\n>\n> + git checkout side4 &&\n> + test_write_lines 0 1 2 3 4 5 >numbers &&\n> + echo yo >greeting &&\n> + git add numbers greeting &&\n> + test_tick &&\n> + git commit -m other-content-modifications &&\n> +\n> git switch --orphan unrelated &&\n> >something-else &&\n> git add something-else &&\n> @@ -97,6 +105,21 @@ test_expect_success 'Content merge and a few conflicts' '\n> test_cmp expect actual\n> '\n>\n> +test_expect_success 'Auto resolve conflicts by \"ours\" strategy option' '\n> + git checkout side1^0 &&\n> +\n> + # make sure merge conflict exists\n> + test_must_fail git merge side4 &&\n> + git merge --abort &&\n> +\n> + git merge -X ours side4 &&\n> + git rev-parse HEAD^{tree} > expected &&\n> +\n> + git merge-tree -X ours side1 side4 > actual &&\n\nPlease fix the problems in these last two lines as pointed out in both\nof my last two reviews.\n"},{"id":"481913","messageId":"CAFWsj_W-p7kqJbCApZXjGv6Ja0xLjdgVZLVhetwLSiZr-sJA8A@mail.gmail.com","threadId":"60069","inReplyTo":"pull.1565.v4.git.1694836025469.gitgitgadget@gmail.com","subject":"Re: [PATCH v4] merge-tree: add -X strategy option","fromName":"唐宇奕","fromEmail":"winglovet@gmail.com","sentAt":"2023-09-16T04:04:51Z","receivedAt":"2023-09-16T04:05:59Z","isPatch":true,"sender":{"key":"winglovet@gmail.com","avatar":"https://avatars.githubusercontent.com/u/16458679?v=4"},"body":"Thanks, what we want to do is to compare two patchsets' diff while\nproviding capabilities like normal commit diff.\nThe existing range-diff command provide an effective solution, however:\n* The output isn't suitable for machine processing\n* The command take's number of commits and commit messages into count,\nwhile what we really want is merely the content diff\n* The command doesn't support features like word diff, ignoring\nwhitespace changes, etc\nSo what comes to our mind is to simulate user behavior. We first merge\nthe old patchset into the new patchset's base commit, then use the\nmerge result to diff against the new patchset's source commit.\nBy doing this, the diff introduced between two patchsets' bases won't\nbe shown. Thus we get the 'real diff' between two patchsets.\nIn the first step, we use git-merge-tree to produce the merge result\nsince its performance's better than git-merge.\nHowever, sometimes there's conflict between the new patchset's base\nand the old patchset's source.So we need automatic conflict resolving\n- only use content from 'their' side specifically.\nHope that makes sense.\n\nOn Sat, Sep 16, 2023 at 11:47 AM Izzy via GitGitGadget\n wrote:\n>\n> From: Tang Yuyi \n>\n> Add merge strategy option to produce more customizable merge result such\n> as automatically resolving conflicts.\n>\n> Signed-off-by: Tang Yuyi \n> ---\n> merge-tree: add -X strategy option\n>\n> Change-Id: I16be592262d13cebcff8726eb043f7ecdb313b76\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1565%2FWingT%2Fmerge_tree_allow_strategy_option-v4\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1565/WingT/merge_tree_allow_strategy_option-v4\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1565\n>\n> Range-diff vs v3:\n>\n> 1: d64a774fa7c ! 1: d2d8fcc2e9b merge-tree: add -X strategy option\n> @@ Commit message\n> merge-tree: add -X strategy option\n>\n> Add merge strategy option to produce more customizable merge result such\n> - as automatically solve conflicts.\n> + as automatically resolving conflicts.\n>\n> Signed-off-by: Tang Yuyi \n>\n>\n>\n> builtin/merge-tree.c | 24 ++++++++++++++++++++++++\n> t/t4301-merge-tree-write-tree.sh | 23 +++++++++++++++++++++++\n> 2 files changed, 47 insertions(+)\n>\n> diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\n> index 0de42aecf4b..2ec6ec0d39a 100644\n> --- a/builtin/merge-tree.c\n> +++ b/builtin/merge-tree.c\n> @@ -19,6 +19,8 @@\n> #include \"tree.h\"\n> #include \"config.h\"\n>\n> +static const char **xopts;\n> +static size_t xopts_nr, xopts_alloc;\n> static int line_termination = '\\n';\n>\n> struct merge_list {\n> @@ -414,6 +416,7 @@ struct merge_tree_options {\n> int show_messages;\n> int name_only;\n> int use_stdin;\n> + struct merge_options merge_options;\n> };\n>\n> static int real_merge(struct merge_tree_options *o,\n> @@ -439,6 +442,8 @@ static int real_merge(struct merge_tree_options *o,\n>\n> init_merge_options(&opt, the_repository);\n>\n> + opt.recursive_variant = o->merge_options.recursive_variant;\n> +\n> opt.show_rename_progress = 0;\n>\n> opt.branch1 = branch1;\n> @@ -510,6 +515,17 @@ static int real_merge(struct merge_tree_options *o,\n> return !result.clean; /* result.clean < 0 handled above */\n> }\n>\n> +static int option_parse_x(const struct option *opt,\n> + const char *arg, int unset)\n> +{\n> + if (unset)\n> + return 0;\n> +\n> + ALLOC_GROW(xopts, xopts_nr + 1, xopts_alloc);\n> + xopts[xopts_nr++] = xstrdup(arg);\n> + return 0;\n> +}\n> +\n> int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> {\n> struct merge_tree_options o = { .show_messages = -1 };\n> @@ -548,6 +564,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> &merge_base,\n> N_(\"commit\"),\n> N_(\"specify a merge-base for the merge\")),\n> + OPT_CALLBACK('X', \"strategy-option\", &xopts,\n> + N_(\"option=value\"),\n> + N_(\"option for selected merge strategy\"),\n> + option_parse_x),\n> OPT_END()\n> };\n>\n> @@ -556,6 +576,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> argc = parse_options(argc, argv, prefix, mt_options,\n> merge_tree_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n>\n> + for (int x = 0; x < xopts_nr; x++)\n> + if (parse_merge_opt(&o.merge_options, xopts[x]))\n> + die(_(\"unknown strategy option: -X%s\"), xopts[x]);\n> +\n> /* Handle --stdin */\n> if (o.use_stdin) {\n> struct strbuf buf = STRBUF_INIT;\n> diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\n> index 250f721795b..4125bb101ec 100755\n> --- a/t/t4301-merge-tree-write-tree.sh\n> +++ b/t/t4301-merge-tree-write-tree.sh\n> @@ -22,6 +22,7 @@ test_expect_success setup '\n> git branch side1 &&\n> git branch side2 &&\n> git branch side3 &&\n> + git branch side4 &&\n>\n> git checkout side1 &&\n> test_write_lines 1 2 3 4 5 6 >numbers &&\n> @@ -46,6 +47,13 @@ test_expect_success setup '\n> test_tick &&\n> git commit -m rename-numbers &&\n>\n> + git checkout side4 &&\n> + test_write_lines 0 1 2 3 4 5 >numbers &&\n> + echo yo >greeting &&\n> + git add numbers greeting &&\n> + test_tick &&\n> + git commit -m other-content-modifications &&\n> +\n> git switch --orphan unrelated &&\n> >something-else &&\n> git add something-else &&\n> @@ -97,6 +105,21 @@ test_expect_success 'Content merge and a few conflicts' '\n> test_cmp expect actual\n> '\n>\n> +test_expect_success 'Auto resolve conflicts by \"ours\" strategy option' '\n> + git checkout side1^0 &&\n> +\n> + # make sure merge conflict exists\n> + test_must_fail git merge side4 &&\n> + git merge --abort &&\n> +\n> + git merge -X ours side4 &&\n> + git rev-parse HEAD^{tree} > expected &&\n> +\n> + git merge-tree -X ours side1 side4 > actual &&\n> +\n> + test_cmp expected actual\n> +'\n> +\n> test_expect_success 'Barf on misspelled option, with exit code other than 0 or 1' '\n> # Mis-spell with single \"s\" instead of double \"s\"\n> test_expect_code 129 git merge-tree --write-tree --mesages FOOBAR side1 side2 2>expect &&\n>\n> base-commit: ac83bc5054c2ac489166072334b4147ce6d0fccb\n> --\n> gitgitgadget\n"},{"id":"481921","messageId":"20230916061114.GF13092@coredump.intra.peff.net","threadId":"60069","inReplyTo":"pull.1565.v4.git.1694836025469.gitgitgadget@gmail.com","subject":"Re: [PATCH v4] merge-tree: add -X strategy option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-09-16T06:11:14Z","receivedAt":"2023-09-16T06:12:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Sep 16, 2023 at 03:47:05AM +0000, Izzy via GitGitGadget wrote:\n\n> +static int option_parse_x(const struct option *opt,\n> +\t\t\t const char *arg, int unset)\n> +{\n> +\tif (unset)\n> +\t\treturn 0;\n> +\n> +\tALLOC_GROW(xopts, xopts_nr + 1, xopts_alloc);\n> +\txopts[xopts_nr++] = xstrdup(arg);\n> +\treturn 0;\n> +}\n\nThis callback was presumably copied from the one in builtin/merge.c, and\nit suffers from the same \"--no-strategy-option\" bug. You should make a\nsimilar change here to the one we did in dee02da826 (merge: make xopts a\nstrvec, 2023-08-31). And as a bonus, it will make your patch even\nshorter. :)\n\nIt would also make it easier to get rid of the global variables, I\nthink. Something like (squashed into your patch):\n\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex 2ec6ec0d39..e13dbc4c79 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -18,9 +18,8 @@\n #include \"quote.h\"\n #include \"tree.h\"\n #include \"config.h\"\n+#include \"strvec.h\"\n \n-static const char **xopts;\n-static size_t xopts_nr, xopts_alloc;\n static int line_termination = '\\n';\n \n struct merge_list {\n@@ -515,20 +514,10 @@ static int real_merge(struct merge_tree_options *o,\n \treturn !result.clean; /* result.clean < 0 handled above */\n }\n \n-static int option_parse_x(const struct option *opt,\n-\t\t\t const char *arg, int unset)\n-{\n-\tif (unset)\n-\t\treturn 0;\n-\n-\tALLOC_GROW(xopts, xopts_nr + 1, xopts_alloc);\n-\txopts[xopts_nr++] = xstrdup(arg);\n-\treturn 0;\n-}\n-\n int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n {\n \tstruct merge_tree_options o = { .show_messages = -1 };\n+\tstruct strvec xopts = STRVEC_INIT;\n \tint expected_remaining_argc;\n \tint original_argc;\n \tconst char *merge_base = NULL;\n@@ -564,10 +553,8 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\t\t &merge_base,\n \t\t\t N_(\"commit\"),\n \t\t\t N_(\"specify a merge-base for the merge\")),\n-\t\tOPT_CALLBACK('X', \"strategy-option\", &xopts,\n-\t\t\tN_(\"option=value\"),\n-\t\t\tN_(\"option for selected merge strategy\"),\n-\t\t\toption_parse_x),\n+\t\tOPT_STRVEC('X', \"strategy-option\", &xopts, N_(\"option=value\"),\n+\t\t\t N_(\"option for selected merge strategy\")),\n \t\tOPT_END()\n \t};\n \n@@ -576,9 +563,9 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, mt_options,\n \t\t\t merge_tree_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n \n-\tfor (int x = 0; x < xopts_nr; x++)\n-\t\tif (parse_merge_opt(&o.merge_options, xopts[x]))\n-\t\t\tdie(_(\"unknown strategy option: -X%s\"), xopts[x]);\n+\tfor (int x = 0; x < xopts.nr; x++)\n+\t\tif (parse_merge_opt(&o.merge_options, xopts.v[x]))\n+\t\t\tdie(_(\"unknown strategy option: -X%s\"), xopts.v[x]);\n \n \t/* Handle --stdin */\n \tif (o.use_stdin) {\n"},{"id":"481924","messageId":"pull.1565.v5.git.1694853437494.gitgitgadget@gmail.com","threadId":"60069","inReplyTo":"pull.1565.v4.git.1694836025469.gitgitgadget@gmail.com","subject":"[PATCH v5] merge-tree: add -X strategy option","fromName":"Izzy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-09-16T08:37:17Z","receivedAt":"2023-09-16T08:38:12Z","isPatch":true,"sender":{"key":"name:Izzy","avatar":null},"body":"From: Tang Yuyi \n\nAdd merge strategy option to produce more customizable merge result such\nas automatically resolving conflicts.\n\nSigned-off-by: Tang Yuyi \n---\n merge-tree: add -X strategy option\n \n Change-Id: I16be592262d13cebcff8726eb043f7ecdb313b76\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1565%2FWingT%2Fmerge_tree_allow_strategy_option-v5\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1565/WingT/merge_tree_allow_strategy_option-v5\nPull-Request: https://github.com/gitgitgadget/git/pull/1565\n\nRange-diff vs v4:\n\n 1: d2d8fcc2e9b ! 1: 28d4282e0d8 merge-tree: add -X strategy option\n @@ Commit message\n \n ## builtin/merge-tree.c ##\n @@\n + #include \"quote.h\"\n #include \"tree.h\"\n #include \"config.h\"\n ++#include \"strvec.h\"\n \n -+static const char **xopts;\n -+static size_t xopts_nr, xopts_alloc;\n static int line_termination = '\\n';\n \n - struct merge_list {\n @@ builtin/merge-tree.c: struct merge_tree_options {\n \tint show_messages;\n \tint name_only;\n @@ builtin/merge-tree.c: static int real_merge(struct merge_tree_options *o,\n \n \topt.branch1 = branch1;\n @@ builtin/merge-tree.c: static int real_merge(struct merge_tree_options *o,\n - \treturn !result.clean; /* result.clean < 0 handled above */\n - }\n - \n -+static int option_parse_x(const struct option *opt,\n -+\t\t\t const char *arg, int unset)\n -+{\n -+\tif (unset)\n -+\t\treturn 0;\n -+\n -+\tALLOC_GROW(xopts, xopts_nr + 1, xopts_alloc);\n -+\txopts[xopts_nr++] = xstrdup(arg);\n -+\treturn 0;\n -+}\n -+\n int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n {\n \tstruct merge_tree_options o = { .show_messages = -1 };\n ++\tstruct strvec xopts = STRVEC_INIT;\n + \tint expected_remaining_argc;\n + \tint original_argc;\n + \tconst char *merge_base = NULL;\n @@ builtin/merge-tree.c: int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\t\t &merge_base,\n \t\t\t N_(\"commit\"),\n \t\t\t N_(\"specify a merge-base for the merge\")),\n -+\t\tOPT_CALLBACK('X', \"strategy-option\", &xopts,\n -+\t\t\tN_(\"option=value\"),\n -+\t\t\tN_(\"option for selected merge strategy\"),\n -+\t\t\toption_parse_x),\n ++\t\tOPT_STRVEC('X', \"strategy-option\", &xopts, N_(\"option=value\"),\n ++\t\t\tN_(\"option for selected merge strategy\")),\n \t\tOPT_END()\n \t};\n \n @@ builtin/merge-tree.c: int cmd_merge_tree(int argc, const char **argv, const char\n \targc = parse_options(argc, argv, prefix, mt_options,\n \t\t\t merge_tree_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n \n -+\tfor (int x = 0; x < xopts_nr; x++)\n -+\t\tif (parse_merge_opt(&o.merge_options, xopts[x]))\n -+\t\t\tdie(_(\"unknown strategy option: -X%s\"), xopts[x]);\n ++\tfor (int x = 0; x < xopts.nr; x++)\n ++\t\tif (parse_merge_opt(&o.merge_options, xopts.v[x]))\n ++\t\t\tdie(_(\"unknown strategy option: -X%s\"), xopts.v[x]);\n +\n \t/* Handle --stdin */\n \tif (o.use_stdin) {\n @@ t/t4301-merge-tree-write-tree.sh: test_expect_success 'Content merge and a few c\n +\tgit merge --abort &&\n +\n +\tgit merge -X ours side4 &&\n -+\tgit rev-parse HEAD^{tree} > expected &&\n ++\tgit rev-parse HEAD^{tree} >expected &&\n +\n -+ git merge-tree -X ours side1 side4 > actual &&\n ++\tgit merge-tree -X ours side1 side4 >actual &&\n +\n +\ttest_cmp expected actual\n +'\n\n\n builtin/merge-tree.c | 11 +++++++++++\n t/t4301-merge-tree-write-tree.sh | 23 +++++++++++++++++++++++\n 2 files changed, 34 insertions(+)\n\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex 0de42aecf4b..97d0fe6c952 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -18,6 +18,7 @@\n #include \"quote.h\"\n #include \"tree.h\"\n #include \"config.h\"\n+#include \"strvec.h\"\n \n static int line_termination = '\\n';\n \n@@ -414,6 +415,7 @@ struct merge_tree_options {\n \tint show_messages;\n \tint name_only;\n \tint use_stdin;\n+\tstruct merge_options merge_options;\n };\n \n static int real_merge(struct merge_tree_options *o,\n@@ -439,6 +441,8 @@ static int real_merge(struct merge_tree_options *o,\n \n \tinit_merge_options(&opt, the_repository);\n \n+\topt.recursive_variant = o->merge_options.recursive_variant;\n+\n \topt.show_rename_progress = 0;\n \n \topt.branch1 = branch1;\n@@ -513,6 +517,7 @@ static int real_merge(struct merge_tree_options *o,\n int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n {\n \tstruct merge_tree_options o = { .show_messages = -1 };\n+\tstruct strvec xopts = STRVEC_INIT;\n \tint expected_remaining_argc;\n \tint original_argc;\n \tconst char *merge_base = NULL;\n@@ -548,6 +553,8 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\t\t &merge_base,\n \t\t\t N_(\"commit\"),\n \t\t\t N_(\"specify a merge-base for the merge\")),\n+\t\tOPT_STRVEC('X', \"strategy-option\", &xopts, N_(\"option=value\"),\n+\t\t\tN_(\"option for selected merge strategy\")),\n \t\tOPT_END()\n \t};\n \n@@ -556,6 +563,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, mt_options,\n \t\t\t merge_tree_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n \n+\tfor (int x = 0; x < xopts.nr; x++)\n+\t\tif (parse_merge_opt(&o.merge_options, xopts.v[x]))\n+\t\t\tdie(_(\"unknown strategy option: -X%s\"), xopts.v[x]);\n+\n \t/* Handle --stdin */\n \tif (o.use_stdin) {\n \t\tstruct strbuf buf = STRBUF_INIT;\ndiff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\nindex 250f721795b..b2c8a43fce3 100755\n--- a/t/t4301-merge-tree-write-tree.sh\n+++ b/t/t4301-merge-tree-write-tree.sh\n@@ -22,6 +22,7 @@ test_expect_success setup '\n \tgit branch side1 &&\n \tgit branch side2 &&\n \tgit branch side3 &&\n+\tgit branch side4 &&\n \n \tgit checkout side1 &&\n \ttest_write_lines 1 2 3 4 5 6 >numbers &&\n@@ -46,6 +47,13 @@ test_expect_success setup '\n \ttest_tick &&\n \tgit commit -m rename-numbers &&\n \n+\tgit checkout side4 &&\n+\ttest_write_lines 0 1 2 3 4 5 >numbers &&\n+\techo yo >greeting &&\n+\tgit add numbers greeting &&\n+\ttest_tick &&\n+\tgit commit -m other-content-modifications &&\n+\n \tgit switch --orphan unrelated &&\n \t>something-else &&\n \tgit add something-else &&\n@@ -97,6 +105,21 @@ test_expect_success 'Content merge and a few conflicts' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'Auto resolve conflicts by \"ours\" strategy option' '\n+\tgit checkout side1^0 &&\n+\n+\t# make sure merge conflict exists\n+\ttest_must_fail git merge side4 &&\n+\tgit merge --abort &&\n+\n+\tgit merge -X ours side4 &&\n+\tgit rev-parse HEAD^{tree} >expected &&\n+\n+\tgit merge-tree -X ours side1 side4 >actual &&\n+\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'Barf on misspelled option, with exit code other than 0 or 1' '\n \t# Mis-spell with single \"s\" instead of double \"s\"\n \ttest_expect_code 129 git merge-tree --write-tree --mesages FOOBAR side1 side2 2>expect &&\n\nbase-commit: ac83bc5054c2ac489166072334b4147ce6d0fccb\n-- \ngitgitgadget\n"},{"id":"481925","messageId":"CAFWsj_UhsK=M7ZL5OzP1n4+hPigjJW4wjHqXDcbpneR9cfvNSw@mail.gmail.com","threadId":"60069","inReplyTo":"pull.1565.v5.git.1694853437494.gitgitgadget@gmail.com","subject":"Re: [PATCH v5] merge-tree: add -X strategy option","fromName":"唐宇奕","fromEmail":"winglovet@gmail.com","sentAt":"2023-09-16T08:38:58Z","receivedAt":"2023-09-16T08:39:47Z","isPatch":true,"sender":{"key":"winglovet@gmail.com","avatar":"https://avatars.githubusercontent.com/u/16458679?v=4"},"body":"Thank you for your advice! I've uploaded new patch.\n\nOn Sat, Sep 16, 2023 at 4:37 PM Izzy via GitGitGadget\n wrote:\n>\n> From: Tang Yuyi \n>\n> Add merge strategy option to produce more customizable merge result such\n> as automatically resolving conflicts.\n>\n> Signed-off-by: Tang Yuyi \n> ---\n> merge-tree: add -X strategy option\n>\n> Change-Id: I16be592262d13cebcff8726eb043f7ecdb313b76\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1565%2FWingT%2Fmerge_tree_allow_strategy_option-v5\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1565/WingT/merge_tree_allow_strategy_option-v5\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1565\n>\n> Range-diff vs v4:\n>\n> 1: d2d8fcc2e9b ! 1: 28d4282e0d8 merge-tree: add -X strategy option\n> @@ Commit message\n>\n> ## builtin/merge-tree.c ##\n> @@\n> + #include \"quote.h\"\n> #include \"tree.h\"\n> #include \"config.h\"\n> ++#include \"strvec.h\"\n>\n> -+static const char **xopts;\n> -+static size_t xopts_nr, xopts_alloc;\n> static int line_termination = '\\n';\n>\n> - struct merge_list {\n> @@ builtin/merge-tree.c: struct merge_tree_options {\n> int show_messages;\n> int name_only;\n> @@ builtin/merge-tree.c: static int real_merge(struct merge_tree_options *o,\n>\n> opt.branch1 = branch1;\n> @@ builtin/merge-tree.c: static int real_merge(struct merge_tree_options *o,\n> - return !result.clean; /* result.clean < 0 handled above */\n> - }\n> -\n> -+static int option_parse_x(const struct option *opt,\n> -+ const char *arg, int unset)\n> -+{\n> -+ if (unset)\n> -+ return 0;\n> -+\n> -+ ALLOC_GROW(xopts, xopts_nr + 1, xopts_alloc);\n> -+ xopts[xopts_nr++] = xstrdup(arg);\n> -+ return 0;\n> -+}\n> -+\n> int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> {\n> struct merge_tree_options o = { .show_messages = -1 };\n> ++ struct strvec xopts = STRVEC_INIT;\n> + int expected_remaining_argc;\n> + int original_argc;\n> + const char *merge_base = NULL;\n> @@ builtin/merge-tree.c: int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> &merge_base,\n> N_(\"commit\"),\n> N_(\"specify a merge-base for the merge\")),\n> -+ OPT_CALLBACK('X', \"strategy-option\", &xopts,\n> -+ N_(\"option=value\"),\n> -+ N_(\"option for selected merge strategy\"),\n> -+ option_parse_x),\n> ++ OPT_STRVEC('X', \"strategy-option\", &xopts, N_(\"option=value\"),\n> ++ N_(\"option for selected merge strategy\")),\n> OPT_END()\n> };\n>\n> @@ builtin/merge-tree.c: int cmd_merge_tree(int argc, const char **argv, const char\n> argc = parse_options(argc, argv, prefix, mt_options,\n> merge_tree_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n>\n> -+ for (int x = 0; x < xopts_nr; x++)\n> -+ if (parse_merge_opt(&o.merge_options, xopts[x]))\n> -+ die(_(\"unknown strategy option: -X%s\"), xopts[x]);\n> ++ for (int x = 0; x < xopts.nr; x++)\n> ++ if (parse_merge_opt(&o.merge_options, xopts.v[x]))\n> ++ die(_(\"unknown strategy option: -X%s\"), xopts.v[x]);\n> +\n> /* Handle --stdin */\n> if (o.use_stdin) {\n> @@ t/t4301-merge-tree-write-tree.sh: test_expect_success 'Content merge and a few c\n> + git merge --abort &&\n> +\n> + git merge -X ours side4 &&\n> -+ git rev-parse HEAD^{tree} > expected &&\n> ++ git rev-parse HEAD^{tree} >expected &&\n> +\n> -+ git merge-tree -X ours side1 side4 > actual &&\n> ++ git merge-tree -X ours side1 side4 >actual &&\n> +\n> + test_cmp expected actual\n> +'\n>\n>\n> builtin/merge-tree.c | 11 +++++++++++\n> t/t4301-merge-tree-write-tree.sh | 23 +++++++++++++++++++++++\n> 2 files changed, 34 insertions(+)\n>\n> diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\n> index 0de42aecf4b..97d0fe6c952 100644\n> --- a/builtin/merge-tree.c\n> +++ b/builtin/merge-tree.c\n> @@ -18,6 +18,7 @@\n> #include \"quote.h\"\n> #include \"tree.h\"\n> #include \"config.h\"\n> +#include \"strvec.h\"\n>\n> static int line_termination = '\\n';\n>\n> @@ -414,6 +415,7 @@ struct merge_tree_options {\n> int show_messages;\n> int name_only;\n> int use_stdin;\n> + struct merge_options merge_options;\n> };\n>\n> static int real_merge(struct merge_tree_options *o,\n> @@ -439,6 +441,8 @@ static int real_merge(struct merge_tree_options *o,\n>\n> init_merge_options(&opt, the_repository);\n>\n> + opt.recursive_variant = o->merge_options.recursive_variant;\n> +\n> opt.show_rename_progress = 0;\n>\n> opt.branch1 = branch1;\n> @@ -513,6 +517,7 @@ static int real_merge(struct merge_tree_options *o,\n> int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> {\n> struct merge_tree_options o = { .show_messages = -1 };\n> + struct strvec xopts = STRVEC_INIT;\n> int expected_remaining_argc;\n> int original_argc;\n> const char *merge_base = NULL;\n> @@ -548,6 +553,8 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> &merge_base,\n> N_(\"commit\"),\n> N_(\"specify a merge-base for the merge\")),\n> + OPT_STRVEC('X', \"strategy-option\", &xopts, N_(\"option=value\"),\n> + N_(\"option for selected merge strategy\")),\n> OPT_END()\n> };\n>\n> @@ -556,6 +563,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> argc = parse_options(argc, argv, prefix, mt_options,\n> merge_tree_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n>\n> + for (int x = 0; x < xopts.nr; x++)\n> + if (parse_merge_opt(&o.merge_options, xopts.v[x]))\n> + die(_(\"unknown strategy option: -X%s\"), xopts.v[x]);\n> +\n> /* Handle --stdin */\n> if (o.use_stdin) {\n> struct strbuf buf = STRBUF_INIT;\n> diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\n> index 250f721795b..b2c8a43fce3 100755\n> --- a/t/t4301-merge-tree-write-tree.sh\n> +++ b/t/t4301-merge-tree-write-tree.sh\n> @@ -22,6 +22,7 @@ test_expect_success setup '\n> git branch side1 &&\n> git branch side2 &&\n> git branch side3 &&\n> + git branch side4 &&\n>\n> git checkout side1 &&\n> test_write_lines 1 2 3 4 5 6 >numbers &&\n> @@ -46,6 +47,13 @@ test_expect_success setup '\n> test_tick &&\n> git commit -m rename-numbers &&\n>\n> + git checkout side4 &&\n> + test_write_lines 0 1 2 3 4 5 >numbers &&\n> + echo yo >greeting &&\n> + git add numbers greeting &&\n> + test_tick &&\n> + git commit -m other-content-modifications &&\n> +\n> git switch --orphan unrelated &&\n> >something-else &&\n> git add something-else &&\n> @@ -97,6 +105,21 @@ test_expect_success 'Content merge and a few conflicts' '\n> test_cmp expect actual\n> '\n>\n> +test_expect_success 'Auto resolve conflicts by \"ours\" strategy option' '\n> + git checkout side1^0 &&\n> +\n> + # make sure merge conflict exists\n> + test_must_fail git merge side4 &&\n> + git merge --abort &&\n> +\n> + git merge -X ours side4 &&\n> + git rev-parse HEAD^{tree} >expected &&\n> +\n> + git merge-tree -X ours side1 side4 >actual &&\n> +\n> + test_cmp expected actual\n> +'\n> +\n> test_expect_success 'Barf on misspelled option, with exit code other than 0 or 1' '\n> # Mis-spell with single \"s\" instead of double \"s\"\n> test_expect_code 129 git merge-tree --write-tree --mesages FOOBAR side1 side2 2>expect &&\n>\n> base-commit: ac83bc5054c2ac489166072334b4147ce6d0fccb\n> --\n> gitgitgadget\n"},{"id":"481946","messageId":"67638fd7-ad63-4e20-87e1-bef121fef197@gmail.com","threadId":"60069","inReplyTo":"pull.1565.v5.git.1694853437494.gitgitgadget@gmail.com","subject":"Re: [PATCH v5] merge-tree: add -X strategy option","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-09-18T09:53:48Z","receivedAt":"2023-09-18T09:54:54Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 16/09/2023 09:37, Izzy via GitGitGadget wrote:\n> From: Tang Yuyi \n> \n> Add merge strategy option to produce more customizable merge result such\n> as automatically resolving conflicts.\n\nI think adding a merge strategy option to merge-tree is a good idea, but \nhave you tested this with anything apart from -Xours or -Xtheirs? It \nlooks to me like those are the only two that this patch supports. If you \nlook at parse_merge_opt() in merge-recursive.c you will see that there \nare many other options. In order to support all the merge options I \nthink this patch needs a bit of refactoring.\n\n> diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\n> index 0de42aecf4b..97d0fe6c952 100644\n> --- a/builtin/merge-tree.c\n> +++ b/builtin/merge-tree.c\n> static int real_merge(struct merge_tree_options *o,\n> @@ -439,6 +441,8 @@ static int real_merge(struct merge_tree_options *o,\n> \n> \tinit_merge_options(&opt, the_repository);\n> \n> +\topt.recursive_variant = o->merge_options.recursive_variant;\n> +\n\nRather that copying across individual members I think you should \ninitialize o->merge_options properly in cmd_merge_tree() by calling \ninit_merge_options() and then use o->merge_options instead of \"opt\" in \nthis function. That way all the strategy options will be supported.\n\n> \topt.show_rename_progress = 0;\n> \n> \topt.branch1 = branch1;\n> @@ -513,6 +517,7 @@ static int real_merge(struct merge_tree_options *o,\n> int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> {\n> \tstruct merge_tree_options o = { .show_messages = -1 };\n> +\tstruct strvec xopts = STRVEC_INIT;\n> \tint expected_remaining_argc;\n> \tint original_argc;\n> \tconst char *merge_base = NULL;\n> @@ -548,6 +553,8 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> \t\t\t &merge_base,\n> \t\t\t N_(\"commit\"),\n> \t\t\t N_(\"specify a merge-base for the merge\")),\n> +\t\tOPT_STRVEC('X', \"strategy-option\", &xopts, N_(\"option=value\"),\n> +\t\t\tN_(\"option for selected merge strategy\")),\n> \t\tOPT_END()\n> \t};\n> \n> @@ -556,6 +563,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n\nYou should add\n\n\tinit_merge_options(&o.merge_options);\n\nhere to ensure it is properly initialized.\n\n> \targc = parse_options(argc, argv, prefix, mt_options,\n> \t\t\t merge_tree_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n\nThis is the right place to call parse_merge_opt() but I think we should \nfirst check that the user has requested a real merge rather than a \ntrivial merge.\n\n\tif (xopts.nr && o.mode == MODE_TRIVIAL)\n\t\tdie(_(\"--trivial-merge is incompatible with all other options\"));\n\nOtherwise if the user passes in invalid strategy option to a trivial \nmerge they'll get an error about an invalid strategy option rather than \nbeing told --strategy-option is not supported with --trivial-merge.\n\nIdeally there would be a preparatory patch that moves the switch \nstatement that is below the \"if(o.use_stdin)\" block up to this point so \nwe'd always have set o.mode before checking if it is a trivial merge. (I \nthink you'd to change the code slightly when it is moved to add a check \nfor o.use_stdin)\n\nBest Wishes\n\nPhillip\n\n> +\tfor (int x = 0; x < xopts.nr; x++)\n> +\t\tif (parse_merge_opt(&o.merge_options, xopts.v[x]))\n> +\t\t\tdie(_(\"unknown strategy option: -X%s\"), xopts.v[x]);\n> +\n> \t/* Handle --stdin */\n> \tif (o.use_stdin) {\n> \t\tstruct strbuf buf = STRBUF_INIT;\n> diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\n> index 250f721795b..b2c8a43fce3 100755\n> --- a/t/t4301-merge-tree-write-tree.sh\n> +++ b/t/t4301-merge-tree-write-tree.sh\n> @@ -22,6 +22,7 @@ test_expect_success setup '\n> \tgit branch side1 &&\n> \tgit branch side2 &&\n> \tgit branch side3 &&\n> +\tgit branch side4 &&\n> \n> \tgit checkout side1 &&\n> \ttest_write_lines 1 2 3 4 5 6 >numbers &&\n> @@ -46,6 +47,13 @@ test_expect_success setup '\n> \ttest_tick &&\n> \tgit commit -m rename-numbers &&\n> \n> +\tgit checkout side4 &&\n> +\ttest_write_lines 0 1 2 3 4 5 >numbers &&\n> +\techo yo >greeting &&\n> +\tgit add numbers greeting &&\n> +\ttest_tick &&\n> +\tgit commit -m other-content-modifications &&\n> +\n> \tgit switch --orphan unrelated &&\n> \t>something-else &&\n> \tgit add something-else &&\n> @@ -97,6 +105,21 @@ test_expect_success 'Content merge and a few conflicts' '\n> \ttest_cmp expect actual\n> '\n> \n> +test_expect_success 'Auto resolve conflicts by \"ours\" strategy option' '\n> +\tgit checkout side1^0 &&\n> +\n> +\t# make sure merge conflict exists\n> +\ttest_must_fail git merge side4 &&\n> +\tgit merge --abort &&\n> +\n> +\tgit merge -X ours side4 &&\n> +\tgit rev-parse HEAD^{tree} >expected &&\n> +\n> +\tgit merge-tree -X ours side1 side4 >actual &&\n> +\n> +\ttest_cmp expected actual\n> +'\n> +\n> test_expect_success 'Barf on misspelled option, with exit code other than 0 or 1' '\n> \t# Mis-spell with single \"s\" instead of double \"s\"\n> \ttest_expect_code 129 git merge-tree --write-tree --mesages FOOBAR side1 side2 2>expect &&\n> \n> base-commit: ac83bc5054c2ac489166072334b4147ce6d0fccb\n\n"},{"id":"481957","messageId":"xmqqcyyfmpl2.fsf@gitster.g","threadId":"60069","inReplyTo":"67638fd7-ad63-4e20-87e1-bef121fef197@gmail.com","subject":"Re: [PATCH v5] merge-tree: add -X strategy option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-09-18T16:08:25Z","receivedAt":"2023-09-18T16:10:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> ...\n> This is the right place to call parse_merge_opt() but I think we\n> should first check that the user has requested a real merge rather\n> than a trivial merge.\n>\n> \tif (xopts.nr && o.mode == MODE_TRIVIAL)\n> \t\tdie(_(\"--trivial-merge is incompatible with all other options\"));\n>\n> Otherwise if the user passes in invalid strategy option to a trivial\n> merge they'll get an error about an invalid strategy option rather\n> than being told --strategy-option is not supported with\n> --trivial-merge.\n\nI presume that another issue with not having that compatibility\nchecking with \"--trivial\" would be when the user passes a valid\nstrategy option and it would be silently ignored.\n\n> Ideally there would be a preparatory patch that moves the switch\n> statement that is below the \"if(o.use_stdin)\" block up to this point\n> so we'd always have set o.mode before checking if it is a trivial\n> merge. (I think you'd to change the code slightly when it is moved to\n> add a check for o.use_stdin)\n\nThat sounds very good.\n\nThanks for a good \"stepping back a bit\" review. Highly appreciated.\n"},{"id":"482215","messageId":"pull.1565.v6.git.1695522222723.gitgitgadget@gmail.com","threadId":"60069","inReplyTo":"pull.1565.v5.git.1694853437494.gitgitgadget@gmail.com","subject":"[PATCH v6] merge-tree: add -X strategy option","fromName":"Izzy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-09-24T02:23:42Z","receivedAt":"2023-09-24T02:23:49Z","isPatch":true,"sender":{"key":"name:Izzy","avatar":null},"body":"From: Tang Yuyi \n\nAdd merge strategy option to produce more customizable merge result such\nas automatically resolving conflicts.\n\nSigned-off-by: Tang Yuyi \n---\n merge-tree: add -X strategy option\n \n Change-Id: I16be592262d13cebcff8726eb043f7ecdb313b76\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1565%2FWingT%2Fmerge_tree_allow_strategy_option-v6\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1565/WingT/merge_tree_allow_strategy_option-v6\nPull-Request: https://github.com/gitgitgadget/git/pull/1565\n\nRange-diff vs v5:\n\n 1: 28d4282e0d8 ! 1: 6a3dd8aeb13 merge-tree: add -X strategy option\n @@ builtin/merge-tree.c: struct merge_tree_options {\n \n static int real_merge(struct merge_tree_options *o,\n @@ builtin/merge-tree.c: static int real_merge(struct merge_tree_options *o,\n + {\n + \tstruct commit *parent1, *parent2;\n + \tstruct commit_list *merge_bases = NULL;\n +-\tstruct merge_options opt;\n ++\tstruct merge_options opt = o->merge_options;\n + \tstruct merge_result result = { 0 };\n + \tint show_messages = o->show_messages;\n \n - \tinit_merge_options(&opt, the_repository);\n +@@ builtin/merge-tree.c: static int real_merge(struct merge_tree_options *o,\n + \t\thelp_unknown_ref(branch2, \"merge-tree\",\n + \t\t\t\t _(\"not something we can merge\"));\n \n -+\topt.recursive_variant = o->merge_options.recursive_variant;\n -+\n +-\tinit_merge_options(&opt, the_repository);\n +-\n \topt.show_rename_progress = 0;\n \n \topt.branch1 = branch1;\n @@ builtin/merge-tree.c: int cmd_merge_tree(int argc, const char **argv, const char\n \t\tOPT_END()\n \t};\n \n -@@ builtin/merge-tree.c: int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n ++\t/* Init merge options */\n ++\tinit_merge_options(&o.merge_options, the_repository);\n ++\n + \t/* Parse arguments */\n + \toriginal_argc = argc - 1; /* ignoring argv[0] */\n \targc = parse_options(argc, argv, prefix, mt_options,\n \t\t\t merge_tree_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n \n ++\tif (xopts.nr && o.mode == MODE_TRIVIAL)\n ++\t\tdie(_(\"--trivial-merge is incompatible with all other options\"));\n +\tfor (int x = 0; x < xopts.nr; x++)\n +\t\tif (parse_merge_opt(&o.merge_options, xopts.v[x]))\n +\t\t\tdie(_(\"unknown strategy option: -X%s\"), xopts.v[x]);\n\n\n builtin/merge-tree.c | 18 +++++++++++++++---\n t/t4301-merge-tree-write-tree.sh | 23 +++++++++++++++++++++++\n 2 files changed, 38 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex 0de42aecf4b..7024b5ce2e4 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -18,6 +18,7 @@\n #include \"quote.h\"\n #include \"tree.h\"\n #include \"config.h\"\n+#include \"strvec.h\"\n \n static int line_termination = '\\n';\n \n@@ -414,6 +415,7 @@ struct merge_tree_options {\n \tint show_messages;\n \tint name_only;\n \tint use_stdin;\n+\tstruct merge_options merge_options;\n };\n \n static int real_merge(struct merge_tree_options *o,\n@@ -423,7 +425,7 @@ static int real_merge(struct merge_tree_options *o,\n {\n \tstruct commit *parent1, *parent2;\n \tstruct commit_list *merge_bases = NULL;\n-\tstruct merge_options opt;\n+\tstruct merge_options opt = o->merge_options;\n \tstruct merge_result result = { 0 };\n \tint show_messages = o->show_messages;\n \n@@ -437,8 +439,6 @@ static int real_merge(struct merge_tree_options *o,\n \t\thelp_unknown_ref(branch2, \"merge-tree\",\n \t\t\t\t _(\"not something we can merge\"));\n \n-\tinit_merge_options(&opt, the_repository);\n-\n \topt.show_rename_progress = 0;\n \n \topt.branch1 = branch1;\n@@ -513,6 +513,7 @@ static int real_merge(struct merge_tree_options *o,\n int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n {\n \tstruct merge_tree_options o = { .show_messages = -1 };\n+\tstruct strvec xopts = STRVEC_INIT;\n \tint expected_remaining_argc;\n \tint original_argc;\n \tconst char *merge_base = NULL;\n@@ -548,14 +549,25 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\t\t &merge_base,\n \t\t\t N_(\"commit\"),\n \t\t\t N_(\"specify a merge-base for the merge\")),\n+\t\tOPT_STRVEC('X', \"strategy-option\", &xopts, N_(\"option=value\"),\n+\t\t\tN_(\"option for selected merge strategy\")),\n \t\tOPT_END()\n \t};\n \n+\t/* Init merge options */\n+\tinit_merge_options(&o.merge_options, the_repository);\n+\n \t/* Parse arguments */\n \toriginal_argc = argc - 1; /* ignoring argv[0] */\n \targc = parse_options(argc, argv, prefix, mt_options,\n \t\t\t merge_tree_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n \n+\tif (xopts.nr && o.mode == MODE_TRIVIAL)\n+\t\tdie(_(\"--trivial-merge is incompatible with all other options\"));\n+\tfor (int x = 0; x < xopts.nr; x++)\n+\t\tif (parse_merge_opt(&o.merge_options, xopts.v[x]))\n+\t\t\tdie(_(\"unknown strategy option: -X%s\"), xopts.v[x]);\n+\n \t/* Handle --stdin */\n \tif (o.use_stdin) {\n \t\tstruct strbuf buf = STRBUF_INIT;\ndiff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\nindex 250f721795b..b2c8a43fce3 100755\n--- a/t/t4301-merge-tree-write-tree.sh\n+++ b/t/t4301-merge-tree-write-tree.sh\n@@ -22,6 +22,7 @@ test_expect_success setup '\n \tgit branch side1 &&\n \tgit branch side2 &&\n \tgit branch side3 &&\n+\tgit branch side4 &&\n \n \tgit checkout side1 &&\n \ttest_write_lines 1 2 3 4 5 6 >numbers &&\n@@ -46,6 +47,13 @@ test_expect_success setup '\n \ttest_tick &&\n \tgit commit -m rename-numbers &&\n \n+\tgit checkout side4 &&\n+\ttest_write_lines 0 1 2 3 4 5 >numbers &&\n+\techo yo >greeting &&\n+\tgit add numbers greeting &&\n+\ttest_tick &&\n+\tgit commit -m other-content-modifications &&\n+\n \tgit switch --orphan unrelated &&\n \t>something-else &&\n \tgit add something-else &&\n@@ -97,6 +105,21 @@ test_expect_success 'Content merge and a few conflicts' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'Auto resolve conflicts by \"ours\" strategy option' '\n+\tgit checkout side1^0 &&\n+\n+\t# make sure merge conflict exists\n+\ttest_must_fail git merge side4 &&\n+\tgit merge --abort &&\n+\n+\tgit merge -X ours side4 &&\n+\tgit rev-parse HEAD^{tree} >expected &&\n+\n+\tgit merge-tree -X ours side1 side4 >actual &&\n+\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'Barf on misspelled option, with exit code other than 0 or 1' '\n \t# Mis-spell with single \"s\" instead of double \"s\"\n \ttest_expect_code 129 git merge-tree --write-tree --mesages FOOBAR side1 side2 2>expect &&\n\nbase-commit: ac83bc5054c2ac489166072334b4147ce6d0fccb\n-- \ngitgitgadget\n"},{"id":"482216","messageId":"CAFWsj_VxkF7rrK3cxWkcs8Xg+qqGEe0N_2AENNbvX7dG1_rgUA@mail.gmail.com","threadId":"60069","inReplyTo":"pull.1565.v6.git.1695522222723.gitgitgadget@gmail.com","subject":"Re: [PATCH v6] merge-tree: add -X strategy option","fromName":"唐宇奕","fromEmail":"winglovet@gmail.com","sentAt":"2023-09-24T02:26:53Z","receivedAt":"2023-09-24T02:27:08Z","isPatch":true,"sender":{"key":"winglovet@gmail.com","avatar":"https://avatars.githubusercontent.com/u/16458679?v=4"},"body":"Thanks for your advice! I've uploaded a new patch to support more\nstrategy options and the parse option issue.\n"},{"id":"482852","messageId":"a482d047-dd40-436d-8daa-0c74780af11f@gmail.com","threadId":"60069","inReplyTo":"pull.1565.v6.git.1695522222723.gitgitgadget@gmail.com","subject":"Re: [PATCH v6] merge-tree: add -X strategy option","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-10-09T09:58:07Z","receivedAt":"2023-10-09T09:58:23Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 24/09/2023 03:23, Izzy via GitGitGadget wrote:\n\nThanks for updating the patch, sorry it has taken me a while to look at \nthis.\n\n> From: Tang Yuyi \n> \n> Add merge strategy option to produce more customizable merge result such\n> as automatically resolving conflicts.\n> \n> Signed-off-by: Tang Yuyi \n> ---\n\n> static int real_merge(struct merge_tree_options *o,\n> @@ -423,7 +425,7 @@ static int real_merge(struct merge_tree_options *o,\n> {\n> \tstruct commit *parent1, *parent2;\n> \tstruct commit_list *merge_bases = NULL;\n> -\tstruct merge_options opt;\n> +\tstruct merge_options opt = o->merge_options;\n\nCopying struct merge_options by value here is unusual in our code base. \nI don't think it introduces a bug (there is no function to free the \nresources allocated in struct merge_options so we do not end up freeing \nthem twice for example) but it would be clearer that it was safe if you did\n\n\tstruct merge_options *opt = &o->merge_options;\n\nand updated the code to reflect that opt is now a pointer or just \nreplaced all uses of \"opt\" with \"o->merge_options\"\n\n> +\tif (xopts.nr && o.mode == MODE_TRIVIAL)\n> +\t\tdie(_(\"--trivial-merge is incompatible with all other options\"));\n\nThis is good, thanks for adding it.\n\n> +\tfor (int x = 0; x < xopts.nr; x++)\n\nThis is not worth a re-roll on its own but as xopts.nr is a size_t we \nshould declare x to be the same type rather than an int.\n\nThe tests are pretty minimal, ideally we would check that \"-X\" works \nwith \"--stdin\", that it is rejected by a trivial merge and that one of \nthe other strategy options works.\n\nBest Wishes\n\nPhillip\n\n"},{"id":"482865","messageId":"20231009155315.GA3252778@coredump.intra.peff.net","threadId":"60069","inReplyTo":"a482d047-dd40-436d-8daa-0c74780af11f@gmail.com","subject":"Re: [PATCH v6] merge-tree: add -X strategy option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-10-09T15:53:15Z","receivedAt":"2023-10-09T15:54:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 09, 2023 at 10:58:07AM +0100, Phillip Wood wrote:\n\n> > @@ -423,7 +425,7 @@ static int real_merge(struct merge_tree_options *o,\n> > {\n> > \tstruct commit *parent1, *parent2;\n> > \tstruct commit_list *merge_bases = NULL;\n> > -\tstruct merge_options opt;\n> > +\tstruct merge_options opt = o->merge_options;\n> \n> Copying struct merge_options by value here is unusual in our code base. I\n> don't think it introduces a bug (there is no function to free the resources\n> allocated in struct merge_options so we do not end up freeing them twice for\n> example) but it would be clearer that it was safe if you did\n> \n> \tstruct merge_options *opt = &o->merge_options;\n> \n> and updated the code to reflect that opt is now a pointer or just replaced\n> all uses of \"opt\" with \"o->merge_options\"\n\nI agree that struct-copying is an unusual pattern, and we'd potentially\nrun into problems with duplication. But I think it is even trickier than\nthat here. We also go on to actually _modify_ opt in this function,\nassigning to various members (both directly, and I think the merge code\nitself will write to opt->priv).\n\nSo if we use a pointer (rather than struct assignment), those changes\nwill persist in the merge_options struct that was passed in. Which is\nalso weird.\n\nBetween the two, I think using a pointer is probably the least-weird.\nThis real_merge() function is only called once, and is a static-local\nhelper for cmd_merge_tree(). So the two functions work as a single unit,\nand munging \"opt\" is not a big deal.\n\n-Peff\n"},{"id":"482877","messageId":"xmqq4jiz3env.fsf@gitster.g","threadId":"60069","inReplyTo":"20231009155315.GA3252778@coredump.intra.peff.net","subject":"Re: [PATCH v6] merge-tree: add -X strategy option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-10-09T17:10:28Z","receivedAt":"2023-10-09T17:10:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I agree that struct-copying is an unusual pattern, and we'd potentially\n> run into problems with duplication. But I think it is even trickier than\n> that here. We also go on to actually _modify_ opt in this function,\n> assigning to various members (both directly, and I think the merge code\n> itself will write to opt->priv).\n>\n> So if we use a pointer (rather than struct assignment), those changes\n> will persist in the merge_options struct that was passed in. Which is\n> also weird.\n>\n> Between the two, I think using a pointer is probably the least-weird.\n> This real_merge() function is only called once, and is a static-local\n> helper for cmd_merge_tree(). So the two functions work as a single unit,\n> and munging \"opt\" is not a big deal.\n\nIt is called once per --stdin input to perform many merges in a row.\nThe most obvious \"structure to pointer to structure\" conversion\nbelow seems to break an assertion (which is not very surprising, as\nit happens inside that --stdin loop), so I am tempted to revert the\nwhole thing for now.\n\nThanks.\n\n\ngit: merge-ort.c:5110: merge_incore_recursive: Assertion `opt->ancestor == NULL' failed.\n./test-lib.sh: line 1067: 738791 Done printf \"c1 c3\\nc2 -- c1 c3\\nc2 c3\"\n 738792 Aborted | git -C repo merge-tree --stdin > actual\n\n\n builtin/merge-tree.c | 18 +++++++++---------\n 1 file changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git c/builtin/merge-tree.c w/builtin/merge-tree.c\nindex 7024b5ce2e..1cb1fba2de 100644\n--- c/builtin/merge-tree.c\n+++ w/builtin/merge-tree.c\n@@ -425,7 +425,7 @@ static int real_merge(struct merge_tree_options *o,\n {\n \tstruct commit *parent1, *parent2;\n \tstruct commit_list *merge_bases = NULL;\n-\tstruct merge_options opt = o->merge_options;\n+\tstruct merge_options *opt = &o->merge_options;\n \tstruct merge_result result = { 0 };\n \tint show_messages = o->show_messages;\n \n@@ -439,10 +439,10 @@ static int real_merge(struct merge_tree_options *o,\n \t\thelp_unknown_ref(branch2, \"merge-tree\",\n \t\t\t\t _(\"not something we can merge\"));\n \n-\topt.show_rename_progress = 0;\n+\topt->show_rename_progress = 0;\n \n-\topt.branch1 = branch1;\n-\topt.branch2 = branch2;\n+\topt->branch1 = branch1;\n+\topt->branch2 = branch2;\n \n \tif (merge_base) {\n \t\tstruct commit *base_commit;\n@@ -452,11 +452,11 @@ static int real_merge(struct merge_tree_options *o,\n \t\tif (!base_commit)\n \t\t\tdie(_(\"could not lookup commit '%s'\"), merge_base);\n \n-\t\topt.ancestor = merge_base;\n+\t\topt->ancestor = merge_base;\n \t\tbase_tree = repo_get_commit_tree(the_repository, base_commit);\n \t\tparent1_tree = repo_get_commit_tree(the_repository, parent1);\n \t\tparent2_tree = repo_get_commit_tree(the_repository, parent2);\n-\t\tmerge_incore_nonrecursive(&opt, base_tree, parent1_tree, parent2_tree, &result);\n+\t\tmerge_incore_nonrecursive(opt, base_tree, parent1_tree, parent2_tree, &result);\n \t} else {\n \t\t/*\n \t\t * Get the merge bases, in reverse order; see comment above\n@@ -467,7 +467,7 @@ static int real_merge(struct merge_tree_options *o,\n \t\tif (!merge_bases && !o->allow_unrelated_histories)\n \t\t\tdie(_(\"refusing to merge unrelated histories\"));\n \t\tmerge_bases = reverse_commit_list(merge_bases);\n-\t\tmerge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);\n+\t\tmerge_incore_recursive(opt, merge_bases, parent1, parent2, &result);\n \t}\n \n \tif (result.clean < 0)\n@@ -501,12 +501,12 @@ static int real_merge(struct merge_tree_options *o,\n \t}\n \tif (show_messages) {\n \t\tputchar(line_termination);\n-\t\tmerge_display_update_messages(&opt, line_termination == '\\0',\n+\t\tmerge_display_update_messages(opt, line_termination == '\\0',\n \t\t\t\t\t &result);\n \t}\n \tif (o->use_stdin)\n \t\tputchar(line_termination);\n-\tmerge_finalize(&opt, &result);\n+\tmerge_finalize(opt, &result);\n \treturn !result.clean; /* result.clean < 0 handled above */\n }\n \n\n"},{"id":"482894","messageId":"20231009185234.GB3270793@coredump.intra.peff.net","threadId":"60069","inReplyTo":"xmqq4jiz3env.fsf@gitster.g","subject":"Re: [PATCH v6] merge-tree: add -X strategy option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-10-09T18:52:34Z","receivedAt":"2023-10-09T18:52:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 09, 2023 at 10:10:28AM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > I agree that struct-copying is an unusual pattern, and we'd potentially\n> > run into problems with duplication. But I think it is even trickier than\n> > that here. We also go on to actually _modify_ opt in this function,\n> > assigning to various members (both directly, and I think the merge code\n> > itself will write to opt->priv).\n> >\n> > So if we use a pointer (rather than struct assignment), those changes\n> > will persist in the merge_options struct that was passed in. Which is\n> > also weird.\n> >\n> > Between the two, I think using a pointer is probably the least-weird.\n> > This real_merge() function is only called once, and is a static-local\n> > helper for cmd_merge_tree(). So the two functions work as a single unit,\n> > and munging \"opt\" is not a big deal.\n> \n> It is called once per --stdin input to perform many merges in a row.\n> The most obvious \"structure to pointer to structure\" conversion\n> below seems to break an assertion (which is not very surprising, as\n> it happens inside that --stdin loop), so I am tempted to revert the\n> whole thing for now.\n\nOops, I totally missed the loop around the call to real_merge(). So\nyeah, I think this is rather tricky.\n\nBefore Izzy's patch, real_merge() always makes its own fresh\nmerge_options. After, we have a template merge_options that we copy, but\nwe are assuming that a shallow struct copy is OK (probably true, but an\nanti-pattern that may bite us later). If we add Phillip's suggestion on\ntop, then we do not copy at all, and end up reusing the same options\nstruct (which is definitely wrong).\n\nI don't think there are any bugs with the state at the current tip of\nty/merge-tree-strategy-options, but if we want to make it safer, I think\nwe have two options:\n\n - delay the conversion of the \"xopts\" list into merge_options until we\n initialize it in real_merge(). This avoids breaking abstraction\n boundaries, but does mean that the sanity-checking of the options\n happens a little later (but not much in practice).\n\n - provide a copy_merge_options() function, which makes this kind of\n \"set up a template and then copy it\" pattern official. It can be a\n struct assignment for now, but it at least alerts anybody adding new\n options to the notion that a deep copy might be required.\n\nOption 1 looks something like this (a lot of the hunks are just\nreverting the tip of that branch, so squashed in it would be even\nsmaller):\n\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex 7024b5ce2e..f9dbbdb867 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -415,7 +415,7 @@ struct merge_tree_options {\n \tint show_messages;\n \tint name_only;\n \tint use_stdin;\n-\tstruct merge_options merge_options;\n+\tstruct strvec xopts;\n };\n \n static int real_merge(struct merge_tree_options *o,\n@@ -425,7 +425,7 @@ static int real_merge(struct merge_tree_options *o,\n {\n \tstruct commit *parent1, *parent2;\n \tstruct commit_list *merge_bases = NULL;\n-\tstruct merge_options opt = o->merge_options;\n+\tstruct merge_options opt;\n \tstruct merge_result result = { 0 };\n \tint show_messages = o->show_messages;\n \n@@ -439,11 +439,17 @@ static int real_merge(struct merge_tree_options *o,\n \t\thelp_unknown_ref(branch2, \"merge-tree\",\n \t\t\t\t _(\"not something we can merge\"));\n \n+\tinit_merge_options(&opt, the_repository);\n+\n \topt.show_rename_progress = 0;\n \n \topt.branch1 = branch1;\n \topt.branch2 = branch2;\n \n+\tfor (size_t x = 0; x < o->xopts.nr; x++)\n+\t\tif (parse_merge_opt(&opt, o->xopts.v[x]))\n+\t\t\tdie(_(\"unknown strategy option: -X%s\"), o->xopts.v[x]);\n+\n \tif (merge_base) {\n \t\tstruct commit *base_commit;\n \t\tstruct tree *base_tree, *parent1_tree, *parent2_tree;\n@@ -512,8 +518,7 @@ static int real_merge(struct merge_tree_options *o,\n \n int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n {\n-\tstruct merge_tree_options o = { .show_messages = -1 };\n-\tstruct strvec xopts = STRVEC_INIT;\n+\tstruct merge_tree_options o = { .show_messages = -1,.xopts = STRVEC_INIT };\n \tint expected_remaining_argc;\n \tint original_argc;\n \tconst char *merge_base = NULL;\n@@ -549,24 +554,18 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\t\t &merge_base,\n \t\t\t N_(\"commit\"),\n \t\t\t N_(\"specify a merge-base for the merge\")),\n-\t\tOPT_STRVEC('X', \"strategy-option\", &xopts, N_(\"option=value\"),\n+\t\tOPT_STRVEC('X', \"strategy-option\", &o.xopts, N_(\"option=value\"),\n \t\t\tN_(\"option for selected merge strategy\")),\n \t\tOPT_END()\n \t};\n \n-\t/* Init merge options */\n-\tinit_merge_options(&o.merge_options, the_repository);\n-\n \t/* Parse arguments */\n \toriginal_argc = argc - 1; /* ignoring argv[0] */\n \targc = parse_options(argc, argv, prefix, mt_options,\n \t\t\t merge_tree_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n \n-\tif (xopts.nr && o.mode == MODE_TRIVIAL)\n+\tif (o.xopts.nr && o.mode == MODE_TRIVIAL)\n \t\tdie(_(\"--trivial-merge is incompatible with all other options\"));\n-\tfor (int x = 0; x < xopts.nr; x++)\n-\t\tif (parse_merge_opt(&o.merge_options, xopts.v[x]))\n-\t\t\tdie(_(\"unknown strategy option: -X%s\"), xopts.v[x]);\n \n \t/* Handle --stdin */\n \tif (o.use_stdin) {\n"},{"id":"483082","messageId":"xmqqlec9kl0k.fsf@gitster.g","threadId":"60069","inReplyTo":"20231009185234.GB3270793@coredump.intra.peff.net","subject":"Re: [PATCH v6] merge-tree: add -X strategy option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-10-11T19:38:03Z","receivedAt":"2023-10-11T19:38:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Oops, I totally missed the loop around the call to real_merge(). So\n> yeah, I think this is rather tricky.\n> ...\n> I don't think there are any bugs with the state at the current tip of\n> ty/merge-tree-strategy-options, but if we want to make it safer, I think\n> we have two options:\n>\n> - delay the conversion of the \"xopts\" list into merge_options until we\n> initialize it in real_merge(). This avoids breaking abstraction\n> boundaries, but does mean that the sanity-checking of the options\n> happens a little later (but not much in practice).\n>\n> - provide a copy_merge_options() function, which makes this kind of\n> \"set up a template and then copy it\" pattern official. It can be a\n> struct assignment for now, but it at least alerts anybody adding new\n> options to the notion that a deep copy might be required.\n>\n> Option 1 looks something like this (a lot of the hunks are just\n> reverting the tip of that branch, so squashed in it would be even\n> smaller):\n\nIf we have no plan and intention to extend \"merge-tree\" even more in\nthe future, then option 1 would be the approach with least patch\nnoise, and as your \"something like this\" shows, it is a nice and\nclean solution. I very much like it.\n\nBut as the renovated \"merge-tree\" is a relatively young thing in our\ntoolbox, I suspect that more and more work may want to go into it.\nAnd the other \"official copy_merge_options()\" approach would be a\nmore healthy solution in the longer run, I would think. If we were\nto go that route, we should also give an interface to free the\nresources held by the copy.\n\nIt is not that much code on top of the commit that is already queued\nin 'next', I suspect. Perhaps something like this?\n\n builtin/merge-tree.c | 4 +++-\n merge-recursive.c | 16 ++++++++++++++++\n merge-recursive.h | 3 +++\n 3 files changed, 22 insertions(+), 1 deletion(-)\n\ndiff --git c/builtin/merge-tree.c w/builtin/merge-tree.c\nindex 7024b5ce2e..a35e0452d6 100644\n--- c/builtin/merge-tree.c\n+++ w/builtin/merge-tree.c\n@@ -425,10 +425,11 @@ static int real_merge(struct merge_tree_options *o,\n {\n \tstruct commit *parent1, *parent2;\n \tstruct commit_list *merge_bases = NULL;\n-\tstruct merge_options opt = o->merge_options;\n \tstruct merge_result result = { 0 };\n \tint show_messages = o->show_messages;\n+\tstruct merge_options opt;\n \n+\tcopy_merge_options(&opt, &o->merge_options);\n \tparent1 = get_merge_parent(branch1);\n \tif (!parent1)\n \t\thelp_unknown_ref(branch1, \"merge-tree\",\n@@ -507,6 +508,7 @@ static int real_merge(struct merge_tree_options *o,\n \tif (o->use_stdin)\n \t\tputchar(line_termination);\n \tmerge_finalize(&opt, &result);\n+\tclear_merge_options(&opt);\n \treturn !result.clean; /* result.clean < 0 handled above */\n }\n \ndiff --git c/merge-recursive.c w/merge-recursive.c\nindex 0d7e57e2df..e3beb0801b 100644\n--- c/merge-recursive.c\n+++ w/merge-recursive.c\n@@ -3912,6 +3912,22 @@ void init_merge_options(struct merge_options *opt,\n \t\topt->buffer_output = 0;\n }\n \n+/*\n+ * For now, members of merge_options do not need deep copying, but\n+ * it may change in the future, in which case we would need to update\n+ * this, and also make a matching change to clear_merge_options() to\n+ * release the resources held by a copied instance.\n+ */\n+void copy_merge_options(struct merge_options *dst, struct merge_options *src)\n+{\n+\t*dst = *src;\n+}\n+\n+void clear_merge_options(struct merge_options *opt UNUSED)\n+{\n+\t; /* no-op as our copy is shallow right now */\n+}\n+\n int parse_merge_opt(struct merge_options *opt, const char *s)\n {\n \tconst char *arg;\ndiff --git c/merge-recursive.h w/merge-recursive.h\nindex b88000e3c2..3d3b3e3c29 100644\n--- c/merge-recursive.h\n+++ w/merge-recursive.h\n@@ -55,6 +55,9 @@ struct merge_options {\n \n void init_merge_options(struct merge_options *opt, struct repository *repo);\n \n+void copy_merge_options(struct merge_options *dst, struct merge_options *src);\n+void clear_merge_options(struct merge_options *opt);\n+\n /* parse the option in s and update the relevant field of opt */\n int parse_merge_opt(struct merge_options *opt, const char *s);\n \n\n"},{"id":"483094","messageId":"20231011214340.GA518221@coredump.intra.peff.net","threadId":"60069","inReplyTo":"xmqqlec9kl0k.fsf@gitster.g","subject":"Re: [PATCH v6] merge-tree: add -X strategy option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-10-11T21:43:40Z","receivedAt":"2023-10-11T21:43:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 11, 2023 at 12:38:03PM -0700, Junio C Hamano wrote:\n\n> If we have no plan and intention to extend \"merge-tree\" even more in\n> the future, then option 1 would be the approach with least patch\n> noise, and as your \"something like this\" shows, it is a nice and\n> clean solution. I very much like it.\n> \n> But as the renovated \"merge-tree\" is a relatively young thing in our\n> toolbox, I suspect that more and more work may want to go into it.\n> And the other \"official copy_merge_options()\" approach would be a\n> more healthy solution in the longer run, I would think. If we were\n> to go that route, we should also give an interface to free the\n> resources held by the copy.\n\nI am happy with either, as they both resolve the \"merge-tree knows\nintimate details about merge_options\" issue. The patch I showed would\nrequire manually passing more details down to real_merge(), which is I\nguess what you are getting at with the \"more work may want to go into\nit\".\n\n> It is not that much code on top of the commit that is already queued\n> in 'next', I suspect. Perhaps something like this?\n\nThis looks OK, though...\n\n> +void clear_merge_options(struct merge_options *opt UNUSED)\n> +{\n> +\t; /* no-op as our copy is shallow right now */\n> +}\n\nClearing is generally not just about copies, but any use of the struct.\nso this invites the question of whether the original non-copy struct\nshould have a call to clear_merge_options() in cmd_merge_tree(). And\nditto for every other user.\n\nI do not mind adding such calls, but of course we know that they are\ncurrently noops (and we don't have any particular plan to change that).\n\n-Peff\n"},{"id":"483097","messageId":"xmqqedi0kdjj.fsf@gitster.g","threadId":"60069","inReplyTo":"20231011214340.GA518221@coredump.intra.peff.net","subject":"Re: [PATCH v6] merge-tree: add -X strategy option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-10-11T22:19:28Z","receivedAt":"2023-10-11T22:19:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I am happy with either, as they both resolve the \"merge-tree knows\n> intimate details about merge_options\" issue. The patch I showed would\n> require manually passing more details down to real_merge(), which is I\n> guess what you are getting at with the \"more work may want to go into\n> it\".\n\nI was alluding more about teaching \"merge-tree\" various optional\nbehaviour merge_options represents. In today's patch it may be\n-X, who knows what tomorrow's patch wants to bring\n\"merge-tree\" to feature-parity with \"merge\". And the first approach\nwould mean we would add xopts today to the struct, but we will be\nrequired passing more details as we add other things.\n\n>> It is not that much code on top of the commit that is already queued\n>> in 'next', I suspect. Perhaps something like this?\n>\n> This looks OK, though...\n>\n>> +void clear_merge_options(struct merge_options *opt UNUSED)\n>> +{\n>> +\t; /* no-op as our copy is shallow right now */\n>> +}\n>\n> Clearing is generally not just about copies, but any use of the struct.\n> so this invites the question of whether the original non-copy struct\n> should have a call to clear_merge_options() in cmd_merge_tree(). And\n> ditto for every other user.\n\nYes, once we start leaking, somebody hopefully notice the lack of a\ncall to this on the original/template copy and add one. Until then...\n"}]}