{"thread":{"id":"58703","subject":"[PATCH] merge-tree.c: add --merge-base= option","startedAt":"2022-10-27T12:12:48Z","lastAt":"2022-11-25T05:28:59Z","messageCount":43,"participants":["Kyle Zhao via GitGitGadget","Junio C Hamano","Elijah Newren","kylezhao (赵柯宇)","Taylor Blau","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"465868","messageId":"pull.1397.git.1666872761355.gitgitgadget@gmail.com","threadId":"58703","inReplyTo":null,"subject":"[PATCH] merge-tree.c: add --merge-base= option","fromName":"Kyle Zhao via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-10-27T12:12:41Z","receivedAt":"2022-10-27T12:12:48Z","isPatch":true,"sender":{"key":"goldofzky@gmail.com","avatar":"https://gravatar.com/avatar/aa4fa27f0c1bed9c986ed596415799518e03a6e71a0931308a4de87e166e183a?d=mp&s=160"},"body":"From: Kyle Zhao \n\nThis option allows users to specify a merge-base commit for the merge.\n\nIt will give our callers more flexibility to use the `git merge-tree`.\nFor example:\n\n git merge-tree --merge-base=^1 source-branch \n\nThis allows us to implement `git cherry-pick` in bare repositories.\n\nSigned-off-by: Kyle Zhao \n---\n merge-tree.c: add --merge-base= option\n \n Thanks for Elijah's work. I'm very excited that merge-ort is integrated\n into the git merge-tree, which means that we can use merge-ort in bare\n repositories to optimize merge performance.\n \n In this patch, I introduce a new --merge-base= option to allow\n callers to specify a merge-base for the merge. This may allow users to\n implement git cherry-pick and git rebase in bare repositories with git\n merge-tree cmd.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1397%2Fkeyu98%2Fkz%2Fmerge-tree-option-merge-base-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1397/keyu98/kz/merge-tree-option-merge-base-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1397\n\n Documentation/git-merge-tree.txt | 4 +++\n builtin/merge-tree.c | 23 +++++++++++++----\n t/t4301-merge-tree-write-tree.sh | 44 ++++++++++++++++++++++++++++++++\n 3 files changed, 66 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\nindex d6c356740ef..e762209b76d 100644\n--- a/Documentation/git-merge-tree.txt\n+++ b/Documentation/git-merge-tree.txt\n@@ -64,6 +64,10 @@ OPTIONS\n \tshare no common history. This flag can be given to override that\n \tcheck and make the merge proceed anyway.\n \n+--merge-base=::\n+\tInstead of finding the merge-bases for and ,\n+\tspecify a merge-base for the merge.\n+\n [[OUTPUT]]\n OUTPUT\n ------\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex ae5782917b9..adc461f00f3 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -3,6 +3,7 @@\n #include \"tree-walk.h\"\n #include \"xdiff-interface.h\"\n #include \"help.h\"\n+#include \"commit.h\"\n #include \"commit-reach.h\"\n #include \"merge-ort.h\"\n #include \"object-store.h\"\n@@ -402,6 +403,7 @@ struct merge_tree_options {\n \tint allow_unrelated_histories;\n \tint show_messages;\n \tint name_only;\n+\tchar* merge_base;\n };\n \n static int real_merge(struct merge_tree_options *o,\n@@ -430,11 +432,18 @@ static int real_merge(struct merge_tree_options *o,\n \topt.branch1 = branch1;\n \topt.branch2 = branch2;\n \n-\t/*\n-\t * Get the merge bases, in reverse order; see comment above\n-\t * merge_incore_recursive in merge-ort.h\n-\t */\n-\tmerge_bases = get_merge_bases(parent1, parent2);\n+\tif (o->merge_base) {\n+\t\tstruct commit *c = lookup_commit_reference_by_name(o->merge_base);\n+\t\tif (!c)\n+\t\t\tdie(_(\"could not lookup commit %s\"), o->merge_base);\n+\t\tcommit_list_insert(c, &merge_bases);\n+\t} else {\n+\t\t/*\n+\t\t * Get the merge bases, in reverse order; see comment above\n+\t\t * merge_incore_recursive in merge-ort.h\n+\t\t */\n+\t\tmerge_bases = get_merge_bases(parent1, parent2);\n+\t}\n \tif (!merge_bases && !o->allow_unrelated_histories)\n \t\tdie(_(\"refusing to merge unrelated histories\"));\n \tmerge_bases = reverse_commit_list(merge_bases);\n@@ -505,6 +514,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\t\t &o.allow_unrelated_histories,\n \t\t\t N_(\"allow merging unrelated histories\"),\n \t\t\t PARSE_OPT_NONEG),\n+\t\tOPT_STRING(0, \"merge-base\",\n+\t\t\t &o.merge_base,\n+\t\t\t N_(\"commit\"),\n+\t\t\t N_(\"specify a merge-base commit for the merge\")),\n \t\tOPT_END()\n \t};\n \ndiff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\nindex 013b77144bd..032ff4a1c3d 100755\n--- a/t/t4301-merge-tree-write-tree.sh\n+++ b/t/t4301-merge-tree-write-tree.sh\n@@ -819,4 +819,48 @@ test_expect_success SANITY 'merge-ort fails gracefully in a read-only repository\n \ttest_must_fail git -C read-only merge-tree side1 side2\n '\n \n+# specify merge-base as parent of branch2.\n+# git merge-tree --merge-base=A O B\n+# Commit O: foo, bar\n+# Commit A: modify foo after Commit O\n+# Commit B: modify bar after Commit A\n+# Expected: foo is unchanged, modify bar\n+\n+test_expect_success 'specify merge-base as parent of branch2' '\n+\t# Setup\n+\tgit init base-b2-p && (\n+\t\tcd base-b2-p &&\n+\t\techo foo >foo &&\n+\t\techo bar >bar &&\n+\t\tgit add foo bar &&\n+\t\tgit commit -m O &&\n+\n+\t\tgit branch O &&\n+\t\tgit branch A &&\n+\n+\t\tgit checkout A &&\n+\t\techo \"A\" >foo &&\n+\t\tgit add foo &&\n+\t\tgit commit -m A &&\n+\n+\t\tgit checkout -b B &&\n+\t\techo \"B\" >bar &&\n+\t\tgit add bar &&\n+\t\tgit commit -m B\n+\t) &&\n+\t# Testing\n+\t(\n+\t\tcd base-b2-p &&\n+\t\tTREE_OID=$(git merge-tree --merge-base=A O B) &&\n+\n+\t\tq_to_tab <<-EOF >expect &&\n+\t\t100644 blob $(git rev-parse B:bar)Qbar\n+\t\t100644 blob $(git rev-parse O:foo)Qfoo\n+\t\tEOF\n+\n+\t\tgit ls-tree $TREE_OID >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n test_done\n\nbase-commit: db29e6bbaed156ae9025ff0474fd788e58230367\n-- \ngitgitgadget\n"},{"id":"465887","messageId":"xmqqczadm93w.fsf@gitster.g","threadId":"58703","inReplyTo":"pull.1397.git.1666872761355.gitgitgadget@gmail.com","subject":"Re: [PATCH] merge-tree.c: add --merge-base= option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-27T18:09:39Z","receivedAt":"2022-10-27T18:09:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kyle Zhao via GitGitGadget\" writes:\n\n> diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\n> index d6c356740ef..e762209b76d 100644\n> --- a/Documentation/git-merge-tree.txt\n> +++ b/Documentation/git-merge-tree.txt\n> @@ -64,6 +64,10 @@ OPTIONS\n> \tshare no common history. This flag can be given to override that\n> \tcheck and make the merge proceed anyway.\n> \n> +--merge-base=::\n> +\tInstead of finding the merge-bases for and ,\n> +\tspecify a merge-base for the merge.\n\nI like adding and exposing this feature to allow the end-user\nspecify which commit to use as the base (instead of allowing the\ntool compute it from the two branches), but I wonder if a new option\nis even needed.\n\nIn the original \"trivial merge\" mode, the command takes three trees\nwithout having to have this new option. In the new \"write-tree\"\nmode, currently it is incapable of taking the base, but it does not\nhave to stay that way. Wouldn't it be sufficient to update the UI\nto\n\n git merge-tree [--write-tree] [] [] \n git merge-tree [--trivial-merge] \n\nIOW, when you want to supply the base, you'd be explicit and ask for\nthe new \"write-tree\" mode, i.e.\n\n $ git merge-tree --write-tree $(git merge-base branch^ branch) HEAD branch \n\nwould be how you would use merge-tree to cherry-pick the commit at\nthe tip of the branch on top of the current commit.\n\n> @@ -402,6 +403,7 @@ struct merge_tree_options {\n> \tint allow_unrelated_histories;\n> \tint show_messages;\n> \tint name_only;\n> +\tchar* merge_base;\n\nStyle. We write in C, not in C++, and our asterisks stick to\nvariables and members of structs, not types.\n\n> -\t/*\n> -\t * Get the merge bases, in reverse order; see comment above\n> -\t * merge_incore_recursive in merge-ort.h\n> -\t */\n> -\tmerge_bases = get_merge_bases(parent1, parent2);\n> +\tif (o->merge_base) {\n> +\t\tstruct commit *c = lookup_commit_reference_by_name(o->merge_base);\n> +\t\tif (!c)\n> +\t\t\tdie(_(\"could not lookup commit %s\"), o->merge_base);\n> +\t\tcommit_list_insert(c, &merge_bases);\n\nCurious. The original code unconditionally assigned merge_bases, so\nthere wasn't a good reason to initialize the variable before this point,\nbut this new code assumes that merge_bases to be initialized to NULL.\n\nLuckily, it is initialized in the original code, even though it\nwasn't necessary at all. So this new code can work correctly.\nGood.\n\n> +\t} else {\n> +\t\t/*\n> +\t\t * Get the merge bases, in reverse order; see comment above\n> +\t\t * merge_incore_recursive in merge-ort.h\n> +\t\t */\n> +\t\tmerge_bases = get_merge_bases(parent1, parent2);\n> +\t}\n\nYes, this feature was very much lacking and is a welcome addition.\n\nI also have to wonder how this should interact with a topic that is\nin-flight to feed multiple merge-tree requests from the standard\ninput to have a single process perform multiple (not necessarily\nrelated) merges. Elijah knows much better, but my gut feeling is\nthat it shouldn't be hard to allow feeding an extra commit on the\nsame line to be used as the base.\n\nThanks.\n"},{"id":"465934","messageId":"CABPp-BGBFyoY7m+KCA9MTifKmpZ0ccLgsYHahMCgPxuTpuUGPg@mail.gmail.com","threadId":"58703","inReplyTo":"pull.1397.git.1666872761355.gitgitgadget@gmail.com","subject":"Re: [PATCH] merge-tree.c: add --merge-base= option","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-10-28T08:13:45Z","receivedAt":"2022-10-28T08:14:04Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi Kyle!\n\nThanks for the interest in this area of the code. There's lots of\ninteresting history you're probably not aware of here...\n\nOn Thu, Oct 27, 2022 at 5:12 AM Kyle Zhao via GitGitGadget\n wrote:\n>\n> From: Kyle Zhao \n>\n> This option allows users to specify a merge-base commit for the merge.\n\nNote that this has been implemented previously. I may have\nimplemented it previously and ripped it out (about a year ago), or\nmaybe I just avoided it once I ran across the reasons below. Dscho\nalso implemented it over at\nhttps://lore.kernel.org/git/nycvar.QRO.7.76.6.2202210956430.26495@tvgsbejvaqbjf.bet/\n\nYou may want to take a look at his code for comparison, especially the\nmerge_incore_nonrecursive() stuff.\n\n> It will give our callers more flexibility to use the `git merge-tree`.\n> For example:\n>\n> git merge-tree --merge-base=^1 source-branch \n>\n> This allows us to implement `git cherry-pick` in bare repositories.\n\nThis capability is something we may ultimately want for completeness,\nbut it definitely should not be marketed as a building block for\nimplementing cherry-pick or rebase. There are several reasons for\nthat:\n\nPerformance reasons:\n * it's a design with a separate fork for every commit to be picked,\nwhich is quite wasteful. cherry-pick and rebase should do better.\n * it is explicitly single-commit replaying, which discards the\nremembering renames optimization I placed into merge-ort. We should\nhave something series-aware so it can take advantage of that.\n\nUsability/Design reasons:\n * with cherry-picking or rebasing you often have a sequence of\ncommits. Picking them one at a time not only has the performance\nissues above, but also introduces usability and design questions\naround the creation of commits from the toplevel trees created by\nmerge-tree. For normal merges, we have no idea what the commit\nmessage should be without explicit directives and user editing; this\nis reflected in the fact that `git merge` has 9 different flags for\ncontrolling how to specify the commit message or components of it or\nhow to tweak it or whatever. I really don't want to copy all of that\nlogic from git-merge into git-merge-tree, and merge-tree is therefore\njust a dry-run merge. However, in the case of cherry-picking or\nrebasing, we *do* have a commit message (and author) that we can just\nre-use. But if we make merge-tree handle the commit message and\ncreate commits, you are fundamentally changing the output style of\nmerge-tree that we have carefully documented (and promised backward\ncompatibility on). Figuring out how to extend the output and when\n(and do we also allow that for non-specified merge bases) probably\nrequires some careful thought.\n * a cherry-pick or rebase tool actually might want to do something\nclever with sequences of replayed commits. For example, if later\ncommits in the series have commit messages that refer to earlier\ncommits in the series by their (abbreviated) commit hash, it would be\nnice to update those (abbreviated) commit hashes to instead refer to\nthe replayed version of the earlier commit. But that fundamentally\nrequires operating on a series rather than an individual commit.\n\nCorrectness/capability reason:\n * this patch is fundamentally limited to replaying regular commits.\nReplaying merge-commits correctly doesn't fit within this design; it\nneeds a different framework.\n\nMaintenance reason:\n * suggesting this code as a basis for cherry-pick or rebase is\nlikely to lead to yet another shell-scripted rebase; we've been trying\nto generally nuke shell scripts from Git for a variety of reasons.\nIt'd be sad to regress here.\n\n> Signed-off-by: Kyle Zhao \n> ---\n> merge-tree.c: add --merge-base= option\n>\n> Thanks for Elijah's work. I'm very excited that merge-ort is integrated\n> into the git merge-tree, which means that we can use merge-ort in bare\n> repositories to optimize merge performance.\n>\n> In this patch, I introduce a new --merge-base= option to allow\n> callers to specify a merge-base for the merge. This may allow users to\n> implement git cherry-pick and git rebase in bare repositories with git\n> merge-tree cmd.\n\nMy apologies for having very limited git time (which is often entirely\nused up just in reviewing/responding to patches and other queries on\nthe list instead of on new features like this, or maybe on making some\nnice slides for a conference); if I had more time, I think git-replay\ncould have easily been done many months ago (or perhaps even last\nyear). Then you'd have the high level tool you need for server side\ncherry-picking and rebasing. But, I haven't had much time. So, it\nmakes sense that folks might want to use an interim hack while waiting\nfor the more robust tool to materialize.\n\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1397%2Fkeyu98%2Fkz%2Fmerge-tree-option-merge-base-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1397/keyu98/kz/merge-tree-option-merge-base-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1397\n>\n> Documentation/git-merge-tree.txt | 4 +++\n> builtin/merge-tree.c | 23 +++++++++++++----\n> t/t4301-merge-tree-write-tree.sh | 44 ++++++++++++++++++++++++++++++++\n> 3 files changed, 66 insertions(+), 5 deletions(-)\n>\n> diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\n> index d6c356740ef..e762209b76d 100644\n> --- a/Documentation/git-merge-tree.txt\n> +++ b/Documentation/git-merge-tree.txt\n> @@ -64,6 +64,10 @@ OPTIONS\n> share no common history. This flag can be given to override that\n> check and make the merge proceed anyway.\n>\n> +--merge-base=::\n> + Instead of finding the merge-bases for and ,\n> + specify a merge-base for the merge.\n\nWe could potentially just handle this as an additional positional\nargument, which might be more in-keeping with merge-tree style.\nHowever...\n\n* it would force the --write-tree option to be specified when three\ncommits are listed...at least until we can declare the deprecation\nperiod for --trivial-merge over and nuke that code and flag.\n* if we ever want to allow octopus merges, the additional positional\nargument does not fly well; the explicit flag for --merge-base might\nbe necessary.\n\nSo, this kind of raises questions on what the deprecation period for\n--trivial-merge should be, and whether we ever want to support octopus\nmerges within git-merge-tree.\n\n> [[OUTPUT]]\n> OUTPUT\n\nNo output changes...makes sense, but kind of reinforces the fact that\nthis really isn't a basis for cherry-pick or rebase (as noted above).\n\n> ------\n> diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\n> index ae5782917b9..adc461f00f3 100644\n> --- a/builtin/merge-tree.c\n> +++ b/builtin/merge-tree.c\n> @@ -3,6 +3,7 @@\n> #include \"tree-walk.h\"\n> #include \"xdiff-interface.h\"\n> #include \"help.h\"\n> +#include \"commit.h\"\n> #include \"commit-reach.h\"\n> #include \"merge-ort.h\"\n> #include \"object-store.h\"\n> @@ -402,6 +403,7 @@ struct merge_tree_options {\n> int allow_unrelated_histories;\n> int show_messages;\n> int name_only;\n> + char* merge_base;\n> };\n>\n> static int real_merge(struct merge_tree_options *o,\n> @@ -430,11 +432,18 @@ static int real_merge(struct merge_tree_options *o,\n> opt.branch1 = branch1;\n> opt.branch2 = branch2;\n>\n> - /*\n> - * Get the merge bases, in reverse order; see comment above\n> - * merge_incore_recursive in merge-ort.h\n> - */\n> - merge_bases = get_merge_bases(parent1, parent2);\n> + if (o->merge_base) {\n> + struct commit *c = lookup_commit_reference_by_name(o->merge_base);\n> + if (!c)\n> + die(_(\"could not lookup commit %s\"), o->merge_base);\n\nWould it make sense to have o->merge_base be a struct commit *, and\nmove this lookup/die logic out to the parsing logic in\ncmd_merge_tree()?\n\nJust a thought.\n\n> + commit_list_insert(c, &merge_bases);\n> + } else {\n> + /*\n> + * Get the merge bases, in reverse order; see comment above\n> + * merge_incore_recursive in merge-ort.h\n> + */\n> + merge_bases = get_merge_bases(parent1, parent2);\n> + }\n> if (!merge_bases && !o->allow_unrelated_histories)\n> die(_(\"refusing to merge unrelated histories\"));\n> merge_bases = reverse_commit_list(merge_bases);\n\nand the next line of code, not showing in the context here, is a call\nto merge_incore_recursive(). While that technically works, it doesn't\nmake logical sense. You're not doing a recursive merge when you have\na specified merge base, so I think the code should instead call\nmerge_incore_nonrecursive() in such a case (see Dscho's code for a\ncomparison here).\n\n> @@ -505,6 +514,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> &o.allow_unrelated_histories,\n> N_(\"allow merging unrelated histories\"),\n> PARSE_OPT_NONEG),\n> + OPT_STRING(0, \"merge-base\",\n> + &o.merge_base,\n> + N_(\"commit\"),\n> + N_(\"specify a merge-base commit for the merge\")),\n> OPT_END()\n> };\n>\n> diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\n> index 013b77144bd..032ff4a1c3d 100755\n> --- a/t/t4301-merge-tree-write-tree.sh\n> +++ b/t/t4301-merge-tree-write-tree.sh\n> @@ -819,4 +819,48 @@ test_expect_success SANITY 'merge-ort fails gracefully in a read-only repository\n> test_must_fail git -C read-only merge-tree side1 side2\n> '\n>\n> +# specify merge-base as parent of branch2.\n> +# git merge-tree --merge-base=A O B\n> +# Commit O: foo, bar\n> +# Commit A: modify foo after Commit O\n> +# Commit B: modify bar after Commit A\n> +# Expected: foo is unchanged, modify bar\n\nSo, the O-A-B thing thing was taken from t6423; it appears I didn't\ncopy the comment at the top of t6423 over to t4301 to explain this.\nSorry about that. Anyway, for these types of comments in these files,\nO is always the merge base of both A and B, and neither A nor B\ncontain each other in their history. From that basis, your\ndescription here makes no sense (A would be the tip of some branch,\nnot the parent of anything). I'd call your commits something else\n(maybe just C1, C2, and C3 or something?).\n\n> +\n> +test_expect_success 'specify merge-base as parent of branch2' '\n> + # Setup\n> + git init base-b2-p && (\n> + cd base-b2-p &&\n> + echo foo >foo &&\n> + echo bar >bar &&\n> + git add foo bar &&\n> + git commit -m O &&\n> +\n> + git branch O &&\n> + git branch A &&\n> +\n> + git checkout A &&\n> + echo \"A\" >foo &&\n> + git add foo &&\n> + git commit -m A &&\n> +\n> + git checkout -b B &&\n> + echo \"B\" >bar &&\n> + git add bar &&\n> + git commit -m B\n\nWould test_commit be useful here, given that you aren't worried about\nthe exact contents of files?\n\n> + ) &&\n> + # Testing\n> + (\n> + cd base-b2-p &&\n> + TREE_OID=$(git merge-tree --merge-base=A O B) &&\n> +\n> + q_to_tab <<-EOF >expect &&\n> + 100644 blob $(git rev-parse B:bar)Qbar\n> + 100644 blob $(git rev-parse O:foo)Qfoo\n> + EOF\n> +\n> + git ls-tree $TREE_OID >actual &&\n> + test_cmp expect actual\n> + )\n> +'\n> +\n> test_done\n>\n> base-commit: db29e6bbaed156ae9025ff0474fd788e58230367\n> --\n> gitgitgadget\n"},{"id":"465935","messageId":"CABPp-BHceBeo6Y8s00gET=4yqCMf3qqsiNMhqJ34HVf8r=bhnw@mail.gmail.com","threadId":"58703","inReplyTo":"xmqqczadm93w.fsf@gitster.g","subject":"Re: [PATCH] merge-tree.c: add --merge-base= option","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-10-28T08:20:18Z","receivedAt":"2022-10-28T08:20:36Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Thu, Oct 27, 2022 at 11:09 AM Junio C Hamano wrote:\n>\n[...]\n> Wouldn't it be sufficient to update the UI to\n>\n> git merge-tree [--write-tree] [] [] \n> git merge-tree [--trivial-merge] \n>\n> IOW, when you want to supply the base, you'd be explicit and ask for\n> the new \"write-tree\" mode, i.e.\n>\n> $ git merge-tree --write-tree $(git merge-base branch^ branch) HEAD branch\n>\n> would be how you would use merge-tree to cherry-pick the commit at\n> the tip of the branch on top of the current commit.\n\nThis was my thought too; but would we be painting ourselves into a\ncorner if we ever want to make merge-tree support octopus merges?\n\nAlso, why did you write $(git merge-base branch^ branch) rather than\njust branch^ ?\n\n> I also have to wonder how this should interact with a topic that is\n> in-flight to feed multiple merge-tree requests from the standard\n> input to have a single process perform multiple (not necessarily\n> related) merges. Elijah knows much better, but my gut feeling is\n> that it shouldn't be hard to allow feeding an extra commit on the\n> same line to be used as the base.\n\nYeah, I don't think that'd be too hard...if we could rule out ever\nsupporting octopus merges in merge-tree (which I'm not so sure is a\ngood assumption). Otherwise, we might need to figure out the\nappropriate backward-compatible input parsing (and output format\nchanges?)\n"},{"id":"465937","messageId":"pull.1397.v2.git.1666958144017.gitgitgadget@gmail.com","threadId":"58703","inReplyTo":"pull.1397.git.1666872761355.gitgitgadget@gmail.com","subject":"[PATCH v2] merge-tree.c: add --merge-base= option","fromName":"Kyle Zhao via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-10-28T11:55:43Z","receivedAt":"2022-10-28T11:55:51Z","isPatch":true,"sender":{"key":"goldofzky@gmail.com","avatar":"https://gravatar.com/avatar/aa4fa27f0c1bed9c986ed596415799518e03a6e71a0931308a4de87e166e183a?d=mp&s=160"},"body":"From: Kyle Zhao \n\nThis patch will give our callers more flexibility to use `git merge-tree`,\nsuch as:\n\n git merge-tree --write-tree --merge-base=branch^ HEAD branch\n\nIt would cherry-pick the commit at the tip of the branch on top of the\ncurrent commit even if the repository is bare.\n\nSigned-off-by: Kyle Zhao \n---\n merge-tree: allow specifying a base commit when --write-tree is passed\n \n Thanks for Elijah's work. I'm very excited that merge-ort is integrated\n into the git merge-tree, which means that we can use merge-ort in bare\n repositories to optimize merge performance.\n \n In this patch, I introduce a new --merge-base= option to allow\n callers to specify a merge-base for the merge. This may allow users to\n implement git cherry-pick and git rebase in bare repositories with git\n merge-tree cmd.\n \n Changes since v1:\n \n * Changed merge_incore_nonrecursive() to merge_incore_recursive() when\n merge-base is specified.\n * Fixed c style problem.\n * Moved commit lookup/die logic out to the parsing logic in\n cmd_merge_tree().\n * use test_commit for test\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1397%2Fkeyu98%2Fkz%2Fmerge-tree-option-merge-base-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1397/keyu98/kz/merge-tree-option-merge-base-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1397\n\nRange-diff vs v1:\n\n 1: 965e544c849 ! 1: ab4e5d5ad08 merge-tree.c: add --merge-base= option\n @@ Metadata\n ## Commit message ##\n merge-tree.c: add --merge-base= option\n \n - This option allows users to specify a merge-base commit for the merge.\n + This patch will give our callers more flexibility to use `git merge-tree`,\n + such as:\n \n - It will give our callers more flexibility to use the `git merge-tree`.\n - For example:\n + git merge-tree --write-tree --merge-base=branch^ HEAD branch\n \n - git merge-tree --merge-base=^1 source-branch \n -\n - This allows us to implement `git cherry-pick` in bare repositories.\n + It would cherry-pick the commit at the tip of the branch on top of the\n + current commit even if the repository is bare.\n \n Signed-off-by: Kyle Zhao \n \n @@ builtin/merge-tree.c: struct merge_tree_options {\n \tint allow_unrelated_histories;\n \tint show_messages;\n \tint name_only;\n -+\tchar* merge_base;\n ++\tconst struct commit *base_commit;\n };\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 -\t * merge_incore_recursive in merge-ort.h\n -\t */\n -\tmerge_bases = get_merge_bases(parent1, parent2);\n -+\tif (o->merge_base) {\n -+\t\tstruct commit *c = lookup_commit_reference_by_name(o->merge_base);\n -+\t\tif (!c)\n -+\t\t\tdie(_(\"could not lookup commit %s\"), o->merge_base);\n -+\t\tcommit_list_insert(c, &merge_bases);\n +-\tif (!merge_bases && !o->allow_unrelated_histories)\n +-\t\tdie(_(\"refusing to merge unrelated histories\"));\n +-\tmerge_bases = reverse_commit_list(merge_bases);\n ++\tif (o->base_commit) {\n ++\t\tstruct tree *base_tree, *parent1_tree, *parent2_tree;\n ++\n ++\t\topt.ancestor = \"specified merge base\";\n ++\t\tbase_tree = get_commit_tree(o->base_commit);\n ++\t\tparent1_tree = get_commit_tree(parent1);\n ++\t\tparent2_tree = get_commit_tree(parent2);\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 +\t\t * merge_incore_recursive in merge-ort.h\n +\t\t */\n +\t\tmerge_bases = get_merge_bases(parent1, parent2);\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}\n - \tif (!merge_bases && !o->allow_unrelated_histories)\n - \t\tdie(_(\"refusing to merge unrelated histories\"));\n - \tmerge_bases = reverse_commit_list(merge_bases);\n + \n +-\tmerge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);\n + \tif (result.clean < 0)\n + \t\tdie(_(\"failure to merge\"));\n + \n +@@ builtin/merge-tree.c: 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 ++\tconst char *merge_base = NULL;\n + \tstruct merge_tree_options o = { .show_messages = -1 };\n + \tint expected_remaining_argc;\n + \tint original_argc;\n @@ builtin/merge-tree.c: int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\t\t &o.allow_unrelated_histories,\n \t\t\t N_(\"allow merging unrelated histories\"),\n \t\t\t PARSE_OPT_NONEG),\n +\t\tOPT_STRING(0, \"merge-base\",\n -+\t\t\t &o.merge_base,\n ++\t\t\t &merge_base,\n +\t\t\t N_(\"commit\"),\n -+\t\t\t N_(\"specify a merge-base commit for the merge\")),\n ++\t\t\t N_(\"specify a merge-base for the merge\")),\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\tusage_with_options(merge_tree_usage, mt_options);\n + \n + \t/* Do the relevant type of merge */\n +-\tif (o.mode == MODE_REAL)\n ++\tif (o.mode == MODE_REAL) {\n ++\t\tif (merge_base) {\n ++\t\t\to.base_commit = lookup_commit_reference_by_name(merge_base);\n ++\t\t\tif (!o.base_commit)\n ++\t\t\t\tdie(_(\"could not lookup commit %s\"), merge_base);\n ++\t\t}\n + \t\treturn real_merge(&o, argv[0], argv[1], prefix);\n ++\t}\n + \telse\n + \t\treturn trivial_merge(argv[0], argv[1], argv[2]);\n + }\n \n ## t/t4301-merge-tree-write-tree.sh ##\n @@ t/t4301-merge-tree-write-tree.sh: test_expect_success SANITY 'merge-ort fails gracefully in a read-only repository\n \ttest_must_fail git -C read-only merge-tree side1 side2\n '\n \n -+# specify merge-base as parent of branch2.\n -+# git merge-tree --merge-base=A O B\n -+# Commit O: foo, bar\n -+# Commit A: modify foo after Commit O\n -+# Commit B: modify bar after Commit A\n -+# Expected: foo is unchanged, modify bar\n -+\n +test_expect_success 'specify merge-base as parent of branch2' '\n +\t# Setup\n +\tgit init base-b2-p && (\n +\t\tcd base-b2-p &&\n -+\t\techo foo >foo &&\n -+\t\techo bar >bar &&\n -+\t\tgit add foo bar &&\n -+\t\tgit commit -m O &&\n -+\n -+\t\tgit branch O &&\n -+\t\tgit branch A &&\n -+\n -+\t\tgit checkout A &&\n -+\t\techo \"A\" >foo &&\n -+\t\tgit add foo &&\n -+\t\tgit commit -m A &&\n -+\n -+\t\tgit checkout -b B &&\n -+\t\techo \"B\" >bar &&\n -+\t\tgit add bar &&\n -+\t\tgit commit -m B\n ++\t\ttest_commit c1 file1 &&\n ++\t\ttest_commit c2 file2 &&\n ++\t\ttest_commit c3 file3\n +\t) &&\n +\t# Testing\n +\t(\n +\t\tcd base-b2-p &&\n -+\t\tTREE_OID=$(git merge-tree --merge-base=A O B) &&\n ++\t\tTREE_OID=$(git merge-tree --write-tree --merge-base=c2 c1 c3) &&\n +\n +\t\tq_to_tab <<-EOF >expect &&\n -+\t\t100644 blob $(git rev-parse B:bar)Qbar\n -+\t\t100644 blob $(git rev-parse O:foo)Qfoo\n ++\t\t100644 blob $(git rev-parse c1:file1)Qfile1\n ++\t\t100644 blob $(git rev-parse c3:file3)Qfile3\n +\t\tEOF\n +\n +\t\tgit ls-tree $TREE_OID >actual &&\n\n\n Documentation/git-merge-tree.txt | 4 +++\n builtin/merge-tree.c | 43 ++++++++++++++++++++++++--------\n t/t4301-merge-tree-write-tree.sh | 23 +++++++++++++++++\n 3 files changed, 60 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\nindex d6c356740ef..e762209b76d 100644\n--- a/Documentation/git-merge-tree.txt\n+++ b/Documentation/git-merge-tree.txt\n@@ -64,6 +64,10 @@ OPTIONS\n \tshare no common history. This flag can be given to override that\n \tcheck and make the merge proceed anyway.\n \n+--merge-base=::\n+\tInstead of finding the merge-bases for and ,\n+\tspecify a merge-base for the merge.\n+\n [[OUTPUT]]\n OUTPUT\n ------\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex ae5782917b9..089ea8fac81 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -3,6 +3,7 @@\n #include \"tree-walk.h\"\n #include \"xdiff-interface.h\"\n #include \"help.h\"\n+#include \"commit.h\"\n #include \"commit-reach.h\"\n #include \"merge-ort.h\"\n #include \"object-store.h\"\n@@ -402,6 +403,7 @@ struct merge_tree_options {\n \tint allow_unrelated_histories;\n \tint show_messages;\n \tint name_only;\n+\tconst struct commit *base_commit;\n };\n \n static int real_merge(struct merge_tree_options *o,\n@@ -430,16 +432,26 @@ static int real_merge(struct merge_tree_options *o,\n \topt.branch1 = branch1;\n \topt.branch2 = branch2;\n \n-\t/*\n-\t * Get the merge bases, in reverse order; see comment above\n-\t * merge_incore_recursive in merge-ort.h\n-\t */\n-\tmerge_bases = get_merge_bases(parent1, parent2);\n-\tif (!merge_bases && !o->allow_unrelated_histories)\n-\t\tdie(_(\"refusing to merge unrelated histories\"));\n-\tmerge_bases = reverse_commit_list(merge_bases);\n+\tif (o->base_commit) {\n+\t\tstruct tree *base_tree, *parent1_tree, *parent2_tree;\n+\n+\t\topt.ancestor = \"specified merge base\";\n+\t\tbase_tree = get_commit_tree(o->base_commit);\n+\t\tparent1_tree = get_commit_tree(parent1);\n+\t\tparent2_tree = get_commit_tree(parent2);\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+\t\t * merge_incore_recursive in merge-ort.h\n+\t\t */\n+\t\tmerge_bases = get_merge_bases(parent1, parent2);\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}\n \n-\tmerge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);\n \tif (result.clean < 0)\n \t\tdie(_(\"failure to merge\"));\n \n@@ -478,6 +490,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+\tconst char *merge_base = NULL;\n \tstruct merge_tree_options o = { .show_messages = -1 };\n \tint expected_remaining_argc;\n \tint original_argc;\n@@ -505,6 +518,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\t\t &o.allow_unrelated_histories,\n \t\t\t N_(\"allow merging unrelated histories\"),\n \t\t\t PARSE_OPT_NONEG),\n+\t\tOPT_STRING(0, \"merge-base\",\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_END()\n \t};\n \n@@ -544,8 +561,14 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\tusage_with_options(merge_tree_usage, mt_options);\n \n \t/* Do the relevant type of merge */\n-\tif (o.mode == MODE_REAL)\n+\tif (o.mode == MODE_REAL) {\n+\t\tif (merge_base) {\n+\t\t\to.base_commit = lookup_commit_reference_by_name(merge_base);\n+\t\t\tif (!o.base_commit)\n+\t\t\t\tdie(_(\"could not lookup commit %s\"), merge_base);\n+\t\t}\n \t\treturn real_merge(&o, argv[0], argv[1], prefix);\n+\t}\n \telse\n \t\treturn trivial_merge(argv[0], argv[1], argv[2]);\n }\ndiff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\nindex 013b77144bd..64bfe6f4a41 100755\n--- a/t/t4301-merge-tree-write-tree.sh\n+++ b/t/t4301-merge-tree-write-tree.sh\n@@ -819,4 +819,27 @@ test_expect_success SANITY 'merge-ort fails gracefully in a read-only repository\n \ttest_must_fail git -C read-only merge-tree side1 side2\n '\n \n+test_expect_success 'specify merge-base as parent of branch2' '\n+\t# Setup\n+\tgit init base-b2-p && (\n+\t\tcd base-b2-p &&\n+\t\ttest_commit c1 file1 &&\n+\t\ttest_commit c2 file2 &&\n+\t\ttest_commit c3 file3\n+\t) &&\n+\t# Testing\n+\t(\n+\t\tcd base-b2-p &&\n+\t\tTREE_OID=$(git merge-tree --write-tree --merge-base=c2 c1 c3) &&\n+\n+\t\tq_to_tab <<-EOF >expect &&\n+\t\t100644 blob $(git rev-parse c1:file1)Qfile1\n+\t\t100644 blob $(git rev-parse c3:file3)Qfile3\n+\t\tEOF\n+\n+\t\tgit ls-tree $TREE_OID >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n test_done\n\nbase-commit: 5af5e54106e20f65c913550c80aec3186b859e9b\n-- \ngitgitgadget\n"},{"id":"465938","messageId":"b517c12bef6d45238b1f88f180b51011@tencent.com","threadId":"58703","inReplyTo":"CABPp-BGBFyoY7m+KCA9MTifKmpZ0ccLgsYHahMCgPxuTpuUGPg@mail.gmail.com","subject":"Re: [Internet]Re: [PATCH] merge-tree.c: add --merge-base= option","fromName":"kylezhao (赵柯宇)","fromEmail":"kylezhao@tencent.com","sentAt":"2022-10-28T11:54:15Z","receivedAt":"2022-10-28T12:01:38Z","isPatch":true,"sender":{"key":"kylezhao@tencent.com","avatar":"https://avatars.githubusercontent.com/u/34176272?v=4"},"body":"> Note that this has been implemented previously. I may have implemented it previously and ripped it out (about a year ago), or maybe I just avoided it once I ran across the reasons below. Dscho also implemented it over at https://lore.kernel.org/git/nycvar.QRO.7.76.6.2202210956430.26495@tvgsbejvaqbjf.bet/\n>\n> You may want to take a look at his code for comparison, especially the\nmerge_incore_nonrecursive() stuff.\n\nThanks. I'll take a look later.\n\n> This capability is something we may ultimately want for completeness,\n> but it definitely should not be marketed as a building block for\n> implementing cherry-pick or rebase. There are several reasons for\n> that:\n> \n> Performance reasons:\n> * it's a design with a separate fork for every commit to be picked,\n> ....\n> Maintenance reason:\n> * suggesting this code as a basis for cherry-pick or rebase is\n> likely to lead to yet another shell-scripted rebase; we've been trying\n> to generally nuke shell scripts from Git for a variety of reasons.\n> It'd be sad to regress here.\n\nThank you for your very detailed explanation, I feel like I have a deeper understanding of rebase and cherry-pick.\n\n> My apologies for having very limited git time (which is often entirely used up just in reviewing/responding to patches and other queries on the list instead of on new features like this, or maybe on making some nice slides for a conference); if I had more time, I think git-replay could have easily been done many months ago (or perhaps even last year). Then you'd have the high level tool you need for server side cherry-picking and rebasing. But, I haven't had much time. So, it makes sense that folks might want to use an interim hack while waiting for the more robust tool to materialize.\n\ngit-replay sounds great, looking forward to it one day.\n\n> Would it make sense to have o->merge_base be a struct commit *, and move this lookup/die logic out to the parsing logic in cmd_merge_tree()?\nDone.\n\n> and the next line of code, not showing in the context here, is a call\n> to merge_incore_recursive(). While that technically works, it doesn't\n> make logical sense. You're not doing a recursive merge when you have\n> a specified merge base, so I think the code should instead call\n> merge_incore_nonrecursive() in such a case (see Dscho's code for a\n> comparison here).\nDone. I've changed it to merge_incore_recursive(). It works, but I don't know if it's written correctly, especially opt.ancestor.\n\n> So, the O-A-B thing thing was taken from t6423; it appears I didn't copy the comment at the top of t6423 over to t4301 to explain this.\n> Sorry about that. Anyway, for these types of comments in these files, O is always the merge base of both A and B, and neither A nor B contain each other in their history. From that basis, your description here makes no sense (A would be the tip of some branch, not the parent of anything). I'd call your commits something else (maybe just C1, C2, and C3 or something?).\n\nDone.\n\n> Would test_commit be useful here, given that you aren't worried about\n> the exact contents of files?\n\nIt's useful, which makes the test clearer."},{"id":"465939","messageId":"ad27a59046e345a68669d3876a75772a@tencent.com","threadId":"58703","inReplyTo":"xmqqczadm93w.fsf@gitster.g","subject":"RE: [Internet]Re: [PATCH] merge-tree.c: add --merge-base= option","fromName":"kylezhao (赵柯宇)","fromEmail":"kylezhao@tencent.com","sentAt":"2022-10-28T11:43:45Z","receivedAt":"2022-10-28T12:21:18Z","isPatch":true,"sender":{"key":"kylezhao@tencent.com","avatar":"https://avatars.githubusercontent.com/u/34176272?v=4"},"body":"Thanks for your reviews.\n\n> In the original \"trivial merge\" mode, the command takes three trees\n> without having to have this new option. In the new \"write-tree\"\n> mode, currently it is incapable of taking the base, but it does not\n> have to stay that way. Wouldn't it be sufficient to update the UI\n> to\n>\n> git merge-tree [--write-tree] [] [] \n> git merge-tree [--trivial-merge] \n>\n> IOW, when you want to supply the base, you'd be explicit and ask for\n> the new \"write-tree\" mode, i.e.\n>\n> $ git merge-tree --write-tree $(git merge-base branch^ branch) HEAD branch \n>\n> would be how you would use merge-tree to cherry-pick the commit at\n> the tip of the branch on top of the current commit.\n\nReferring to Newren's reply, if we need to implement octopus merges for git-merge-tree in the future, still need a new option, \nso I haven't modified it yet.\n\n>> @@ -402,6 +403,7 @@ struct merge_tree_options {\n>> \tint allow_unrelated_histories;\n>> \tint show_messages;\n>> \tint name_only;\n>> +\tchar* merge_base;\n>\n> Style. We write in C, not in C++, and our asterisks stick to variables and members of structs, not types.\n\nDone.\n\n\n"},{"id":"465955","messageId":"xmqqfsf8j5p5.fsf@gitster.g","threadId":"58703","inReplyTo":"CABPp-BHceBeo6Y8s00gET=4yqCMf3qqsiNMhqJ34HVf8r=bhnw@mail.gmail.com","subject":"Re: [PATCH] merge-tree.c: add --merge-base= option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-28T16:03:50Z","receivedAt":"2022-10-28T16:05:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n> On Thu, Oct 27, 2022 at 11:09 AM Junio C Hamano wrote:\n>>\n> [...]\n>> Wouldn't it be sufficient to update the UI to\n>>\n>> git merge-tree [--write-tree] [] [] \n>> git merge-tree [--trivial-merge] \n>>\n>> IOW, when you want to supply the base, you'd be explicit and ask for\n>> the new \"write-tree\" mode, i.e.\n>>\n>> $ git merge-tree --write-tree $(git merge-base branch^ branch) HEAD branch\n>>\n>> would be how you would use merge-tree to cherry-pick the commit at\n>> the tip of the branch on top of the current commit.\n>\n> This was my thought too; but would we be painting ourselves into a\n> corner if we ever want to make merge-tree support octopus merges?\n\nTrue, the above suggestion was especially bad as it would not be\npossible to extend to support multiple bases *and* octopus at the\nsame time. Consider it scrapped. Taking zero or more --base-commit\noptions explicitly from the command line would be a better\ninterface.\n\n> Also, why did you write $(git merge-base branch^ branch) rather than\n> just branch^ ?\n\nJust to be explicit which one is doing what.\n\n> Yeah, I don't think that'd be too hard...if we could rule out ever\n> supporting octopus merges in merge-tree (which I'm not so sure is a\n> good assumption). Otherwise, we might need to figure out the\n> appropriate backward-compatible input parsing (and output format\n> changes?)\n\nI'd prefer an approach to tackle one thing at a time while making\nsure we leave the door open for the future ones. I think the\nbackend interface from \"merge\" to external strategies use a\nseparator to signal the boundary between the heads being merged\n(i.e. branchN above) and the bases being used, which is easy to\nparse and support multiple bases and octopus at the same time.\n\nAs to making it easy to implement \"cherry-pick\", I do not think you\nshould dogmatically forbid it on the basis for merge-tree from the\ncommand line is inherently one mergy operation per invocation. You\nwill have the --stdin interface, and a way forward may be a notation\nto refer to previous results in the --stdin input stream. That way,\na single invocation of merge-tree \"server\" can be fed a sequence of\n3-way merges that are actually steps of multi-commit cherry-pick,\nthat merge the differences of multiple commits on top of the\nprevious result, one step at a time.\n\nIOW, as long as you make sure --stdin interface is rich enough to\nallow you do what people would do a sequence of \"git merge-tree\"\nsingle shot invocations (perhaps even driven by a script), you would\nbe OK, I would think.\n\n"},{"id":"465997","messageId":"CABPp-BHiqW2zgt4UfnH5iJbOf6RzPnnw+=bzvRYdrLQs08hU5g@mail.gmail.com","threadId":"58703","inReplyTo":"pull.1397.v2.git.1666958144017.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] merge-tree.c: add --merge-base= option","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-10-29T01:32:11Z","receivedAt":"2022-10-29T01:32:29Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nOn Fri, Oct 28, 2022 at 4:55 AM Kyle Zhao via GitGitGadget\n wrote:\n>\n> From: Kyle Zhao \n>\n> This patch will give our callers more flexibility to use `git merge-tree`,\n> such as:\n>\n> git merge-tree --write-tree --merge-base=branch^ HEAD branch\n>\n> It would cherry-pick the commit at the tip of the branch on top of the\n> current commit even if the repository is bare.\n\nPerhaps just \"This does a merge of HEAD and branch, but uses branch^\nas the merge-base.\"\n\nAlso, given that both Junio and I thought a positional argument might\nbe better, it might be worth calling out that the reason for using an\noption flag instead of a positional argument is to allow additional\ncommits passed to merge-tree to be handled via an octopus merge in the\nfuture.\n\n> Signed-off-by: Kyle Zhao \n> ---\n> merge-tree: allow specifying a base commit when --write-tree is passed\n>\n> Thanks for Elijah's work. I'm very excited that merge-ort is integrated\n> into the git merge-tree, which means that we can use merge-ort in bare\n> repositories to optimize merge performance.\n>\n> In this patch, I introduce a new --merge-base= option to allow\n> callers to specify a merge-base for the merge. This may allow users to\n> implement git cherry-pick and git rebase in bare repositories with git\n> merge-tree cmd.\n>\n> Changes since v1:\n>\n> * Changed merge_incore_nonrecursive() to merge_incore_recursive() when\n> merge-base is specified.\n\nI think you mean the other way around (using\nmerge_incore_nonrecursive() instead of merge_incore_recursive() when\nmerge-base is specified).\n\n> + if (o->base_commit) {\n> + struct tree *base_tree, *parent1_tree, *parent2_tree;\n> +\n> + opt.ancestor = \"specified merge base\";\n\nIt is a specified merge base, but that won't help the user much when\nthey get conflict markers if they attempt to investigate them. Since\nthe specified merge base is a commit the user will know, and in fact\none they named on the command line, we should use that name. So, I'd\nexpect this to be set to o->merge_base.\n\n> @@ -544,8 +561,14 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> usage_with_options(merge_tree_usage, mt_options);\n>\n> /* Do the relevant type of merge */\n> - if (o.mode == MODE_REAL)\n> + if (o.mode == MODE_REAL) {\n> + if (merge_base) {\n> + o.base_commit = lookup_commit_reference_by_name(merge_base);\n> + if (!o.base_commit)\n> + die(_(\"could not lookup commit %s\"), merge_base);\n> + }\n\nPersonally, I think of the code before \"/* Do the relevant type of\nmerge */\" as a continuation of option parsing (i.e. sanity checking\narguments and determining defaults and whatnot), and I think the last\nfive lines above are more option parsing. From that angle, I think\nit'd make sense to move these lines out above this block (before or\nafter determining o.mode).\n\nBut this may well just be personal preference; what you have certainly works.\n\n> return real_merge(&o, argv[0], argv[1], prefix);\n> + }\n> else\n> return trivial_merge(argv[0], argv[1], argv[2]);\n> }\n> diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\n> index 013b77144bd..64bfe6f4a41 100755\n> --- a/t/t4301-merge-tree-write-tree.sh\n> +++ b/t/t4301-merge-tree-write-tree.sh\n> @@ -819,4 +819,27 @@ test_expect_success SANITY 'merge-ort fails gracefully in a read-only repository\n> test_must_fail git -C read-only merge-tree side1 side2\n> '\n>\n> +test_expect_success 'specify merge-base as parent of branch2' '\n> + # Setup\n> + git init base-b2-p && (\n> + cd base-b2-p &&\n> + test_commit c1 file1 &&\n> + test_commit c2 file2 &&\n> + test_commit c3 file3\n\nMuch simpler. :-)\n\n> + ) &&\n> + # Testing\n> + (\n> + cd base-b2-p &&\n> + TREE_OID=$(git merge-tree --write-tree --merge-base=c2 c1 c3) &&\n> +\n> + q_to_tab <<-EOF >expect &&\n> + 100644 blob $(git rev-parse c1:file1)Qfile1\n> + 100644 blob $(git rev-parse c3:file3)Qfile3\n> + EOF\n> +\n> + git ls-tree $TREE_OID >actual &&\n> + test_cmp expect actual\n\nIn particular, you are testing here that file2 does NOT appear despite\nthe fact that it was part of c3. That makes sense, but I'm not sure\ncasual readers of this code would catch that. It might be worth\nadding a comment to make it easier to follow for future test readers.\n\n> + )\n> +'\n> +\n> test_done\n>\n> base-commit: 5af5e54106e20f65c913550c80aec3186b859e9b\n\nThis should be rebased on top of en/merge-tree-sequence. Then, you\nneed to either figure out how to modify the new --stdin flag to also\naccept a specified merge base in a way that permits future handling of\noctopus merges (it looks like Junio might have a good suggestion\nelsewhere in this thread), or else explicitly call out in the commit\nmessage why specified merge base support is only being added when\n--stdin is not specified.\n"},{"id":"465998","messageId":"CABPp-BGXM=iRAgjNbZ0o3FSjj583GpkuFB6emUYwWjdFWb9-jQ@mail.gmail.com","threadId":"58703","inReplyTo":"xmqqfsf8j5p5.fsf@gitster.g","subject":"Re: [PATCH] merge-tree.c: add --merge-base= option","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-10-29T01:52:00Z","receivedAt":"2022-10-29T01:53:24Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, Oct 28, 2022 at 9:03 AM Junio C Hamano wrote:\n>\n> > Yeah, I don't think that'd be too hard...if we could rule out ever\n> > supporting octopus merges in merge-tree (which I'm not so sure is a\n> > good assumption). Otherwise, we might need to figure out the\n> > appropriate backward-compatible input parsing (and output format\n> > changes?)\n>\n> I'd prefer an approach to tackle one thing at a time while making\n> sure we leave the door open for the future ones. I think the\n> backend interface from \"merge\" to external strategies use a\n> separator to signal the boundary between the heads being merged\n> (i.e. branchN above) and the bases being used, which is easy to\n> parse and support multiple bases and octopus at the same time.\n\nOoh, the separator idea sounds like a good solution.\n\n> As to making it easy to implement \"cherry-pick\", I do not think you\n> should dogmatically forbid it on the basis for merge-tree from the\n> command line is inherently one mergy operation per invocation. You\n> will have the --stdin interface, and a way forward may be a notation\n> to refer to previous results in the --stdin input stream. That way,\n> a single invocation of merge-tree \"server\" can be fed a sequence of\n> 3-way merges that are actually steps of multi-commit cherry-pick,\n> that merge the differences of multiple commits on top of the\n> previous result, one step at a time.\n>\n> IOW, as long as you make sure --stdin interface is rich enough to\n> allow you do what people would do a sequence of \"git merge-tree\"\n> single shot invocations (perhaps even driven by a script), you would\n> be OK, I would think.\n\nI agree that this kind of functionality probably makes sense for\ninclusion from a completeness perspective, but I still maintain that\nif its sole purpose is implementing a cherry-pick command then it's at\nbest an ill-advised or interim hack and we should instead do something\nbetter. While combining this patch with --stdin does solve the\n\"single operation per invocation\" issue, that's only one of six issues\nI listed in my email to Kyle about why this is the wrong base for\nbuilding a cherry-pick alternative.\n"},{"id":"466004","messageId":"pull.1397.v3.git.1667014975042.gitgitgadget@gmail.com","threadId":"58703","inReplyTo":"pull.1397.v2.git.1666958144017.gitgitgadget@gmail.com","subject":"[PATCH v3] merge-tree.c: add --merge-base= option","fromName":"Kyle Zhao via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-10-29T03:42:54Z","receivedAt":"2022-10-29T03:43:16Z","isPatch":true,"sender":{"key":"goldofzky@gmail.com","avatar":"https://gravatar.com/avatar/aa4fa27f0c1bed9c986ed596415799518e03a6e71a0931308a4de87e166e183a?d=mp&s=160"},"body":"From: Kyle Zhao \n\nThis patch will give our callers more flexibility to use `git merge-tree`,\nsuch as:\n\n git merge-tree --write-tree --merge-base=branch^ HEAD branch\n\nThis does a merge of HEAD and branch, but uses branch^ as the merge-base.\n\nAnd the reason why using an option flag instead of a positional argument\nis to allow additional commits passed to merge-tree to be handled via an\noctopus merge in the future.\n\nSigned-off-by: Kyle Zhao \n---\n merge-tree: allow specifying a base commit when --write-tree is passed\n \n Thanks for Elijah's work. I'm very excited that merge-ort is integrated\n into the git merge-tree, which means that we can use merge-ort in bare\n repositories to optimize merge performance.\n \n In this patch, I introduce a new --merge-base= option to allow\n callers to specify a merge-base for the merge. This may allow users to\n implement git cherry-pick and git rebase in bare repositories with git\n merge-tree cmd.\n \n Changes since v1:\n \n * Changed merge_incore_recursive() to merge_incore_nonrecursive() when\n merge-base is specified.\n * Fixed c style problem.\n * Moved commit lookup/die logic out to the parsing logic in\n cmd_merge_tree().\n * use test_commit for test\n \n Changes since v2:\n \n * commit message\n * Rebased on top of en/merge-tree-sequence.\n * Set opt.ancestor to o->merge_base. Because opt.ancestor is a *char.\n To make it easier to pass parameters, I moved\n lookup_commit_reference_by_name() to real_ merge() again.\n * Added test comment.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1397%2Fkeyu98%2Fkz%2Fmerge-tree-option-merge-base-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1397/keyu98/kz/merge-tree-option-merge-base-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/1397\n\nRange-diff vs v2:\n\n 1: ab4e5d5ad08 ! 1: 554cbde49e8 merge-tree.c: add --merge-base= option\n @@ Commit message\n \n git merge-tree --write-tree --merge-base=branch^ HEAD branch\n \n - It would cherry-pick the commit at the tip of the branch on top of the\n - current commit even if the repository is bare.\n + This does a merge of HEAD and branch, but uses branch^ as the merge-base.\n +\n + And the reason why using an option flag instead of a positional argument\n + is to allow additional commits passed to merge-tree to be handled via an\n + octopus merge in the future.\n \n Signed-off-by: Kyle Zhao \n \n @@ builtin/merge-tree.c\n #include \"merge-ort.h\"\n #include \"object-store.h\"\n @@ builtin/merge-tree.c: struct merge_tree_options {\n - \tint allow_unrelated_histories;\n \tint show_messages;\n \tint name_only;\n -+\tconst struct commit *base_commit;\n + \tint use_stdin;\n ++\tconst char *merge_base;\n };\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 -\tif (!merge_bases && !o->allow_unrelated_histories)\n -\t\tdie(_(\"refusing to merge unrelated histories\"));\n -\tmerge_bases = reverse_commit_list(merge_bases);\n -+\tif (o->base_commit) {\n ++\tif (o->merge_base) {\n ++\t\tstruct commit *base_commit;\n +\t\tstruct tree *base_tree, *parent1_tree, *parent2_tree;\n +\n -+\t\topt.ancestor = \"specified merge base\";\n -+\t\tbase_tree = get_commit_tree(o->base_commit);\n ++\t\tbase_commit = lookup_commit_reference_by_name(o->merge_base);\n ++\t\tif (!base_commit)\n ++\t\t\tdie(_(\"could not lookup commit %s\"), o->merge_base);\n ++\n ++\t\topt.ancestor = o->merge_base;\n ++\t\tbase_tree = get_commit_tree(base_commit);\n +\t\tparent1_tree = get_commit_tree(parent1);\n +\t\tparent2_tree = get_commit_tree(parent2);\n +\t\tmerge_incore_nonrecursive(&opt, base_tree, parent1_tree, parent2_tree, &result);\n @@ builtin/merge-tree.c: static int real_merge(struct merge_tree_options *o,\n \tif (result.clean < 0)\n \t\tdie(_(\"failure to merge\"));\n \n -@@ builtin/merge-tree.c: 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 -+\tconst char *merge_base = NULL;\n - \tstruct merge_tree_options o = { .show_messages = -1 };\n - \tint expected_remaining_argc;\n - \tint original_argc;\n @@ builtin/merge-tree.c: int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n - \t\t\t &o.allow_unrelated_histories,\n - \t\t\t N_(\"allow merging unrelated histories\"),\n + \t\t\t &o.use_stdin,\n + \t\t\t N_(\"perform multiple merges, one per line of input\"),\n \t\t\t PARSE_OPT_NONEG),\n +\t\tOPT_STRING(0, \"merge-base\",\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\t\t &o.merge_base,\n ++\t\t\t N_(\"commit\"),\n ++\t\t\t N_(\"specify a merge-base for the merge\")),\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\tusage_with_options(merge_tree_usage, mt_options);\n - \n - \t/* Do the relevant type of merge */\n --\tif (o.mode == MODE_REAL)\n -+\tif (o.mode == MODE_REAL) {\n -+\t\tif (merge_base) {\n -+\t\t\to.base_commit = lookup_commit_reference_by_name(merge_base);\n -+\t\t\tif (!o.base_commit)\n -+\t\t\t\tdie(_(\"could not lookup commit %s\"), merge_base);\n -+\t\t}\n - \t\treturn real_merge(&o, argv[0], argv[1], prefix);\n -+\t}\n - \telse\n - \t\treturn trivial_merge(argv[0], argv[1], argv[2]);\n - }\n \n ## t/t4301-merge-tree-write-tree.sh ##\n -@@ t/t4301-merge-tree-write-tree.sh: test_expect_success SANITY 'merge-ort fails gracefully in a read-only repository\n - \ttest_must_fail git -C read-only merge-tree side1 side2\n +@@ t/t4301-merge-tree-write-tree.sh: test_expect_success '--stdin with both a successful and a conflicted merge' '\n + \ttest_cmp expect actual\n '\n \n ++# specify merge-base as parent of branch2\n ++# git merge-tree --write-tree --merge-base=c2 c1 c3\n ++# Commit c1: add file1\n ++# Commit c2: add file2 after c1\n ++# Commit c3: add file3 after c2\n ++# Expected: add file3, and file2 does NOT appear\n ++\n +test_expect_success 'specify merge-base as parent of branch2' '\n +\t# Setup\n +\tgit init base-b2-p && (\n\n\n Documentation/git-merge-tree.txt | 4 ++++\n builtin/merge-tree.c | 39 ++++++++++++++++++++++++--------\n t/t4301-merge-tree-write-tree.sh | 30 ++++++++++++++++++++++++\n 3 files changed, 64 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\nindex 04bcc416e6e..d9dacb2ce54 100644\n--- a/Documentation/git-merge-tree.txt\n+++ b/Documentation/git-merge-tree.txt\n@@ -64,6 +64,10 @@ OPTIONS\n \tshare no common history. This flag can be given to override that\n \tcheck and make the merge proceed anyway.\n \n+--merge-base=::\n+\tInstead of finding the merge-bases for and ,\n+\tspecify a merge-base for the merge.\n+\n [[OUTPUT]]\n OUTPUT\n ------\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex fe853aa8f91..cd25aac6ba6 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -3,6 +3,7 @@\n #include \"tree-walk.h\"\n #include \"xdiff-interface.h\"\n #include \"help.h\"\n+#include \"commit.h\"\n #include \"commit-reach.h\"\n #include \"merge-ort.h\"\n #include \"object-store.h\"\n@@ -403,6 +404,7 @@ struct merge_tree_options {\n \tint show_messages;\n \tint name_only;\n \tint use_stdin;\n+\tconst char *merge_base;\n };\n \n static int real_merge(struct merge_tree_options *o,\n@@ -432,16 +434,31 @@ static int real_merge(struct merge_tree_options *o,\n \topt.branch1 = branch1;\n \topt.branch2 = branch2;\n \n-\t/*\n-\t * Get the merge bases, in reverse order; see comment above\n-\t * merge_incore_recursive in merge-ort.h\n-\t */\n-\tmerge_bases = get_merge_bases(parent1, parent2);\n-\tif (!merge_bases && !o->allow_unrelated_histories)\n-\t\tdie(_(\"refusing to merge unrelated histories\"));\n-\tmerge_bases = reverse_commit_list(merge_bases);\n+\tif (o->merge_base) {\n+\t\tstruct commit *base_commit;\n+\t\tstruct tree *base_tree, *parent1_tree, *parent2_tree;\n+\n+\t\tbase_commit = lookup_commit_reference_by_name(o->merge_base);\n+\t\tif (!base_commit)\n+\t\t\tdie(_(\"could not lookup commit %s\"), o->merge_base);\n+\n+\t\topt.ancestor = o->merge_base;\n+\t\tbase_tree = get_commit_tree(base_commit);\n+\t\tparent1_tree = get_commit_tree(parent1);\n+\t\tparent2_tree = get_commit_tree(parent2);\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+\t\t * merge_incore_recursive in merge-ort.h\n+\t\t */\n+\t\tmerge_bases = get_merge_bases(parent1, parent2);\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}\n \n-\tmerge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);\n \tif (result.clean < 0)\n \t\tdie(_(\"failure to merge\"));\n \n@@ -515,6 +532,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\t\t &o.use_stdin,\n \t\t\t N_(\"perform multiple merges, one per line of input\"),\n \t\t\t PARSE_OPT_NONEG),\n+\t\tOPT_STRING(0, \"merge-base\",\n+\t\t\t &o.merge_base,\n+\t\t\t N_(\"commit\"),\n+\t\t\t N_(\"specify a merge-base for the merge\")),\n \t\tOPT_END()\n \t};\n \ndiff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\nindex cac85591b52..5b0073d3dcd 100755\n--- a/t/t4301-merge-tree-write-tree.sh\n+++ b/t/t4301-merge-tree-write-tree.sh\n@@ -860,4 +860,34 @@ test_expect_success '--stdin with both a successful and a conflicted merge' '\n \ttest_cmp expect actual\n '\n \n+# specify merge-base as parent of branch2\n+# git merge-tree --write-tree --merge-base=c2 c1 c3\n+# Commit c1: add file1\n+# Commit c2: add file2 after c1\n+# Commit c3: add file3 after c2\n+# Expected: add file3, and file2 does NOT appear\n+\n+test_expect_success 'specify merge-base as parent of branch2' '\n+\t# Setup\n+\tgit init base-b2-p && (\n+\t\tcd base-b2-p &&\n+\t\ttest_commit c1 file1 &&\n+\t\ttest_commit c2 file2 &&\n+\t\ttest_commit c3 file3\n+\t) &&\n+\t# Testing\n+\t(\n+\t\tcd base-b2-p &&\n+\t\tTREE_OID=$(git merge-tree --write-tree --merge-base=c2 c1 c3) &&\n+\n+\t\tq_to_tab <<-EOF >expect &&\n+\t\t100644 blob $(git rev-parse c1:file1)Qfile1\n+\t\t100644 blob $(git rev-parse c3:file3)Qfile3\n+\t\tEOF\n+\n+\t\tgit ls-tree $TREE_OID >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n test_done\n\nbase-commit: ec1edbcb56ac05e9980299b05924c5c1b51d68b4\n-- \ngitgitgadget\n"},{"id":"466157","messageId":"CABPp-BF1kjWMS7Te31FqGrHMNOQEGhG=e1g9ah29ZHWGYufbXA@mail.gmail.com","threadId":"58703","inReplyTo":"pull.1397.v3.git.1667014975042.gitgitgadget@gmail.com","subject":"Re: [PATCH v3] merge-tree.c: add --merge-base= option","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-11-01T01:08:00Z","receivedAt":"2022-11-01T01:09:15Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nOn Fri, Oct 28, 2022 at 8:42 PM Kyle Zhao via GitGitGadget\n wrote:\n>\n> From: Kyle Zhao \n>\n> This patch will give our callers more flexibility to use `git merge-tree`,\n> such as:\n>\n> git merge-tree --write-tree --merge-base=branch^ HEAD branch\n>\n> This does a merge of HEAD and branch, but uses branch^ as the merge-base.\n>\n> And the reason why using an option flag instead of a positional argument\n> is to allow additional commits passed to merge-tree to be handled via an\n> octopus merge in the future.\n>\n> Signed-off-by: Kyle Zhao \n> ---\n> merge-tree: allow specifying a base commit when --write-tree is passed\n>\n> Thanks for Elijah's work. I'm very excited that merge-ort is integrated\n> into the git merge-tree, which means that we can use merge-ort in bare\n> repositories to optimize merge performance.\n>\n> In this patch, I introduce a new --merge-base= option to allow\n> callers to specify a merge-base for the merge. This may allow users to\n> implement git cherry-pick and git rebase in bare repositories with git\n> merge-tree cmd.\n>\n> Changes since v1:\n>\n> * Changed merge_incore_recursive() to merge_incore_nonrecursive() when\n> merge-base is specified.\n> * Fixed c style problem.\n> * Moved commit lookup/die logic out to the parsing logic in\n> cmd_merge_tree().\n> * use test_commit for test\n>\n> Changes since v2:\n>\n> * commit message\n> * Rebased on top of en/merge-tree-sequence.\n> * Set opt.ancestor to o->merge_base. Because opt.ancestor is a *char.\n> To make it easier to pass parameters, I moved\n> lookup_commit_reference_by_name() to real_ merge() again.\n> * Added test comment.\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1397%2Fkeyu98%2Fkz%2Fmerge-tree-option-merge-base-v3\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1397/keyu98/kz/merge-tree-option-merge-base-v3\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1397\n>\n[...]\n> Documentation/git-merge-tree.txt | 4 ++++\n> builtin/merge-tree.c | 39 ++++++++++++++++++++++++--------\n> t/t4301-merge-tree-write-tree.sh | 30 ++++++++++++++++++++++++\n> 3 files changed, 64 insertions(+), 9 deletions(-)\n>\n> diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\n> index 04bcc416e6e..d9dacb2ce54 100644\n> --- a/Documentation/git-merge-tree.txt\n> +++ b/Documentation/git-merge-tree.txt\n> @@ -64,6 +64,10 @@ OPTIONS\n> share no common history. This flag can be given to override that\n> check and make the merge proceed anyway.\n>\n> +--merge-base=::\n> + Instead of finding the merge-bases for and ,\n> + specify a merge-base for the merge.\n> +\n> [[OUTPUT]]\n> OUTPUT\n> ------\n> diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\n> index fe853aa8f91..cd25aac6ba6 100644\n> --- a/builtin/merge-tree.c\n> +++ b/builtin/merge-tree.c\n> @@ -3,6 +3,7 @@\n> #include \"tree-walk.h\"\n> #include \"xdiff-interface.h\"\n> #include \"help.h\"\n> +#include \"commit.h\"\n> #include \"commit-reach.h\"\n> #include \"merge-ort.h\"\n> #include \"object-store.h\"\n> @@ -403,6 +404,7 @@ struct merge_tree_options {\n> int show_messages;\n> int name_only;\n> int use_stdin;\n> + const char *merge_base;\n> };\n>\n> static int real_merge(struct merge_tree_options *o,\n> @@ -432,16 +434,31 @@ static int real_merge(struct merge_tree_options *o,\n> opt.branch1 = branch1;\n> opt.branch2 = branch2;\n>\n> - /*\n> - * Get the merge bases, in reverse order; see comment above\n> - * merge_incore_recursive in merge-ort.h\n> - */\n> - merge_bases = get_merge_bases(parent1, parent2);\n> - if (!merge_bases && !o->allow_unrelated_histories)\n> - die(_(\"refusing to merge unrelated histories\"));\n> - merge_bases = reverse_commit_list(merge_bases);\n> + if (o->merge_base) {\n> + struct commit *base_commit;\n> + struct tree *base_tree, *parent1_tree, *parent2_tree;\n> +\n> + base_commit = lookup_commit_reference_by_name(o->merge_base);\n> + if (!base_commit)\n> + die(_(\"could not lookup commit %s\"), o->merge_base);\n> +\n> + opt.ancestor = o->merge_base;\n> + base_tree = get_commit_tree(base_commit);\n> + parent1_tree = get_commit_tree(parent1);\n> + parent2_tree = get_commit_tree(parent2);\n> + merge_incore_nonrecursive(&opt, base_tree, parent1_tree, parent2_tree, &result);\n> + } else {\n> + /*\n> + * Get the merge bases, in reverse order; see comment above\n> + * merge_incore_recursive in merge-ort.h\n> + */\n> + merge_bases = get_merge_bases(parent1, parent2);\n> + if (!merge_bases && !o->allow_unrelated_histories)\n> + die(_(\"refusing to merge unrelated histories\"));\n> + merge_bases = reverse_commit_list(merge_bases);\n> + merge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);\n> + }\n>\n> - merge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);\n> if (result.clean < 0)\n> die(_(\"failure to merge\"));\n>\n> @@ -515,6 +532,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> &o.use_stdin,\n> N_(\"perform multiple merges, one per line of input\"),\n> PARSE_OPT_NONEG),\n> + OPT_STRING(0, \"merge-base\",\n> + &o.merge_base,\n> + N_(\"commit\"),\n> + N_(\"specify a merge-base for the merge\")),\n> OPT_END()\n> };\n>\n> diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\n> index cac85591b52..5b0073d3dcd 100755\n> --- a/t/t4301-merge-tree-write-tree.sh\n> +++ b/t/t4301-merge-tree-write-tree.sh\n> @@ -860,4 +860,34 @@ test_expect_success '--stdin with both a successful and a conflicted merge' '\n> test_cmp expect actual\n> '\n>\n> +# specify merge-base as parent of branch2\n> +# git merge-tree --write-tree --merge-base=c2 c1 c3\n> +# Commit c1: add file1\n> +# Commit c2: add file2 after c1\n> +# Commit c3: add file3 after c2\n> +# Expected: add file3, and file2 does NOT appear\n> +\n> +test_expect_success 'specify merge-base as parent of branch2' '\n> + # Setup\n> + git init base-b2-p && (\n> + cd base-b2-p &&\n> + test_commit c1 file1 &&\n> + test_commit c2 file2 &&\n> + test_commit c3 file3\n> + ) &&\n> + # Testing\n> + (\n> + cd base-b2-p &&\n> + TREE_OID=$(git merge-tree --write-tree --merge-base=c2 c1 c3) &&\n> +\n> + q_to_tab <<-EOF >expect &&\n> + 100644 blob $(git rev-parse c1:file1)Qfile1\n> + 100644 blob $(git rev-parse c3:file3)Qfile3\n> + EOF\n> +\n> + git ls-tree $TREE_OID >actual &&\n> + test_cmp expect actual\n> + )\n> +'\n> +\n> test_done\n>\n> base-commit: ec1edbcb56ac05e9980299b05924c5c1b51d68b4\n\nThis looks pretty good; thanks for the many changes. One thing is\nstill missing: you are now building on a commit that adds a --stdin\noption to merge-tree, which allows the user to specify what to merge\non stdin rather than via other arguments on the command line. Since\nyou're making merge-tree support specified merge base(s), it should\nalso support that in conjunction with --stdin. So, see the \"/* Handle\nstdin */\" block and adjust it appropriately.\n\nOnce you do that, I think your series will be good to go.\n"},{"id":"466188","messageId":"pull.1397.v4.git.1667292904.gitgitgadget@gmail.com","threadId":"58703","inReplyTo":"pull.1397.v3.git.1667014975042.gitgitgadget@gmail.com","subject":"[PATCH v4 0/2] merge-tree: allow specifying a base commit when --write-tree is passed","fromName":"Kyle Zhao via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-11-01T08:55:02Z","receivedAt":"2022-11-01T08:55:11Z","isPatch":true,"sender":{"key":"goldofzky@gmail.com","avatar":"https://gravatar.com/avatar/aa4fa27f0c1bed9c986ed596415799518e03a6e71a0931308a4de87e166e183a?d=mp&s=160"},"body":"Thanks for Elijah's work. I'm very excited that merge-ort is integrated into\nthe git merge-tree, which means that we can use merge-ort in bare\nrepositories to optimize merge performance.\n\nIn this patch, I introduce a new --merge-base= option to allow\ncallers to specify a merge-base for the merge. This may allow users to\nimplement git cherry-pick and git rebase in bare repositories with git\nmerge-tree cmd.\n\nChanges since v1:\n\n * Changed merge_incore_recursive() to merge_incore_nonrecursive() when\n merge-base is specified.\n * Fixed c style problem.\n * Moved commit lookup/die logic out to the parsing logic in\n cmd_merge_tree().\n * use test_commit for test\n\nChanges since v2:\n\n * commit message\n * Rebased on top of en/merge-tree-sequence.\n * Set opt.ancestor to o->merge_base. Because opt.ancestor is a *char. To\n make it easier to pass parameters, I moved\n lookup_commit_reference_by_name() to real_ merge() again.\n * Added test comment.\n\nChanges since v3:\n\n * support --merge-base in conjunction with --stdin\n\nKyle Zhao (2):\n merge-tree.c: add --merge-base= option\n merge-tree.c: support --merge-base in conjunction with --stdin\n\n Documentation/git-merge-tree.txt | 5 +++\n builtin/merge-tree.c | 64 ++++++++++++++++++++++++++------\n t/t4301-merge-tree-write-tree.sh | 51 +++++++++++++++++++++++++\n 3 files changed, 108 insertions(+), 12 deletions(-)\n\n\nbase-commit: ec1edbcb56ac05e9980299b05924c5c1b51d68b4\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1397%2Fkeyu98%2Fkz%2Fmerge-tree-option-merge-base-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1397/keyu98/kz/merge-tree-option-merge-base-v4\nPull-Request: https://github.com/gitgitgadget/git/pull/1397\n\nRange-diff vs v3:\n\n 1: 554cbde49e8 ! 1: bba854fc8fa merge-tree.c: add --merge-base= option\n @@ builtin/merge-tree.c\n #include \"merge-ort.h\"\n #include \"object-store.h\"\n @@ builtin/merge-tree.c: struct merge_tree_options {\n - \tint show_messages;\n - \tint name_only;\n - \tint use_stdin;\n -+\tconst char *merge_base;\n };\n \n static int real_merge(struct merge_tree_options *o,\n ++\t\t const char *merge_base,\n + \t\t const char *branch1, const char *branch2,\n + \t\t const char *prefix)\n + {\n @@ builtin/merge-tree.c: static int real_merge(struct merge_tree_options *o,\n \topt.branch1 = branch1;\n \topt.branch2 = branch2;\n @@ builtin/merge-tree.c: static int real_merge(struct merge_tree_options *o,\n -\tif (!merge_bases && !o->allow_unrelated_histories)\n -\t\tdie(_(\"refusing to merge unrelated histories\"));\n -\tmerge_bases = reverse_commit_list(merge_bases);\n -+\tif (o->merge_base) {\n ++\tif (merge_base) {\n +\t\tstruct commit *base_commit;\n +\t\tstruct tree *base_tree, *parent1_tree, *parent2_tree;\n +\n -+\t\tbase_commit = lookup_commit_reference_by_name(o->merge_base);\n ++\t\tbase_commit = lookup_commit_reference_by_name(merge_base);\n +\t\tif (!base_commit)\n -+\t\t\tdie(_(\"could not lookup commit %s\"), o->merge_base);\n ++\t\t\tdie(_(\"could not lookup commit %s\"), merge_base);\n +\n -+\t\topt.ancestor = o->merge_base;\n ++\t\topt.ancestor = merge_base;\n +\t\tbase_tree = get_commit_tree(base_commit);\n +\t\tparent1_tree = get_commit_tree(parent1);\n +\t\tparent2_tree = get_commit_tree(parent2);\n @@ builtin/merge-tree.c: static int real_merge(struct merge_tree_options *o,\n \tif (result.clean < 0)\n \t\tdie(_(\"failure to merge\"));\n \n +@@ builtin/merge-tree.c: int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n + \tstruct merge_tree_options o = { .show_messages = -1 };\n + \tint expected_remaining_argc;\n + \tint original_argc;\n ++\tconst char *merge_base = NULL;\n + \n + \tconst char * const merge_tree_usage[] = {\n + \t\tN_(\"git merge-tree [--write-tree] [] \"),\n @@ builtin/merge-tree.c: int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\t\t &o.use_stdin,\n \t\t\t N_(\"perform multiple merges, one per line of input\"),\n \t\t\t PARSE_OPT_NONEG),\n +\t\tOPT_STRING(0, \"merge-base\",\n -+\t\t\t &o.merge_base,\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_END()\n \t};\n \n +@@ builtin/merge-tree.c: int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n + \t\t\tif (!split[0] || !split[1] || split[2])\n + \t\t\t\tdie(_(\"malformed input line: '%s'.\"), buf.buf);\n + \t\t\tstrbuf_rtrim(split[0]);\n +-\t\t\tresult = real_merge(&o, split[0]->buf, split[1]->buf, prefix);\n ++\t\t\tresult = real_merge(&o, merge_base, split[0]->buf, split[1]->buf, prefix);\n + \t\t\tif (result < 0)\n + \t\t\t\tdie(_(\"merging cannot continue; got unclean result of %d\"), result);\n + \t\t\tstrbuf_list_free(split);\n +@@ builtin/merge-tree.c: int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n + \n + \t/* Do the relevant type of merge */\n + \tif (o.mode == MODE_REAL)\n +-\t\treturn real_merge(&o, argv[0], argv[1], prefix);\n ++\t\treturn real_merge(&o, merge_base, argv[0], argv[1], prefix);\n + \telse\n + \t\treturn trivial_merge(argv[0], argv[1], argv[2]);\n + }\n \n ## t/t4301-merge-tree-write-tree.sh ##\n @@ t/t4301-merge-tree-write-tree.sh: test_expect_success '--stdin with both a successful and a conflicted merge' '\n -: ----------- > 2: db47fbc663e merge-tree.c: support --merge-base in conjunction with --stdin\n\n-- \ngitgitgadget\n"},{"id":"466190","messageId":"db47fbc663eb0f3a1fc9a063dfb1051bc64601af.1667292904.git.gitgitgadget@gmail.com","threadId":"58703","inReplyTo":"pull.1397.v4.git.1667292904.gitgitgadget@gmail.com","subject":"[PATCH v4 2/2] merge-tree.c: support --merge-base in conjunction with --stdin","fromName":"Kyle Zhao via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-11-01T08:55:04Z","receivedAt":"2022-11-01T08:55:13Z","isPatch":true,"sender":{"key":"goldofzky@gmail.com","avatar":"https://gravatar.com/avatar/aa4fa27f0c1bed9c986ed596415799518e03a6e71a0931308a4de87e166e183a?d=mp&s=160"},"body":"From: Kyle Zhao \n\nThe previous change add \"--merge-base\" option in order to allow users to\nspecify merge-base for the merge. But it doesn't compatible well with\n--stdin, because multiple batched merges can only have the same specified\nbase.\n\nThis patch allows users to pass --merge-base option into the input line,\nsuch as:\n\n printf \"--merge-base= \" | git merge-tree --stdin\n\nThis does a merge of b1 and b2, and uses b3 as the merge-base.\n\nSigned-off-by: Kyle Zhao \n---\n Documentation/git-merge-tree.txt | 3 ++-\n builtin/merge-tree.c | 22 ++++++++++++++++++++--\n t/t4301-merge-tree-write-tree.sh | 21 +++++++++++++++++++++\n 3 files changed, 43 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\nindex d9dacb2ce54..be6a11bbaec 100644\n--- a/Documentation/git-merge-tree.txt\n+++ b/Documentation/git-merge-tree.txt\n@@ -66,7 +66,8 @@ OPTIONS\n \n --merge-base=::\n \tInstead of finding the merge-bases for and ,\n-\tspecify a merge-base for the merge.\n+\tspecify a merge-base for the merge. When --stdin is passed, this\n+\toption should be passed into the input line.\n \n [[OUTPUT]]\n OUTPUT\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex f402b807c0f..7a8049e7b0c 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -551,16 +551,34 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \n \t\tif (o.mode == MODE_TRIVIAL)\n \t\t\tdie(_(\"--trivial-merge is incompatible with all other options\"));\n+\t\tif (merge_base)\n+\t\t\tdie(_(\"--merge-base should be passed into the input line\"));\n \t\tline_termination = '\\0';\n \t\twhile (strbuf_getline_lf(&buf, stdin) != EOF) {\n \t\t\tstruct strbuf **split;\n \t\t\tint result;\n+\t\t\tconst char *input_merge_base = NULL;\n+\t\t\tconst char *arg;\n \n \t\t\tsplit = strbuf_split(&buf, ' ');\n-\t\t\tif (!split[0] || !split[1] || split[2])\n+\t\t\tif (!split[0] || !split[1])\n \t\t\t\tdie(_(\"malformed input line: '%s'.\"), buf.buf);\n \t\t\tstrbuf_rtrim(split[0]);\n-\t\t\tresult = real_merge(&o, merge_base, split[0]->buf, split[1]->buf, prefix);\n+\n+\t\t\t/* parse --merge-base= option */\n+\t\t\targ = split[0]->buf;\n+\t\t\tif (skip_prefix(arg, \"--merge-base=\", &arg))\n+\t\t\t\tinput_merge_base = arg;\n+\n+\t\t\tif (input_merge_base && split[2] && !split[3]) {\n+\t\t\t\tstrbuf_rtrim(split[1]);\n+\t\t\t\tresult = real_merge(&o, input_merge_base, split[1]->buf, split[2]->buf, prefix);\n+\t\t\t} else if (!input_merge_base && !split[2]) {\n+\t\t\t\tresult = real_merge(&o, NULL, split[0]->buf, split[1]->buf, prefix);\n+\t\t\t} else {\n+\t\t\t\tdie(_(\"malformed input line: '%s'.\"), buf.buf);\n+\t\t\t}\n+\n \t\t\tif (result < 0)\n \t\t\t\tdie(_(\"merging cannot continue; got unclean result of %d\"), result);\n \t\t\tstrbuf_list_free(split);\ndiff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\nindex 5b0073d3dcd..aec2c00b91f 100755\n--- a/t/t4301-merge-tree-write-tree.sh\n+++ b/t/t4301-merge-tree-write-tree.sh\n@@ -860,6 +860,13 @@ test_expect_success '--stdin with both a successful and a conflicted merge' '\n \ttest_cmp expect actual\n '\n \n+\n+test_expect_success '--merge-base is incompatible with --stdin' '\n+\ttest_must_fail git merge-tree --merge-base=side1 --stdin 2>expect &&\n+\n+\tgrep \"^fatal: --merge-base should be passed into the input line\" expect\n+'\n+\n # specify merge-base as parent of branch2\n # git merge-tree --write-tree --merge-base=c2 c1 c3\n # Commit c1: add file1\n@@ -890,4 +897,18 @@ test_expect_success 'specify merge-base as parent of branch2' '\n \t)\n '\n \n+test_expect_success '--stdin with both a normal merge and a merge-base specified merge' '\n+\tcd base-b2-p &&\n+\tprintf \"c1 c3\\n--merge-base=c2 c1 c3\" | git merge-tree --stdin >actual &&\n+\n+\tprintf \"1\\0\" >expect &&\n+\tgit merge-tree --write-tree -z c1 c3 >>expect &&\n+\tprintf \"\\0\" >>expect &&\n+\n+\tprintf \"1\\0\" >>expect &&\n+\tgit merge-tree --write-tree -z --merge-base=c2 c1 c3 >>expect &&\n+\tprintf \"\\0\" >>expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \ngitgitgadget\n"},{"id":"466189","messageId":"bba854fc8fac39bb5e560b012a78fc48bdf13b15.1667292904.git.gitgitgadget@gmail.com","threadId":"58703","inReplyTo":"pull.1397.v4.git.1667292904.gitgitgadget@gmail.com","subject":"[PATCH v4 1/2] merge-tree.c: add --merge-base= option","fromName":"Kyle Zhao via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-11-01T08:55:03Z","receivedAt":"2022-11-01T08:55:15Z","isPatch":true,"sender":{"key":"goldofzky@gmail.com","avatar":"https://gravatar.com/avatar/aa4fa27f0c1bed9c986ed596415799518e03a6e71a0931308a4de87e166e183a?d=mp&s=160"},"body":"From: Kyle Zhao \n\nThis patch will give our callers more flexibility to use `git merge-tree`,\nsuch as:\n\n git merge-tree --write-tree --merge-base=branch^ HEAD branch\n\nThis does a merge of HEAD and branch, but uses branch^ as the merge-base.\n\nAnd the reason why using an option flag instead of a positional argument\nis to allow additional commits passed to merge-tree to be handled via an\noctopus merge in the future.\n\nSigned-off-by: Kyle Zhao \n---\n Documentation/git-merge-tree.txt | 4 +++\n builtin/merge-tree.c | 44 ++++++++++++++++++++++++--------\n t/t4301-merge-tree-write-tree.sh | 30 ++++++++++++++++++++++\n 3 files changed, 67 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\nindex 04bcc416e6e..d9dacb2ce54 100644\n--- a/Documentation/git-merge-tree.txt\n+++ b/Documentation/git-merge-tree.txt\n@@ -64,6 +64,10 @@ OPTIONS\n \tshare no common history. This flag can be given to override that\n \tcheck and make the merge proceed anyway.\n \n+--merge-base=::\n+\tInstead of finding the merge-bases for and ,\n+\tspecify a merge-base for the merge.\n+\n [[OUTPUT]]\n OUTPUT\n ------\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex fe853aa8f91..f402b807c0f 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -3,6 +3,7 @@\n #include \"tree-walk.h\"\n #include \"xdiff-interface.h\"\n #include \"help.h\"\n+#include \"commit.h\"\n #include \"commit-reach.h\"\n #include \"merge-ort.h\"\n #include \"object-store.h\"\n@@ -406,6 +407,7 @@ struct merge_tree_options {\n };\n \n static int real_merge(struct merge_tree_options *o,\n+\t\t const char *merge_base,\n \t\t const char *branch1, const char *branch2,\n \t\t const char *prefix)\n {\n@@ -432,16 +434,31 @@ static int real_merge(struct merge_tree_options *o,\n \topt.branch1 = branch1;\n \topt.branch2 = branch2;\n \n-\t/*\n-\t * Get the merge bases, in reverse order; see comment above\n-\t * merge_incore_recursive in merge-ort.h\n-\t */\n-\tmerge_bases = get_merge_bases(parent1, parent2);\n-\tif (!merge_bases && !o->allow_unrelated_histories)\n-\t\tdie(_(\"refusing to merge unrelated histories\"));\n-\tmerge_bases = reverse_commit_list(merge_bases);\n+\tif (merge_base) {\n+\t\tstruct commit *base_commit;\n+\t\tstruct tree *base_tree, *parent1_tree, *parent2_tree;\n+\n+\t\tbase_commit = lookup_commit_reference_by_name(merge_base);\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\tbase_tree = get_commit_tree(base_commit);\n+\t\tparent1_tree = get_commit_tree(parent1);\n+\t\tparent2_tree = get_commit_tree(parent2);\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+\t\t * merge_incore_recursive in merge-ort.h\n+\t\t */\n+\t\tmerge_bases = get_merge_bases(parent1, parent2);\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}\n \n-\tmerge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);\n \tif (result.clean < 0)\n \t\tdie(_(\"failure to merge\"));\n \n@@ -487,6 +504,7 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \tstruct merge_tree_options o = { .show_messages = -1 };\n \tint expected_remaining_argc;\n \tint original_argc;\n+\tconst char *merge_base = NULL;\n \n \tconst char * const merge_tree_usage[] = {\n \t\tN_(\"git merge-tree [--write-tree] [] \"),\n@@ -515,6 +533,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\t\t &o.use_stdin,\n \t\t\t N_(\"perform multiple merges, one per line of input\"),\n \t\t\t PARSE_OPT_NONEG),\n+\t\tOPT_STRING(0, \"merge-base\",\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_END()\n \t};\n \n@@ -538,7 +560,7 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\t\tif (!split[0] || !split[1] || split[2])\n \t\t\t\tdie(_(\"malformed input line: '%s'.\"), buf.buf);\n \t\t\tstrbuf_rtrim(split[0]);\n-\t\t\tresult = real_merge(&o, split[0]->buf, split[1]->buf, prefix);\n+\t\t\tresult = real_merge(&o, merge_base, split[0]->buf, split[1]->buf, prefix);\n \t\t\tif (result < 0)\n \t\t\t\tdie(_(\"merging cannot continue; got unclean result of %d\"), result);\n \t\t\tstrbuf_list_free(split);\n@@ -581,7 +603,7 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \n \t/* Do the relevant type of merge */\n \tif (o.mode == MODE_REAL)\n-\t\treturn real_merge(&o, argv[0], argv[1], prefix);\n+\t\treturn real_merge(&o, merge_base, argv[0], argv[1], prefix);\n \telse\n \t\treturn trivial_merge(argv[0], argv[1], argv[2]);\n }\ndiff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\nindex cac85591b52..5b0073d3dcd 100755\n--- a/t/t4301-merge-tree-write-tree.sh\n+++ b/t/t4301-merge-tree-write-tree.sh\n@@ -860,4 +860,34 @@ test_expect_success '--stdin with both a successful and a conflicted merge' '\n \ttest_cmp expect actual\n '\n \n+# specify merge-base as parent of branch2\n+# git merge-tree --write-tree --merge-base=c2 c1 c3\n+# Commit c1: add file1\n+# Commit c2: add file2 after c1\n+# Commit c3: add file3 after c2\n+# Expected: add file3, and file2 does NOT appear\n+\n+test_expect_success 'specify merge-base as parent of branch2' '\n+\t# Setup\n+\tgit init base-b2-p && (\n+\t\tcd base-b2-p &&\n+\t\ttest_commit c1 file1 &&\n+\t\ttest_commit c2 file2 &&\n+\t\ttest_commit c3 file3\n+\t) &&\n+\t# Testing\n+\t(\n+\t\tcd base-b2-p &&\n+\t\tTREE_OID=$(git merge-tree --write-tree --merge-base=c2 c1 c3) &&\n+\n+\t\tq_to_tab <<-EOF >expect &&\n+\t\t100644 blob $(git rev-parse c1:file1)Qfile1\n+\t\t100644 blob $(git rev-parse c3:file3)Qfile3\n+\t\tEOF\n+\n+\t\tgit ls-tree $TREE_OID >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"466219","messageId":"Y2GNeqdn3oDy+OFJ@nand.local","threadId":"58703","inReplyTo":"pull.1397.v4.git.1667292904.gitgitgadget@gmail.com","subject":"Re: [PATCH v4 0/2] merge-tree: allow specifying a base commit when --write-tree is passed","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-11-01T21:19:54Z","receivedAt":"2022-11-01T21:22:11Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Nov 01, 2022 at 08:55:02AM +0000, Kyle Zhao via GitGitGadget wrote:\n> Thanks for Elijah's work. I'm very excited that merge-ort is integrated into\n> the git merge-tree, which means that we can use merge-ort in bare\n> repositories to optimize merge performance.\n>\n> In this patch, I introduce a new --merge-base= option to allow\n> callers to specify a merge-base for the merge. This may allow users to\n> implement git cherry-pick and git rebase in bare repositories with git\n> merge-tree cmd.\n\nThanks for the update. I haven't taken a close look, but queued what you\nhave as a replacement on top of en/merge-tree-sequence.\n\nLet's see what Elijah thinks...\n\nThanks,\nTaylor\n"},{"id":"466368","messageId":"CABPp-BG4in6hPnniHoE+au0XyVXwgs9pNUJdpLDTmP=dxvKjqw@mail.gmail.com","threadId":"58703","inReplyTo":"db47fbc663eb0f3a1fc9a063dfb1051bc64601af.1667292904.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v4 2/2] merge-tree.c: support --merge-base in conjunction with --stdin","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-11-03T01:13:00Z","receivedAt":"2022-11-03T01:13:33Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Nov 1, 2022 at 1:55 AM Kyle Zhao via GitGitGadget\n wrote:\n>\n> From: Kyle Zhao \n>\n> The previous change add \"--merge-base\" option in order to allow users to\n\ns/add/added/ ?\n\n> specify merge-base for the merge. But it doesn't compatible well with\n> --stdin, because multiple batched merges can only have the same specified\n> base.\n\n\"it doesn't compatible well\" doesn't parse for me.\n\n> This patch allows users to pass --merge-base option into the input line,\n> such as:\n\nQuoting from Documentation/SubmittingPatches:\n\n\"\"\"\nDescribe your changes in imperative mood, e.g. \"make xyzzy do frotz\"\ninstead of \"[This patch] makes xyzzy do frotz\" or \"[I] changed xyzzy\nto do frotz\", as if you are giving orders to the codebase to change\nits behavior.\n\"\"\"\n\n>\n> printf \"--merge-base= \" | git merge-tree --stdin\n>\n> This does a merge of b1 and b2, and uses b3 as the merge-base.\n\nPerhaps something like:\n\n\"\"\"\nThe previous commit added a `--merge-base` option in order to allow\nusing a specified merge-base for the merge. Extend the input accepted\nby `--stdin` to also allow a specified merge-base with each merge\nrequested. For example:\n\n printf \"--merge-base= \" | git merge-tree --stdin\n\ndoes a merge of b1 and b2, and uses b3 as the merge-base.\n\"\"\"\n\nHowever, I'm a bit curious about using `--merge-base=` on the input\nline as opposed to just using a simpler marker; something like\n\n printf \" -- \" | git merge-tree --stdin\n\n(which follows a precedent set by git-merge-recursive). Your version\nis a bit more self-documenting, but what if we want to allow users to\nspecify multiple merge bases in the future (for use in passing to\nmerge_incore_recursive())? Is it annoying to need to prefix each one?\n\n> Signed-off-by: Kyle Zhao \n> ---\n> Documentation/git-merge-tree.txt | 3 ++-\n> builtin/merge-tree.c | 22 ++++++++++++++++++++--\n> t/t4301-merge-tree-write-tree.sh | 21 +++++++++++++++++++++\n> 3 files changed, 43 insertions(+), 3 deletions(-)\n>\n> diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\n> index d9dacb2ce54..be6a11bbaec 100644\n> --- a/Documentation/git-merge-tree.txt\n> +++ b/Documentation/git-merge-tree.txt\n> @@ -66,7 +66,8 @@ OPTIONS\n>\n> --merge-base=::\n> Instead of finding the merge-bases for and ,\n> - specify a merge-base for the merge.\n> + specify a merge-base for the merge. When --stdin is passed, this\n> + option should be passed into the input line.\n\nI'm not sure \"passed into the input line\" will be clear to readers.\nPerhaps we want to have --stdin documented (looks like I forgot to do\nthat in my series; oops), mentioning the input format.\n\n> [[OUTPUT]]\n> OUTPUT\n> diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\n> index f402b807c0f..7a8049e7b0c 100644\n> --- a/builtin/merge-tree.c\n> +++ b/builtin/merge-tree.c\n> @@ -551,16 +551,34 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n>\n> if (o.mode == MODE_TRIVIAL)\n> die(_(\"--trivial-merge is incompatible with all other options\"));\n> + if (merge_base)\n> + die(_(\"--merge-base should be passed into the input line\"));\n\nIf we change the input as noted above, the wording here may need to change too.\n\n> line_termination = '\\0';\n> while (strbuf_getline_lf(&buf, stdin) != EOF) {\n> struct strbuf **split;\n> int result;\n> + const char *input_merge_base = NULL;\n> + const char *arg;\n>\n> split = strbuf_split(&buf, ' ');\n> - if (!split[0] || !split[1] || split[2])\n> + if (!split[0] || !split[1])\n> die(_(\"malformed input line: '%s'.\"), buf.buf);\n> strbuf_rtrim(split[0]);\n> - result = real_merge(&o, merge_base, split[0]->buf, split[1]->buf, prefix);\n> +\n> + /* parse --merge-base= option */\n> + arg = split[0]->buf;\n> + if (skip_prefix(arg, \"--merge-base=\", &arg))\n> + input_merge_base = arg;\n> +\n> + if (input_merge_base && split[2] && !split[3]) {\n> + strbuf_rtrim(split[1]);\n> + result = real_merge(&o, input_merge_base, split[1]->buf, split[2]->buf, prefix);\n> + } else if (!input_merge_base && !split[2]) {\n> + result = real_merge(&o, NULL, split[0]->buf, split[1]->buf, prefix);\n> + } else {\n> + die(_(\"malformed input line: '%s'.\"), buf.buf);\n> + }\n> +\n> if (result < 0)\n> die(_(\"merging cannot continue; got unclean result of %d\"), result);\n> strbuf_list_free(split);\n> diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\n> index 5b0073d3dcd..aec2c00b91f 100755\n> --- a/t/t4301-merge-tree-write-tree.sh\n> +++ b/t/t4301-merge-tree-write-tree.sh\n> @@ -860,6 +860,13 @@ test_expect_success '--stdin with both a successful and a conflicted merge' '\n> test_cmp expect actual\n> '\n>\n> +\n> +test_expect_success '--merge-base is incompatible with --stdin' '\n> + test_must_fail git merge-tree --merge-base=side1 --stdin 2>expect &&\n> +\n> + grep \"^fatal: --merge-base should be passed into the input line\" expect\n> +'\n\nYeah, most merge-tree options are for controlling the output, and as\nsuch they are not incompatible with --stdin. This option is, so it\nmakes sense you need a special check for it. Looks good.\n\n> +\n> # specify merge-base as parent of branch2\n> # git merge-tree --write-tree --merge-base=c2 c1 c3\n> # Commit c1: add file1\n> @@ -890,4 +897,18 @@ test_expect_success 'specify merge-base as parent of branch2' '\n> )\n> '\n>\n> +test_expect_success '--stdin with both a normal merge and a merge-base specified merge' '\n> + cd base-b2-p &&\n> + printf \"c1 c3\\n--merge-base=c2 c1 c3\" | git merge-tree --stdin >actual &&\n> +\n> + printf \"1\\0\" >expect &&\n> + git merge-tree --write-tree -z c1 c3 >>expect &&\n> + printf \"\\0\" >>expect &&\n> +\n> + printf \"1\\0\" >>expect &&\n> + git merge-tree --write-tree -z --merge-base=c2 c1 c3 >>expect &&\n> + printf \"\\0\" >>expect &&\n> + test_cmp expect actual\n> +'\n\nThis last test seems odd. You are merely testing that the output of\n\"git merge-tree --stdin\" matches the output of repeated calls to \"git\nmerge-tree\", not that the merges involved produce correct results?\nMaybe that's fine since earlier tests verify that individual\nmerge-tree calls are doing the right thing, I was just a bit\nsurprised. Maybe a comment in the code that this was your intent\nwould be helpful?\n"},{"id":"466373","messageId":"pull.1397.v5.git.1667446182.gitgitgadget@gmail.com","threadId":"58703","inReplyTo":"pull.1397.v4.git.1667292904.gitgitgadget@gmail.com","subject":"[PATCH v5 0/2] merge-tree: allow specifying a base commit when --write-tree is passed","fromName":"Kyle Zhao via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-11-03T03:29:40Z","receivedAt":"2022-11-03T03:29:59Z","isPatch":true,"sender":{"key":"goldofzky@gmail.com","avatar":"https://gravatar.com/avatar/aa4fa27f0c1bed9c986ed596415799518e03a6e71a0931308a4de87e166e183a?d=mp&s=160"},"body":"Thanks for Elijah's work. I'm very excited that merge-ort is integrated into\nthe git merge-tree, which means that we can use merge-ort in bare\nrepositories to optimize merge performance.\n\nIn this patch, I introduce a new --merge-base= option to allow\ncallers to specify a merge-base for the merge. This may allow users to\nimplement git cherry-pick and git rebase in bare repositories with git\nmerge-tree cmd.\n\nChanges since v1:\n\n * Changed merge_incore_recursive() to merge_incore_nonrecursive() when\n merge-base is specified.\n * Fixed c style problem.\n * Moved commit lookup/die logic out to the parsing logic in\n cmd_merge_tree().\n * use test_commit for test\n\nChanges since v2:\n\n * commit message\n * Rebased on top of en/merge-tree-sequence.\n * Set opt.ancestor to o->merge_base. Because opt.ancestor is a *char. To\n make it easier to pass parameters, I moved\n lookup_commit_reference_by_name() to real_ merge() again.\n * Added test comment.\n\nChanges since v3:\n\n * support --merge-base in conjunction with --stdin\n\nKyle Zhao (2):\n merge-tree.c: add --merge-base= option\n merge-tree.c: allow specifying the merge-base when --stdin is passed\n\n Documentation/git-merge-tree.txt | 16 ++++++++\n builtin/merge-tree.c | 65 ++++++++++++++++++++++++++------\n t/t4301-merge-tree-write-tree.sh | 55 +++++++++++++++++++++++++++\n 3 files changed, 124 insertions(+), 12 deletions(-)\n\n\nbase-commit: ec1edbcb56ac05e9980299b05924c5c1b51d68b4\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1397%2Fkeyu98%2Fkz%2Fmerge-tree-option-merge-base-v5\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1397/keyu98/kz/merge-tree-option-merge-base-v5\nPull-Request: https://github.com/gitgitgadget/git/pull/1397\n\nRange-diff vs v4:\n\n 1: bba854fc8fa ! 1: 01df0d1a6a7 merge-tree.c: add --merge-base= option\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 + \n + \t\tif (o.mode == MODE_TRIVIAL)\n + \t\t\tdie(_(\"--trivial-merge is incompatible with all other options\"));\n ++\t\tif (merge_base)\n ++\t\t\tdie(_(\"--merge-base is incompatible with --stdin\"));\n + \t\tline_termination = '\\0';\n + \t\twhile (strbuf_getline_lf(&buf, stdin) != EOF) {\n + \t\t\tstruct strbuf **split;\n @@ builtin/merge-tree.c: int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\t\tif (!split[0] || !split[1] || split[2])\n \t\t\t\tdie(_(\"malformed input line: '%s'.\"), buf.buf);\n 2: db47fbc663e ! 2: 3bdfad03cca merge-tree.c: support --merge-base in conjunction with --stdin\n @@ Metadata\n Author: Kyle Zhao \n \n ## Commit message ##\n - merge-tree.c: support --merge-base in conjunction with --stdin\n + merge-tree.c: allow specifying the merge-base when --stdin is passed\n \n - The previous change add \"--merge-base\" option in order to allow users to\n - specify merge-base for the merge. But it doesn't compatible well with\n - --stdin, because multiple batched merges can only have the same specified\n - base.\n + The previous commit added a `--merge-base` option in order to allow\n + using a specified merge-base for the merge. Extend the input accepted\n + by `--stdin` to also allow a specified merge-base with each merge\n + requested. For example:\n \n - This patch allows users to pass --merge-base option into the input line,\n - such as:\n + printf \" -- \" | git merge-tree --stdin\n \n - printf \"--merge-base= \" | git merge-tree --stdin\n -\n - This does a merge of b1 and b2, and uses b3 as the merge-base.\n + does a merge of b1 and b2, and uses b3 as the merge-base.\n \n Signed-off-by: Kyle Zhao \n \n @@ Documentation/git-merge-tree.txt: OPTIONS\n --merge-base=::\n \tInstead of finding the merge-bases for and ,\n -\tspecify a merge-base for the merge.\n -+\tspecify a merge-base for the merge. When --stdin is passed, this\n -+\toption should be passed into the input line.\n ++\tspecify a merge-base for the merge. This option is incompatible\n ++\twith `--stdin`.\n \n [[OUTPUT]]\n OUTPUT\n +@@ Documentation/git-merge-tree.txt: with linkgit:git-merge[1]:\n + * any messages that would have been printed to stdout (the\n + <>)\n + \n ++INPUT FORMAT\n ++------------\n ++'git merge-tree --stdin' input format is fully text based. Each line\n ++has this format:\n ++\n ++\t[ -- ] \n ++\n ++If one line is separated by `--`, the string before the separator is\n ++used for specifying a merge-base for the merge and the string after\n ++the separator describes the branches to be merged.\n ++\n + MISTAKES TO AVOID\n + -----------------\n + \n \n ## builtin/merge-tree.c ##\n @@ builtin/merge-tree.c: int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n - \n - \t\tif (o.mode == MODE_TRIVIAL)\n - \t\t\tdie(_(\"--trivial-merge is incompatible with all other options\"));\n -+\t\tif (merge_base)\n -+\t\t\tdie(_(\"--merge-base should be passed into the input line\"));\n - \t\tline_termination = '\\0';\n \t\twhile (strbuf_getline_lf(&buf, stdin) != EOF) {\n \t\t\tstruct strbuf **split;\n \t\t\tint result;\n +\t\t\tconst char *input_merge_base = NULL;\n -+\t\t\tconst char *arg;\n \n \t\t\tsplit = strbuf_split(&buf, ' ');\n -\t\t\tif (!split[0] || !split[1] || split[2])\n @@ builtin/merge-tree.c: int cmd_merge_tree(int argc, const char **argv, const char\n \t\t\t\tdie(_(\"malformed input line: '%s'.\"), buf.buf);\n \t\t\tstrbuf_rtrim(split[0]);\n -\t\t\tresult = real_merge(&o, merge_base, split[0]->buf, split[1]->buf, prefix);\n ++\t\t\tstrbuf_rtrim(split[1]);\n +\n -+\t\t\t/* parse --merge-base= option */\n -+\t\t\targ = split[0]->buf;\n -+\t\t\tif (skip_prefix(arg, \"--merge-base=\", &arg))\n -+\t\t\t\tinput_merge_base = arg;\n ++\t\t\t/* parse the merge-base */\n ++\t\t\tif (!strcmp(split[1]->buf, \"--\")) {\n ++\t\t\t\tinput_merge_base = split[0]->buf;\n ++\t\t\t}\n +\n -+\t\t\tif (input_merge_base && split[2] && !split[3]) {\n -+\t\t\t\tstrbuf_rtrim(split[1]);\n -+\t\t\t\tresult = real_merge(&o, input_merge_base, split[1]->buf, split[2]->buf, prefix);\n ++\t\t\tif (input_merge_base && split[2] && split[3] && !split[4]) {\n ++\t\t\t\tstrbuf_rtrim(split[2]);\n ++\t\t\t\tstrbuf_rtrim(split[3]);\n ++\t\t\t\tresult = real_merge(&o, input_merge_base, split[2]->buf, split[3]->buf, prefix);\n +\t\t\t} else if (!input_merge_base && !split[2]) {\n +\t\t\t\tresult = real_merge(&o, NULL, split[0]->buf, split[1]->buf, prefix);\n +\t\t\t} else {\n @@ t/t4301-merge-tree-write-tree.sh: test_expect_success '--stdin with both a succe\n +test_expect_success '--merge-base is incompatible with --stdin' '\n +\ttest_must_fail git merge-tree --merge-base=side1 --stdin 2>expect &&\n +\n -+\tgrep \"^fatal: --merge-base should be passed into the input line\" expect\n ++\tgrep \"^fatal: --merge-base is incompatible with --stdin\" expect\n +'\n +\n # specify merge-base as parent of branch2\n @@ t/t4301-merge-tree-write-tree.sh: test_expect_success 'specify merge-base as par\n \t)\n '\n \n ++# Since the earlier tests have verified that individual merge-tree calls\n ++# are doing the right thing, this test case is only used to test whether\n ++# the input format is available.\n ++\n +test_expect_success '--stdin with both a normal merge and a merge-base specified merge' '\n +\tcd base-b2-p &&\n -+\tprintf \"c1 c3\\n--merge-base=c2 c1 c3\" | git merge-tree --stdin >actual &&\n ++\tprintf \"c1 c3\\nc2 -- c1 c3\" | git merge-tree --stdin >actual &&\n +\n +\tprintf \"1\\0\" >expect &&\n +\tgit merge-tree --write-tree -z c1 c3 >>expect &&\n\n-- \ngitgitgadget\n"},{"id":"466374","messageId":"01df0d1a6a7319beada3a3a94b8e36818be2a4d2.1667446182.git.gitgitgadget@gmail.com","threadId":"58703","inReplyTo":"pull.1397.v5.git.1667446182.gitgitgadget@gmail.com","subject":"[PATCH v5 1/2] merge-tree.c: add --merge-base= option","fromName":"Kyle Zhao via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-11-03T03:29:41Z","receivedAt":"2022-11-03T03:30:05Z","isPatch":true,"sender":{"key":"goldofzky@gmail.com","avatar":"https://gravatar.com/avatar/aa4fa27f0c1bed9c986ed596415799518e03a6e71a0931308a4de87e166e183a?d=mp&s=160"},"body":"From: Kyle Zhao \n\nThis patch will give our callers more flexibility to use `git merge-tree`,\nsuch as:\n\n git merge-tree --write-tree --merge-base=branch^ HEAD branch\n\nThis does a merge of HEAD and branch, but uses branch^ as the merge-base.\n\nAnd the reason why using an option flag instead of a positional argument\nis to allow additional commits passed to merge-tree to be handled via an\noctopus merge in the future.\n\nSigned-off-by: Kyle Zhao \n---\n Documentation/git-merge-tree.txt | 4 +++\n builtin/merge-tree.c | 46 ++++++++++++++++++++++++--------\n t/t4301-merge-tree-write-tree.sh | 30 +++++++++++++++++++++\n 3 files changed, 69 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\nindex 04bcc416e6e..d9dacb2ce54 100644\n--- a/Documentation/git-merge-tree.txt\n+++ b/Documentation/git-merge-tree.txt\n@@ -64,6 +64,10 @@ OPTIONS\n \tshare no common history. This flag can be given to override that\n \tcheck and make the merge proceed anyway.\n \n+--merge-base=::\n+\tInstead of finding the merge-bases for and ,\n+\tspecify a merge-base for the merge.\n+\n [[OUTPUT]]\n OUTPUT\n ------\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex fe853aa8f91..62c6d43cdb9 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -3,6 +3,7 @@\n #include \"tree-walk.h\"\n #include \"xdiff-interface.h\"\n #include \"help.h\"\n+#include \"commit.h\"\n #include \"commit-reach.h\"\n #include \"merge-ort.h\"\n #include \"object-store.h\"\n@@ -406,6 +407,7 @@ struct merge_tree_options {\n };\n \n static int real_merge(struct merge_tree_options *o,\n+\t\t const char *merge_base,\n \t\t const char *branch1, const char *branch2,\n \t\t const char *prefix)\n {\n@@ -432,16 +434,31 @@ static int real_merge(struct merge_tree_options *o,\n \topt.branch1 = branch1;\n \topt.branch2 = branch2;\n \n-\t/*\n-\t * Get the merge bases, in reverse order; see comment above\n-\t * merge_incore_recursive in merge-ort.h\n-\t */\n-\tmerge_bases = get_merge_bases(parent1, parent2);\n-\tif (!merge_bases && !o->allow_unrelated_histories)\n-\t\tdie(_(\"refusing to merge unrelated histories\"));\n-\tmerge_bases = reverse_commit_list(merge_bases);\n+\tif (merge_base) {\n+\t\tstruct commit *base_commit;\n+\t\tstruct tree *base_tree, *parent1_tree, *parent2_tree;\n+\n+\t\tbase_commit = lookup_commit_reference_by_name(merge_base);\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\tbase_tree = get_commit_tree(base_commit);\n+\t\tparent1_tree = get_commit_tree(parent1);\n+\t\tparent2_tree = get_commit_tree(parent2);\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+\t\t * merge_incore_recursive in merge-ort.h\n+\t\t */\n+\t\tmerge_bases = get_merge_bases(parent1, parent2);\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}\n \n-\tmerge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);\n \tif (result.clean < 0)\n \t\tdie(_(\"failure to merge\"));\n \n@@ -487,6 +504,7 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \tstruct merge_tree_options o = { .show_messages = -1 };\n \tint expected_remaining_argc;\n \tint original_argc;\n+\tconst char *merge_base = NULL;\n \n \tconst char * const merge_tree_usage[] = {\n \t\tN_(\"git merge-tree [--write-tree] [] \"),\n@@ -515,6 +533,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\t\t &o.use_stdin,\n \t\t\t N_(\"perform multiple merges, one per line of input\"),\n \t\t\t PARSE_OPT_NONEG),\n+\t\tOPT_STRING(0, \"merge-base\",\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_END()\n \t};\n \n@@ -529,6 +551,8 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \n \t\tif (o.mode == MODE_TRIVIAL)\n \t\t\tdie(_(\"--trivial-merge is incompatible with all other options\"));\n+\t\tif (merge_base)\n+\t\t\tdie(_(\"--merge-base is incompatible with --stdin\"));\n \t\tline_termination = '\\0';\n \t\twhile (strbuf_getline_lf(&buf, stdin) != EOF) {\n \t\t\tstruct strbuf **split;\n@@ -538,7 +562,7 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\t\tif (!split[0] || !split[1] || split[2])\n \t\t\t\tdie(_(\"malformed input line: '%s'.\"), buf.buf);\n \t\t\tstrbuf_rtrim(split[0]);\n-\t\t\tresult = real_merge(&o, split[0]->buf, split[1]->buf, prefix);\n+\t\t\tresult = real_merge(&o, merge_base, split[0]->buf, split[1]->buf, prefix);\n \t\t\tif (result < 0)\n \t\t\t\tdie(_(\"merging cannot continue; got unclean result of %d\"), result);\n \t\t\tstrbuf_list_free(split);\n@@ -581,7 +605,7 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \n \t/* Do the relevant type of merge */\n \tif (o.mode == MODE_REAL)\n-\t\treturn real_merge(&o, argv[0], argv[1], prefix);\n+\t\treturn real_merge(&o, merge_base, argv[0], argv[1], prefix);\n \telse\n \t\treturn trivial_merge(argv[0], argv[1], argv[2]);\n }\ndiff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\nindex cac85591b52..5b0073d3dcd 100755\n--- a/t/t4301-merge-tree-write-tree.sh\n+++ b/t/t4301-merge-tree-write-tree.sh\n@@ -860,4 +860,34 @@ test_expect_success '--stdin with both a successful and a conflicted merge' '\n \ttest_cmp expect actual\n '\n \n+# specify merge-base as parent of branch2\n+# git merge-tree --write-tree --merge-base=c2 c1 c3\n+# Commit c1: add file1\n+# Commit c2: add file2 after c1\n+# Commit c3: add file3 after c2\n+# Expected: add file3, and file2 does NOT appear\n+\n+test_expect_success 'specify merge-base as parent of branch2' '\n+\t# Setup\n+\tgit init base-b2-p && (\n+\t\tcd base-b2-p &&\n+\t\ttest_commit c1 file1 &&\n+\t\ttest_commit c2 file2 &&\n+\t\ttest_commit c3 file3\n+\t) &&\n+\t# Testing\n+\t(\n+\t\tcd base-b2-p &&\n+\t\tTREE_OID=$(git merge-tree --write-tree --merge-base=c2 c1 c3) &&\n+\n+\t\tq_to_tab <<-EOF >expect &&\n+\t\t100644 blob $(git rev-parse c1:file1)Qfile1\n+\t\t100644 blob $(git rev-parse c3:file3)Qfile3\n+\t\tEOF\n+\n+\t\tgit ls-tree $TREE_OID >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"466375","messageId":"3bdfad03ccade6527bbe74b62ef166df3fcdc13d.1667446182.git.gitgitgadget@gmail.com","threadId":"58703","inReplyTo":"pull.1397.v5.git.1667446182.gitgitgadget@gmail.com","subject":"[PATCH v5 2/2] merge-tree.c: allow specifying the merge-base when --stdin is passed","fromName":"Kyle Zhao via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-11-03T03:29:42Z","receivedAt":"2022-11-03T03:30:06Z","isPatch":true,"sender":{"key":"goldofzky@gmail.com","avatar":"https://gravatar.com/avatar/aa4fa27f0c1bed9c986ed596415799518e03a6e71a0931308a4de87e166e183a?d=mp&s=160"},"body":"From: Kyle Zhao \n\nThe previous commit added a `--merge-base` option in order to allow\nusing a specified merge-base for the merge. Extend the input accepted\nby `--stdin` to also allow a specified merge-base with each merge\nrequested. For example:\n\n printf \" -- \" | git merge-tree --stdin\n\ndoes a merge of b1 and b2, and uses b3 as the merge-base.\n\nSigned-off-by: Kyle Zhao \n---\n Documentation/git-merge-tree.txt | 14 +++++++++++++-\n builtin/merge-tree.c | 21 +++++++++++++++++++--\n t/t4301-merge-tree-write-tree.sh | 25 +++++++++++++++++++++++++\n 3 files changed, 57 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\nindex d9dacb2ce54..298c133fdb6 100644\n--- a/Documentation/git-merge-tree.txt\n+++ b/Documentation/git-merge-tree.txt\n@@ -66,7 +66,8 @@ OPTIONS\n \n --merge-base=::\n \tInstead of finding the merge-bases for and ,\n-\tspecify a merge-base for the merge.\n+\tspecify a merge-base for the merge. This option is incompatible\n+\twith `--stdin`.\n \n [[OUTPUT]]\n OUTPUT\n@@ -220,6 +221,17 @@ with linkgit:git-merge[1]:\n * any messages that would have been printed to stdout (the\n <>)\n \n+INPUT FORMAT\n+------------\n+'git merge-tree --stdin' input format is fully text based. Each line\n+has this format:\n+\n+\t[ -- ] \n+\n+If one line is separated by `--`, the string before the separator is\n+used for specifying a merge-base for the merge and the string after\n+the separator describes the branches to be merged.\n+\n MISTAKES TO AVOID\n -----------------\n \ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex 62c6d43cdb9..330f779e8bc 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -557,12 +557,29 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\twhile (strbuf_getline_lf(&buf, stdin) != EOF) {\n \t\t\tstruct strbuf **split;\n \t\t\tint result;\n+\t\t\tconst char *input_merge_base = NULL;\n \n \t\t\tsplit = strbuf_split(&buf, ' ');\n-\t\t\tif (!split[0] || !split[1] || split[2])\n+\t\t\tif (!split[0] || !split[1])\n \t\t\t\tdie(_(\"malformed input line: '%s'.\"), buf.buf);\n \t\t\tstrbuf_rtrim(split[0]);\n-\t\t\tresult = real_merge(&o, merge_base, split[0]->buf, split[1]->buf, prefix);\n+\t\t\tstrbuf_rtrim(split[1]);\n+\n+\t\t\t/* parse the merge-base */\n+\t\t\tif (!strcmp(split[1]->buf, \"--\")) {\n+\t\t\t\tinput_merge_base = split[0]->buf;\n+\t\t\t}\n+\n+\t\t\tif (input_merge_base && split[2] && split[3] && !split[4]) {\n+\t\t\t\tstrbuf_rtrim(split[2]);\n+\t\t\t\tstrbuf_rtrim(split[3]);\n+\t\t\t\tresult = real_merge(&o, input_merge_base, split[2]->buf, split[3]->buf, prefix);\n+\t\t\t} else if (!input_merge_base && !split[2]) {\n+\t\t\t\tresult = real_merge(&o, NULL, split[0]->buf, split[1]->buf, prefix);\n+\t\t\t} else {\n+\t\t\t\tdie(_(\"malformed input line: '%s'.\"), buf.buf);\n+\t\t\t}\n+\n \t\t\tif (result < 0)\n \t\t\t\tdie(_(\"merging cannot continue; got unclean result of %d\"), result);\n \t\t\tstrbuf_list_free(split);\ndiff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\nindex 5b0073d3dcd..03bb1cb8685 100755\n--- a/t/t4301-merge-tree-write-tree.sh\n+++ b/t/t4301-merge-tree-write-tree.sh\n@@ -860,6 +860,13 @@ test_expect_success '--stdin with both a successful and a conflicted merge' '\n \ttest_cmp expect actual\n '\n \n+\n+test_expect_success '--merge-base is incompatible with --stdin' '\n+\ttest_must_fail git merge-tree --merge-base=side1 --stdin 2>expect &&\n+\n+\tgrep \"^fatal: --merge-base is incompatible with --stdin\" expect\n+'\n+\n # specify merge-base as parent of branch2\n # git merge-tree --write-tree --merge-base=c2 c1 c3\n # Commit c1: add file1\n@@ -890,4 +897,22 @@ test_expect_success 'specify merge-base as parent of branch2' '\n \t)\n '\n \n+# Since the earlier tests have verified that individual merge-tree calls\n+# are doing the right thing, this test case is only used to test whether\n+# the input format is available.\n+\n+test_expect_success '--stdin with both a normal merge and a merge-base specified merge' '\n+\tcd base-b2-p &&\n+\tprintf \"c1 c3\\nc2 -- c1 c3\" | git merge-tree --stdin >actual &&\n+\n+\tprintf \"1\\0\" >expect &&\n+\tgit merge-tree --write-tree -z c1 c3 >>expect &&\n+\tprintf \"\\0\" >>expect &&\n+\n+\tprintf \"1\\0\" >>expect &&\n+\tgit merge-tree --write-tree -z --merge-base=c2 c1 c3 >>expect &&\n+\tprintf \"\\0\" >>expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \ngitgitgadget\n"},{"id":"466376","messageId":"3cb3cd03860349b5b38b3ca3b3ce4a0b@tencent.com","threadId":"58703","inReplyTo":"CABPp-BG4in6hPnniHoE+au0XyVXwgs9pNUJdpLDTmP=dxvKjqw@mail.gmail.com","subject":"RE: [Internet]Re: [PATCH v4 2/2] merge-tree.c: support --merge-base in conjunction with --stdin","fromName":"kylezhao (赵柯宇)","fromEmail":"kylezhao@tencent.com","sentAt":"2022-11-03T03:28:49Z","receivedAt":"2022-11-03T03:35:22Z","isPatch":true,"sender":{"key":"kylezhao@tencent.com","avatar":"https://avatars.githubusercontent.com/u/34176272?v=4"},"body":"Thanks for your review.\n\n>>\n>> The previous change add \"--merge-base\" option in order to allow users \n>> to\n>\n> s/add/added/ ?\n\nDone.\n\n\n> Perhaps something like:\n> \n> \"\"\"\n> The previous commit added a `--merge-base` option in order to allow using a specified merge-base for the merge. Extend the input accepted by `--stdin` to also allow a specified merge-base with each merge requested. For example:\n> \n > printf \"--merge-base= \" | git merge-tree --stdin\n>\n\nDone.\n\n\n> However, I'm a bit curious about using `--merge-base=` on the input line as opposed to just using a simpler marker; something like\n> \n> printf \" -- \" | git merge-tree --stdin\n> \n> (which follows a precedent set by git-merge-recursive). Your version is a bit more self-documenting, but what if we want to allow users to specify multiple merge bases in the future (for use in passing to merge_incore_recursive())? Is it annoying to need to prefix each one?\n\nActually, I thought about it at the time, but didn't think of any good input format. Your idea is great, I think I'll use it.\n\n> I'm not sure \"passed into the input line\" will be clear to readers.\n> Perhaps we want to have --stdin documented (looks like I forgot to do that in my series; oops), mentioning the input format.\n\nI just added the documentation for --input, please see what else needs to be improved.\n\n> > + printf \"1\\0\" >>expect &&\n> > + git merge-tree --write-tree -z --merge-base=c2 c1 c3 >>expect &&\n> > + printf \"\\0\" >>expect &&\n> > + test_cmp expect actual\n> > +'\n>\n> This last test seems odd. You are merely testing that the output of \"git merge-tree --stdin\" matches the output of repeated calls to \"git merge-tree\", not that the merges involved produce correct results?\n> Maybe that's fine since earlier tests verify that individual merge-tree calls are doing the right thing, I was just a bit surprised. Maybe a comment in the code that this was your intent would be helpful?\n\nYeah, I have added the comment.\n"},{"id":"466379","messageId":"221103.868rks301x.gmgdl@evledraar.gmail.com","threadId":"58703","inReplyTo":"01df0d1a6a7319beada3a3a94b8e36818be2a4d2.1667446182.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v5 1/2] merge-tree.c: add --merge-base= option","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-03T08:36:38Z","receivedAt":"2022-11-03T08:38:31Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Nov 03 2022, Kyle Zhao via GitGitGadget wrote:\n\n> From: Kyle Zhao \n\n> [...]\n> +test_expect_success 'specify merge-base as parent of branch2' '\n> +\t# Setup\n> +\tgit init base-b2-p && (\n\nIt's good practice to nuke it after the test:\n\n\ttest_when_finished \"rm -rf base-b2-p\" &&\n\tgit init base-b2-p\n\n\n\n> +\t\tcd base-b2-p &&\n> +\t\ttest_commit c1 file1 &&\n> +\t\ttest_commit c2 file2 &&\n> +\t\ttest_commit c3 file3\n> +\t) &&\n\nCould avoid the subshell-ing with:\n\n\ttest_commit -C base-b2-p c1 file1 &&\n\t[...]\n\n\t\n\n> +\t# Testing\n> +\t(\n> +\t\tcd base-b2-p &&\n> +\t\tTREE_OID=$(git merge-tree --write-tree --merge-base=c2 c1 c3) &&\n\nDitto sub-shell.\n\n> +\n> +\t\tq_to_tab <<-EOF >expect &&\n> +\t\t100644 blob $(git rev-parse c1:file1)Qfile1\n> +\t\t100644 blob $(git rev-parse c3:file3)Qfile3\n> +\t\tEOF\n> +\n> +\t\tgit ls-tree $TREE_OID >actual &&\n> +\t\ttest_cmp expect actual\n> +\t)\n> +'\n> +\n> test_done\n\n"},{"id":"466391","messageId":"11e8532a5ba746e2b25e689c5d1230cf@tencent.com","threadId":"58703","inReplyTo":"221103.868rks301x.gmgdl@evledraar.gmail.com","subject":"RE: [Internet]Re: [PATCH v5 1/2] merge-tree.c: add --merge-base= option","fromName":"kylezhao (赵柯宇)","fromEmail":"kylezhao@tencent.com","sentAt":"2022-11-03T09:43:49Z","receivedAt":"2022-11-03T09:44:11Z","isPatch":true,"sender":{"key":"kylezhao@tencent.com","avatar":"https://avatars.githubusercontent.com/u/34176272?v=4"},"body":"> \n> > [...]\n> > +test_expect_success 'specify merge-base as parent of branch2' '\n> > +\t# Setup\n> > +\tgit init base-b2-p && (\n> \n> It's good practice to nuke it after the test:\n> \n> \ttest_when_finished \"rm -rf base-b2-p\" &&\n> \tgit init base-b2-p\n> \n> \n\nThanks for the reviews, I'll fix it in the next patch.\n\n> \n> Could avoid the subshell-ing with:\n> \n> \ttest_commit -C base-b2-p c1 file1 &&\n> \t[...]\n\nNice.\n\n\n"},{"id":"466396","messageId":"pull.1397.v6.git.1667472621.gitgitgadget@gmail.com","threadId":"58703","inReplyTo":"pull.1397.v5.git.1667446182.gitgitgadget@gmail.com","subject":"[PATCH v6 0/2] merge-tree: allow specifying a base commit when --write-tree is passed","fromName":"Kyle Zhao via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-11-03T10:50:19Z","receivedAt":"2022-11-03T10:50:30Z","isPatch":true,"sender":{"key":"goldofzky@gmail.com","avatar":"https://gravatar.com/avatar/aa4fa27f0c1bed9c986ed596415799518e03a6e71a0931308a4de87e166e183a?d=mp&s=160"},"body":"Thanks for Elijah's work. I'm very excited that merge-ort is integrated into\nthe git merge-tree, which means that we can use merge-ort in bare\nrepositories to optimize merge performance.\n\nIn this patch, I introduce a new --merge-base= option to allow\ncallers to specify a merge-base for the merge. This may allow users to\nimplement git cherry-pick and git rebase in bare repositories with git\nmerge-tree cmd.\n\nChanges since v1:\n\n * Changed merge_incore_recursive() to merge_incore_nonrecursive() when\n merge-base is specified.\n * Fixed c style problem.\n * Moved commit lookup/die logic out to the parsing logic in\n cmd_merge_tree().\n * use test_commit for test\n\nChanges since v2:\n\n * commit message\n * Rebased on top of en/merge-tree-sequence.\n * Set opt.ancestor to o->merge_base. Because opt.ancestor is a *char. To\n make it easier to pass parameters, I moved\n lookup_commit_reference_by_name() to real_ merge() again.\n * Added test comment.\n\nChanges since v3:\n\n * support --merge-base in conjunction with --stdin\n\nChanges since v4:\n\n * commit message\n * added input format document\n * changed the input format for specifying the merge-base when --stdin is\n passed\n * changed the output when --stdin and --merge-base are used at the same\n time\n * add comment for test\n\nChanges since v5:\n\n * improved test: remove the test repo after the test; avoid sub-shell.\n\nKyle Zhao (2):\n merge-tree.c: add --merge-base= option\n merge-tree.c: allow specifying the merge-base when --stdin is passed\n\n Documentation/git-merge-tree.txt | 16 ++++++++\n builtin/merge-tree.c | 65 ++++++++++++++++++++++++++------\n t/t4301-merge-tree-write-tree.sh | 61 ++++++++++++++++++++++++++++++\n 3 files changed, 130 insertions(+), 12 deletions(-)\n\n\nbase-commit: ec1edbcb56ac05e9980299b05924c5c1b51d68b4\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1397%2Fkeyu98%2Fkz%2Fmerge-tree-option-merge-base-v6\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1397/keyu98/kz/merge-tree-option-merge-base-v6\nPull-Request: https://github.com/gitgitgadget/git/pull/1397\n\nRange-diff vs v5:\n\n 1: 01df0d1a6a7 ! 1: 1cf1c69b8e8 merge-tree.c: add --merge-base= option\n @@ t/t4301-merge-tree-write-tree.sh: test_expect_success '--stdin with both a succe\n +\n +test_expect_success 'specify merge-base as parent of branch2' '\n +\t# Setup\n -+\tgit init base-b2-p && (\n -+\t\tcd base-b2-p &&\n -+\t\ttest_commit c1 file1 &&\n -+\t\ttest_commit c2 file2 &&\n -+\t\ttest_commit c3 file3\n -+\t) &&\n ++\ttest_when_finished \"rm -rf base-b2-p\" &&\n ++\tgit init base-b2-p &&\n ++\ttest_commit -C base-b2-p c1 file1 &&\n ++\ttest_commit -C base-b2-p c2 file2 &&\n ++\ttest_commit -C base-b2-p c3 file3 &&\n ++\n +\t# Testing\n -+\t(\n -+\t\tcd base-b2-p &&\n -+\t\tTREE_OID=$(git merge-tree --write-tree --merge-base=c2 c1 c3) &&\n ++\tTREE_OID=$(git -C base-b2-p merge-tree --write-tree --merge-base=c2 c1 c3) &&\n +\n -+\t\tq_to_tab <<-EOF >expect &&\n -+\t\t100644 blob $(git rev-parse c1:file1)Qfile1\n -+\t\t100644 blob $(git rev-parse c3:file3)Qfile3\n -+\t\tEOF\n ++\tq_to_tab <<-EOF >expect &&\n ++\t100644 blob $(git -C base-b2-p rev-parse c1:file1)Qfile1\n ++\t100644 blob $(git -C base-b2-p rev-parse c3:file3)Qfile3\n ++\tEOF\n +\n -+\t\tgit ls-tree $TREE_OID >actual &&\n -+\t\ttest_cmp expect actual\n -+\t)\n ++\tgit -C base-b2-p ls-tree $TREE_OID >actual &&\n ++\ttest_cmp expect actual\n +'\n +\n test_done\n 2: 3bdfad03cca ! 2: 40d56544e6e merge-tree.c: allow specifying the merge-base when --stdin is passed\n @@ t/t4301-merge-tree-write-tree.sh: test_expect_success '--stdin with both a succe\n # git merge-tree --write-tree --merge-base=c2 c1 c3\n # Commit c1: add file1\n @@ t/t4301-merge-tree-write-tree.sh: test_expect_success 'specify merge-base as parent of branch2' '\n - \t)\n + \ttest_cmp expect actual\n '\n \n +# Since the earlier tests have verified that individual merge-tree calls\n +# are doing the right thing, this test case is only used to test whether\n +# the input format is available.\n +\n -+test_expect_success '--stdin with both a normal merge and a merge-base specified merge' '\n -+\tcd base-b2-p &&\n -+\tprintf \"c1 c3\\nc2 -- c1 c3\" | git merge-tree --stdin >actual &&\n ++test_expect_success 'check the input format when --stdin is passed' '\n ++\ttest_when_finished \"rm -rf repo\" &&\n ++\tgit init repo &&\n ++\ttest_commit -C repo c1 &&\n ++\ttest_commit -C repo c2 &&\n ++\ttest_commit -C repo c3 &&\n ++\tprintf \"c1 c3\\nc2 -- c1 c3\\nc2 c3\" | git -C repo merge-tree --stdin >actual &&\n +\n +\tprintf \"1\\0\" >expect &&\n -+\tgit merge-tree --write-tree -z c1 c3 >>expect &&\n ++\tgit -C repo merge-tree --write-tree -z c1 c3 >>expect &&\n +\tprintf \"\\0\" >>expect &&\n +\n +\tprintf \"1\\0\" >>expect &&\n -+\tgit merge-tree --write-tree -z --merge-base=c2 c1 c3 >>expect &&\n ++\tgit -C repo merge-tree --write-tree -z --merge-base=c2 c1 c3 >>expect &&\n +\tprintf \"\\0\" >>expect &&\n ++\n ++\tprintf \"1\\0\" >>expect &&\n ++\tgit -C repo merge-tree --write-tree -z c2 c3 >>expect &&\n ++\tprintf \"\\0\" >>expect &&\n ++\n +\ttest_cmp expect actual\n +'\n +\n\n-- \ngitgitgadget\n"},{"id":"466397","messageId":"1cf1c69b8e8e8e81eccc42b5d8efc605a36ab7eb.1667472621.git.gitgitgadget@gmail.com","threadId":"58703","inReplyTo":"pull.1397.v6.git.1667472621.gitgitgadget@gmail.com","subject":"[PATCH v6 1/2] merge-tree.c: add --merge-base= option","fromName":"Kyle Zhao via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-11-03T10:50:20Z","receivedAt":"2022-11-03T10:50:34Z","isPatch":true,"sender":{"key":"goldofzky@gmail.com","avatar":"https://gravatar.com/avatar/aa4fa27f0c1bed9c986ed596415799518e03a6e71a0931308a4de87e166e183a?d=mp&s=160"},"body":"From: Kyle Zhao \n\nThis patch will give our callers more flexibility to use `git merge-tree`,\nsuch as:\n\n git merge-tree --write-tree --merge-base=branch^ HEAD branch\n\nThis does a merge of HEAD and branch, but uses branch^ as the merge-base.\n\nAnd the reason why using an option flag instead of a positional argument\nis to allow additional commits passed to merge-tree to be handled via an\noctopus merge in the future.\n\nSigned-off-by: Kyle Zhao \n---\n Documentation/git-merge-tree.txt | 4 +++\n builtin/merge-tree.c | 46 ++++++++++++++++++++++++--------\n t/t4301-merge-tree-write-tree.sh | 27 +++++++++++++++++++\n 3 files changed, 66 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\nindex 04bcc416e6e..d9dacb2ce54 100644\n--- a/Documentation/git-merge-tree.txt\n+++ b/Documentation/git-merge-tree.txt\n@@ -64,6 +64,10 @@ OPTIONS\n \tshare no common history. This flag can be given to override that\n \tcheck and make the merge proceed anyway.\n \n+--merge-base=::\n+\tInstead of finding the merge-bases for and ,\n+\tspecify a merge-base for the merge.\n+\n [[OUTPUT]]\n OUTPUT\n ------\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex fe853aa8f91..62c6d43cdb9 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -3,6 +3,7 @@\n #include \"tree-walk.h\"\n #include \"xdiff-interface.h\"\n #include \"help.h\"\n+#include \"commit.h\"\n #include \"commit-reach.h\"\n #include \"merge-ort.h\"\n #include \"object-store.h\"\n@@ -406,6 +407,7 @@ struct merge_tree_options {\n };\n \n static int real_merge(struct merge_tree_options *o,\n+\t\t const char *merge_base,\n \t\t const char *branch1, const char *branch2,\n \t\t const char *prefix)\n {\n@@ -432,16 +434,31 @@ static int real_merge(struct merge_tree_options *o,\n \topt.branch1 = branch1;\n \topt.branch2 = branch2;\n \n-\t/*\n-\t * Get the merge bases, in reverse order; see comment above\n-\t * merge_incore_recursive in merge-ort.h\n-\t */\n-\tmerge_bases = get_merge_bases(parent1, parent2);\n-\tif (!merge_bases && !o->allow_unrelated_histories)\n-\t\tdie(_(\"refusing to merge unrelated histories\"));\n-\tmerge_bases = reverse_commit_list(merge_bases);\n+\tif (merge_base) {\n+\t\tstruct commit *base_commit;\n+\t\tstruct tree *base_tree, *parent1_tree, *parent2_tree;\n+\n+\t\tbase_commit = lookup_commit_reference_by_name(merge_base);\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\tbase_tree = get_commit_tree(base_commit);\n+\t\tparent1_tree = get_commit_tree(parent1);\n+\t\tparent2_tree = get_commit_tree(parent2);\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+\t\t * merge_incore_recursive in merge-ort.h\n+\t\t */\n+\t\tmerge_bases = get_merge_bases(parent1, parent2);\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}\n \n-\tmerge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);\n \tif (result.clean < 0)\n \t\tdie(_(\"failure to merge\"));\n \n@@ -487,6 +504,7 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \tstruct merge_tree_options o = { .show_messages = -1 };\n \tint expected_remaining_argc;\n \tint original_argc;\n+\tconst char *merge_base = NULL;\n \n \tconst char * const merge_tree_usage[] = {\n \t\tN_(\"git merge-tree [--write-tree] [] \"),\n@@ -515,6 +533,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\t\t &o.use_stdin,\n \t\t\t N_(\"perform multiple merges, one per line of input\"),\n \t\t\t PARSE_OPT_NONEG),\n+\t\tOPT_STRING(0, \"merge-base\",\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_END()\n \t};\n \n@@ -529,6 +551,8 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \n \t\tif (o.mode == MODE_TRIVIAL)\n \t\t\tdie(_(\"--trivial-merge is incompatible with all other options\"));\n+\t\tif (merge_base)\n+\t\t\tdie(_(\"--merge-base is incompatible with --stdin\"));\n \t\tline_termination = '\\0';\n \t\twhile (strbuf_getline_lf(&buf, stdin) != EOF) {\n \t\t\tstruct strbuf **split;\n@@ -538,7 +562,7 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\t\tif (!split[0] || !split[1] || split[2])\n \t\t\t\tdie(_(\"malformed input line: '%s'.\"), buf.buf);\n \t\t\tstrbuf_rtrim(split[0]);\n-\t\t\tresult = real_merge(&o, split[0]->buf, split[1]->buf, prefix);\n+\t\t\tresult = real_merge(&o, merge_base, split[0]->buf, split[1]->buf, prefix);\n \t\t\tif (result < 0)\n \t\t\t\tdie(_(\"merging cannot continue; got unclean result of %d\"), result);\n \t\t\tstrbuf_list_free(split);\n@@ -581,7 +605,7 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \n \t/* Do the relevant type of merge */\n \tif (o.mode == MODE_REAL)\n-\t\treturn real_merge(&o, argv[0], argv[1], prefix);\n+\t\treturn real_merge(&o, merge_base, argv[0], argv[1], prefix);\n \telse\n \t\treturn trivial_merge(argv[0], argv[1], argv[2]);\n }\ndiff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\nindex cac85591b52..6db96ccbaae 100755\n--- a/t/t4301-merge-tree-write-tree.sh\n+++ b/t/t4301-merge-tree-write-tree.sh\n@@ -860,4 +860,31 @@ test_expect_success '--stdin with both a successful and a conflicted merge' '\n \ttest_cmp expect actual\n '\n \n+# specify merge-base as parent of branch2\n+# git merge-tree --write-tree --merge-base=c2 c1 c3\n+# Commit c1: add file1\n+# Commit c2: add file2 after c1\n+# Commit c3: add file3 after c2\n+# Expected: add file3, and file2 does NOT appear\n+\n+test_expect_success 'specify merge-base as parent of branch2' '\n+\t# Setup\n+\ttest_when_finished \"rm -rf base-b2-p\" &&\n+\tgit init base-b2-p &&\n+\ttest_commit -C base-b2-p c1 file1 &&\n+\ttest_commit -C base-b2-p c2 file2 &&\n+\ttest_commit -C base-b2-p c3 file3 &&\n+\n+\t# Testing\n+\tTREE_OID=$(git -C base-b2-p merge-tree --write-tree --merge-base=c2 c1 c3) &&\n+\n+\tq_to_tab <<-EOF >expect &&\n+\t100644 blob $(git -C base-b2-p rev-parse c1:file1)Qfile1\n+\t100644 blob $(git -C base-b2-p rev-parse c3:file3)Qfile3\n+\tEOF\n+\n+\tgit -C base-b2-p ls-tree $TREE_OID >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"466398","messageId":"40d56544e6e319605d02bab743a6e957ff0a5926.1667472621.git.gitgitgadget@gmail.com","threadId":"58703","inReplyTo":"pull.1397.v6.git.1667472621.gitgitgadget@gmail.com","subject":"[PATCH v6 2/2] merge-tree.c: allow specifying the merge-base when --stdin is passed","fromName":"Kyle Zhao via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-11-03T10:50:21Z","receivedAt":"2022-11-03T10:50:36Z","isPatch":true,"sender":{"key":"goldofzky@gmail.com","avatar":"https://gravatar.com/avatar/aa4fa27f0c1bed9c986ed596415799518e03a6e71a0931308a4de87e166e183a?d=mp&s=160"},"body":"From: Kyle Zhao \n\nThe previous commit added a `--merge-base` option in order to allow\nusing a specified merge-base for the merge. Extend the input accepted\nby `--stdin` to also allow a specified merge-base with each merge\nrequested. For example:\n\n printf \" -- \" | git merge-tree --stdin\n\ndoes a merge of b1 and b2, and uses b3 as the merge-base.\n\nSigned-off-by: Kyle Zhao \n---\n Documentation/git-merge-tree.txt | 14 ++++++++++++-\n builtin/merge-tree.c | 21 ++++++++++++++++++--\n t/t4301-merge-tree-write-tree.sh | 34 ++++++++++++++++++++++++++++++++\n 3 files changed, 66 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\nindex d9dacb2ce54..298c133fdb6 100644\n--- a/Documentation/git-merge-tree.txt\n+++ b/Documentation/git-merge-tree.txt\n@@ -66,7 +66,8 @@ OPTIONS\n \n --merge-base=::\n \tInstead of finding the merge-bases for and ,\n-\tspecify a merge-base for the merge.\n+\tspecify a merge-base for the merge. This option is incompatible\n+\twith `--stdin`.\n \n [[OUTPUT]]\n OUTPUT\n@@ -220,6 +221,17 @@ with linkgit:git-merge[1]:\n * any messages that would have been printed to stdout (the\n <>)\n \n+INPUT FORMAT\n+------------\n+'git merge-tree --stdin' input format is fully text based. Each line\n+has this format:\n+\n+\t[ -- ] \n+\n+If one line is separated by `--`, the string before the separator is\n+used for specifying a merge-base for the merge and the string after\n+the separator describes the branches to be merged.\n+\n MISTAKES TO AVOID\n -----------------\n \ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex 62c6d43cdb9..330f779e8bc 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -557,12 +557,29 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\twhile (strbuf_getline_lf(&buf, stdin) != EOF) {\n \t\t\tstruct strbuf **split;\n \t\t\tint result;\n+\t\t\tconst char *input_merge_base = NULL;\n \n \t\t\tsplit = strbuf_split(&buf, ' ');\n-\t\t\tif (!split[0] || !split[1] || split[2])\n+\t\t\tif (!split[0] || !split[1])\n \t\t\t\tdie(_(\"malformed input line: '%s'.\"), buf.buf);\n \t\t\tstrbuf_rtrim(split[0]);\n-\t\t\tresult = real_merge(&o, merge_base, split[0]->buf, split[1]->buf, prefix);\n+\t\t\tstrbuf_rtrim(split[1]);\n+\n+\t\t\t/* parse the merge-base */\n+\t\t\tif (!strcmp(split[1]->buf, \"--\")) {\n+\t\t\t\tinput_merge_base = split[0]->buf;\n+\t\t\t}\n+\n+\t\t\tif (input_merge_base && split[2] && split[3] && !split[4]) {\n+\t\t\t\tstrbuf_rtrim(split[2]);\n+\t\t\t\tstrbuf_rtrim(split[3]);\n+\t\t\t\tresult = real_merge(&o, input_merge_base, split[2]->buf, split[3]->buf, prefix);\n+\t\t\t} else if (!input_merge_base && !split[2]) {\n+\t\t\t\tresult = real_merge(&o, NULL, split[0]->buf, split[1]->buf, prefix);\n+\t\t\t} else {\n+\t\t\t\tdie(_(\"malformed input line: '%s'.\"), buf.buf);\n+\t\t\t}\n+\n \t\t\tif (result < 0)\n \t\t\t\tdie(_(\"merging cannot continue; got unclean result of %d\"), result);\n \t\t\tstrbuf_list_free(split);\ndiff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\nindex 6db96ccbaae..22b43f0003e 100755\n--- a/t/t4301-merge-tree-write-tree.sh\n+++ b/t/t4301-merge-tree-write-tree.sh\n@@ -860,6 +860,13 @@ test_expect_success '--stdin with both a successful and a conflicted merge' '\n \ttest_cmp expect actual\n '\n \n+\n+test_expect_success '--merge-base is incompatible with --stdin' '\n+\ttest_must_fail git merge-tree --merge-base=side1 --stdin 2>expect &&\n+\n+\tgrep \"^fatal: --merge-base is incompatible with --stdin\" expect\n+'\n+\n # specify merge-base as parent of branch2\n # git merge-tree --write-tree --merge-base=c2 c1 c3\n # Commit c1: add file1\n@@ -887,4 +894,31 @@ test_expect_success 'specify merge-base as parent of branch2' '\n \ttest_cmp expect actual\n '\n \n+# Since the earlier tests have verified that individual merge-tree calls\n+# are doing the right thing, this test case is only used to test whether\n+# the input format is available.\n+\n+test_expect_success 'check the input format when --stdin is passed' '\n+\ttest_when_finished \"rm -rf repo\" &&\n+\tgit init repo &&\n+\ttest_commit -C repo c1 &&\n+\ttest_commit -C repo c2 &&\n+\ttest_commit -C repo c3 &&\n+\tprintf \"c1 c3\\nc2 -- c1 c3\\nc2 c3\" | git -C repo merge-tree --stdin >actual &&\n+\n+\tprintf \"1\\0\" >expect &&\n+\tgit -C repo merge-tree --write-tree -z c1 c3 >>expect &&\n+\tprintf \"\\0\" >>expect &&\n+\n+\tprintf \"1\\0\" >>expect &&\n+\tgit -C repo merge-tree --write-tree -z --merge-base=c2 c1 c3 >>expect &&\n+\tprintf \"\\0\" >>expect &&\n+\n+\tprintf \"1\\0\" >>expect &&\n+\tgit -C repo merge-tree --write-tree -z c2 c3 >>expect &&\n+\tprintf \"\\0\" >>expect &&\n+\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \ngitgitgadget\n"},{"id":"467159","messageId":"CABPp-BFgjT7b5EyVU7P4YpGvGZk9xfV+nDJG2UBtGc2c3VqeFw@mail.gmail.com","threadId":"58703","inReplyTo":"40d56544e6e319605d02bab743a6e957ff0a5926.1667472621.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v6 2/2] merge-tree.c: allow specifying the merge-base when --stdin is passed","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-11-11T19:44:43Z","receivedAt":"2022-11-11T19:45:01Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Sorry for the delay; my Git time has sadly been quite limited. :-(\n\nOn Thu, Nov 3, 2022 at 3:50 AM Kyle Zhao via GitGitGadget\n wrote:\n> +# Since the earlier tests have verified that individual merge-tree calls\n> +# are doing the right thing, this test case is only used to test whether\n> +# the input format is available.\n\n\"the input format is available\"? I'm not sure exactly what that\nmeans, but it seems almost certainly to not be the only thing it is\ntesting. Perhaps you meant something like:\n\n# Since the earlier tests have verified that individual merge-tree calls\n# are doing the right thing, this test case is only used to verify that\n# we can also trigger merges via --stdin, and that when we do we get\n# the same answer as running a bunch of separate merges.\n\n> +\n> +test_expect_success 'check the input format when --stdin is passed' '\n> + test_when_finished \"rm -rf repo\" &&\n> + git init repo &&\n> + test_commit -C repo c1 &&\n> + test_commit -C repo c2 &&\n> + test_commit -C repo c3 &&\n> + printf \"c1 c3\\nc2 -- c1 c3\\nc2 c3\" | git -C repo merge-tree --stdin >actual &&\n> +\n> + printf \"1\\0\" >expect &&\n> + git -C repo merge-tree --write-tree -z c1 c3 >>expect &&\n> + printf \"\\0\" >>expect &&\n> +\n> + printf \"1\\0\" >>expect &&\n> + git -C repo merge-tree --write-tree -z --merge-base=c2 c1 c3 >>expect &&\n> + printf \"\\0\" >>expect &&\n> +\n> + printf \"1\\0\" >>expect &&\n> + git -C repo merge-tree --write-tree -z c2 c3 >>expect &&\n> + printf \"\\0\" >>expect &&\n> +\n> + test_cmp expect actual\n> +'\n> +\n> test_done\n\nMy above nit on your comment is my only remaining issue with your\nimplementation. Looks good.\n\nAs an aside, I am still a little disappointed that the sole reason for\nthis series is limited to a usecase where this solution is at best an\ninterim hack[1][2]...but since I have had very limited time to work on\nGit stuff including providing a proper solution for that usecase (in\nthe form of git-replay), and since it makes sense to include this\ncapability from a completeness perspective.\n\nAnyway, thanks for patiently fixing everything up. I think this\nseries should be ready to merge down once the comment is fixed up.\n\n[1] https://lore.kernel.org/git/CABPp-BGBFyoY7m+KCA9MTifKmpZ0ccLgsYHahMCgPxuTpuUGPg@mail.gmail.com/\n[2] https://lore.kernel.org/git/CABPp-BGXM=iRAgjNbZ0o3FSjj583GpkuFB6emUYwWjdFWb9-jQ@mail.gmail.com/\n"},{"id":"467179","messageId":"pull.1397.v7.git.1668210314.gitgitgadget@gmail.com","threadId":"58703","inReplyTo":"pull.1397.v6.git.1667472621.gitgitgadget@gmail.com","subject":"[PATCH v7 0/2] merge-tree: allow specifying a base commit when --write-tree is passed","fromName":"Kyle Zhao via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-11-11T23:45:12Z","receivedAt":"2022-11-11T23:45:22Z","isPatch":true,"sender":{"key":"goldofzky@gmail.com","avatar":"https://gravatar.com/avatar/aa4fa27f0c1bed9c986ed596415799518e03a6e71a0931308a4de87e166e183a?d=mp&s=160"},"body":"Thanks for everyone's careful reviews.\n\nIn this patch, I introduce a new --merge-base= option to allow\ncallers to specify a merge-base for the merge and extend the input accepted\nby --stdin to also allow a specified merge-base with each merge requested.\n\nRegards, Kyle\n\nChanges since v1:\n\n * Changed merge_incore_recursive() to merge_incore_nonrecursive() when\n merge-base is specified.\n * Fixed c style problem.\n * Moved commit lookup/die logic out to the parsing logic in\n cmd_merge_tree().\n * use test_commit for test\n\nChanges since v2:\n\n * commit message\n * Rebased on top of en/merge-tree-sequence.\n * Set opt.ancestor to o->merge_base. Because opt.ancestor is a *char. To\n make it easier to pass parameters, I moved\n lookup_commit_reference_by_name() to real_ merge() again.\n * Added test comment.\n\nChanges since v3:\n\n * support --merge-base in conjunction with --stdin\n\nChanges since v4:\n\n * commit message\n * added input format document\n * changed the input format for specifying the merge-base when --stdin is\n passed\n * changed the output when --stdin and --merge-base are used at the same\n time\n * add comment for test\n\nChanges since v5:\n\n * improved test: remove the test repo after the test; avoid sub-shell.\n\nChanges since v6:\n\n * fixed comment of test\n\nKyle Zhao (2):\n merge-tree.c: add --merge-base= option\n merge-tree.c: allow specifying the merge-base when --stdin is passed\n\n Documentation/git-merge-tree.txt | 16 ++++++++\n builtin/merge-tree.c | 65 ++++++++++++++++++++++++++------\n t/t4301-merge-tree-write-tree.sh | 62 ++++++++++++++++++++++++++++++\n 3 files changed, 131 insertions(+), 12 deletions(-)\n\n\nbase-commit: ec1edbcb56ac05e9980299b05924c5c1b51d68b4\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1397%2Fkeyu98%2Fkz%2Fmerge-tree-option-merge-base-v7\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1397/keyu98/kz/merge-tree-option-merge-base-v7\nPull-Request: https://github.com/gitgitgadget/git/pull/1397\n\nRange-diff vs v6:\n\n 1: 1cf1c69b8e8 = 1: 1cf1c69b8e8 merge-tree.c: add --merge-base= option\n 2: 40d56544e6e ! 2: 48e55d4e97c merge-tree.c: allow specifying the merge-base when --stdin is passed\n @@ t/t4301-merge-tree-write-tree.sh: test_expect_success 'specify merge-base as par\n '\n \n +# Since the earlier tests have verified that individual merge-tree calls\n -+# are doing the right thing, this test case is only used to test whether\n -+# the input format is available.\n ++# are doing the right thing, this test case is only used to verify that\n ++# we can also trigger merges via --stdin, and that when we do we get\n ++# the same answer as running a bunch of separate merges.\n +\n +test_expect_success 'check the input format when --stdin is passed' '\n +\ttest_when_finished \"rm -rf repo\" &&\n\n-- \ngitgitgadget\n"},{"id":"467180","messageId":"1cf1c69b8e8e8e81eccc42b5d8efc605a36ab7eb.1668210314.git.gitgitgadget@gmail.com","threadId":"58703","inReplyTo":"pull.1397.v7.git.1668210314.gitgitgadget@gmail.com","subject":"[PATCH v7 1/2] merge-tree.c: add --merge-base= option","fromName":"Kyle Zhao via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-11-11T23:45:13Z","receivedAt":"2022-11-11T23:45:26Z","isPatch":true,"sender":{"key":"goldofzky@gmail.com","avatar":"https://gravatar.com/avatar/aa4fa27f0c1bed9c986ed596415799518e03a6e71a0931308a4de87e166e183a?d=mp&s=160"},"body":"From: Kyle Zhao \n\nThis patch will give our callers more flexibility to use `git merge-tree`,\nsuch as:\n\n git merge-tree --write-tree --merge-base=branch^ HEAD branch\n\nThis does a merge of HEAD and branch, but uses branch^ as the merge-base.\n\nAnd the reason why using an option flag instead of a positional argument\nis to allow additional commits passed to merge-tree to be handled via an\noctopus merge in the future.\n\nSigned-off-by: Kyle Zhao \n---\n Documentation/git-merge-tree.txt | 4 +++\n builtin/merge-tree.c | 46 ++++++++++++++++++++++++--------\n t/t4301-merge-tree-write-tree.sh | 27 +++++++++++++++++++\n 3 files changed, 66 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\nindex 04bcc416e6e..d9dacb2ce54 100644\n--- a/Documentation/git-merge-tree.txt\n+++ b/Documentation/git-merge-tree.txt\n@@ -64,6 +64,10 @@ OPTIONS\n \tshare no common history. This flag can be given to override that\n \tcheck and make the merge proceed anyway.\n \n+--merge-base=::\n+\tInstead of finding the merge-bases for and ,\n+\tspecify a merge-base for the merge.\n+\n [[OUTPUT]]\n OUTPUT\n ------\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex fe853aa8f91..62c6d43cdb9 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -3,6 +3,7 @@\n #include \"tree-walk.h\"\n #include \"xdiff-interface.h\"\n #include \"help.h\"\n+#include \"commit.h\"\n #include \"commit-reach.h\"\n #include \"merge-ort.h\"\n #include \"object-store.h\"\n@@ -406,6 +407,7 @@ struct merge_tree_options {\n };\n \n static int real_merge(struct merge_tree_options *o,\n+\t\t const char *merge_base,\n \t\t const char *branch1, const char *branch2,\n \t\t const char *prefix)\n {\n@@ -432,16 +434,31 @@ static int real_merge(struct merge_tree_options *o,\n \topt.branch1 = branch1;\n \topt.branch2 = branch2;\n \n-\t/*\n-\t * Get the merge bases, in reverse order; see comment above\n-\t * merge_incore_recursive in merge-ort.h\n-\t */\n-\tmerge_bases = get_merge_bases(parent1, parent2);\n-\tif (!merge_bases && !o->allow_unrelated_histories)\n-\t\tdie(_(\"refusing to merge unrelated histories\"));\n-\tmerge_bases = reverse_commit_list(merge_bases);\n+\tif (merge_base) {\n+\t\tstruct commit *base_commit;\n+\t\tstruct tree *base_tree, *parent1_tree, *parent2_tree;\n+\n+\t\tbase_commit = lookup_commit_reference_by_name(merge_base);\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\tbase_tree = get_commit_tree(base_commit);\n+\t\tparent1_tree = get_commit_tree(parent1);\n+\t\tparent2_tree = get_commit_tree(parent2);\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+\t\t * merge_incore_recursive in merge-ort.h\n+\t\t */\n+\t\tmerge_bases = get_merge_bases(parent1, parent2);\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}\n \n-\tmerge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);\n \tif (result.clean < 0)\n \t\tdie(_(\"failure to merge\"));\n \n@@ -487,6 +504,7 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \tstruct merge_tree_options o = { .show_messages = -1 };\n \tint expected_remaining_argc;\n \tint original_argc;\n+\tconst char *merge_base = NULL;\n \n \tconst char * const merge_tree_usage[] = {\n \t\tN_(\"git merge-tree [--write-tree] [] \"),\n@@ -515,6 +533,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\t\t &o.use_stdin,\n \t\t\t N_(\"perform multiple merges, one per line of input\"),\n \t\t\t PARSE_OPT_NONEG),\n+\t\tOPT_STRING(0, \"merge-base\",\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_END()\n \t};\n \n@@ -529,6 +551,8 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \n \t\tif (o.mode == MODE_TRIVIAL)\n \t\t\tdie(_(\"--trivial-merge is incompatible with all other options\"));\n+\t\tif (merge_base)\n+\t\t\tdie(_(\"--merge-base is incompatible with --stdin\"));\n \t\tline_termination = '\\0';\n \t\twhile (strbuf_getline_lf(&buf, stdin) != EOF) {\n \t\t\tstruct strbuf **split;\n@@ -538,7 +562,7 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\t\tif (!split[0] || !split[1] || split[2])\n \t\t\t\tdie(_(\"malformed input line: '%s'.\"), buf.buf);\n \t\t\tstrbuf_rtrim(split[0]);\n-\t\t\tresult = real_merge(&o, split[0]->buf, split[1]->buf, prefix);\n+\t\t\tresult = real_merge(&o, merge_base, split[0]->buf, split[1]->buf, prefix);\n \t\t\tif (result < 0)\n \t\t\t\tdie(_(\"merging cannot continue; got unclean result of %d\"), result);\n \t\t\tstrbuf_list_free(split);\n@@ -581,7 +605,7 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \n \t/* Do the relevant type of merge */\n \tif (o.mode == MODE_REAL)\n-\t\treturn real_merge(&o, argv[0], argv[1], prefix);\n+\t\treturn real_merge(&o, merge_base, argv[0], argv[1], prefix);\n \telse\n \t\treturn trivial_merge(argv[0], argv[1], argv[2]);\n }\ndiff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\nindex cac85591b52..6db96ccbaae 100755\n--- a/t/t4301-merge-tree-write-tree.sh\n+++ b/t/t4301-merge-tree-write-tree.sh\n@@ -860,4 +860,31 @@ test_expect_success '--stdin with both a successful and a conflicted merge' '\n \ttest_cmp expect actual\n '\n \n+# specify merge-base as parent of branch2\n+# git merge-tree --write-tree --merge-base=c2 c1 c3\n+# Commit c1: add file1\n+# Commit c2: add file2 after c1\n+# Commit c3: add file3 after c2\n+# Expected: add file3, and file2 does NOT appear\n+\n+test_expect_success 'specify merge-base as parent of branch2' '\n+\t# Setup\n+\ttest_when_finished \"rm -rf base-b2-p\" &&\n+\tgit init base-b2-p &&\n+\ttest_commit -C base-b2-p c1 file1 &&\n+\ttest_commit -C base-b2-p c2 file2 &&\n+\ttest_commit -C base-b2-p c3 file3 &&\n+\n+\t# Testing\n+\tTREE_OID=$(git -C base-b2-p merge-tree --write-tree --merge-base=c2 c1 c3) &&\n+\n+\tq_to_tab <<-EOF >expect &&\n+\t100644 blob $(git -C base-b2-p rev-parse c1:file1)Qfile1\n+\t100644 blob $(git -C base-b2-p rev-parse c3:file3)Qfile3\n+\tEOF\n+\n+\tgit -C base-b2-p ls-tree $TREE_OID >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"467181","messageId":"48e55d4e97cfddee25e1cd5dd662c4f93a59b460.1668210315.git.gitgitgadget@gmail.com","threadId":"58703","inReplyTo":"pull.1397.v7.git.1668210314.gitgitgadget@gmail.com","subject":"[PATCH v7 2/2] merge-tree.c: allow specifying the merge-base when --stdin is passed","fromName":"Kyle Zhao via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-11-11T23:45:14Z","receivedAt":"2022-11-11T23:45:28Z","isPatch":true,"sender":{"key":"goldofzky@gmail.com","avatar":"https://gravatar.com/avatar/aa4fa27f0c1bed9c986ed596415799518e03a6e71a0931308a4de87e166e183a?d=mp&s=160"},"body":"From: Kyle Zhao \n\nThe previous commit added a `--merge-base` option in order to allow\nusing a specified merge-base for the merge. Extend the input accepted\nby `--stdin` to also allow a specified merge-base with each merge\nrequested. For example:\n\n printf \" -- \" | git merge-tree --stdin\n\ndoes a merge of b1 and b2, and uses b3 as the merge-base.\n\nSigned-off-by: Kyle Zhao \n---\n Documentation/git-merge-tree.txt | 14 ++++++++++++-\n builtin/merge-tree.c | 21 +++++++++++++++++--\n t/t4301-merge-tree-write-tree.sh | 35 ++++++++++++++++++++++++++++++++\n 3 files changed, 67 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\nindex d9dacb2ce54..298c133fdb6 100644\n--- a/Documentation/git-merge-tree.txt\n+++ b/Documentation/git-merge-tree.txt\n@@ -66,7 +66,8 @@ OPTIONS\n \n --merge-base=::\n \tInstead of finding the merge-bases for and ,\n-\tspecify a merge-base for the merge.\n+\tspecify a merge-base for the merge. This option is incompatible\n+\twith `--stdin`.\n \n [[OUTPUT]]\n OUTPUT\n@@ -220,6 +221,17 @@ with linkgit:git-merge[1]:\n * any messages that would have been printed to stdout (the\n <>)\n \n+INPUT FORMAT\n+------------\n+'git merge-tree --stdin' input format is fully text based. Each line\n+has this format:\n+\n+\t[ -- ] \n+\n+If one line is separated by `--`, the string before the separator is\n+used for specifying a merge-base for the merge and the string after\n+the separator describes the branches to be merged.\n+\n MISTAKES TO AVOID\n -----------------\n \ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex 62c6d43cdb9..330f779e8bc 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -557,12 +557,29 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\twhile (strbuf_getline_lf(&buf, stdin) != EOF) {\n \t\t\tstruct strbuf **split;\n \t\t\tint result;\n+\t\t\tconst char *input_merge_base = NULL;\n \n \t\t\tsplit = strbuf_split(&buf, ' ');\n-\t\t\tif (!split[0] || !split[1] || split[2])\n+\t\t\tif (!split[0] || !split[1])\n \t\t\t\tdie(_(\"malformed input line: '%s'.\"), buf.buf);\n \t\t\tstrbuf_rtrim(split[0]);\n-\t\t\tresult = real_merge(&o, merge_base, split[0]->buf, split[1]->buf, prefix);\n+\t\t\tstrbuf_rtrim(split[1]);\n+\n+\t\t\t/* parse the merge-base */\n+\t\t\tif (!strcmp(split[1]->buf, \"--\")) {\n+\t\t\t\tinput_merge_base = split[0]->buf;\n+\t\t\t}\n+\n+\t\t\tif (input_merge_base && split[2] && split[3] && !split[4]) {\n+\t\t\t\tstrbuf_rtrim(split[2]);\n+\t\t\t\tstrbuf_rtrim(split[3]);\n+\t\t\t\tresult = real_merge(&o, input_merge_base, split[2]->buf, split[3]->buf, prefix);\n+\t\t\t} else if (!input_merge_base && !split[2]) {\n+\t\t\t\tresult = real_merge(&o, NULL, split[0]->buf, split[1]->buf, prefix);\n+\t\t\t} else {\n+\t\t\t\tdie(_(\"malformed input line: '%s'.\"), buf.buf);\n+\t\t\t}\n+\n \t\t\tif (result < 0)\n \t\t\t\tdie(_(\"merging cannot continue; got unclean result of %d\"), result);\n \t\t\tstrbuf_list_free(split);\ndiff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\nindex 6db96ccbaae..a8983a0edcb 100755\n--- a/t/t4301-merge-tree-write-tree.sh\n+++ b/t/t4301-merge-tree-write-tree.sh\n@@ -860,6 +860,13 @@ test_expect_success '--stdin with both a successful and a conflicted merge' '\n \ttest_cmp expect actual\n '\n \n+\n+test_expect_success '--merge-base is incompatible with --stdin' '\n+\ttest_must_fail git merge-tree --merge-base=side1 --stdin 2>expect &&\n+\n+\tgrep \"^fatal: --merge-base is incompatible with --stdin\" expect\n+'\n+\n # specify merge-base as parent of branch2\n # git merge-tree --write-tree --merge-base=c2 c1 c3\n # Commit c1: add file1\n@@ -887,4 +894,32 @@ test_expect_success 'specify merge-base as parent of branch2' '\n \ttest_cmp expect actual\n '\n \n+# Since the earlier tests have verified that individual merge-tree calls\n+# are doing the right thing, this test case is only used to verify that\n+# we can also trigger merges via --stdin, and that when we do we get\n+# the same answer as running a bunch of separate merges.\n+\n+test_expect_success 'check the input format when --stdin is passed' '\n+\ttest_when_finished \"rm -rf repo\" &&\n+\tgit init repo &&\n+\ttest_commit -C repo c1 &&\n+\ttest_commit -C repo c2 &&\n+\ttest_commit -C repo c3 &&\n+\tprintf \"c1 c3\\nc2 -- c1 c3\\nc2 c3\" | git -C repo merge-tree --stdin >actual &&\n+\n+\tprintf \"1\\0\" >expect &&\n+\tgit -C repo merge-tree --write-tree -z c1 c3 >>expect &&\n+\tprintf \"\\0\" >>expect &&\n+\n+\tprintf \"1\\0\" >>expect &&\n+\tgit -C repo merge-tree --write-tree -z --merge-base=c2 c1 c3 >>expect &&\n+\tprintf \"\\0\" >>expect &&\n+\n+\tprintf \"1\\0\" >>expect &&\n+\tgit -C repo merge-tree --write-tree -z c2 c3 >>expect &&\n+\tprintf \"\\0\" >>expect &&\n+\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \ngitgitgadget\n"},{"id":"467183","messageId":"CABPp-BE=XgrQxnyPp3-xi+CuU8kBarwJNfNvEtK3HscQh9sSEw@mail.gmail.com","threadId":"58703","inReplyTo":"pull.1397.v7.git.1668210314.gitgitgadget@gmail.com","subject":"Re: [PATCH v7 0/2] merge-tree: allow specifying a base commit when --write-tree is passed","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-11-12T00:32:35Z","receivedAt":"2022-11-12T00:32:52Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, Nov 11, 2022 at 3:45 PM Kyle Zhao via GitGitGadget\n wrote:\n>\n> Thanks for everyone's careful reviews.\n>\n> In this patch, I introduce a new --merge-base= option to allow\n> callers to specify a merge-base for the merge and extend the input accepted\n> by --stdin to also allow a specified merge-base with each merge requested.\n>\n> Regards, Kyle\n>\n> Changes since v1:\n>\n> * Changed merge_incore_recursive() to merge_incore_nonrecursive() when\n> merge-base is specified.\n> * Fixed c style problem.\n> * Moved commit lookup/die logic out to the parsing logic in\n> cmd_merge_tree().\n> * use test_commit for test\n>\n> Changes since v2:\n>\n> * commit message\n> * Rebased on top of en/merge-tree-sequence.\n> * Set opt.ancestor to o->merge_base. Because opt.ancestor is a *char. To\n> make it easier to pass parameters, I moved\n> lookup_commit_reference_by_name() to real_ merge() again.\n> * Added test comment.\n>\n> Changes since v3:\n>\n> * support --merge-base in conjunction with --stdin\n>\n> Changes since v4:\n>\n> * commit message\n> * added input format document\n> * changed the input format for specifying the merge-base when --stdin is\n> passed\n> * changed the output when --stdin and --merge-base are used at the same\n> time\n> * add comment for test\n>\n> Changes since v5:\n>\n> * improved test: remove the test repo after the test; avoid sub-shell.\n>\n> Changes since v6:\n>\n> * fixed comment of test\n\nThanks; this version looks good to me and is ready to merge down.\n\n> Kyle Zhao (2):\n> merge-tree.c: add --merge-base= option\n> merge-tree.c: allow specifying the merge-base when --stdin is passed\n>\n> Documentation/git-merge-tree.txt | 16 ++++++++\n> builtin/merge-tree.c | 65 ++++++++++++++++++++++++++------\n> t/t4301-merge-tree-write-tree.sh | 62 ++++++++++++++++++++++++++++++\n> 3 files changed, 131 insertions(+), 12 deletions(-)\n>\n>\n> base-commit: ec1edbcb56ac05e9980299b05924c5c1b51d68b4\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1397%2Fkeyu98%2Fkz%2Fmerge-tree-option-merge-base-v7\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1397/keyu98/kz/merge-tree-option-merge-base-v7\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1397\n>\n> Range-diff vs v6:\n>\n> 1: 1cf1c69b8e8 = 1: 1cf1c69b8e8 merge-tree.c: add --merge-base= option\n> 2: 40d56544e6e ! 2: 48e55d4e97c merge-tree.c: allow specifying the merge-base when --stdin is passed\n> @@ t/t4301-merge-tree-write-tree.sh: test_expect_success 'specify merge-base as par\n> '\n>\n> +# Since the earlier tests have verified that individual merge-tree calls\n> -+# are doing the right thing, this test case is only used to test whether\n> -+# the input format is available.\n> ++# are doing the right thing, this test case is only used to verify that\n> ++# we can also trigger merges via --stdin, and that when we do we get\n> ++# the same answer as running a bunch of separate merges.\n> +\n> +test_expect_success 'check the input format when --stdin is passed' '\n> + test_when_finished \"rm -rf repo\" &&\n>\n> --\n> gitgitgadget\n"},{"id":"467210","messageId":"Y3B4Tl+luEPJgVJP@nand.local","threadId":"58703","inReplyTo":"pull.1397.v7.git.1668210314.gitgitgadget@gmail.com","subject":"Re: [PATCH v7 0/2] merge-tree: allow specifying a base commit when --write-tree is passed","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-11-13T04:53:34Z","receivedAt":"2022-11-13T04:54:32Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Fri, Nov 11, 2022 at 11:45:12PM +0000, Kyle Zhao via GitGitGadget wrote:\n> Thanks for everyone's careful reviews.\n>\n> In this patch, I introduce a new --merge-base= option to allow\n> callers to specify a merge-base for the merge and extend the input accepted\n> by --stdin to also allow a specified merge-base with each merge requested.\n\nThanks for the updated round. I applied it locally, now let's see what\nreviewers think of it...\n\nThanks,\nTaylor\n"},{"id":"467211","messageId":"Y3B4gGWgnlCpAFDP@nand.local","threadId":"58703","inReplyTo":"Y3B4Tl+luEPJgVJP@nand.local","subject":"Re: [PATCH v7 0/2] merge-tree: allow specifying a base commit when --write-tree is passed","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-11-13T04:54:24Z","receivedAt":"2022-11-13T04:54:32Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Sat, Nov 12, 2022 at 11:53:34PM -0500, Taylor Blau wrote:\n> On Fri, Nov 11, 2022 at 11:45:12PM +0000, Kyle Zhao via GitGitGadget wrote:\n> > Thanks for everyone's careful reviews.\n> >\n> > In this patch, I introduce a new --merge-base= option to allow\n> > callers to specify a merge-base for the merge and extend the input accepted\n> > by --stdin to also allow a specified merge-base with each merge requested.\n>\n> Thanks for the updated round. I applied it locally, now let's see what\n> reviewers think of it...\n\nOops ;-). I should have looked further down in my inbox before replying.\nLooks like this is ready to merge down according to Elijah, so let's\nstart cooking this in 'next'.\n\nThanks again.\n\nThanks,\nTaylor\n"},{"id":"467743","messageId":"xmqqsfibsn9o.fsf@gitster.g","threadId":"58703","inReplyTo":"1cf1c69b8e8e8e81eccc42b5d8efc605a36ab7eb.1668210314.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v7 1/2] merge-tree.c: add --merge-base= option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-11-22T03:04:19Z","receivedAt":"2022-11-22T03:04:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kyle Zhao via GitGitGadget\" writes:\n\n> +--merge-base=::\n> +\tInstead of finding the merge-bases for and ,\n> +\tspecify a merge-base for the merge.\n\nOK.\n\n> +\tconst char *merge_base = NULL;\n> \n> \tconst char * const merge_tree_usage[] = {\n> \t\tN_(\"git merge-tree [--write-tree] [] \"),\n> @@ -515,6 +533,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> \t\t\t &o.use_stdin,\n> \t\t\t N_(\"perform multiple merges, one per line of input\"),\n> \t\t\t PARSE_OPT_NONEG),\n> +\t\tOPT_STRING(0, \"merge-base\",\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_END()\n> \t};\n\nThis looks wrong, though.\n\nShouldn't \"git merge-tree --merge-base=X --merge-base=Y A B\"\nallow you to compute the merge between A and B in a history\nwhere there are two merge bases?\n\nUnfortunately this is already in 'next', so let's see an incremental\nfix on top.\n\nThanks.\n"},{"id":"467745","messageId":"3e5cdd224d76499bbddf55b0b649439b@tencent.com","threadId":"58703","inReplyTo":"xmqqsfibsn9o.fsf@gitster.g","subject":"RE: [Internet]Re: [PATCH v7 1/2] merge-tree.c: add --merge-base= option","fromName":"kylezhao (赵柯宇)","fromEmail":"kylezhao@tencent.com","sentAt":"2022-11-22T03:52:21Z","receivedAt":"2022-11-22T03:52:55Z","isPatch":true,"sender":{"key":"kylezhao@tencent.com","avatar":"https://avatars.githubusercontent.com/u/34176272?v=4"},"body":"> > +\tconst char *merge_base = NULL;\n> >\n> > \tconst char * const merge_tree_usage[] = {\n> > \t\tN_(\"git merge-tree [--write-tree] [] \n> > \"), @@ -515,6 +533,10 @@ int cmd_merge_tree(int argc, const\n> char **argv, const char *prefix)\n> > \t\t\t &o.use_stdin,\n> > \t\t\t N_(\"perform multiple merges, one per line of input\"),\n> > \t\t\t PARSE_OPT_NONEG),\n> > +\t\tOPT_STRING(0, \"merge-base\",\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_END()\n> > \t};\n> \n> This looks wrong, though.\n> \n> Shouldn't \"git merge-tree --merge-base=X --merge-base=Y A B\"\n> allow you to compute the merge between A and B in a history where there are\n> two merge bases?\n> \n> Unfortunately this is already in 'next', so let's see an incremental fix on top.\n> \n> Thanks.\n\nI agree.\n\nOPT_STRING only use the last value of \"--merge-base\".\nIt will mislead users, they may specify the merge-base multiple times, but found it doesn't work.\n\nI went to check the api-parse-option.txt, but I didn't found an elegant solution to stop when the users uses the\nsecond \"--merge-base\". Did I miss it? Or we just need to mention this in the git-merge-tree.txt,\nsuch as:\n --merge-base=::\n \tInstead of finding the merge-bases for and ,\n-\tspecify a merge-base for the merge.\n+ specify a merge-base for the merge. This option doesn't support\n+ being specified multiple times, only the last value you provide will be used.\n\n\nThanks,\nKyle\n\n"},{"id":"467748","messageId":"xmqqcz9fsjdr.fsf@gitster.g","threadId":"58703","inReplyTo":"3e5cdd224d76499bbddf55b0b649439b@tencent.com","subject":"Re: [Internet]Re: [PATCH v7 1/2] merge-tree.c: add --merge-base= option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-11-22T04:28:16Z","receivedAt":"2022-11-22T04:28:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"kylezhao(赵柯宇)\" writes:\n\n> I went to check the api-parse-option.txt, but I didn't found an\n> elegant solution to stop when the users uses the second\n> \"--merge-base\".\n\nThat's not even a fix, as it does not allow specifying more than one\nmerge bases, is it?\n\nJust like how builtin/merge-tree.c::real_merge() is prepared to\nhandle parent1 and parent2 that have multiple merge bases and pass\nall of them to the underlying merge machinery, you can accumulate\nmultiple strings using OPT_STRING_LIST() and use all of them as\nmerge_bases, no?\n"},{"id":"467750","messageId":"xmqqr0xvr1e5.fsf@gitster.g","threadId":"58703","inReplyTo":"xmqqcz9fsjdr.fsf@gitster.g","subject":"Re: [Internet]Re: [PATCH v7 1/2] merge-tree.c: add --merge-base= option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-11-22T05:42:10Z","receivedAt":"2022-11-22T05:42:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> \"kylezhao(赵柯宇)\" writes:\n>\n>> I went to check the api-parse-option.txt, but I didn't found an\n>> elegant solution to stop when the users uses the second\n>> \"--merge-base\".\n>\n> That's not even a fix, as it does not allow specifying more than one\n> merge bases, is it?\n>\n> Just like how builtin/merge-tree.c::real_merge() is prepared to\n> handle parent1 and parent2 that have multiple merge bases and pass\n> all of them to the underlying merge machinery, you can accumulate\n> multiple strings using OPT_STRING_LIST() and use all of them as\n> merge_bases, no?\n\nI am a bit torn on this, though. \n\nBecause it uses lookup_commit_reference_by_name() to obtain\nbase_commit in addition to parent1 and parent2, and then\nget_commit_tree() on them to get their trees, the real_merge()\nfunction in the posted patch is incapable of accepting a single\n\"pretend as if this tree object is the common ancestor tree\" and\nmerging the two tree objects. But once that flaw is fixed, using\nmerge_incore_nonrecursive() with an explicitly given \"base\", we can\nmerge totally trees regardless of how the commits they are found in\nare related in the history, which is a very logical thing to do.\nAnd while operating in that mode, there is no way to accept more\nthan one \"base\".\n\nSo, I would be PERFECTLY HAPPY if this new mode of operation can\ntake only one \"base\" tree object, if it allows BASE, PARENT1,\nand PARENT2 to be all tree objects, not necessarily commit objects.\n\nBut if we insist that PARENT1 and PARENT2 must be commits, then I\nwould find it very unsatisfying if we only took a single BASE that\nalso must be a commit. If the merge-base has to be a tree or trees,\nthen there is no way to perform recursive merge (because you cannot\ncompute common ancestors across tree objects) , so it is perfectly\njustifiable to take only a single base (and error out upon seeing a\nsecond --merge-base=X option).\n\nBut it has to be a commit, then there is no justification to forbid\nrecursive operation, and I do not see a good reason to take only one\nCOMMON thing.\n\nSo, it is easy to say \"let's take the current patch as a good first\nstep\", but it is unclear what the good second step would be.\n\nWe could correct the code to allow PARENT1, PARENT2 and BASE to be\ntree objects, not necessarily commits, when only a single merge-base\nis specified. That corrects the current flaw that tree objects\ncannot be used. And then when more than one BASE is given (or no\nBASE is given---which is the original code), we could correct the\ncode to call merge_incore_recursive() instead.\n\nBut the amount of the effort to get there from the current codebase\n(without your patch) feels more or less the same ballpark as the\npatch in question, so...\n"},{"id":"467753","messageId":"ea04a44ca4a6475f99619c0fb997ecff@tencent.com","threadId":"58703","inReplyTo":"xmqqr0xvr1e5.fsf@gitster.g","subject":"RE: [Internet]Re: Re: [PATCH v7 1/2] merge-tree.c: add --merge-base= option","fromName":"kylezhao (赵柯宇)","fromEmail":"kylezhao@tencent.com","sentAt":"2022-11-22T07:47:36Z","receivedAt":"2022-11-22T07:47:58Z","isPatch":true,"sender":{"key":"kylezhao@tencent.com","avatar":"https://avatars.githubusercontent.com/u/34176272?v=4"},"body":"> I am a bit torn on this, though.\n> \n> Because it uses lookup_commit_reference_by_name() to obtain base_commit\n> in addition to parent1 and parent2, and then\n> get_commit_tree() on them to get their trees, the real_merge() function in the\n> posted patch is incapable of accepting a single \"pretend as if this tree object is\n> the common ancestor tree\" and merging the two tree objects. But once that\n> flaw is fixed, using\n> merge_incore_nonrecursive() with an explicitly given \"base\", we can merge\n> totally trees regardless of how the commits they are found in are related in the\n> history, which is a very logical thing to do.\n> And while operating in that mode, there is no way to accept more than one\n> \"base\".\n> \n> So, I would be PERFECTLY HAPPY if this new mode of operation can take only\n> one \"base\" tree object, if it allows BASE, PARENT1, and PARENT2 to be all tree\n> objects, not necessarily commit objects.\n> \n> But if we insist that PARENT1 and PARENT2 must be commits, then I would find\n> it very unsatisfying if we only took a single BASE that also must be a commit. If\n> the merge-base has to be a tree or trees, then there is no way to perform\n> recursive merge (because you cannot compute common ancestors across tree\n> objects) , so it is perfectly justifiable to take only a single base (and error out\n> upon seeing a second --merge-base=X option).\n> \n> But it has to be a commit, then there is no justification to forbid recursive\n> operation, and I do not see a good reason to take only one COMMON thing.\n> \n> So, it is easy to say \"let's take the current patch as a good first step\", but it is\n> unclear what the good second step would be.\n\nYeah, I think so. \nIn fact, I planned to implement a version that specifies *only one* merge-base\nat the beginning and made the format of this option would be possible to\nextend to support multiple bases and octopus at the same time.\n\n> \n> We could correct the code to allow PARENT1, PARENT2 and BASE to be tree\n> objects, not necessarily commits, when only a single merge-base is specified.\n> That corrects the current flaw that tree objects cannot be used. And then when\n> more than one BASE is given (or no BASE is given---which is the original code),\n> we could correct the code to call merge_incore_recursive() instead.\n\nI prefer this solution.\n\n> \n> But the amount of the effort to get there from the current codebase (without\n> your patch) feels more or less the same ballpark as the patch in question, so...\n\nIt means we need to support specifying multiple bases in the first version, which makes\nmerge-tree command more complete. Since individual merge-tree support multiple bases, \n--stdin mode seems to need to support this too.\n\nHowever, I can't think of when the user needs to manually specify multiple bases for a merge ;).\nIn other words, do we really need to support multiple bases in the first version?\n\nThanks,\nKyle\n\n\n\n"},{"id":"467901","messageId":"pull.1397.v8.git.1669261026.gitgitgadget@gmail.com","threadId":"58703","inReplyTo":"pull.1397.v7.git.1668210314.gitgitgadget@gmail.com","subject":"[PATCH v8 0/3] merge-tree: allow specifying a base commit when --write-tree is passed","fromName":"Kyle Zhao via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-11-24T03:37:03Z","receivedAt":"2022-11-24T03:37:14Z","isPatch":true,"sender":{"key":"goldofzky@gmail.com","avatar":"https://gravatar.com/avatar/aa4fa27f0c1bed9c986ed596415799518e03a6e71a0931308a4de87e166e183a?d=mp&s=160"},"body":"Users may specify the merge-base multiple times, such as \"git merge-tree\n--merge-base=X --merge-base=Y A B\", but found it doesn't work.\n\nThis patch I fix the description of option \"--merge-base\".\n\nBy the way, the amount of the effort support specifying multiple bases from\nthe codebase without these patches feels more or less the same ballpark as\nthese patches.\n\nOne point worth discussing is whether manually specifying multiple bases is\na rarely used feature and do we need to support it in the first version.\n\nIf needed, It might take a while for me to implement it, and the feature\nmight be delayed to the version after 2.39.\n\nThanks, Kyle\n\nChanges since v1:\n\n * Changed merge_incore_recursive() to merge_incore_nonrecursive() when\n merge-base is specified.\n * Fixed c style problem.\n * Moved commit lookup/die logic out to the parsing logic in\n cmd_merge_tree().\n * use test_commit for test\n\nChanges since v2:\n\n * commit message\n * Rebased on top of en/merge-tree-sequence.\n * Set opt.ancestor to o->merge_base. Because opt.ancestor is a *char. To\n make it easier to pass parameters, I moved\n lookup_commit_reference_by_name() to real_ merge() again.\n * Added test comment.\n\nChanges since v3:\n\n * support --merge-base in conjunction with --stdin\n\nChanges since v4:\n\n * commit message\n * added input format document\n * changed the input format for specifying the merge-base when --stdin is\n passed\n * changed the output when --stdin and --merge-base are used at the same\n time\n * add comment for test\n\nChanges since v5:\n\n * improved test: remove the test repo after the test; avoid sub-shell.\n\nChanges since v6:\n\n * fixed comment of test\n\nChanges since v7:\n\n * docs: fix description of the --merge-base option\n\nKyle Zhao (3):\n merge-tree.c: add --merge-base= option\n merge-tree.c: allow specifying the merge-base when --stdin is passed\n docs: fix description of the `--merge-base` option\n\n Documentation/git-merge-tree.txt | 16 ++++++++\n builtin/merge-tree.c | 65 ++++++++++++++++++++++++++------\n t/t4301-merge-tree-write-tree.sh | 62 ++++++++++++++++++++++++++++++\n 3 files changed, 131 insertions(+), 12 deletions(-)\n\n\nbase-commit: ec1edbcb56ac05e9980299b05924c5c1b51d68b4\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1397%2Fkeyu98%2Fkz%2Fmerge-tree-option-merge-base-v8\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1397/keyu98/kz/merge-tree-option-merge-base-v8\nPull-Request: https://github.com/gitgitgadget/git/pull/1397\n\nRange-diff vs v7:\n\n 1: 1cf1c69b8e8 = 1: 1cf1c69b8e8 merge-tree.c: add --merge-base= option\n 2: 48e55d4e97c = 2: 48e55d4e97c merge-tree.c: allow specifying the merge-base when --stdin is passed\n -: ----------- > 3: c21466d1db0 docs: fix description of the `--merge-base` option\n\n-- \ngitgitgadget\n"},{"id":"467902","messageId":"1cf1c69b8e8e8e81eccc42b5d8efc605a36ab7eb.1669261026.git.gitgitgadget@gmail.com","threadId":"58703","inReplyTo":"pull.1397.v8.git.1669261026.gitgitgadget@gmail.com","subject":"[PATCH v8 1/3] merge-tree.c: add --merge-base= option","fromName":"Kyle Zhao via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-11-24T03:37:04Z","receivedAt":"2022-11-24T03:37:18Z","isPatch":true,"sender":{"key":"goldofzky@gmail.com","avatar":"https://gravatar.com/avatar/aa4fa27f0c1bed9c986ed596415799518e03a6e71a0931308a4de87e166e183a?d=mp&s=160"},"body":"From: Kyle Zhao \n\nThis patch will give our callers more flexibility to use `git merge-tree`,\nsuch as:\n\n git merge-tree --write-tree --merge-base=branch^ HEAD branch\n\nThis does a merge of HEAD and branch, but uses branch^ as the merge-base.\n\nAnd the reason why using an option flag instead of a positional argument\nis to allow additional commits passed to merge-tree to be handled via an\noctopus merge in the future.\n\nSigned-off-by: Kyle Zhao \n---\n Documentation/git-merge-tree.txt | 4 +++\n builtin/merge-tree.c | 46 ++++++++++++++++++++++++--------\n t/t4301-merge-tree-write-tree.sh | 27 +++++++++++++++++++\n 3 files changed, 66 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\nindex 04bcc416e6e..d9dacb2ce54 100644\n--- a/Documentation/git-merge-tree.txt\n+++ b/Documentation/git-merge-tree.txt\n@@ -64,6 +64,10 @@ OPTIONS\n \tshare no common history. This flag can be given to override that\n \tcheck and make the merge proceed anyway.\n \n+--merge-base=::\n+\tInstead of finding the merge-bases for and ,\n+\tspecify a merge-base for the merge.\n+\n [[OUTPUT]]\n OUTPUT\n ------\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex fe853aa8f91..62c6d43cdb9 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -3,6 +3,7 @@\n #include \"tree-walk.h\"\n #include \"xdiff-interface.h\"\n #include \"help.h\"\n+#include \"commit.h\"\n #include \"commit-reach.h\"\n #include \"merge-ort.h\"\n #include \"object-store.h\"\n@@ -406,6 +407,7 @@ struct merge_tree_options {\n };\n \n static int real_merge(struct merge_tree_options *o,\n+\t\t const char *merge_base,\n \t\t const char *branch1, const char *branch2,\n \t\t const char *prefix)\n {\n@@ -432,16 +434,31 @@ static int real_merge(struct merge_tree_options *o,\n \topt.branch1 = branch1;\n \topt.branch2 = branch2;\n \n-\t/*\n-\t * Get the merge bases, in reverse order; see comment above\n-\t * merge_incore_recursive in merge-ort.h\n-\t */\n-\tmerge_bases = get_merge_bases(parent1, parent2);\n-\tif (!merge_bases && !o->allow_unrelated_histories)\n-\t\tdie(_(\"refusing to merge unrelated histories\"));\n-\tmerge_bases = reverse_commit_list(merge_bases);\n+\tif (merge_base) {\n+\t\tstruct commit *base_commit;\n+\t\tstruct tree *base_tree, *parent1_tree, *parent2_tree;\n+\n+\t\tbase_commit = lookup_commit_reference_by_name(merge_base);\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\tbase_tree = get_commit_tree(base_commit);\n+\t\tparent1_tree = get_commit_tree(parent1);\n+\t\tparent2_tree = get_commit_tree(parent2);\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+\t\t * merge_incore_recursive in merge-ort.h\n+\t\t */\n+\t\tmerge_bases = get_merge_bases(parent1, parent2);\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}\n \n-\tmerge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);\n \tif (result.clean < 0)\n \t\tdie(_(\"failure to merge\"));\n \n@@ -487,6 +504,7 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \tstruct merge_tree_options o = { .show_messages = -1 };\n \tint expected_remaining_argc;\n \tint original_argc;\n+\tconst char *merge_base = NULL;\n \n \tconst char * const merge_tree_usage[] = {\n \t\tN_(\"git merge-tree [--write-tree] [] \"),\n@@ -515,6 +533,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\t\t &o.use_stdin,\n \t\t\t N_(\"perform multiple merges, one per line of input\"),\n \t\t\t PARSE_OPT_NONEG),\n+\t\tOPT_STRING(0, \"merge-base\",\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_END()\n \t};\n \n@@ -529,6 +551,8 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \n \t\tif (o.mode == MODE_TRIVIAL)\n \t\t\tdie(_(\"--trivial-merge is incompatible with all other options\"));\n+\t\tif (merge_base)\n+\t\t\tdie(_(\"--merge-base is incompatible with --stdin\"));\n \t\tline_termination = '\\0';\n \t\twhile (strbuf_getline_lf(&buf, stdin) != EOF) {\n \t\t\tstruct strbuf **split;\n@@ -538,7 +562,7 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\t\tif (!split[0] || !split[1] || split[2])\n \t\t\t\tdie(_(\"malformed input line: '%s'.\"), buf.buf);\n \t\t\tstrbuf_rtrim(split[0]);\n-\t\t\tresult = real_merge(&o, split[0]->buf, split[1]->buf, prefix);\n+\t\t\tresult = real_merge(&o, merge_base, split[0]->buf, split[1]->buf, prefix);\n \t\t\tif (result < 0)\n \t\t\t\tdie(_(\"merging cannot continue; got unclean result of %d\"), result);\n \t\t\tstrbuf_list_free(split);\n@@ -581,7 +605,7 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \n \t/* Do the relevant type of merge */\n \tif (o.mode == MODE_REAL)\n-\t\treturn real_merge(&o, argv[0], argv[1], prefix);\n+\t\treturn real_merge(&o, merge_base, argv[0], argv[1], prefix);\n \telse\n \t\treturn trivial_merge(argv[0], argv[1], argv[2]);\n }\ndiff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\nindex cac85591b52..6db96ccbaae 100755\n--- a/t/t4301-merge-tree-write-tree.sh\n+++ b/t/t4301-merge-tree-write-tree.sh\n@@ -860,4 +860,31 @@ test_expect_success '--stdin with both a successful and a conflicted merge' '\n \ttest_cmp expect actual\n '\n \n+# specify merge-base as parent of branch2\n+# git merge-tree --write-tree --merge-base=c2 c1 c3\n+# Commit c1: add file1\n+# Commit c2: add file2 after c1\n+# Commit c3: add file3 after c2\n+# Expected: add file3, and file2 does NOT appear\n+\n+test_expect_success 'specify merge-base as parent of branch2' '\n+\t# Setup\n+\ttest_when_finished \"rm -rf base-b2-p\" &&\n+\tgit init base-b2-p &&\n+\ttest_commit -C base-b2-p c1 file1 &&\n+\ttest_commit -C base-b2-p c2 file2 &&\n+\ttest_commit -C base-b2-p c3 file3 &&\n+\n+\t# Testing\n+\tTREE_OID=$(git -C base-b2-p merge-tree --write-tree --merge-base=c2 c1 c3) &&\n+\n+\tq_to_tab <<-EOF >expect &&\n+\t100644 blob $(git -C base-b2-p rev-parse c1:file1)Qfile1\n+\t100644 blob $(git -C base-b2-p rev-parse c3:file3)Qfile3\n+\tEOF\n+\n+\tgit -C base-b2-p ls-tree $TREE_OID >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"467903","messageId":"48e55d4e97cfddee25e1cd5dd662c4f93a59b460.1669261026.git.gitgitgadget@gmail.com","threadId":"58703","inReplyTo":"pull.1397.v8.git.1669261026.gitgitgadget@gmail.com","subject":"[PATCH v8 2/3] merge-tree.c: allow specifying the merge-base when --stdin is passed","fromName":"Kyle Zhao via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-11-24T03:37:05Z","receivedAt":"2022-11-24T03:37:20Z","isPatch":true,"sender":{"key":"goldofzky@gmail.com","avatar":"https://gravatar.com/avatar/aa4fa27f0c1bed9c986ed596415799518e03a6e71a0931308a4de87e166e183a?d=mp&s=160"},"body":"From: Kyle Zhao \n\nThe previous commit added a `--merge-base` option in order to allow\nusing a specified merge-base for the merge. Extend the input accepted\nby `--stdin` to also allow a specified merge-base with each merge\nrequested. For example:\n\n printf \" -- \" | git merge-tree --stdin\n\ndoes a merge of b1 and b2, and uses b3 as the merge-base.\n\nSigned-off-by: Kyle Zhao \n---\n Documentation/git-merge-tree.txt | 14 ++++++++++++-\n builtin/merge-tree.c | 21 +++++++++++++++++--\n t/t4301-merge-tree-write-tree.sh | 35 ++++++++++++++++++++++++++++++++\n 3 files changed, 67 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\nindex d9dacb2ce54..298c133fdb6 100644\n--- a/Documentation/git-merge-tree.txt\n+++ b/Documentation/git-merge-tree.txt\n@@ -66,7 +66,8 @@ OPTIONS\n \n --merge-base=::\n \tInstead of finding the merge-bases for and ,\n-\tspecify a merge-base for the merge.\n+\tspecify a merge-base for the merge. This option is incompatible\n+\twith `--stdin`.\n \n [[OUTPUT]]\n OUTPUT\n@@ -220,6 +221,17 @@ with linkgit:git-merge[1]:\n * any messages that would have been printed to stdout (the\n <>)\n \n+INPUT FORMAT\n+------------\n+'git merge-tree --stdin' input format is fully text based. Each line\n+has this format:\n+\n+\t[ -- ] \n+\n+If one line is separated by `--`, the string before the separator is\n+used for specifying a merge-base for the merge and the string after\n+the separator describes the branches to be merged.\n+\n MISTAKES TO AVOID\n -----------------\n \ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex 62c6d43cdb9..330f779e8bc 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -557,12 +557,29 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\twhile (strbuf_getline_lf(&buf, stdin) != EOF) {\n \t\t\tstruct strbuf **split;\n \t\t\tint result;\n+\t\t\tconst char *input_merge_base = NULL;\n \n \t\t\tsplit = strbuf_split(&buf, ' ');\n-\t\t\tif (!split[0] || !split[1] || split[2])\n+\t\t\tif (!split[0] || !split[1])\n \t\t\t\tdie(_(\"malformed input line: '%s'.\"), buf.buf);\n \t\t\tstrbuf_rtrim(split[0]);\n-\t\t\tresult = real_merge(&o, merge_base, split[0]->buf, split[1]->buf, prefix);\n+\t\t\tstrbuf_rtrim(split[1]);\n+\n+\t\t\t/* parse the merge-base */\n+\t\t\tif (!strcmp(split[1]->buf, \"--\")) {\n+\t\t\t\tinput_merge_base = split[0]->buf;\n+\t\t\t}\n+\n+\t\t\tif (input_merge_base && split[2] && split[3] && !split[4]) {\n+\t\t\t\tstrbuf_rtrim(split[2]);\n+\t\t\t\tstrbuf_rtrim(split[3]);\n+\t\t\t\tresult = real_merge(&o, input_merge_base, split[2]->buf, split[3]->buf, prefix);\n+\t\t\t} else if (!input_merge_base && !split[2]) {\n+\t\t\t\tresult = real_merge(&o, NULL, split[0]->buf, split[1]->buf, prefix);\n+\t\t\t} else {\n+\t\t\t\tdie(_(\"malformed input line: '%s'.\"), buf.buf);\n+\t\t\t}\n+\n \t\t\tif (result < 0)\n \t\t\t\tdie(_(\"merging cannot continue; got unclean result of %d\"), result);\n \t\t\tstrbuf_list_free(split);\ndiff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\nindex 6db96ccbaae..a8983a0edcb 100755\n--- a/t/t4301-merge-tree-write-tree.sh\n+++ b/t/t4301-merge-tree-write-tree.sh\n@@ -860,6 +860,13 @@ test_expect_success '--stdin with both a successful and a conflicted merge' '\n \ttest_cmp expect actual\n '\n \n+\n+test_expect_success '--merge-base is incompatible with --stdin' '\n+\ttest_must_fail git merge-tree --merge-base=side1 --stdin 2>expect &&\n+\n+\tgrep \"^fatal: --merge-base is incompatible with --stdin\" expect\n+'\n+\n # specify merge-base as parent of branch2\n # git merge-tree --write-tree --merge-base=c2 c1 c3\n # Commit c1: add file1\n@@ -887,4 +894,32 @@ test_expect_success 'specify merge-base as parent of branch2' '\n \ttest_cmp expect actual\n '\n \n+# Since the earlier tests have verified that individual merge-tree calls\n+# are doing the right thing, this test case is only used to verify that\n+# we can also trigger merges via --stdin, and that when we do we get\n+# the same answer as running a bunch of separate merges.\n+\n+test_expect_success 'check the input format when --stdin is passed' '\n+\ttest_when_finished \"rm -rf repo\" &&\n+\tgit init repo &&\n+\ttest_commit -C repo c1 &&\n+\ttest_commit -C repo c2 &&\n+\ttest_commit -C repo c3 &&\n+\tprintf \"c1 c3\\nc2 -- c1 c3\\nc2 c3\" | git -C repo merge-tree --stdin >actual &&\n+\n+\tprintf \"1\\0\" >expect &&\n+\tgit -C repo merge-tree --write-tree -z c1 c3 >>expect &&\n+\tprintf \"\\0\" >>expect &&\n+\n+\tprintf \"1\\0\" >>expect &&\n+\tgit -C repo merge-tree --write-tree -z --merge-base=c2 c1 c3 >>expect &&\n+\tprintf \"\\0\" >>expect &&\n+\n+\tprintf \"1\\0\" >>expect &&\n+\tgit -C repo merge-tree --write-tree -z c2 c3 >>expect &&\n+\tprintf \"\\0\" >>expect &&\n+\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"467904","messageId":"c21466d1db0e7f7fcd7308b61aa1e3cd4e3d67c4.1669261026.git.gitgitgadget@gmail.com","threadId":"58703","inReplyTo":"pull.1397.v8.git.1669261026.gitgitgadget@gmail.com","subject":"[PATCH v8 3/3] docs: fix description of the `--merge-base` option","fromName":"Kyle Zhao via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-11-24T03:37:06Z","receivedAt":"2022-11-24T03:37:23Z","isPatch":true,"sender":{"key":"goldofzky@gmail.com","avatar":"https://gravatar.com/avatar/aa4fa27f0c1bed9c986ed596415799518e03a6e71a0931308a4de87e166e183a?d=mp&s=160"},"body":"From: Kyle Zhao \n\nSigned-off-by: Kyle Zhao \n---\n Documentation/git-merge-tree.txt | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\nindex 298c133fdb6..88ee942101a 100644\n--- a/Documentation/git-merge-tree.txt\n+++ b/Documentation/git-merge-tree.txt\n@@ -66,8 +66,8 @@ OPTIONS\n \n --merge-base=::\n \tInstead of finding the merge-bases for and ,\n-\tspecify a merge-base for the merge. This option is incompatible\n-\twith `--stdin`.\n+\tspecify a merge-base for the merge, and specifying multiple bases is\n+\tcurrently not supported. This option is incompatible with `--stdin`.\n \n [[OUTPUT]]\n OUTPUT\n-- \ngitgitgadget\n"},{"id":"467969","messageId":"xmqqtu2neh60.fsf@gitster.g","threadId":"58703","inReplyTo":"c21466d1db0e7f7fcd7308b61aa1e3cd4e3d67c4.1669261026.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v8 3/3] docs: fix description of the `--merge-base` option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-11-25T05:28:55Z","receivedAt":"2022-11-25T05:28:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kyle Zhao via GitGitGadget\" writes:\n\n> From: Kyle Zhao \n>\n> Signed-off-by: Kyle Zhao \n> ---\n> Documentation/git-merge-tree.txt | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\n> index 298c133fdb6..88ee942101a 100644\n> --- a/Documentation/git-merge-tree.txt\n> +++ b/Documentation/git-merge-tree.txt\n> @@ -66,8 +66,8 @@ OPTIONS\n> \n> --merge-base=::\n> \tInstead of finding the merge-bases for and ,\n> -\tspecify a merge-base for the merge. This option is incompatible\n> -\twith `--stdin`.\n> +\tspecify a merge-base for the merge, and specifying multiple bases is\n> +\tcurrently not supported. This option is incompatible with `--stdin`.\n\nMakes sense. Will queue.\n\nThanks.\n"}]}