{"thread":{"id":"57169","subject":"[PATCH 1/8] merge-tree: rename merge_trees() to trivial_merge_trees()","startedAt":"2021-12-31T05:04:14Z","lastAt":"2022-02-22T13:05:43Z","messageCount":57,"participants":["Elijah Newren via GitGitGadget","Johannes Altmanninger","Elijah Newren","Fabian Stelzer","Ramsay Jones","Junio C Hamano","Johannes Schindelin","Christian Couder","René Scharfe"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"445269","messageId":"a7c7910d834b2e46ee6c5063db48f9cf551d0a35.1640927044.git.gitgitgadget@gmail.com","threadId":"57169","inReplyTo":"pull.1114.git.git.1640927044.gitgitgadget@gmail.com","subject":"[PATCH 1/8] merge-tree: rename merge_trees() to trivial_merge_trees()","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-12-31T05:03:57Z","receivedAt":"2021-12-31T05:04:14Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nmerge-recursive.h defined its own merge_trees() function, different than\nthe one found in builtin/merge-tree.c. That was okay in the past, but\nwe want merge-tree to be able to use the merge-ort functions, which will\nend up including merge-recursive.h. Rename the function found in\nbuiltin/merge-tree.c to avoid the conflict.\n\nSigned-off-by: Elijah Newren \n---\n builtin/merge-tree.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex 5dc94d6f880..06f9eee9f78 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -28,7 +28,7 @@ static void add_merge_entry(struct merge_list *entry)\n \tmerge_result_end = &entry->next;\n }\n \n-static void merge_trees(struct tree_desc t[3], const char *base);\n+static void trivial_merge_trees(struct tree_desc t[3], const char *base);\n \n static const char *explanation(struct merge_list *entry)\n {\n@@ -225,7 +225,7 @@ static void unresolved_directory(const struct traverse_info *info,\n \tbuf2 = fill_tree_descriptor(r, t + 2, ENTRY_OID(n + 2));\n #undef ENTRY_OID\n \n-\tmerge_trees(t, newbase);\n+\ttrivial_merge_trees(t, newbase);\n \n \tfree(buf0);\n \tfree(buf1);\n@@ -342,7 +342,7 @@ static int threeway_callback(int n, unsigned long mask, unsigned long dirmask, s\n \treturn mask;\n }\n \n-static void merge_trees(struct tree_desc t[3], const char *base)\n+static void trivial_merge_trees(struct tree_desc t[3], const char *base)\n {\n \tstruct traverse_info info;\n \n@@ -378,7 +378,7 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \tbuf1 = get_tree_descriptor(r, t+0, argv[1]);\n \tbuf2 = get_tree_descriptor(r, t+1, argv[2]);\n \tbuf3 = get_tree_descriptor(r, t+2, argv[3]);\n-\tmerge_trees(t, \"\");\n+\ttrivial_merge_trees(t, \"\");\n \tfree(buf1);\n \tfree(buf2);\n \tfree(buf3);\n-- \ngitgitgadget\n\n"},{"id":"445270","messageId":"pull.1114.git.git.1640927044.gitgitgadget@gmail.com","threadId":"57169","inReplyTo":null,"subject":"[PATCH 0/8] RFC: Server side merges (no ref updating, no commit creating, no touching worktree or index)","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-12-31T05:03:56Z","receivedAt":"2021-12-31T05:04:14Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"(NOTE for Junio: This series has a minor conflict with en/remerge-diff --\nthis series moves a code block into a new function, but en/remerge-diff adds\na BUG() message to that code block. But this series is just RFC, so you may\nwant to wait to pick it up.)\n\nNOTE2: A preliminary version of this series was discussed here:\nhttps://lore.kernel.org/git/nycvar.QRO.7.76.6.2110211147490.56@tvgsbejvaqbjf.bet/\n\nThis series introduces a new option to git-merge-tree: --real (best name I\ncould come up with). This new option is designed to allow a server-side\n\"real\" merge (or allow folks client-side to do merges with branches they\ndon't even have checked out). Real merges differ from trivial merges in that\nthey handle:\n\n * three way content merges\n * recursive ancestor consolidation\n * renames\n * proper directory/file conflict handling\n * etc.\n\nThe reason this is different from merge is that merge-tree does NOT:\n\n * Read/write/update any working tree (and assumes there probably isn't one)\n * Read/write/update any index (and assumes there probably isn't one)\n * Create a commit object\n * Update any refs\n\nThis series attempts to guess what kind of output would be wanted, basically\nchoosing:\n\n * clean merge or conflict signalled via exit status\n * stdout consists solely of printing the hash of the resulting tree (though\n that tree may include files that have conflict markers)\n * new optional --messages flag for specifying a file where informational\n messages (e.g. conflict notices and files involved in three-way-content\n merges) can be written; by default, this output is simply discarded\n * new optional --conflicted-list flag for specifying a file where the names\n of conflicted-files can be written in a NUL-character-separated list\n\nThis design means it's basically just a low-level tool that other scripts\nwould use and do additional work with. Perhaps something like this:\n\n NEWTREE=$(git merge-tree --real $BRANCH1 $BRANCH2)\n test $? -eq 0 || die \"There were conflicts...\"\n NEWCOMMIT=$(git commit-tree $NEWTREE -p $BRANCH1 $BRANCH2)\n git update-ref $BRANCH1 $NEWCOMMIT\n\n\nElijah Newren (8):\n merge-tree: rename merge_trees() to trivial_merge_trees()\n merge-tree: move logic for existing merge into new function\n merge-tree: add option parsing and initial shell for real merge\n function\n merge-tree: implement real merges\n merge-ort: split out a separate display_update_messages() function\n merge-ort: allow update messages to be written to different file\n stream\n merge-tree: support saving merge messages to a separate file\n merge-tree: provide an easy way to access which files have conflicts\n\n Documentation/git-merge-tree.txt | 32 +++++--\n builtin/merge-tree.c | 152 ++++++++++++++++++++++++++++---\n git.c | 2 +-\n merge-ort.c | 85 ++++++++++-------\n merge-ort.h | 12 +++\n t/t4301-merge-tree-real.sh | 108 ++++++++++++++++++++++\n 6 files changed, 333 insertions(+), 58 deletions(-)\n create mode 100755 t/t4301-merge-tree-real.sh\n\n\nbase-commit: 2ae0a9cb8298185a94e5998086f380a355dd8907\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1114%2Fnewren%2Fmerge-into-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1114/newren/merge-into-v1\nPull-Request: https://github.com/git/git/pull/1114\n-- \ngitgitgadget\n"},{"id":"445271","messageId":"9da8e77c1d7c3645fdad74080c0093f420dcfef4.1640927044.git.gitgitgadget@gmail.com","threadId":"57169","inReplyTo":"pull.1114.git.git.1640927044.gitgitgadget@gmail.com","subject":"[PATCH 2/8] merge-tree: move logic for existing merge into new function","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-12-31T05:03:58Z","receivedAt":"2021-12-31T05:04:15Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nIn preparation for adding a non-trivial merge capability to merge-tree,\nmove the existing merge logic for trivial merges into a new function.\n\nSigned-off-by: Elijah Newren \n---\n builtin/merge-tree.c | 13 ++++++++-----\n 1 file changed, 8 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex 06f9eee9f78..9fe5b99f623 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -366,15 +366,11 @@ static void *get_tree_descriptor(struct repository *r,\n \treturn buf;\n }\n \n-int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n-{\n+static int trivial_merge(int argc, const char **argv) {\n \tstruct repository *r = the_repository;\n \tstruct tree_desc t[3];\n \tvoid *buf1, *buf2, *buf3;\n \n-\tif (argc != 4)\n-\t\tusage(merge_tree_usage);\n-\n \tbuf1 = get_tree_descriptor(r, t+0, argv[1]);\n \tbuf2 = get_tree_descriptor(r, t+1, argv[2]);\n \tbuf3 = get_tree_descriptor(r, t+2, argv[3]);\n@@ -386,3 +382,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \tshow_result();\n \treturn 0;\n }\n+\n+int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n+{\n+\tif (argc != 4)\n+\t\tusage(merge_tree_usage);\n+\treturn trivial_merge(argc, argv);\n+}\n-- \ngitgitgadget\n\n"},{"id":"445272","messageId":"9d03d3f56ab9ab01281b63cc2ecff195ab8089ae.1640927044.git.gitgitgadget@gmail.com","threadId":"57169","inReplyTo":"pull.1114.git.git.1640927044.gitgitgadget@gmail.com","subject":"[PATCH 3/8] merge-tree: add option parsing and initial shell for real merge function","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-12-31T05:03:59Z","receivedAt":"2021-12-31T05:04:16Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nLet merge-tree accept a `--real` parameter for choosing real merges\ninstead of trivial merges. Note that real merges differ from trivial\nmerges in that they handle:\n - three way content merges\n - recursive ancestor consolidation\n - renames\n - proper directory/file conflict handling\n - etc.\nBasically all the stuff you'd expect from `git merge`, just without\nupdating the index and working tree. The initial shell added here does\nnothing more than die with \"real merges are not yet implemented\", but\nthat will be fixed in subsequent commits.\n\nSigned-off-by: Elijah Newren \n---\n builtin/merge-tree.c | 56 +++++++++++++++++++++++++++++++++++++-------\n git.c | 2 +-\n 2 files changed, 48 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex 9fe5b99f623..f04b1eaad0a 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -3,13 +3,12 @@\n #include \"tree-walk.h\"\n #include \"xdiff-interface.h\"\n #include \"object-store.h\"\n+#include \"parse-options.h\"\n #include \"repository.h\"\n #include \"blob.h\"\n #include \"exec-cmd.h\"\n #include \"merge-blobs.h\"\n \n-static const char merge_tree_usage[] = \"git merge-tree \";\n-\n struct merge_list {\n \tstruct merge_list *next;\n \tstruct merge_list *link;\t/* other stages for this object */\n@@ -366,14 +365,16 @@ static void *get_tree_descriptor(struct repository *r,\n \treturn buf;\n }\n \n-static int trivial_merge(int argc, const char **argv) {\n+static int trivial_merge(const char *base,\n+\t\t\t const char *branch1,\n+\t\t\t const char *branch2) {\n \tstruct repository *r = the_repository;\n \tstruct tree_desc t[3];\n \tvoid *buf1, *buf2, *buf3;\n \n-\tbuf1 = get_tree_descriptor(r, t+0, argv[1]);\n-\tbuf2 = get_tree_descriptor(r, t+1, argv[2]);\n-\tbuf3 = get_tree_descriptor(r, t+2, argv[3]);\n+\tbuf1 = get_tree_descriptor(r, t+0, base);\n+\tbuf2 = get_tree_descriptor(r, t+1, branch1);\n+\tbuf3 = get_tree_descriptor(r, t+2, branch2);\n \ttrivial_merge_trees(t, \"\");\n \tfree(buf1);\n \tfree(buf2);\n@@ -383,9 +384,46 @@ static int trivial_merge(int argc, const char **argv) {\n \treturn 0;\n }\n \n+struct merge_tree_options {\n+\tint real;\n+};\n+\n+static int real_merge(struct merge_tree_options *o,\n+\t\t const char *branch1, const char *branch2)\n+{\n+\tdie(_(\"real merges are not yet implemented\"));\n+}\n+\n int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n {\n-\tif (argc != 4)\n-\t\tusage(merge_tree_usage);\n-\treturn trivial_merge(argc, argv);\n+\tstruct merge_tree_options o = { 0 };\n+\tint expected_remaining_argc;\n+\n+\tconst char * const merge_tree_usage[] = {\n+\t\tN_(\"git merge-tree --real \"),\n+\t\tN_(\"git merge-tree \"),\n+\t\tNULL\n+\t};\n+\tstruct option mt_options[] = {\n+\t\tOPT_BOOL(0, \"real\", &o.real,\n+\t\t\t N_(\"do a real merge instead of a trivial merge\")),\n+\t\tOPT_END()\n+\t};\n+\n+\t/* Check for a request for basic help */\n+\tif (argc == 2 && !strcmp(argv[1], \"-h\"))\n+\t\tusage_with_options(merge_tree_usage, mt_options);\n+\n+\t/* Parse arguments */\n+\targc = parse_options(argc, argv, prefix, mt_options,\n+\t\t\t merge_tree_usage, 0);\n+\texpected_remaining_argc = (o.real ? 2 : 3);\n+\tif (argc != expected_remaining_argc)\n+\t\tusage_with_options(merge_tree_usage, mt_options);\n+\n+\t/* Do the relevant type of merge */\n+\tif (o.real)\n+\t\treturn real_merge(&o, argv[0], argv[1]);\n+\telse\n+\t\treturn trivial_merge(argv[0], argv[1], argv[2]);\n }\ndiff --git a/git.c b/git.c\nindex 7edafd8ecff..0124c053878 100644\n--- a/git.c\n+++ b/git.c\n@@ -561,7 +561,7 @@ static struct cmd_struct commands[] = {\n \t{ \"merge-recursive-ours\", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT },\n \t{ \"merge-recursive-theirs\", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT },\n \t{ \"merge-subtree\", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT },\n-\t{ \"merge-tree\", cmd_merge_tree, RUN_SETUP | NO_PARSEOPT },\n+\t{ \"merge-tree\", cmd_merge_tree, RUN_SETUP },\n \t{ \"mktag\", cmd_mktag, RUN_SETUP | NO_PARSEOPT },\n \t{ \"mktree\", cmd_mktree, RUN_SETUP },\n \t{ \"multi-pack-index\", cmd_multi_pack_index, RUN_SETUP },\n-- \ngitgitgadget\n\n"},{"id":"445273","messageId":"9fc71f4511b163bec53616d82e8fe5214facf060.1640927044.git.gitgitgadget@gmail.com","threadId":"57169","inReplyTo":"pull.1114.git.git.1640927044.gitgitgadget@gmail.com","subject":"[PATCH 4/8] merge-tree: implement real merges","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-12-31T05:04:00Z","receivedAt":"2021-12-31T05:04:21Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nThis adds the ability to perform real merges rather than just trivial\nmerges (meaning handling three way content merges, recursive ancestor\nconsolidation, renames, proper directory/file conflict handling, and so\nforth). However, unlike `git merge`, the working tree and index are\nleft alone and no branch is updated.\n\nThe only output is:\n - the toplevel resulting tree printed on stdout\n - exit status of 0 (clean) or 1 (conflicts present)\n\nThis output is mean to be used by some higher level script, perhaps in a\nsequence of steps like this:\n\n NEWTREE=$(git merge-tree --real $BRANCH1 $BRANCH2)\n test $? -eq 0 || die \"There were conflicts...\"\n NEWCOMMIT=$(git commit-tree $NEWTREE -p $BRANCH1 $BRANCH2)\n git update-ref $BRANCH1 $NEWCOMMIT\n\nNote that higher level scripts may also want to access the\nconflict/warning messages normally output during a merge, or have quick\naccess to a list of files with conflicts. That is not available in this\npreliminary implementation, but subsequent commits will add that\nability.\n\nSigned-off-by: Elijah Newren \n---\n Documentation/git-merge-tree.txt | 28 +++++++----\n builtin/merge-tree.c | 55 +++++++++++++++++++++-\n t/t4301-merge-tree-real.sh | 81 ++++++++++++++++++++++++++++++++\n 3 files changed, 153 insertions(+), 11 deletions(-)\n create mode 100755 t/t4301-merge-tree-real.sh\n\ndiff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\nindex 58731c19422..5823938937f 100644\n--- a/Documentation/git-merge-tree.txt\n+++ b/Documentation/git-merge-tree.txt\n@@ -3,26 +3,34 @@ git-merge-tree(1)\n \n NAME\n ----\n-git-merge-tree - Show three-way merge without touching index\n+git-merge-tree - Perform merge without touching index or working tree\n \n \n SYNOPSIS\n --------\n [verse]\n+'git merge-tree' --real \n 'git merge-tree' \n \n DESCRIPTION\n -----------\n-Reads three tree-ish, and output trivial merge results and\n-conflicting stages to the standard output. This is similar to\n-what three-way 'git read-tree -m' does, but instead of storing the\n-results in the index, the command outputs the entries to the\n-standard output.\n+Performs a merge, but does not make any new commits and does not read\n+from or write to either the working tree or index.\n \n-This is meant to be used by higher level scripts to compute\n-merge results outside of the index, and stuff the results back into the\n-index. For this reason, the output from the command omits\n-entries that match the tree.\n+The first form will merge the two branches, doing a full recursive\n+merge with rename detection. If the merge is clean, the exit status\n+will be `0`, and if the merge has conflicts, the exit status will be\n+`1`. The output will consist solely of the resulting toplevel tree\n+(which may have files including conflict markers).\n+\n+The second form is meant for backward compatibility and will only do a\n+trival merge. It reads three tree-ish, and outputs trivial merge\n+results and conflicting stages to the standard output in a semi-diff\n+format. Since this was designed for higher level scripts to consume\n+and merge the results back into the index, it omits entries that match\n+. The result of this second form is is similar to what\n+three-way 'git read-tree -m' does, but instead of storing the results\n+in the index, the command outputs the entries to the standard output.\n \n GIT\n ---\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex f04b1eaad0a..c5757bed5bb 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -2,6 +2,9 @@\n #include \"builtin.h\"\n #include \"tree-walk.h\"\n #include \"xdiff-interface.h\"\n+#include \"help.h\"\n+#include \"commit-reach.h\"\n+#include \"merge-ort.h\"\n #include \"object-store.h\"\n #include \"parse-options.h\"\n #include \"repository.h\"\n@@ -391,7 +394,57 @@ struct merge_tree_options {\n static int real_merge(struct merge_tree_options *o,\n \t\t const char *branch1, const char *branch2)\n {\n-\tdie(_(\"real merges are not yet implemented\"));\n+\tstruct commit *parent1, *parent2;\n+\tstruct commit_list *common;\n+\tstruct commit_list *merge_bases = NULL;\n+\tstruct commit_list *j;\n+\tstruct merge_options opt;\n+\tstruct merge_result result = { 0 };\n+\n+\tparent1 = get_merge_parent(branch1);\n+\tif (!parent1)\n+\t\thelp_unknown_ref(branch1, \"merge\",\n+\t\t\t\t _(\"not something we can merge\"));\n+\n+\tparent2 = get_merge_parent(branch2);\n+\tif (!parent2)\n+\t\thelp_unknown_ref(branch2, \"merge\",\n+\t\t\t\t _(\"not something we can merge\"));\n+\n+\tinit_merge_options(&opt, the_repository);\n+\t/*\n+\t * TODO: Support subtree and other -X options?\n+\tif (use_strategies_nr == 1 &&\n+\t !strcmp(use_strategies[0]->name, \"subtree\"))\n+\t\topt.subtree_shift = \"\";\n+\tfor (x = 0; x < xopts_nr; x++)\n+\t\tif (parse_merge_opt(&opt, xopts[x]))\n+\t\t\tdie(_(\"Unknown strategy option: -X%s\"), xopts[x]);\n+\t*/\n+\n+\topt.show_rename_progress = 0;\n+\n+\topt.branch1 = merge_remote_util(parent1)->name; /* or just branch1? */\n+\topt.branch2 = merge_remote_util(parent2)->name; /* or just 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+\tcommon = get_merge_bases(parent1, parent2);\n+\tfor (j = common; j; j = j->next)\n+\t\tcommit_list_insert(j->item, &merge_bases);\n+\n+\t/*\n+\t * TODO: notify if merging unrelated histories?\n+\tif (!common)\n+\t\tfprintf(stderr, _(\"merging unrelated histories\"));\n+\t */\n+\n+\tmerge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);\n+\tprintf(\"%s\\n\", oid_to_hex(&result.tree->object.oid));\n+\tmerge_switch_to_result(&opt, NULL, &result, 0, 0);\n+\treturn result.clean ? 0 : 1;\n }\n \n int cmd_merge_tree(int argc, const char **argv, const char *prefix)\ndiff --git a/t/t4301-merge-tree-real.sh b/t/t4301-merge-tree-real.sh\nnew file mode 100755\nindex 00000000000..9fb617ccc7f\n--- /dev/null\n+++ b/t/t4301-merge-tree-real.sh\n@@ -0,0 +1,81 @@\n+#!/bin/sh\n+\n+test_description='git merge-tree --real'\n+\n+. ./test-lib.sh\n+\n+# This test is ort-specific\n+GIT_TEST_MERGE_ALGORITHM=ort\n+export GIT_TEST_MERGE_ALGORITHM\n+\n+test_expect_success setup '\n+\ttest_write_lines 1 2 3 4 5 >numbers &&\n+\techo hello >greeting &&\n+\techo foo >whatever &&\n+\tgit add numbers greeting whatever &&\n+\tgit commit -m initial &&\n+\n+\tgit branch side1 &&\n+\tgit branch side2 &&\n+\n+\tgit checkout side1 &&\n+\ttest_write_lines 1 2 3 4 5 6 >numbers &&\n+\techo hi >greeting &&\n+\techo bar >whatever &&\n+\tgit add numbers greeting whatever &&\n+\tgit commit -m rename-and-modify &&\n+\n+\tgit checkout side2 &&\n+\ttest_write_lines 0 1 2 3 4 5 >numbers &&\n+\techo yo >greeting &&\n+\tgit rm whatever &&\n+\tmkdir whatever &&\n+\t>whatever/empty &&\n+\tgit add numbers greeting whatever/empty &&\n+\tgit commit -m remove-and-rename\n+'\n+\n+test_expect_success 'Content merge and a few conflicts' '\n+\tgit checkout side1^0 &&\n+\ttest_must_fail git merge side2 &&\n+\tcp .git/AUTO_MERGE EXPECT &&\n+\tE_TREE=$(cat EXPECT) &&\n+\n+\tgit reset --hard &&\n+\ttest_must_fail git merge-tree --real side1 side2 >RESULT &&\n+\tR_TREE=$(cat RESULT) &&\n+\n+\t# Due to differences of e.g. \"HEAD\" vs \"side1\", the results will not\n+\t# exactly match. Dig into individual files.\n+\n+\t# Numbers should have three-way merged cleanly\n+\ttest_write_lines 0 1 2 3 4 5 6 >expect &&\n+\tgit show ${R_TREE}:numbers >actual &&\n+\ttest_cmp expect actual &&\n+\n+\t# whatever and whatever~ should have same HASHES\n+\tgit rev-parse ${E_TREE}:whatever ${E_TREE}:whatever~HEAD >expect &&\n+\tgit rev-parse ${R_TREE}:whatever ${R_TREE}:whatever~side1 >actual &&\n+\ttest_cmp expect actual &&\n+\n+\t# greeting should have a merge conflict\n+\tgit show ${E_TREE}:greeting >tmp &&\n+\tcat tmp | sed -e s/HEAD/side1/ >expect &&\n+\tgit show ${R_TREE}:greeting >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'Barf on misspelled option' '\n+\t# Mis-spell with single \"s\" instead of double \"s\"\n+\ttest_expect_code 129 git merge-tree --real --mesages FOOBAR side1 side2 2>expect &&\n+\n+\tgrep \"error: unknown option.*mesages\" expect\n+'\n+\n+test_expect_success 'Barf on too many arguments' '\n+\ttest_expect_code 129 git merge-tree --real side1 side2 side3 2>expect &&\n+\n+\tgrep \"^usage: git merge-tree\" expect\n+'\n+\n+test_done\n-- \ngitgitgadget\n\n"},{"id":"445274","messageId":"aa816e766e9eb747be466bba3b74439aadc3332b.1640927044.git.gitgitgadget@gmail.com","threadId":"57169","inReplyTo":"pull.1114.git.git.1640927044.gitgitgadget@gmail.com","subject":"[PATCH 5/8] merge-ort: split out a separate display_update_messages() function","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-12-31T05:04:01Z","receivedAt":"2021-12-31T05:04:22Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nNo functional changes included in this patch; it's just a preparatory\nstep in anticipation of wanting to handle the printed messages\ndifferently in `git merge-tree --real`.\n\nSigned-off-by: Elijah Newren \n---\n merge-ort.c | 69 ++++++++++++++++++++++++++++-------------------------\n merge-ort.h | 8 +++++++\n 2 files changed, 44 insertions(+), 33 deletions(-)\n\ndiff --git a/merge-ort.c b/merge-ort.c\nindex 0342f104836..6237e2fb7fe 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -4197,6 +4197,42 @@ static int record_conflicted_index_entries(struct merge_options *opt)\n \treturn errs;\n }\n \n+void merge_display_update_messages(struct merge_options *opt,\n+\t\t\t\t struct merge_result *result)\n+{\n+\tstruct merge_options_internal *opti = result->priv;\n+\tstruct hashmap_iter iter;\n+\tstruct strmap_entry *e;\n+\tstruct string_list olist = STRING_LIST_INIT_NODUP;\n+\tint i;\n+\n+\ttrace2_region_enter(\"merge\", \"display messages\", opt->repo);\n+\n+\t/* Hack to pre-allocate olist to the desired size */\n+\tALLOC_GROW(olist.items, strmap_get_size(&opti->output),\n+\t\t olist.alloc);\n+\n+\t/* Put every entry from output into olist, then sort */\n+\tstrmap_for_each_entry(&opti->output, &iter, e) {\n+\t\tstring_list_append(&olist, e->key)->util = e->value;\n+\t}\n+\tstring_list_sort(&olist);\n+\n+\t/* Iterate over the items, printing them */\n+\tfor (i = 0; i < olist.nr; ++i) {\n+\t\tstruct strbuf *sb = olist.items[i].util;\n+\n+\t\tprintf(\"%s\", sb->buf);\n+\t}\n+\tstring_list_clear(&olist, 0);\n+\n+\t/* Also include needed rename limit adjustment now */\n+\tdiff_warn_rename_limit(\"merge.renamelimit\",\n+\t\t\t opti->renames.needed_limit, 0);\n+\n+\ttrace2_region_leave(\"merge\", \"display messages\", opt->repo);\n+}\n+\n void merge_switch_to_result(struct merge_options *opt,\n \t\t\t struct tree *head,\n \t\t\t struct merge_result *result,\n@@ -4235,39 +4271,6 @@ void merge_switch_to_result(struct merge_options *opt,\n \t\ttrace2_region_leave(\"merge\", \"write_auto_merge\", opt->repo);\n \t}\n \n-\tif (display_update_msgs) {\n-\t\tstruct merge_options_internal *opti = result->priv;\n-\t\tstruct hashmap_iter iter;\n-\t\tstruct strmap_entry *e;\n-\t\tstruct string_list olist = STRING_LIST_INIT_NODUP;\n-\t\tint i;\n-\n-\t\ttrace2_region_enter(\"merge\", \"display messages\", opt->repo);\n-\n-\t\t/* Hack to pre-allocate olist to the desired size */\n-\t\tALLOC_GROW(olist.items, strmap_get_size(&opti->output),\n-\t\t\t olist.alloc);\n-\n-\t\t/* Put every entry from output into olist, then sort */\n-\t\tstrmap_for_each_entry(&opti->output, &iter, e) {\n-\t\t\tstring_list_append(&olist, e->key)->util = e->value;\n-\t\t}\n-\t\tstring_list_sort(&olist);\n-\n-\t\t/* Iterate over the items, printing them */\n-\t\tfor (i = 0; i < olist.nr; ++i) {\n-\t\t\tstruct strbuf *sb = olist.items[i].util;\n-\n-\t\t\tprintf(\"%s\", sb->buf);\n-\t\t}\n-\t\tstring_list_clear(&olist, 0);\n-\n-\t\t/* Also include needed rename limit adjustment now */\n-\t\tdiff_warn_rename_limit(\"merge.renamelimit\",\n-\t\t\t\t opti->renames.needed_limit, 0);\n-\n-\t\ttrace2_region_leave(\"merge\", \"display messages\", opt->repo);\n-\t}\n \n \tmerge_finalize(opt, result);\n }\ndiff --git a/merge-ort.h b/merge-ort.h\nindex c011864ffeb..1b93555a60b 100644\n--- a/merge-ort.h\n+++ b/merge-ort.h\n@@ -70,6 +70,14 @@ void merge_switch_to_result(struct merge_options *opt,\n \t\t\t int update_worktree_and_index,\n \t\t\t int display_update_msgs);\n \n+/*\n+ * Display messages about conflicts and which files were 3-way merged.\n+ * Automatically called by merge_switch_to_result() with stream == stdout,\n+ * so only call this when bypassing merge_switch_to_result().\n+ */\n+void merge_display_update_messages(struct merge_options *opt,\n+\t\t\t\t struct merge_result *result);\n+\n /* Do needed cleanup when not calling merge_switch_to_result() */\n void merge_finalize(struct merge_options *opt,\n \t\t struct merge_result *result);\n-- \ngitgitgadget\n\n"},{"id":"445275","messageId":"1d24a4f4070de81e7850cc220cd784116ec33718.1640927044.git.gitgitgadget@gmail.com","threadId":"57169","inReplyTo":"pull.1114.git.git.1640927044.gitgitgadget@gmail.com","subject":"[PATCH 8/8] merge-tree: provide an easy way to access which files have conflicts","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-12-31T05:04:04Z","receivedAt":"2021-12-31T05:04:23Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nCallers of `git merge-tree --real` might want an easy way to determine\nwhich files conflicted. While they could potentially use the --messages\noption and parse the resulting messages written to that file, those\nmessages are not meant to be machine readable. Provide a simpler\nmechanism of having the user specify --unmerged-list=$FILENAME, and then\nwrite a NUL-separated list of unmerged filenames to the specified file.\n\nSigned-off-by: Elijah Newren \n---\n Documentation/git-merge-tree.txt | 6 ++++--\n builtin/merge-tree.c | 16 ++++++++++++++++\n merge-ort.c | 13 +++++++++++++\n merge-ort.h | 3 +++\n t/t4301-merge-tree-real.sh | 9 +++++++++\n 5 files changed, 45 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\nindex 4d5857b390b..542cea1a1a8 100644\n--- a/Documentation/git-merge-tree.txt\n+++ b/Documentation/git-merge-tree.txt\n@@ -9,7 +9,7 @@ git-merge-tree - Perform merge without touching index or working tree\n SYNOPSIS\n --------\n [verse]\n-'git merge-tree' --real [--messages=] \n+'git merge-tree' --real [--messages=] [--conflicted-list=] \n 'git merge-tree' \n \n DESCRIPTION\n@@ -23,7 +23,9 @@ will be `0`, and if the merge has conflicts, the exit status will be\n `1`. The output will consist solely of the resulting toplevel tree\n (which may have files including conflict markers). With `--messages`,\n it will write any informational messages (such as \"Auto-merging\n-\" and conflict notices) to the given file.\n+\" and conflict notices) to the given file. With\n+`--conflicted-list`, it will write a list of unmerged files, one per\n+line, to the given file.\n \n The second form is meant for backward compatibility and will only do a\n trival merge. It reads three tree-ish, and outputs trivial merge\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex 47deef0b199..90bd1e92135 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -390,6 +390,7 @@ static int trivial_merge(const char *base,\n struct merge_tree_options {\n \tint real;\n \tchar *messages_file;\n+\tchar *conflicted_file;\n };\n \n static int real_merge(struct merge_tree_options *o,\n@@ -449,6 +450,19 @@ static int real_merge(struct merge_tree_options *o,\n \t\tmerge_display_update_messages(&opt, &result, fp);\n \t\tfclose(fp);\n \t}\n+\tif (o->conflicted_file) {\n+\t\tstruct string_list conflicted_files = STRING_LIST_INIT_NODUP;\n+\t\tFILE *fp = xfopen(o->conflicted_file, \"w\");\n+\t\tint i;\n+\n+\t\tmerge_get_conflicted_files(&result, &conflicted_files);\n+\t\tfor (i = 0; i < conflicted_files.nr; i++) {\n+\t\t\tfprintf(fp, \"%s\", conflicted_files.items[i].string);\n+\t\t\tfputc('\\0', fp);\n+\t\t}\n+\t\tfclose(fp);\n+\t\tstring_list_clear(&conflicted_files, 0);\n+\t}\n \tprintf(\"%s\\n\", oid_to_hex(&result.tree->object.oid));\n \n \tmerge_finalize(&opt, &result);\n@@ -471,6 +485,8 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"do a real merge instead of a trivial merge\")),\n \t\tOPT_STRING(0, \"messages\", &o.messages_file, N_(\"file\"),\n \t\t\t N_(\"filename to write informational/conflict messages to\")),\n+\t\tOPT_STRING(0, \"conflicted-list\", &o.conflicted_file, N_(\"file\"),\n+\t\t\t N_(\"filename to write list of unmerged files\")),\n \t\tOPT_END()\n \t};\n \ndiff --git a/merge-ort.c b/merge-ort.c\nindex 86eebf39166..3d6dd1b234c 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -4234,6 +4234,19 @@ void merge_display_update_messages(struct merge_options *opt,\n \ttrace2_region_leave(\"merge\", \"display messages\", opt->repo);\n }\n \n+void merge_get_conflicted_files(struct merge_result *result,\n+\t\t\t\tstruct string_list *conflicted_files)\n+{\n+\tstruct hashmap_iter iter;\n+\tstruct strmap_entry *e;\n+\tstruct merge_options_internal *opti = result->priv;\n+\n+\tstrmap_for_each_entry(&opti->conflicted, &iter, e) {\n+\t\tstring_list_append(conflicted_files, e->key);\n+\t}\n+\tstring_list_sort(conflicted_files);\n+}\n+\n void merge_switch_to_result(struct merge_options *opt,\n \t\t\t struct tree *head,\n \t\t\t struct merge_result *result,\ndiff --git a/merge-ort.h b/merge-ort.h\nindex 55819a57da8..165cef6616f 100644\n--- a/merge-ort.h\n+++ b/merge-ort.h\n@@ -79,6 +79,9 @@ void merge_display_update_messages(struct merge_options *opt,\n \t\t\t\t struct merge_result *result,\n \t\t\t\t FILE *stream);\n \n+void merge_get_conflicted_files(struct merge_result *result,\n+\t\t\t\tstruct string_list *conflicted_files);\n+\n /* Do needed cleanup when not calling merge_switch_to_result() */\n void merge_finalize(struct merge_options *opt,\n \t\t struct merge_result *result);\ndiff --git a/t/t4301-merge-tree-real.sh b/t/t4301-merge-tree-real.sh\nindex 42218cdc019..0b725eef9fc 100755\n--- a/t/t4301-merge-tree-real.sh\n+++ b/t/t4301-merge-tree-real.sh\n@@ -96,4 +96,13 @@ test_expect_success '--messages gives us the conflict notices and such' '\n \ttest_cmp expect MSG_FILE\n '\n \n+test_expect_success '--messages gives us the conflict notices and such' '\n+\ttest_must_fail git merge-tree --real --conflicted-list=UNMERGED side1 side2 &&\n+\n+\tcat UNMERGED | tr \"\\0\" \"\\n\" >actual &&\n+\ttest_write_lines greeting whatever~side1 >expect &&\n+\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \ngitgitgadget\n"},{"id":"445276","messageId":"777de92d9f166793cddbb383f497518a5dedb9f4.1640927044.git.gitgitgadget@gmail.com","threadId":"57169","inReplyTo":"pull.1114.git.git.1640927044.gitgitgadget@gmail.com","subject":"[PATCH 7/8] merge-tree: support saving merge messages to a separate file","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-12-31T05:04:03Z","receivedAt":"2021-12-31T05:04:27Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nWhen running `git merge-tree --real`, we previously would only return an\nexit status reflecting the cleanness of a merge, and print out the\ntoplevel tree of the resulting merge. Merges also have informational\nmessages, (\"Auto-merging \", \"CONFLICT (content): ...\", \"CONFLICT\n(file/directory)\", etc.) In fact, when non-content conflicts occur\n(such as file/directory, modify/delete, add/add with differing modes,\nrename/rename (1to2), etc.), these informational messages are often the\nonly notification since these conflicts are not representable in the\ncontents of the file.\n\nAdd a --messages option which names a file so that callers can request\nthese messages be recorded somewhere.\n\nSigned-off-by: Elijah Newren \n---\n Documentation/git-merge-tree.txt | 6 ++++--\n builtin/merge-tree.c | 18 ++++++++++++++++--\n t/t4301-merge-tree-real.sh | 18 ++++++++++++++++++\n 3 files changed, 38 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\nindex 5823938937f..4d5857b390b 100644\n--- a/Documentation/git-merge-tree.txt\n+++ b/Documentation/git-merge-tree.txt\n@@ -9,7 +9,7 @@ git-merge-tree - Perform merge without touching index or working tree\n SYNOPSIS\n --------\n [verse]\n-'git merge-tree' --real \n+'git merge-tree' --real [--messages=] \n 'git merge-tree' \n \n DESCRIPTION\n@@ -21,7 +21,9 @@ The first form will merge the two branches, doing a full recursive\n merge with rename detection. If the merge is clean, the exit status\n will be `0`, and if the merge has conflicts, the exit status will be\n `1`. The output will consist solely of the resulting toplevel tree\n-(which may have files including conflict markers).\n+(which may have files including conflict markers). With `--messages`,\n+it will write any informational messages (such as \"Auto-merging\n+\" and conflict notices) to the given file.\n \n The second form is meant for backward compatibility and will only do a\n trival merge. It reads three tree-ish, and outputs trivial merge\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex c5757bed5bb..47deef0b199 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -389,6 +389,7 @@ static int trivial_merge(const char *base,\n \n struct merge_tree_options {\n \tint real;\n+\tchar *messages_file;\n };\n \n static int real_merge(struct merge_tree_options *o,\n@@ -442,8 +443,15 @@ static int real_merge(struct merge_tree_options *o,\n \t */\n \n \tmerge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);\n+\n+\tif (o->messages_file) {\n+\t\tFILE *fp = xfopen(o->messages_file, \"w\");\n+\t\tmerge_display_update_messages(&opt, &result, fp);\n+\t\tfclose(fp);\n+\t}\n \tprintf(\"%s\\n\", oid_to_hex(&result.tree->object.oid));\n-\tmerge_switch_to_result(&opt, NULL, &result, 0, 0);\n+\n+\tmerge_finalize(&opt, &result);\n \treturn result.clean ? 0 : 1;\n }\n \n@@ -451,15 +459,18 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n {\n \tstruct merge_tree_options o = { 0 };\n \tint expected_remaining_argc;\n+\tint original_argc;\n \n \tconst char * const merge_tree_usage[] = {\n-\t\tN_(\"git merge-tree --real \"),\n+\t\tN_(\"git merge-tree --real [] \"),\n \t\tN_(\"git merge-tree \"),\n \t\tNULL\n \t};\n \tstruct option mt_options[] = {\n \t\tOPT_BOOL(0, \"real\", &o.real,\n \t\t\t N_(\"do a real merge instead of a trivial merge\")),\n+\t\tOPT_STRING(0, \"messages\", &o.messages_file, N_(\"file\"),\n+\t\t\t N_(\"filename to write informational/conflict messages to\")),\n \t\tOPT_END()\n \t};\n \n@@ -468,8 +479,11 @@ 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/* Parse arguments */\n+\toriginal_argc = argc;\n \targc = parse_options(argc, argv, prefix, mt_options,\n \t\t\t merge_tree_usage, 0);\n+\tif (!o.real && original_argc < argc)\n+\t\tdie(_(\"--real must be specified if any other options are\"));\n \texpected_remaining_argc = (o.real ? 2 : 3);\n \tif (argc != expected_remaining_argc)\n \t\tusage_with_options(merge_tree_usage, mt_options);\ndiff --git a/t/t4301-merge-tree-real.sh b/t/t4301-merge-tree-real.sh\nindex 9fb617ccc7f..42218cdc019 100755\n--- a/t/t4301-merge-tree-real.sh\n+++ b/t/t4301-merge-tree-real.sh\n@@ -78,4 +78,22 @@ test_expect_success 'Barf on too many arguments' '\n \tgrep \"^usage: git merge-tree\" expect\n '\n \n+test_expect_success '--messages gives us the conflict notices and such' '\n+\ttest_must_fail git merge-tree --real --messages=MSG_FILE side1 side2 &&\n+\n+\t# Expected results:\n+\t# \"greeting\" should merge with conflicts\n+\t# \"numbers\" should merge cleanly\n+\t# \"whatever\" has *both* a modify/delete and a file/directory conflict\n+\tcat <<-EOF >expect &&\n+\tAuto-merging greeting\n+\tCONFLICT (content): Merge conflict in greeting\n+\tAuto-merging numbers\n+\tCONFLICT (file/directory): directory in the way of whatever from side1; moving it to whatever~side1 instead.\n+\tCONFLICT (modify/delete): whatever~side1 deleted in side2 and modified in side1. Version side1 of whatever~side1 left in tree.\n+\tEOF\n+\n+\ttest_cmp expect MSG_FILE\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"445277","messageId":"32ad5b5c10da7204dc4a2d3ca74f8d73745925a7.1640927044.git.gitgitgadget@gmail.com","threadId":"57169","inReplyTo":"pull.1114.git.git.1640927044.gitgitgadget@gmail.com","subject":"[PATCH 6/8] merge-ort: allow update messages to be written to different file stream","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-12-31T05:04:02Z","receivedAt":"2021-12-31T05:04:29Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nThis modifies the new display_update_messages() function to allow\nprinting to somewhere other than stdout.\n\nSigned-off-by: Elijah Newren \n---\n merge-ort.c | 7 +++++--\n merge-ort.h | 3 ++-\n 2 files changed, 7 insertions(+), 3 deletions(-)\n\ndiff --git a/merge-ort.c b/merge-ort.c\nindex 6237e2fb7fe..86eebf39166 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -4198,7 +4198,8 @@ static int record_conflicted_index_entries(struct merge_options *opt)\n }\n \n void merge_display_update_messages(struct merge_options *opt,\n-\t\t\t\t struct merge_result *result)\n+\t\t\t\t struct merge_result *result,\n+\t\t\t\t FILE *stream)\n {\n \tstruct merge_options_internal *opti = result->priv;\n \tstruct hashmap_iter iter;\n@@ -4222,7 +4223,7 @@ void merge_display_update_messages(struct merge_options *opt,\n \tfor (i = 0; i < olist.nr; ++i) {\n \t\tstruct strbuf *sb = olist.items[i].util;\n \n-\t\tprintf(\"%s\", sb->buf);\n+\t\tfprintf(stream, \"%s\", sb->buf);\n \t}\n \tstring_list_clear(&olist, 0);\n \n@@ -4271,6 +4272,8 @@ void merge_switch_to_result(struct merge_options *opt,\n \t\ttrace2_region_leave(\"merge\", \"write_auto_merge\", opt->repo);\n \t}\n \n+\tif (display_update_msgs)\n+\t\tmerge_display_update_messages(opt, result, stdout);\n \n \tmerge_finalize(opt, result);\n }\ndiff --git a/merge-ort.h b/merge-ort.h\nindex 1b93555a60b..55819a57da8 100644\n--- a/merge-ort.h\n+++ b/merge-ort.h\n@@ -76,7 +76,8 @@ void merge_switch_to_result(struct merge_options *opt,\n * so only call this when bypassing merge_switch_to_result().\n */\n void merge_display_update_messages(struct merge_options *opt,\n-\t\t\t\t struct merge_result *result);\n+\t\t\t\t struct merge_result *result,\n+\t\t\t\t FILE *stream);\n \n /* Do needed cleanup when not calling merge_switch_to_result() */\n void merge_finalize(struct merge_options *opt,\n-- \ngitgitgadget\n\n"},{"id":"445315","messageId":"20220101200824.isvinnb2zmobhfqq@gmail.com","threadId":"57169","inReplyTo":"9fc71f4511b163bec53616d82e8fe5214facf060.1640927044.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 4/8] merge-tree: implement real merges","fromName":"Johannes Altmanninger","fromEmail":"aclopte@gmail.com","sentAt":"2022-01-01T20:08:24Z","receivedAt":"2022-01-01T20:08:41Z","isPatch":true,"sender":{"key":"aclopte@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6853872?v=4"},"body":"On Fri, Dec 31, 2021 at 05:04:00AM +0000, Elijah Newren via GitGitGadget wrote:\n> From: Elijah Newren \n> \n> This adds the ability to perform real merges rather than just trivial\n> merges (meaning handling three way content merges, recursive ancestor\n> consolidation, renames, proper directory/file conflict handling, and so\n> forth). However, unlike `git merge`, the working tree and index are\n> left alone and no branch is updated.\n> \n> The only output is:\n> - the toplevel resulting tree printed on stdout\n> - exit status of 0 (clean) or 1 (conflicts present)\n> \n> This output is mean to be used by some higher level script, perhaps in a\n> sequence of steps like this:\n> \n> NEWTREE=$(git merge-tree --real $BRANCH1 $BRANCH2)\n> test $? -eq 0 || die \"There were conflicts...\"\n> NEWCOMMIT=$(git commit-tree $NEWTREE -p $BRANCH1 $BRANCH2)\n> git update-ref $BRANCH1 $NEWCOMMIT\n> \n> Note that higher level scripts may also want to access the\n> conflict/warning messages normally output during a merge, or have quick\n> access to a list of files with conflicts. That is not available in this\n> preliminary implementation, but subsequent commits will add that\n> ability.\n> \n> Signed-off-by: Elijah Newren \n> ---\n> Documentation/git-merge-tree.txt | 28 +++++++----\n> builtin/merge-tree.c | 55 +++++++++++++++++++++-\n> t/t4301-merge-tree-real.sh | 81 ++++++++++++++++++++++++++++++++\n> 3 files changed, 153 insertions(+), 11 deletions(-)\n> create mode 100755 t/t4301-merge-tree-real.sh\n> \n> diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\n> index 58731c19422..5823938937f 100644\n> --- a/Documentation/git-merge-tree.txt\n> +++ b/Documentation/git-merge-tree.txt\n> @@ -3,26 +3,34 @@ git-merge-tree(1)\n> \n> NAME\n> ----\n> -git-merge-tree - Show three-way merge without touching index\n> +git-merge-tree - Perform merge without touching index or working tree\n> \n> \n> SYNOPSIS\n> --------\n> [verse]\n> +'git merge-tree' --real \n> 'git merge-tree' \n\nThis is really exciting. It could replace the merge-machinery of git-revise\n(which is a \"fast rebase\" tool).\nI think for cherry-pick/rebase we need to specify a custom merge base,\nwould that suit the new form?\n"},{"id":"445316","messageId":"20220101200836.msaewswefs5uvkyq@gmail.com","threadId":"57169","inReplyTo":"32ad5b5c10da7204dc4a2d3ca74f8d73745925a7.1640927044.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 6/8] merge-ort: allow update messages to be written to different file stream","fromName":"Johannes Altmanninger","fromEmail":"aclopte@gmail.com","sentAt":"2022-01-01T20:08:36Z","receivedAt":"2022-01-01T20:08:41Z","isPatch":true,"sender":{"key":"aclopte@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6853872?v=4"},"body":"On Fri, Dec 31, 2021 at 05:04:02AM +0000, Elijah Newren via GitGitGadget wrote:\n> From: Elijah Newren \n> \n> This modifies the new display_update_messages() function to allow\n> printing to somewhere other than stdout.\n> \n> Signed-off-by: Elijah Newren \n> ---\n> merge-ort.c | 7 +++++--\n> merge-ort.h | 3 ++-\n> 2 files changed, 7 insertions(+), 3 deletions(-)\n> \n> diff --git a/merge-ort.c b/merge-ort.c\n> index 6237e2fb7fe..86eebf39166 100644\n> --- a/merge-ort.c\n> +++ b/merge-ort.c\n> [...]\n> @@ -4271,6 +4272,8 @@ void merge_switch_to_result(struct merge_options *opt,\n> \t\ttrace2_region_leave(\"merge\", \"write_auto_merge\", opt->repo);\n> \t}\n> \n> +\tif (display_update_msgs)\n> +\t\tmerge_display_update_messages(opt, result, stdout);\n\nis it intentional that the previous patch doesn't have this call?\n"},{"id":"445317","messageId":"20220101201139.elxj76zr2ihrjkdr@gmail.com","threadId":"57169","inReplyTo":"9da8e77c1d7c3645fdad74080c0093f420dcfef4.1640927044.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/8] merge-tree: move logic for existing merge into new function","fromName":"Johannes Altmanninger","fromEmail":"aclopte@gmail.com","sentAt":"2022-01-01T20:11:39Z","receivedAt":"2022-01-01T20:11:46Z","isPatch":true,"sender":{"key":"aclopte@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6853872?v=4"},"body":"On Fri, Dec 31, 2021 at 05:03:58AM +0000, Elijah Newren via GitGitGadget wrote:\n> From: Elijah Newren \n> \n> In preparation for adding a non-trivial merge capability to merge-tree,\n> move the existing merge logic for trivial merges into a new function.\n> \n> Signed-off-by: Elijah Newren \n> ---\n> builtin/merge-tree.c | 13 ++++++++-----\n> 1 file changed, 8 insertions(+), 5 deletions(-)\n> \n> diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\n> index 06f9eee9f78..9fe5b99f623 100644\n> --- a/builtin/merge-tree.c\n> +++ b/builtin/merge-tree.c\n> @@ -366,15 +366,11 @@ static void *get_tree_descriptor(struct repository *r,\n> \treturn buf;\n> }\n> \n> -int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> -{\n> +static int trivial_merge(int argc, const char **argv) {\n\nI guess the brace should probably stay on its own line\n"},{"id":"445318","messageId":"CABPp-BFyw-=L2UAXuQcdAUDK1UYuOKdMd1UfUav7nUwyhFypnA@mail.gmail.com","threadId":"57169","inReplyTo":"20220101201139.elxj76zr2ihrjkdr@gmail.com","subject":"Re: [PATCH 2/8] merge-tree: move logic for existing merge into new function","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-01-01T20:17:54Z","receivedAt":"2022-01-01T20:18:09Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Sat, Jan 1, 2022 at 12:11 PM Johannes Altmanninger wrote:\n>\n> On Fri, Dec 31, 2021 at 05:03:58AM +0000, Elijah Newren via GitGitGadget wrote:\n> > From: Elijah Newren \n> >\n> > In preparation for adding a non-trivial merge capability to merge-tree,\n> > move the existing merge logic for trivial merges into a new function.\n> >\n> > Signed-off-by: Elijah Newren \n> > ---\n> > builtin/merge-tree.c | 13 ++++++++-----\n> > 1 file changed, 8 insertions(+), 5 deletions(-)\n> >\n> > diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\n> > index 06f9eee9f78..9fe5b99f623 100644\n> > --- a/builtin/merge-tree.c\n> > +++ b/builtin/merge-tree.c\n> > @@ -366,15 +366,11 @@ static void *get_tree_descriptor(struct repository *r,\n> > return buf;\n> > }\n> >\n> > -int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> > -{\n> > +static int trivial_merge(int argc, const char **argv) {\n>\n> I guess the brace should probably stay on its own line\n\nWhoops, indeed. Thanks for spotting.\n"},{"id":"445319","messageId":"CABPp-BE8+kRLtEG2OVULGJ7UWX6FFKdoAU_=sMAbmiXPMuafAA@mail.gmail.com","threadId":"57169","inReplyTo":"20220101200836.msaewswefs5uvkyq@gmail.com","subject":"Re: [PATCH 6/8] merge-ort: allow update messages to be written to different file stream","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-01-01T20:19:13Z","receivedAt":"2022-01-01T20:19:27Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Sat, Jan 1, 2022 at 12:08 PM Johannes Altmanninger wrote:\n>\n> On Fri, Dec 31, 2021 at 05:04:02AM +0000, Elijah Newren via GitGitGadget wrote:\n> > From: Elijah Newren \n> >\n> > This modifies the new display_update_messages() function to allow\n> > printing to somewhere other than stdout.\n> >\n> > Signed-off-by: Elijah Newren \n> > ---\n> > merge-ort.c | 7 +++++--\n> > merge-ort.h | 3 ++-\n> > 2 files changed, 7 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/merge-ort.c b/merge-ort.c\n> > index 6237e2fb7fe..86eebf39166 100644\n> > --- a/merge-ort.c\n> > +++ b/merge-ort.c\n> > [...]\n> > @@ -4271,6 +4272,8 @@ void merge_switch_to_result(struct merge_options *opt,\n> > trace2_region_leave(\"merge\", \"write_auto_merge\", opt->repo);\n> > }\n> >\n> > + if (display_update_msgs)\n> > + merge_display_update_messages(opt, result, stdout);\n>\n> is it intentional that the previous patch doesn't have this call?\n\nUgh, oops. Yeah, bad split of the patches; this should have been part\nof the previous one.\n"},{"id":"445320","messageId":"CABPp-BHpK8hPsiuHoYsf5D_rjcGLSW-_faL3ODoh56pG_2Luwg@mail.gmail.com","threadId":"57169","inReplyTo":"20220101200824.isvinnb2zmobhfqq@gmail.com","subject":"Re: [PATCH 4/8] merge-tree: implement real merges","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-01-01T21:11:07Z","receivedAt":"2022-01-01T21:11:22Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Sat, Jan 1, 2022 at 12:08 PM Johannes Altmanninger wrote:\n>\n> On Fri, Dec 31, 2021 at 05:04:00AM +0000, Elijah Newren via GitGitGadget wrote:\n> > From: Elijah Newren \n> >\n> > This adds the ability to perform real merges rather than just trivial\n> > merges (meaning handling three way content merges, recursive ancestor\n> > consolidation, renames, proper directory/file conflict handling, and so\n> > forth). However, unlike `git merge`, the working tree and index are\n> > left alone and no branch is updated.\n> >\n> > The only output is:\n> > - the toplevel resulting tree printed on stdout\n> > - exit status of 0 (clean) or 1 (conflicts present)\n> >\n> > This output is mean to be used by some higher level script, perhaps in a\n> > sequence of steps like this:\n> >\n> > NEWTREE=$(git merge-tree --real $BRANCH1 $BRANCH2)\n> > test $? -eq 0 || die \"There were conflicts...\"\n> > NEWCOMMIT=$(git commit-tree $NEWTREE -p $BRANCH1 $BRANCH2)\n> > git update-ref $BRANCH1 $NEWCOMMIT\n> >\n> > Note that higher level scripts may also want to access the\n> > conflict/warning messages normally output during a merge, or have quick\n> > access to a list of files with conflicts. That is not available in this\n> > preliminary implementation, but subsequent commits will add that\n> > ability.\n> >\n> > Signed-off-by: Elijah Newren \n> > ---\n> > Documentation/git-merge-tree.txt | 28 +++++++----\n> > builtin/merge-tree.c | 55 +++++++++++++++++++++-\n> > t/t4301-merge-tree-real.sh | 81 ++++++++++++++++++++++++++++++++\n> > 3 files changed, 153 insertions(+), 11 deletions(-)\n> > create mode 100755 t/t4301-merge-tree-real.sh\n> >\n> > diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\n> > index 58731c19422..5823938937f 100644\n> > --- a/Documentation/git-merge-tree.txt\n> > +++ b/Documentation/git-merge-tree.txt\n> > @@ -3,26 +3,34 @@ git-merge-tree(1)\n> >\n> > NAME\n> > ----\n> > -git-merge-tree - Show three-way merge without touching index\n> > +git-merge-tree - Perform merge without touching index or working tree\n> >\n> >\n> > SYNOPSIS\n> > --------\n> > [verse]\n> > +'git merge-tree' --real \n> > 'git merge-tree' \n>\n> This is really exciting. It could replace the merge-machinery of git-revise\n> (which is a \"fast rebase\" tool).\n> I think for cherry-pick/rebase we need to specify a custom merge base,\n> would that suit the new form?\n\nI'm glad you're excited about it. :-)\n\nI think having a server side tool for replaying commits (which can\ndouble as a fast rebase/cherry-pick tool on the client side) is also\nimportant, but I think it should be part of a proper builtin, not some\nscript that calls out to `merge-tree --real`.\n\n`merge-tree --real` is simpler, though, so I implemented and submitted it first.\n"},{"id":"445347","messageId":"20220103121527.rhgteepqvx2may2f@fs","threadId":"57169","inReplyTo":"aa816e766e9eb747be466bba3b74439aadc3332b.1640927044.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 5/8] merge-ort: split out a separate display_update_messages() function","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2022-01-03T12:15:27Z","receivedAt":"2022-01-03T12:15:33Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"On 31.12.2021 05:04, Elijah Newren via GitGitGadget wrote:\n>From: Elijah Newren \n>\n>No functional changes included in this patch; it's just a preparatory\n>step in anticipation of wanting to handle the printed messages\n>differently in `git merge-tree --real`.\n\nNot quite. You are missing:\n + if (display_update_msgs)\n + merge_display_update_messages(opt, result, stdout);\n\nin merge_switch_to_result(), which you added in the next commit. Not really \nrelevant for the series as a whole but this commit alone breaks some \nfunctionality.\n\n>\n>Signed-off-by: Elijah Newren \n>---\n> merge-ort.c | 69 ++++++++++++++++++++++++++++-------------------------\n> merge-ort.h | 8 +++++++\n> 2 files changed, 44 insertions(+), 33 deletions(-)\n>\n>diff --git a/merge-ort.c b/merge-ort.c\n>index 0342f104836..6237e2fb7fe 100644\n>--- a/merge-ort.c\n>+++ b/merge-ort.c\n>@@ -4197,6 +4197,42 @@ static int record_conflicted_index_entries(struct merge_options *opt)\n> \treturn errs;\n> }\n>\n>+void merge_display_update_messages(struct merge_options *opt,\n>+\t\t\t\t struct merge_result *result)\n>+{\n>+\tstruct merge_options_internal *opti = result->priv;\n>+\tstruct hashmap_iter iter;\n>+\tstruct strmap_entry *e;\n>+\tstruct string_list olist = STRING_LIST_INIT_NODUP;\n>+\tint i;\n>+\n>+\ttrace2_region_enter(\"merge\", \"display messages\", opt->repo);\n>+\n>+\t/* Hack to pre-allocate olist to the desired size */\n>+\tALLOC_GROW(olist.items, strmap_get_size(&opti->output),\n>+\t\t olist.alloc);\n>+\n>+\t/* Put every entry from output into olist, then sort */\n>+\tstrmap_for_each_entry(&opti->output, &iter, e) {\n>+\t\tstring_list_append(&olist, e->key)->util = e->value;\n>+\t}\n>+\tstring_list_sort(&olist);\n>+\n>+\t/* Iterate over the items, printing them */\n>+\tfor (i = 0; i < olist.nr; ++i) {\n>+\t\tstruct strbuf *sb = olist.items[i].util;\n>+\n>+\t\tprintf(\"%s\", sb->buf);\n>+\t}\n>+\tstring_list_clear(&olist, 0);\n>+\n>+\t/* Also include needed rename limit adjustment now */\n>+\tdiff_warn_rename_limit(\"merge.renamelimit\",\n>+\t\t\t opti->renames.needed_limit, 0);\n>+\n>+\ttrace2_region_leave(\"merge\", \"display messages\", opt->repo);\n>+}\n>+\n> void merge_switch_to_result(struct merge_options *opt,\n> \t\t\t struct tree *head,\n> \t\t\t struct merge_result *result,\n>@@ -4235,39 +4271,6 @@ void merge_switch_to_result(struct merge_options *opt,\n> \t\ttrace2_region_leave(\"merge\", \"write_auto_merge\", opt->repo);\n> \t}\n>\n>-\tif (display_update_msgs) {\n>-\t\tstruct merge_options_internal *opti = result->priv;\n>-\t\tstruct hashmap_iter iter;\n>-\t\tstruct strmap_entry *e;\n>-\t\tstruct string_list olist = STRING_LIST_INIT_NODUP;\n>-\t\tint i;\n>-\n>-\t\ttrace2_region_enter(\"merge\", \"display messages\", opt->repo);\n>-\n>-\t\t/* Hack to pre-allocate olist to the desired size */\n>-\t\tALLOC_GROW(olist.items, strmap_get_size(&opti->output),\n>-\t\t\t olist.alloc);\n>-\n>-\t\t/* Put every entry from output into olist, then sort */\n>-\t\tstrmap_for_each_entry(&opti->output, &iter, e) {\n>-\t\t\tstring_list_append(&olist, e->key)->util = e->value;\n>-\t\t}\n>-\t\tstring_list_sort(&olist);\n>-\n>-\t\t/* Iterate over the items, printing them */\n>-\t\tfor (i = 0; i < olist.nr; ++i) {\n>-\t\t\tstruct strbuf *sb = olist.items[i].util;\n>-\n>-\t\t\tprintf(\"%s\", sb->buf);\n>-\t\t}\n>-\t\tstring_list_clear(&olist, 0);\n>-\n>-\t\t/* Also include needed rename limit adjustment now */\n>-\t\tdiff_warn_rename_limit(\"merge.renamelimit\",\n>-\t\t\t\t opti->renames.needed_limit, 0);\n>-\n>-\t\ttrace2_region_leave(\"merge\", \"display messages\", opt->repo);\n>-\t}\n>\n> \tmerge_finalize(opt, result);\n> }\n>diff --git a/merge-ort.h b/merge-ort.h\n>index c011864ffeb..1b93555a60b 100644\n>--- a/merge-ort.h\n>+++ b/merge-ort.h\n>@@ -70,6 +70,14 @@ void merge_switch_to_result(struct merge_options *opt,\n> \t\t\t int update_worktree_and_index,\n> \t\t\t int display_update_msgs);\n>\n>+/*\n>+ * Display messages about conflicts and which files were 3-way merged.\n>+ * Automatically called by merge_switch_to_result() with stream == stdout,\n>+ * so only call this when bypassing merge_switch_to_result().\n>+ */\n>+void merge_display_update_messages(struct merge_options *opt,\n>+\t\t\t\t struct merge_result *result);\n>+\n> /* Do needed cleanup when not calling merge_switch_to_result() */\n> void merge_finalize(struct merge_options *opt,\n> \t\t struct merge_result *result);\n>-- \n>gitgitgadget\n>\n"},{"id":"445348","messageId":"20220103122347.uba66kusy3ft7g2h@fs","threadId":"57169","inReplyTo":"9fc71f4511b163bec53616d82e8fe5214facf060.1640927044.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 4/8] merge-tree: implement real merges","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2022-01-03T12:23:47Z","receivedAt":"2022-01-03T12:23:51Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"On 31.12.2021 05:04, Elijah Newren via GitGitGadget wrote:\n>From: Elijah Newren \n>\n>This adds the ability to perform real merges rather than just trivial\n>merges (meaning handling three way content merges, recursive ancestor\n>consolidation, renames, proper directory/file conflict handling, and so\n>forth). However, unlike `git merge`, the working tree and index are\n>left alone and no branch is updated.\n>\n>The only output is:\n> - the toplevel resulting tree printed on stdout\n> - exit status of 0 (clean) or 1 (conflicts present)\n>\n>This output is mean to be used by some higher level script, perhaps in a\n>sequence of steps like this:\n>\n> NEWTREE=$(git merge-tree --real $BRANCH1 $BRANCH2)\n> test $? -eq 0 || die \"There were conflicts...\"\n> NEWCOMMIT=$(git commit-tree $NEWTREE -p $BRANCH1 $BRANCH2)\n> git update-ref $BRANCH1 $NEWCOMMIT\n>\n>Note that higher level scripts may also want to access the\n>conflict/warning messages normally output during a merge, or have quick\n>access to a list of files with conflicts. That is not available in this\n>preliminary implementation, but subsequent commits will add that\n>ability.\n>\n>Signed-off-by: Elijah Newren \n>---\n> Documentation/git-merge-tree.txt | 28 +++++++----\n> builtin/merge-tree.c | 55 +++++++++++++++++++++-\n> t/t4301-merge-tree-real.sh | 81 ++++++++++++++++++++++++++++++++\n> 3 files changed, 153 insertions(+), 11 deletions(-)\n> create mode 100755 t/t4301-merge-tree-real.sh\n>\n>diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\n>index 58731c19422..5823938937f 100644\n>--- a/Documentation/git-merge-tree.txt\n>+++ b/Documentation/git-merge-tree.txt\n>@@ -3,26 +3,34 @@ git-merge-tree(1)\n>\n> NAME\n> ----\n>-git-merge-tree - Show three-way merge without touching index\n>+git-merge-tree - Perform merge without touching index or working tree\n>\n>\n> SYNOPSIS\n> --------\n> [verse]\n>+'git merge-tree' --real \n> 'git merge-tree' \n>\n> DESCRIPTION\n> -----------\n>-Reads three tree-ish, and output trivial merge results and\n>-conflicting stages to the standard output. This is similar to\n>-what three-way 'git read-tree -m' does, but instead of storing the\n>-results in the index, the command outputs the entries to the\n>-standard output.\n>+Performs a merge, but does not make any new commits and does not read\n>+from or write to either the working tree or index.\n>\n>-This is meant to be used by higher level scripts to compute\n>-merge results outside of the index, and stuff the results back into the\n>-index. For this reason, the output from the command omits\n>-entries that match the tree.\n>+The first form will merge the two branches, doing a full recursive\n>+merge with rename detection. If the merge is clean, the exit status\n>+will be `0`, and if the merge has conflicts, the exit status will be\n>+`1`. The output will consist solely of the resulting toplevel tree\n>+(which may have files including conflict markers).\n>+\n>+The second form is meant for backward compatibility and will only do a\n>+trival merge. It reads three tree-ish, and outputs trivial merge\n>+results and conflicting stages to the standard output in a semi-diff\n>+format. Since this was designed for higher level scripts to consume\n>+and merge the results back into the index, it omits entries that match\n>+. The result of this second form is is similar to what\n>+three-way 'git read-tree -m' does, but instead of storing the results\n>+in the index, the command outputs the entries to the standard output.\n>\n> GIT\n> ---\n>diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\n>index f04b1eaad0a..c5757bed5bb 100644\n>--- a/builtin/merge-tree.c\n>+++ b/builtin/merge-tree.c\n>@@ -2,6 +2,9 @@\n> #include \"builtin.h\"\n> #include \"tree-walk.h\"\n> #include \"xdiff-interface.h\"\n>+#include \"help.h\"\n>+#include \"commit-reach.h\"\n>+#include \"merge-ort.h\"\n> #include \"object-store.h\"\n> #include \"parse-options.h\"\n> #include \"repository.h\"\n>@@ -391,7 +394,57 @@ struct merge_tree_options {\n> static int real_merge(struct merge_tree_options *o,\n> \t\t const char *branch1, const char *branch2)\n> {\n>-\tdie(_(\"real merges are not yet implemented\"));\n>+\tstruct commit *parent1, *parent2;\n>+\tstruct commit_list *common;\n>+\tstruct commit_list *merge_bases = NULL;\n>+\tstruct commit_list *j;\n>+\tstruct merge_options opt;\n>+\tstruct merge_result result = { 0 };\n>+\n>+\tparent1 = get_merge_parent(branch1);\n>+\tif (!parent1)\n>+\t\thelp_unknown_ref(branch1, \"merge\",\n>+\t\t\t\t _(\"not something we can merge\"));\n>+\n>+\tparent2 = get_merge_parent(branch2);\n>+\tif (!parent2)\n>+\t\thelp_unknown_ref(branch2, \"merge\",\n>+\t\t\t\t _(\"not something we can merge\"));\n>+\n>+\tinit_merge_options(&opt, the_repository);\n>+\t/*\n>+\t * TODO: Support subtree and other -X options?\n>+\tif (use_strategies_nr == 1 &&\n>+\t !strcmp(use_strategies[0]->name, \"subtree\"))\n>+\t\topt.subtree_shift = \"\";\n>+\tfor (x = 0; x < xopts_nr; x++)\n>+\t\tif (parse_merge_opt(&opt, xopts[x]))\n>+\t\t\tdie(_(\"Unknown strategy option: -X%s\"), xopts[x]);\n>+\t*/\n>+\n>+\topt.show_rename_progress = 0;\n>+\n>+\topt.branch1 = merge_remote_util(parent1)->name; /* or just branch1? */\n>+\topt.branch2 = merge_remote_util(parent2)->name; /* or just 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>+\tcommon = get_merge_bases(parent1, parent2);\n>+\tfor (j = common; j; j = j->next)\n>+\t\tcommit_list_insert(j->item, &merge_bases);\n>+\n>+\t/*\n>+\t * TODO: notify if merging unrelated histories?\n>+\tif (!common)\n>+\t\tfprintf(stderr, _(\"merging unrelated histories\"));\n>+\t */\n>+\n>+\tmerge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);\n>+\tprintf(\"%s\\n\", oid_to_hex(&result.tree->object.oid));\n>+\tmerge_switch_to_result(&opt, NULL, &result, 0, 0);\n>+\treturn result.clean ? 0 : 1;\n> }\n>\n> int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n>diff --git a/t/t4301-merge-tree-real.sh b/t/t4301-merge-tree-real.sh\n>new file mode 100755\n>index 00000000000..9fb617ccc7f\n>--- /dev/null\n>+++ b/t/t4301-merge-tree-real.sh\n>@@ -0,0 +1,81 @@\n>+#!/bin/sh\n>+\n>+test_description='git merge-tree --real'\n>+\n>+. ./test-lib.sh\n>+\n>+# This test is ort-specific\n>+GIT_TEST_MERGE_ALGORITHM=ort\n>+export GIT_TEST_MERGE_ALGORITHM\n>+\n>+test_expect_success setup '\n>+\ttest_write_lines 1 2 3 4 5 >numbers &&\n>+\techo hello >greeting &&\n>+\techo foo >whatever &&\n>+\tgit add numbers greeting whatever &&\n>+\tgit commit -m initial &&\n>+\n>+\tgit branch side1 &&\n>+\tgit branch side2 &&\n>+\n>+\tgit checkout side1 &&\n>+\ttest_write_lines 1 2 3 4 5 6 >numbers &&\n>+\techo hi >greeting &&\n>+\techo bar >whatever &&\n>+\tgit add numbers greeting whatever &&\n>+\tgit commit -m rename-and-modify &&\n\nThe commit implies a rename as well which I think is missing.\n\n>+\n>+\tgit checkout side2 &&\n>+\ttest_write_lines 0 1 2 3 4 5 >numbers &&\n>+\techo yo >greeting &&\n>+\tgit rm whatever &&\n>+\tmkdir whatever &&\n>+\t>whatever/empty &&\n>+\tgit add numbers greeting whatever/empty &&\n>+\tgit commit -m remove-and-rename\n\nAnd this looks more like a remove-and-modify. Is it still a rename when we \nempty the files content?\n\n>+'\n>+\n>+test_expect_success 'Content merge and a few conflicts' '\n>+\tgit checkout side1^0 &&\n>+\ttest_must_fail git merge side2 &&\n>+\tcp .git/AUTO_MERGE EXPECT &&\n>+\tE_TREE=$(cat EXPECT) &&\n>+\n>+\tgit reset --hard &&\n>+\ttest_must_fail git merge-tree --real side1 side2 >RESULT &&\n>+\tR_TREE=$(cat RESULT) &&\n>+\n>+\t# Due to differences of e.g. \"HEAD\" vs \"side1\", the results will not\n>+\t# exactly match. Dig into individual files.\n>+\n>+\t# Numbers should have three-way merged cleanly\n>+\ttest_write_lines 0 1 2 3 4 5 6 >expect &&\n>+\tgit show ${R_TREE}:numbers >actual &&\n>+\ttest_cmp expect actual &&\n>+\n>+\t# whatever and whatever~ should have same HASHES\n>+\tgit rev-parse ${E_TREE}:whatever ${E_TREE}:whatever~HEAD >expect &&\n>+\tgit rev-parse ${R_TREE}:whatever ${R_TREE}:whatever~side1 >actual &&\n>+\ttest_cmp expect actual &&\n>+\n>+\t# greeting should have a merge conflict\n>+\tgit show ${E_TREE}:greeting >tmp &&\n>+\tcat tmp | sed -e s/HEAD/side1/ >expect &&\n>+\tgit show ${R_TREE}:greeting >actual &&\n>+\ttest_cmp expect actual\n>+'\n>+\n>+test_expect_success 'Barf on misspelled option' '\n>+\t# Mis-spell with single \"s\" instead of double \"s\"\n>+\ttest_expect_code 129 git merge-tree --real --mesages FOOBAR side1 side2 2>expect &&\n>+\n>+\tgrep \"error: unknown option.*mesages\" expect\n>+'\n>+\n>+test_expect_success 'Barf on too many arguments' '\n>+\ttest_expect_code 129 git merge-tree --real side1 side2 side3 2>expect &&\n>+\n>+\tgrep \"^usage: git merge-tree\" expect\n>+'\n>+\n>+test_done\n>-- \n>gitgitgadget\n>\n"},{"id":"445349","messageId":"20220103122505.q2unvqsdjomkkurn@fs","threadId":"57169","inReplyTo":"20220103121527.rhgteepqvx2may2f@fs","subject":"Re: [PATCH 5/8] merge-ort: split out a separate display_update_messages() function","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2022-01-03T12:25:05Z","receivedAt":"2022-01-03T12:25:09Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"On 03.01.2022 13:15, Fabian Stelzer wrote:\n>On 31.12.2021 05:04, Elijah Newren via GitGitGadget wrote:\n>>From: Elijah Newren \n>>\n>>No functional changes included in this patch; it's just a preparatory\n>>step in anticipation of wanting to handle the printed messages\n>>differently in `git merge-tree --real`.\n>\n>Not quite. You are missing:\n> + if (display_update_msgs)\n> + merge_display_update_messages(opt, result, stdout);\n>\n>in merge_switch_to_result(), which you added in the next commit. Not \n>really relevant for the series as a whole but this commit alone breaks \n>some functionality.\n\nSorry, just saw this was already noted on the next patch.\n\n>\n>>\n>>Signed-off-by: Elijah Newren \n>>---\n>>merge-ort.c | 69 ++++++++++++++++++++++++++++-------------------------\n>>merge-ort.h | 8 +++++++\n>>2 files changed, 44 insertions(+), 33 deletions(-)\n>>\n>>diff --git a/merge-ort.c b/merge-ort.c\n>>index 0342f104836..6237e2fb7fe 100644\n>>--- a/merge-ort.c\n>>+++ b/merge-ort.c\n>>@@ -4197,6 +4197,42 @@ static int record_conflicted_index_entries(struct merge_options *opt)\n>>\treturn errs;\n>>}\n>>\n>>+void merge_display_update_messages(struct merge_options *opt,\n>>+\t\t\t\t struct merge_result *result)\n>>+{\n>>+\tstruct merge_options_internal *opti = result->priv;\n>>+\tstruct hashmap_iter iter;\n>>+\tstruct strmap_entry *e;\n>>+\tstruct string_list olist = STRING_LIST_INIT_NODUP;\n>>+\tint i;\n>>+\n>>+\ttrace2_region_enter(\"merge\", \"display messages\", opt->repo);\n>>+\n>>+\t/* Hack to pre-allocate olist to the desired size */\n>>+\tALLOC_GROW(olist.items, strmap_get_size(&opti->output),\n>>+\t\t olist.alloc);\n>>+\n>>+\t/* Put every entry from output into olist, then sort */\n>>+\tstrmap_for_each_entry(&opti->output, &iter, e) {\n>>+\t\tstring_list_append(&olist, e->key)->util = e->value;\n>>+\t}\n>>+\tstring_list_sort(&olist);\n>>+\n>>+\t/* Iterate over the items, printing them */\n>>+\tfor (i = 0; i < olist.nr; ++i) {\n>>+\t\tstruct strbuf *sb = olist.items[i].util;\n>>+\n>>+\t\tprintf(\"%s\", sb->buf);\n>>+\t}\n>>+\tstring_list_clear(&olist, 0);\n>>+\n>>+\t/* Also include needed rename limit adjustment now */\n>>+\tdiff_warn_rename_limit(\"merge.renamelimit\",\n>>+\t\t\t opti->renames.needed_limit, 0);\n>>+\n>>+\ttrace2_region_leave(\"merge\", \"display messages\", opt->repo);\n>>+}\n>>+\n>>void merge_switch_to_result(struct merge_options *opt,\n>>\t\t\t struct tree *head,\n>>\t\t\t struct merge_result *result,\n>>@@ -4235,39 +4271,6 @@ void merge_switch_to_result(struct merge_options *opt,\n>>\t\ttrace2_region_leave(\"merge\", \"write_auto_merge\", opt->repo);\n>>\t}\n>>\n>>-\tif (display_update_msgs) {\n>>-\t\tstruct merge_options_internal *opti = result->priv;\n>>-\t\tstruct hashmap_iter iter;\n>>-\t\tstruct strmap_entry *e;\n>>-\t\tstruct string_list olist = STRING_LIST_INIT_NODUP;\n>>-\t\tint i;\n>>-\n>>-\t\ttrace2_region_enter(\"merge\", \"display messages\", opt->repo);\n>>-\n>>-\t\t/* Hack to pre-allocate olist to the desired size */\n>>-\t\tALLOC_GROW(olist.items, strmap_get_size(&opti->output),\n>>-\t\t\t olist.alloc);\n>>-\n>>-\t\t/* Put every entry from output into olist, then sort */\n>>-\t\tstrmap_for_each_entry(&opti->output, &iter, e) {\n>>-\t\t\tstring_list_append(&olist, e->key)->util = e->value;\n>>-\t\t}\n>>-\t\tstring_list_sort(&olist);\n>>-\n>>-\t\t/* Iterate over the items, printing them */\n>>-\t\tfor (i = 0; i < olist.nr; ++i) {\n>>-\t\t\tstruct strbuf *sb = olist.items[i].util;\n>>-\n>>-\t\t\tprintf(\"%s\", sb->buf);\n>>-\t\t}\n>>-\t\tstring_list_clear(&olist, 0);\n>>-\n>>-\t\t/* Also include needed rename limit adjustment now */\n>>-\t\tdiff_warn_rename_limit(\"merge.renamelimit\",\n>>-\t\t\t\t opti->renames.needed_limit, 0);\n>>-\n>>-\t\ttrace2_region_leave(\"merge\", \"display messages\", opt->repo);\n>>-\t}\n>>\n>>\tmerge_finalize(opt, result);\n>>}\n>>diff --git a/merge-ort.h b/merge-ort.h\n>>index c011864ffeb..1b93555a60b 100644\n>>--- a/merge-ort.h\n>>+++ b/merge-ort.h\n>>@@ -70,6 +70,14 @@ void merge_switch_to_result(struct merge_options *opt,\n>>\t\t\t int update_worktree_and_index,\n>>\t\t\t int display_update_msgs);\n>>\n>>+/*\n>>+ * Display messages about conflicts and which files were 3-way merged.\n>>+ * Automatically called by merge_switch_to_result() with stream == stdout,\n>>+ * so only call this when bypassing merge_switch_to_result().\n>>+ */\n>>+void merge_display_update_messages(struct merge_options *opt,\n>>+\t\t\t\t struct merge_result *result);\n>>+\n>>/* Do needed cleanup when not calling merge_switch_to_result() */\n>>void merge_finalize(struct merge_options *opt,\n>>\t\t struct merge_result *result);\n>>-- \n>>gitgitgadget\n>>\n"},{"id":"445350","messageId":"20220103123114.uuvpk4nley22gfkg@fs","threadId":"57169","inReplyTo":"777de92d9f166793cddbb383f497518a5dedb9f4.1640927044.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 7/8] merge-tree: support saving merge messages to a separate file","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2022-01-03T12:31:14Z","receivedAt":"2022-01-03T12:31:19Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"On 31.12.2021 05:04, Elijah Newren via GitGitGadget wrote:\n>From: Elijah Newren \n>\n>When running `git merge-tree --real`, we previously would only return an\n>exit status reflecting the cleanness of a merge, and print out the\n>toplevel tree of the resulting merge. Merges also have informational\n>messages, (\"Auto-merging \", \"CONFLICT (content): ...\", \"CONFLICT\n>(file/directory)\", etc.) In fact, when non-content conflicts occur\n>(such as file/directory, modify/delete, add/add with differing modes,\n>rename/rename (1to2), etc.), these informational messages are often the\n>only notification since these conflicts are not representable in the\n>contents of the file.\n>\n>Add a --messages option which names a file so that callers can request\n>these messages be recorded somewhere.\n>\n>Signed-off-by: Elijah Newren \n>---\n> Documentation/git-merge-tree.txt | 6 ++++--\n> builtin/merge-tree.c | 18 ++++++++++++++++--\n> t/t4301-merge-tree-real.sh | 18 ++++++++++++++++++\n> 3 files changed, 38 insertions(+), 4 deletions(-)\n>\n>diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\n>index 5823938937f..4d5857b390b 100644\n>--- a/Documentation/git-merge-tree.txt\n>+++ b/Documentation/git-merge-tree.txt\n>@@ -9,7 +9,7 @@ git-merge-tree - Perform merge without touching index or working tree\n> SYNOPSIS\n> --------\n> [verse]\n>-'git merge-tree' --real \n>+'git merge-tree' --real [--messages=] \n> 'git merge-tree' \n>\n> DESCRIPTION\n>@@ -21,7 +21,9 @@ The first form will merge the two branches, doing a full recursive\n> merge with rename detection. If the merge is clean, the exit status\n> will be `0`, and if the merge has conflicts, the exit status will be\n> `1`. The output will consist solely of the resulting toplevel tree\n>-(which may have files including conflict markers).\n>+(which may have files including conflict markers). With `--messages`,\n>+it will write any informational messages (such as \"Auto-merging\n>+\" and conflict notices) to the given file.\n>\n> The second form is meant for backward compatibility and will only do a\n> trival merge. It reads three tree-ish, and outputs trivial merge\n>diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\n>index c5757bed5bb..47deef0b199 100644\n>--- a/builtin/merge-tree.c\n>+++ b/builtin/merge-tree.c\n>@@ -389,6 +389,7 @@ static int trivial_merge(const char *base,\n>\n> struct merge_tree_options {\n> \tint real;\n>+\tchar *messages_file;\n> };\n>\n> static int real_merge(struct merge_tree_options *o,\n>@@ -442,8 +443,15 @@ static int real_merge(struct merge_tree_options *o,\n> \t */\n>\n> \tmerge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);\n>+\n>+\tif (o->messages_file) {\n>+\t\tFILE *fp = xfopen(o->messages_file, \"w\");\n>+\t\tmerge_display_update_messages(&opt, &result, fp);\n>+\t\tfclose(fp);\n\nI don't know enough about how merge-ort works internally, but it looks to me\nlike at this point the merge already happened and we just didn't clean up \n(finalize) yet. It feels wrong to die() at this point just because we can't \nopen messages_file.\n\n>+\t}\n> \tprintf(\"%s\\n\", oid_to_hex(&result.tree->object.oid));\n>-\tmerge_switch_to_result(&opt, NULL, &result, 0, 0);\n>+\n>+\tmerge_finalize(&opt, &result);\n> \treturn result.clean ? 0 : 1;\n> }\n>\n>@@ -451,15 +459,18 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> {\n> \tstruct merge_tree_options o = { 0 };\n> \tint expected_remaining_argc;\n>+\tint original_argc;\n>\n> \tconst char * const merge_tree_usage[] = {\n>-\t\tN_(\"git merge-tree --real \"),\n>+\t\tN_(\"git merge-tree --real [] \"),\n> \t\tN_(\"git merge-tree \"),\n> \t\tNULL\n> \t};\n> \tstruct option mt_options[] = {\n> \t\tOPT_BOOL(0, \"real\", &o.real,\n> \t\t\t N_(\"do a real merge instead of a trivial merge\")),\n>+\t\tOPT_STRING(0, \"messages\", &o.messages_file, N_(\"file\"),\n>+\t\t\t N_(\"filename to write informational/conflict messages to\")),\n> \t\tOPT_END()\n> \t};\n>\n>@@ -468,8 +479,11 @@ 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/* Parse arguments */\n>+\toriginal_argc = argc;\n> \targc = parse_options(argc, argv, prefix, mt_options,\n> \t\t\t merge_tree_usage, 0);\n>+\tif (!o.real && original_argc < argc)\n>+\t\tdie(_(\"--real must be specified if any other options are\"));\n> \texpected_remaining_argc = (o.real ? 2 : 3);\n> \tif (argc != expected_remaining_argc)\n> \t\tusage_with_options(merge_tree_usage, mt_options);\n>diff --git a/t/t4301-merge-tree-real.sh b/t/t4301-merge-tree-real.sh\n>index 9fb617ccc7f..42218cdc019 100755\n>--- a/t/t4301-merge-tree-real.sh\n>+++ b/t/t4301-merge-tree-real.sh\n>@@ -78,4 +78,22 @@ test_expect_success 'Barf on too many arguments' '\n> \tgrep \"^usage: git merge-tree\" expect\n> '\n>\n>+test_expect_success '--messages gives us the conflict notices and such' '\n>+\ttest_must_fail git merge-tree --real --messages=MSG_FILE side1 side2 &&\n>+\n>+\t# Expected results:\n>+\t# \"greeting\" should merge with conflicts\n>+\t# \"numbers\" should merge cleanly\n>+\t# \"whatever\" has *both* a modify/delete and a file/directory conflict\n>+\tcat <<-EOF >expect &&\n>+\tAuto-merging greeting\n>+\tCONFLICT (content): Merge conflict in greeting\n>+\tAuto-merging numbers\n>+\tCONFLICT (file/directory): directory in the way of whatever from side1; moving it to whatever~side1 instead.\n>+\tCONFLICT (modify/delete): whatever~side1 deleted in side2 and modified in side1. Version side1 of whatever~side1 left in tree.\n>+\tEOF\n>+\n>+\ttest_cmp expect MSG_FILE\n>+'\n>+\n> test_done\n>-- \n>gitgitgadget\n>\n"},{"id":"445351","messageId":"20220103123539.kldjq3hrcagqjzwc@fs","threadId":"57169","inReplyTo":"777de92d9f166793cddbb383f497518a5dedb9f4.1640927044.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 7/8] merge-tree: support saving merge messages to a separate file","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2022-01-03T12:35:39Z","receivedAt":"2022-01-03T12:35:43Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"On 31.12.2021 05:04, Elijah Newren via GitGitGadget wrote:\n>From: Elijah Newren \n>\n>When running `git merge-tree --real`, we previously would only return an\n>exit status reflecting the cleanness of a merge, and print out the\n>toplevel tree of the resulting merge. Merges also have informational\n>messages, (\"Auto-merging \", \"CONFLICT (content): ...\", \"CONFLICT\n>(file/directory)\", etc.) In fact, when non-content conflicts occur\n>(such as file/directory, modify/delete, add/add with differing modes,\n>rename/rename (1to2), etc.), these informational messages are often the\n>only notification since these conflicts are not representable in the\n>contents of the file.\n>\n>Add a --messages option which names a file so that callers can request\n>these messages be recorded somewhere.\n>\n>Signed-off-by: Elijah Newren \n>---\n> Documentation/git-merge-tree.txt | 6 ++++--\n> builtin/merge-tree.c | 18 ++++++++++++++++--\n> t/t4301-merge-tree-real.sh | 18 ++++++++++++++++++\n> 3 files changed, 38 insertions(+), 4 deletions(-)\n>\n>diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\n>index 5823938937f..4d5857b390b 100644\n>--- a/Documentation/git-merge-tree.txt\n>+++ b/Documentation/git-merge-tree.txt\n>@@ -9,7 +9,7 @@ git-merge-tree - Perform merge without touching index or working tree\n> SYNOPSIS\n> --------\n> [verse]\n>-'git merge-tree' --real \n>+'git merge-tree' --real [--messages=] \n> 'git merge-tree' \n>\n> DESCRIPTION\n>@@ -21,7 +21,9 @@ The first form will merge the two branches, doing a full recursive\n> merge with rename detection. If the merge is clean, the exit status\n> will be `0`, and if the merge has conflicts, the exit status will be\n> `1`. The output will consist solely of the resulting toplevel tree\n>-(which may have files including conflict markers).\n>+(which may have files including conflict markers). With `--messages`,\n>+it will write any informational messages (such as \"Auto-merging\n>+\" and conflict notices) to the given file.\n>\n> The second form is meant for backward compatibility and will only do a\n> trival merge. It reads three tree-ish, and outputs trivial merge\n>diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\n>index c5757bed5bb..47deef0b199 100644\n>--- a/builtin/merge-tree.c\n>+++ b/builtin/merge-tree.c\n>@@ -389,6 +389,7 @@ static int trivial_merge(const char *base,\n>\n> struct merge_tree_options {\n> \tint real;\n>+\tchar *messages_file;\n> };\n>\n> static int real_merge(struct merge_tree_options *o,\n>@@ -442,8 +443,15 @@ static int real_merge(struct merge_tree_options *o,\n> \t */\n>\n> \tmerge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);\n>+\n>+\tif (o->messages_file) {\n>+\t\tFILE *fp = xfopen(o->messages_file, \"w\");\n>+\t\tmerge_display_update_messages(&opt, &result, fp);\n>+\t\tfclose(fp);\n>+\t}\n\nSomething else I just wondered. Can the user differentiate between the die()\nin xfopen() and a failed/unclean merge?\nBoth just exit(1) don't they?\n\n> \tprintf(\"%s\\n\", oid_to_hex(&result.tree->object.oid));\n>-\tmerge_switch_to_result(&opt, NULL, &result, 0, 0);\n>+\n>+\tmerge_finalize(&opt, &result);\n> \treturn result.clean ? 0 : 1;\n> }\n>\n>@@ -451,15 +459,18 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> {\n> \tstruct merge_tree_options o = { 0 };\n> \tint expected_remaining_argc;\n>+\tint original_argc;\n>\n> \tconst char * const merge_tree_usage[] = {\n>-\t\tN_(\"git merge-tree --real \"),\n>+\t\tN_(\"git merge-tree --real [] \"),\n> \t\tN_(\"git merge-tree \"),\n> \t\tNULL\n> \t};\n> \tstruct option mt_options[] = {\n> \t\tOPT_BOOL(0, \"real\", &o.real,\n> \t\t\t N_(\"do a real merge instead of a trivial merge\")),\n>+\t\tOPT_STRING(0, \"messages\", &o.messages_file, N_(\"file\"),\n>+\t\t\t N_(\"filename to write informational/conflict messages to\")),\n> \t\tOPT_END()\n> \t};\n>\n>@@ -468,8 +479,11 @@ 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/* Parse arguments */\n>+\toriginal_argc = argc;\n> \targc = parse_options(argc, argv, prefix, mt_options,\n> \t\t\t merge_tree_usage, 0);\n>+\tif (!o.real && original_argc < argc)\n>+\t\tdie(_(\"--real must be specified if any other options are\"));\n> \texpected_remaining_argc = (o.real ? 2 : 3);\n> \tif (argc != expected_remaining_argc)\n> \t\tusage_with_options(merge_tree_usage, mt_options);\n>diff --git a/t/t4301-merge-tree-real.sh b/t/t4301-merge-tree-real.sh\n>index 9fb617ccc7f..42218cdc019 100755\n>--- a/t/t4301-merge-tree-real.sh\n>+++ b/t/t4301-merge-tree-real.sh\n>@@ -78,4 +78,22 @@ test_expect_success 'Barf on too many arguments' '\n> \tgrep \"^usage: git merge-tree\" expect\n> '\n>\n>+test_expect_success '--messages gives us the conflict notices and such' '\n>+\ttest_must_fail git merge-tree --real --messages=MSG_FILE side1 side2 &&\n>+\n>+\t# Expected results:\n>+\t# \"greeting\" should merge with conflicts\n>+\t# \"numbers\" should merge cleanly\n>+\t# \"whatever\" has *both* a modify/delete and a file/directory conflict\n>+\tcat <<-EOF >expect &&\n>+\tAuto-merging greeting\n>+\tCONFLICT (content): Merge conflict in greeting\n>+\tAuto-merging numbers\n>+\tCONFLICT (file/directory): directory in the way of whatever from side1; moving it to whatever~side1 instead.\n>+\tCONFLICT (modify/delete): whatever~side1 deleted in side2 and modified in side1. Version side1 of whatever~side1 left in tree.\n>+\tEOF\n>+\n>+\ttest_cmp expect MSG_FILE\n>+'\n>+\n> test_done\n>-- \n>gitgitgadget\n>\n"},{"id":"445360","messageId":"CABPp-BHWAfeqPyhBehf=37kfMmfi2LtV2tOrqRwTztcQr4fsDw@mail.gmail.com","threadId":"57169","inReplyTo":"20220103122347.uba66kusy3ft7g2h@fs","subject":"Re: [PATCH 4/8] merge-tree: implement real merges","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-01-03T16:37:29Z","receivedAt":"2022-01-03T16:37:44Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Jan 3, 2022 at 4:23 AM Fabian Stelzer wrote:\n>\n> On 31.12.2021 05:04, Elijah Newren via GitGitGadget wrote:\n> >From: Elijah Newren \n> >\n> >This adds the ability to perform real merges rather than just trivial\n> >merges (meaning handling three way content merges, recursive ancestor\n> >consolidation, renames, proper directory/file conflict handling, and so\n> >forth). However, unlike `git merge`, the working tree and index are\n> >left alone and no branch is updated.\n> >\n...\n> >+test_expect_success setup '\n> >+ test_write_lines 1 2 3 4 5 >numbers &&\n> >+ echo hello >greeting &&\n> >+ echo foo >whatever &&\n> >+ git add numbers greeting whatever &&\n> >+ git commit -m initial &&\n> >+\n> >+ git branch side1 &&\n> >+ git branch side2 &&\n> >+\n> >+ git checkout side1 &&\n> >+ test_write_lines 1 2 3 4 5 6 >numbers &&\n> >+ echo hi >greeting &&\n> >+ echo bar >whatever &&\n> >+ git add numbers greeting whatever &&\n> >+ git commit -m rename-and-modify &&\n>\n> The commit implies a rename as well which I think is missing.\n\nSorry, I revised the testcase (multiple times) and forgot to update\nthis commit message string.\n\n> >+\n> >+ git checkout side2 &&\n> >+ test_write_lines 0 1 2 3 4 5 >numbers &&\n> >+ echo yo >greeting &&\n> >+ git rm whatever &&\n> >+ mkdir whatever &&\n> >+ >whatever/empty &&\n> >+ git add numbers greeting whatever/empty &&\n> >+ git commit -m remove-and-rename\n>\n> And this looks more like a remove-and-modify.\n\nLikewise.\n\n\nI'll fix these up; thanks for pointing them out.\n"},{"id":"445361","messageId":"CABPp-BH4okfDXVC418HwfHVR2_NtbKFBOfyYGZ9mWnABMzSruw@mail.gmail.com","threadId":"57169","inReplyTo":"20220103123114.uuvpk4nley22gfkg@fs","subject":"Re: [PATCH 7/8] merge-tree: support saving merge messages to a separate file","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-01-03T16:51:18Z","receivedAt":"2022-01-03T16:51:32Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Jan 3, 2022 at 4:31 AM Fabian Stelzer wrote:\n>\n> On 31.12.2021 05:04, Elijah Newren via GitGitGadget wrote:\n> >From: Elijah Newren \n> >\n> >When running `git merge-tree --real`, we previously would only return an\n> >exit status reflecting the cleanness of a merge, and print out the\n> >toplevel tree of the resulting merge. Merges also have informational\n> >messages, (\"Auto-merging \", \"CONFLICT (content): ...\", \"CONFLICT\n> >(file/directory)\", etc.) In fact, when non-content conflicts occur\n> >(such as file/directory, modify/delete, add/add with differing modes,\n> >rename/rename (1to2), etc.), these informational messages are often the\n> >only notification since these conflicts are not representable in the\n> >contents of the file.\n> >\n> >Add a --messages option which names a file so that callers can request\n> >these messages be recorded somewhere.\n> >\n> >Signed-off-by: Elijah Newren \n> >---\n> > Documentation/git-merge-tree.txt | 6 ++++--\n> > builtin/merge-tree.c | 18 ++++++++++++++++--\n> > t/t4301-merge-tree-real.sh | 18 ++++++++++++++++++\n> > 3 files changed, 38 insertions(+), 4 deletions(-)\n> >\n> >diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\n> >index 5823938937f..4d5857b390b 100644\n> >--- a/Documentation/git-merge-tree.txt\n> >+++ b/Documentation/git-merge-tree.txt\n> >@@ -9,7 +9,7 @@ git-merge-tree - Perform merge without touching index or working tree\n> > SYNOPSIS\n> > --------\n> > [verse]\n> >-'git merge-tree' --real \n> >+'git merge-tree' --real [--messages=] \n> > 'git merge-tree' \n> >\n> > DESCRIPTION\n> >@@ -21,7 +21,9 @@ The first form will merge the two branches, doing a full recursive\n> > merge with rename detection. If the merge is clean, the exit status\n> > will be `0`, and if the merge has conflicts, the exit status will be\n> > `1`. The output will consist solely of the resulting toplevel tree\n> >-(which may have files including conflict markers).\n> >+(which may have files including conflict markers). With `--messages`,\n> >+it will write any informational messages (such as \"Auto-merging\n> >+\" and conflict notices) to the given file.\n> >\n> > The second form is meant for backward compatibility and will only do a\n> > trival merge. It reads three tree-ish, and outputs trivial merge\n> >diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\n> >index c5757bed5bb..47deef0b199 100644\n> >--- a/builtin/merge-tree.c\n> >+++ b/builtin/merge-tree.c\n> >@@ -389,6 +389,7 @@ static int trivial_merge(const char *base,\n> >\n> > struct merge_tree_options {\n> > int real;\n> >+ char *messages_file;\n> > };\n> >\n> > static int real_merge(struct merge_tree_options *o,\n> >@@ -442,8 +443,15 @@ static int real_merge(struct merge_tree_options *o,\n> > */\n> >\n> > merge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);\n> >+\n> >+ if (o->messages_file) {\n> >+ FILE *fp = xfopen(o->messages_file, \"w\");\n> >+ merge_display_update_messages(&opt, &result, fp);\n> >+ fclose(fp);\n>\n> I don't know enough about how merge-ort works internally, but it looks to me\n> like at this point the merge already happened and we just didn't clean up\n> (finalize) yet. It feels wrong to die() at this point just because we can't\n> open messages_file.\n\nYes, the merge already happened; there now exists a new toplevel tree\n(that nothing references). I'm not sure I understand what's wrong\nwith die'ing here, though. I can't tell if you want to defer the\ndie-ing until later, or just avoid the die-ing and return some kind of\nsuccess despite failing to complete what the user requested.\n\n>\n> >+ }\n> > printf(\"%s\\n\", oid_to_hex(&result.tree->object.oid));\n> >- merge_switch_to_result(&opt, NULL, &result, 0, 0);\n> >+\n> >+ merge_finalize(&opt, &result);\n> > return result.clean ? 0 : 1;\n> > }\n> >\n"},{"id":"445362","messageId":"CABPp-BHGRaJX=fqfQ5vHDGzVJiNHpuU4dZY9h3AN=iPPc90pAA@mail.gmail.com","threadId":"57169","inReplyTo":"20220103123539.kldjq3hrcagqjzwc@fs","subject":"Re: [PATCH 7/8] merge-tree: support saving merge messages to a separate file","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-01-03T16:55:46Z","receivedAt":"2022-01-03T16:56:01Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Jan 3, 2022 at 4:35 AM Fabian Stelzer wrote:\n>\n> On 31.12.2021 05:04, Elijah Newren via GitGitGadget wrote:\n> >From: Elijah Newren \n> >\n> >When running `git merge-tree --real`, we previously would only return an\n> >exit status reflecting the cleanness of a merge, and print out the\n> >toplevel tree of the resulting merge. Merges also have informational\n> >messages, (\"Auto-merging \", \"CONFLICT (content): ...\", \"CONFLICT\n> >(file/directory)\", etc.) In fact, when non-content conflicts occur\n> >(such as file/directory, modify/delete, add/add with differing modes,\n> >rename/rename (1to2), etc.), these informational messages are often the\n> >only notification since these conflicts are not representable in the\n> >contents of the file.\n> >\n> >Add a --messages option which names a file so that callers can request\n> >these messages be recorded somewhere.\n> >\n> >Signed-off-by: Elijah Newren \n> >---\n> > Documentation/git-merge-tree.txt | 6 ++++--\n> > builtin/merge-tree.c | 18 ++++++++++++++++--\n> > t/t4301-merge-tree-real.sh | 18 ++++++++++++++++++\n> > 3 files changed, 38 insertions(+), 4 deletions(-)\n> >\n> >diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\n> >index 5823938937f..4d5857b390b 100644\n> >--- a/Documentation/git-merge-tree.txt\n> >+++ b/Documentation/git-merge-tree.txt\n> >@@ -9,7 +9,7 @@ git-merge-tree - Perform merge without touching index or working tree\n> > SYNOPSIS\n> > --------\n> > [verse]\n> >-'git merge-tree' --real \n> >+'git merge-tree' --real [--messages=] \n> > 'git merge-tree' \n> >\n> > DESCRIPTION\n> >@@ -21,7 +21,9 @@ The first form will merge the two branches, doing a full recursive\n> > merge with rename detection. If the merge is clean, the exit status\n> > will be `0`, and if the merge has conflicts, the exit status will be\n> > `1`. The output will consist solely of the resulting toplevel tree\n> >-(which may have files including conflict markers).\n> >+(which may have files including conflict markers). With `--messages`,\n> >+it will write any informational messages (such as \"Auto-merging\n> >+\" and conflict notices) to the given file.\n> >\n> > The second form is meant for backward compatibility and will only do a\n> > trival merge. It reads three tree-ish, and outputs trivial merge\n> >diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\n> >index c5757bed5bb..47deef0b199 100644\n> >--- a/builtin/merge-tree.c\n> >+++ b/builtin/merge-tree.c\n> >@@ -389,6 +389,7 @@ static int trivial_merge(const char *base,\n> >\n> > struct merge_tree_options {\n> > int real;\n> >+ char *messages_file;\n> > };\n> >\n> > static int real_merge(struct merge_tree_options *o,\n> >@@ -442,8 +443,15 @@ static int real_merge(struct merge_tree_options *o,\n> > */\n> >\n> > merge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);\n> >+\n> >+ if (o->messages_file) {\n> >+ FILE *fp = xfopen(o->messages_file, \"w\");\n> >+ merge_display_update_messages(&opt, &result, fp);\n> >+ fclose(fp);\n> >+ }\n>\n> Something else I just wondered. Can the user differentiate between the die()\n> in xfopen() and a failed/unclean merge?\n> Both just exit(1) don't they?\n\nxfopen() calls die_errno(), which calls die_routine(), which will be\npointing at die_builtin() since we don't change it in\nbuiltin/merge-tree.c, and die_builtin() calls exit(128).\n\nSo, a different error code.\n\nBut good question...perhaps I should mention exit codes other than 0\nand 1 in the documentation of merge-tree for other failures.\n\n>\n> > printf(\"%s\\n\", oid_to_hex(&result.tree->object.oid));\n> >- merge_switch_to_result(&opt, NULL, &result, 0, 0);\n> >+\n> >+ merge_finalize(&opt, &result);\n> > return result.clean ? 0 : 1;\n> > }\n> >\n> >@@ -451,15 +459,18 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> > {\n> > struct merge_tree_options o = { 0 };\n> > int expected_remaining_argc;\n> >+ int original_argc;\n> >\n> > const char * const merge_tree_usage[] = {\n> >- N_(\"git merge-tree --real \"),\n> >+ N_(\"git merge-tree --real [] \"),\n> > N_(\"git merge-tree \"),\n> > NULL\n> > };\n> > struct option mt_options[] = {\n> > OPT_BOOL(0, \"real\", &o.real,\n> > N_(\"do a real merge instead of a trivial merge\")),\n> >+ OPT_STRING(0, \"messages\", &o.messages_file, N_(\"file\"),\n> >+ N_(\"filename to write informational/conflict messages to\")),\n> > OPT_END()\n> > };\n> >\n> >@@ -468,8 +479,11 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> > usage_with_options(merge_tree_usage, mt_options);\n> >\n> > /* Parse arguments */\n> >+ original_argc = argc;\n> > argc = parse_options(argc, argv, prefix, mt_options,\n> > merge_tree_usage, 0);\n> >+ if (!o.real && original_argc < argc)\n> >+ die(_(\"--real must be specified if any other options are\"));\n> > expected_remaining_argc = (o.real ? 2 : 3);\n> > if (argc != expected_remaining_argc)\n> > usage_with_options(merge_tree_usage, mt_options);\n> >diff --git a/t/t4301-merge-tree-real.sh b/t/t4301-merge-tree-real.sh\n> >index 9fb617ccc7f..42218cdc019 100755\n> >--- a/t/t4301-merge-tree-real.sh\n> >+++ b/t/t4301-merge-tree-real.sh\n> >@@ -78,4 +78,22 @@ test_expect_success 'Barf on too many arguments' '\n> > grep \"^usage: git merge-tree\" expect\n> > '\n> >\n> >+test_expect_success '--messages gives us the conflict notices and such' '\n> >+ test_must_fail git merge-tree --real --messages=MSG_FILE side1 side2 &&\n> >+\n> >+ # Expected results:\n> >+ # \"greeting\" should merge with conflicts\n> >+ # \"numbers\" should merge cleanly\n> >+ # \"whatever\" has *both* a modify/delete and a file/directory conflict\n> >+ cat <<-EOF >expect &&\n> >+ Auto-merging greeting\n> >+ CONFLICT (content): Merge conflict in greeting\n> >+ Auto-merging numbers\n> >+ CONFLICT (file/directory): directory in the way of whatever from side1; moving it to whatever~side1 instead.\n> >+ CONFLICT (modify/delete): whatever~side1 deleted in side2 and modified in side1. Version side1 of whatever~side1 left in tree.\n> >+ EOF\n> >+\n> >+ test_cmp expect MSG_FILE\n> >+'\n> >+\n> > test_done\n> >--\n> >gitgitgadget\n> >\n"},{"id":"445364","messageId":"20220103172255.54psh2e5iqzd37sy@fs","threadId":"57169","inReplyTo":"CABPp-BH4okfDXVC418HwfHVR2_NtbKFBOfyYGZ9mWnABMzSruw@mail.gmail.com","subject":"Re: [PATCH 7/8] merge-tree: support saving merge messages to a separate file","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2022-01-03T17:22:55Z","receivedAt":"2022-01-03T17:23:00Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"On 03.01.2022 08:51, Elijah Newren wrote:\n>On Mon, Jan 3, 2022 at 4:31 AM Fabian Stelzer wrote:\n>>\n>> On 31.12.2021 05:04, Elijah Newren via GitGitGadget wrote:\n>> >From: Elijah Newren \n[...]\n>> >\n>> > static int real_merge(struct merge_tree_options *o,\n>> >@@ -442,8 +443,15 @@ static int real_merge(struct merge_tree_options *o,\n>> > */\n>> >\n>> > merge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);\n>> >+\n>> >+ if (o->messages_file) {\n>> >+ FILE *fp = xfopen(o->messages_file, \"w\");\n>> >+ merge_display_update_messages(&opt, &result, fp);\n>> >+ fclose(fp);\n>>\n>> I don't know enough about how merge-ort works internally, but it looks to me\n>> like at this point the merge already happened and we just didn't clean up\n>> (finalize) yet. It feels wrong to die() at this point just because we can't\n>> open messages_file.\n>\n>Yes, the merge already happened; there now exists a new toplevel tree\n>(that nothing references). I'm not sure I understand what's wrong\n>with die'ing here, though. I can't tell if you want to defer the\n>die-ing until later, or just avoid the die-ing and return some kind of\n>success despite failing to complete what the user requested.\n>\n\nI think i would prefer the merge operation to abort before actually merging \nwhen not being able to write its logfile. Otherwise we possibly do a whole \nlot of work that`s inaccessible afterwards isn't it? (since we don`t print \nthe hash)\n\nThanks for your work on this feature. I think this could open a lot of new \npossibilities.\n"},{"id":"445366","messageId":"CABPp-BEneaLnaTVo-Yb7fooLK8sQsDq_MdeBu-R2EporqCSqoQ@mail.gmail.com","threadId":"57169","inReplyTo":"20220103172255.54psh2e5iqzd37sy@fs","subject":"Re: [PATCH 7/8] merge-tree: support saving merge messages to a separate file","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-01-03T19:46:53Z","receivedAt":"2022-01-03T19:47:07Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Jan 3, 2022 at 9:23 AM Fabian Stelzer wrote:\n>\n> On 03.01.2022 08:51, Elijah Newren wrote:\n> >On Mon, Jan 3, 2022 at 4:31 AM Fabian Stelzer wrote:\n> >>\n> >> On 31.12.2021 05:04, Elijah Newren via GitGitGadget wrote:\n> >> >From: Elijah Newren \n> [...]\n> >> >\n> >> > static int real_merge(struct merge_tree_options *o,\n> >> >@@ -442,8 +443,15 @@ static int real_merge(struct merge_tree_options *o,\n> >> > */\n> >> >\n> >> > merge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);\n> >> >+\n> >> >+ if (o->messages_file) {\n> >> >+ FILE *fp = xfopen(o->messages_file, \"w\");\n> >> >+ merge_display_update_messages(&opt, &result, fp);\n> >> >+ fclose(fp);\n> >>\n> >> I don't know enough about how merge-ort works internally, but it looks to me\n> >> like at this point the merge already happened and we just didn't clean up\n> >> (finalize) yet. It feels wrong to die() at this point just because we can't\n> >> open messages_file.\n> >\n> >Yes, the merge already happened; there now exists a new toplevel tree\n> >(that nothing references). I'm not sure I understand what's wrong\n> >with die'ing here, though. I can't tell if you want to defer the\n> >die-ing until later, or just avoid the die-ing and return some kind of\n> >success despite failing to complete what the user requested.\n> >\n>\n> I think i would prefer the merge operation to abort before actually merging\n> when not being able to write its logfile. Otherwise we possibly do a whole\n> lot of work that`s inaccessible afterwards isn't it? (since we don`t print\n> the hash)\n\nI see where you're coming from, but I don't see this as worth worrying\nabout. For two reasons:\n\n(1) I'm not sure I buy the \"whole lot of work\" concern.\n\nmerge-ort is pretty snappy. For a simple example of rebasing a single\npatch in linux.git across a branch with 28000 renames, I get 176\nmilliseconds for merge_incore_nonrecursive(). Granted, linux.git is\npretty small in terms of number of files, but Stolee did some\nmeasurements a while back on the Microsoft repos with millions of\nfiles at HEAD. For those, for a trivial merge he saw\nmerge_incore_recursive() complete in 2 milliseconds, and for a trivial\nrebase he saw merge_incore_nonrecursive() complete in 4 milliseconds\n(See https://lore.kernel.org/git/CABPp-BHO7bZ3H7A=E9TudhvBoNfwPvRiDMm8S9kq3mYeSXrpXw@mail.gmail.com/).\nSo huge numbers of files pose much less of a problem than lots of\ninteresting work like renames, and merge-ort is pretty fast in either\ncase. Sure, if we were talking about traditional merge-recursive\nwhich would have taken 150000 milliseconds on the same single patch in\nlinux.git testcase (due to the 28000 renames), then we might worry\nmore about not letting work get tossed, but at only 176 milliseconds\neven with a crazy number of renames, it's just not worth worrying\nabout.\n\n(2) Even if there is a lot of computation, I don't see why this error\npath merits extra coding work to salvage the computation somehow\n\nBy way of comparison, a regular `git merge` will abort after\ncompleting the same amount of merge work (i.e. after creating a new\ntree) when the user has a dirty working tree involving a path that\nwould need to be updated by the merge operation. And that is not a\nbug; it's a requirement -- we cannot first check if the user has\ndirtied such a path before performing the merge because it's\nimpossible to do so accurately in the face of renames.\nmerge-recursive tried to do that and had early aborts that fell in the\nfalse-positive category and some that fell in the false-negative\ncategory. It was impossible to fix the false-positives and\nfalse-negatives without either (a) disallowing ever doing a merge with\na dirty working tree under any conditions (a backwards compatibility\nbreak), or (b) waiting to do the notification of\ndirty-files-in-the-way until after the merge tree has been computed.\nI wasn't about to break that feature, so merge-ort had to delay error\nnotifications instead.\n\nNow, the dirty-file-in-the-way condition is for a very common case\n(either for users who intentionally like keeping dirty changes around\nand doing merges but the branch they are merging happens to touch a\nfile they didn't know about, or users who just forgot that they had\nlocal modifications). In contrast, this case here is for when we\ncannot open a file for writing -- with the filename explicitly just\nspecified by the user.\n\n\nSo, I'd rather keep the code nice and simple as it currently stands.\n\n> Thanks for your work on this feature. I think this could open a lot of new\n> possibilities.\n\nI hope people do interesting things with it, and with the server-side\ncommit replaying I'm working on as well.\n"},{"id":"445410","messageId":"20220104130546.tk36zg4iju6ivdng@fs","threadId":"57169","inReplyTo":"CABPp-BEneaLnaTVo-Yb7fooLK8sQsDq_MdeBu-R2EporqCSqoQ@mail.gmail.com","subject":"Re: [PATCH 7/8] merge-tree: support saving merge messages to a separate file","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2022-01-04T13:05:46Z","receivedAt":"2022-01-04T13:05:51Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"On 03.01.2022 11:46, Elijah Newren wrote:\n>On Mon, Jan 3, 2022 at 9:23 AM Fabian Stelzer wrote:\n>>\n>> On 03.01.2022 08:51, Elijah Newren wrote:\n>> >On Mon, Jan 3, 2022 at 4:31 AM Fabian Stelzer wrote:\n>> >>\n>> >> On 31.12.2021 05:04, Elijah Newren via GitGitGadget wrote:\n>> >> >From: Elijah Newren \n>> [...]\n>> >> >\n>> >> > static int real_merge(struct merge_tree_options *o,\n>> >> >@@ -442,8 +443,15 @@ static int real_merge(struct merge_tree_options *o,\n>> >> > */\n>> >> >\n>> >> > merge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);\n>> >> >+\n>> >> >+ if (o->messages_file) {\n>> >> >+ FILE *fp = xfopen(o->messages_file, \"w\");\n>> >> >+ merge_display_update_messages(&opt, &result, fp);\n>> >> >+ fclose(fp);\n>> >>\n>> >> I don't know enough about how merge-ort works internally, but it looks to me\n>> >> like at this point the merge already happened and we just didn't clean up\n>> >> (finalize) yet. It feels wrong to die() at this point just because we can't\n>> >> open messages_file.\n>> >\n>> >Yes, the merge already happened; there now exists a new toplevel tree\n>> >(that nothing references). I'm not sure I understand what's wrong\n>> >with die'ing here, though. I can't tell if you want to defer the\n>> >die-ing until later, or just avoid the die-ing and return some kind of\n>> >success despite failing to complete what the user requested.\n>> >\n>>\n>> I think i would prefer the merge operation to abort before actually merging\n>> when not being able to write its logfile. Otherwise we possibly do a whole\n>> lot of work that`s inaccessible afterwards isn't it? (since we don`t print\n>> the hash)\n>\n>I see where you're coming from, but I don't see this as worth worrying\n>about. For two reasons:\n>\n>(1) I'm not sure I buy the \"whole lot of work\" concern.\n>\n>merge-ort is pretty snappy. For a simple example of rebasing a single\n>patch in linux.git across a branch with 28000 renames, I get 176\n>milliseconds for merge_incore_nonrecursive(). Granted, linux.git is\n>pretty small in terms of number of files, but Stolee did some\n>measurements a while back on the Microsoft repos with millions of\n>files at HEAD. For those, for a trivial merge he saw\n>merge_incore_recursive() complete in 2 milliseconds, and for a trivial\n>rebase he saw merge_incore_nonrecursive() complete in 4 milliseconds\n>(See https://lore.kernel.org/git/CABPp-BHO7bZ3H7A=E9TudhvBoNfwPvRiDMm8S9kq3mYeSXrpXw@mail.gmail.com/).\n>So huge numbers of files pose much less of a problem than lots of\n>interesting work like renames, and merge-ort is pretty fast in either\n>case. Sure, if we were talking about traditional merge-recursive\n>which would have taken 150000 milliseconds on the same single patch in\n>linux.git testcase (due to the 28000 renames), then we might worry\n>more about not letting work get tossed, but at only 176 milliseconds\n>even with a crazy number of renames, it's just not worth worrying\n>about.\n\nI guess I'm just not used to how good ort is yet. I was still expecting \ncases like the merge-recursive ones :)\n\n>\n>(2) Even if there is a lot of computation, I don't see why this error\n>path merits extra coding work to salvage the computation somehow\n>\n>By way of comparison, a regular `git merge` will abort after\n>completing the same amount of merge work (i.e. after creating a new\n>tree) when the user has a dirty working tree involving a path that\n>would need to be updated by the merge operation. And that is not a\n>bug; it's a requirement -- we cannot first check if the user has\n>dirtied such a path before performing the merge because it's\n>impossible to do so accurately in the face of renames.\n>merge-recursive tried to do that and had early aborts that fell in the\n>false-positive category and some that fell in the false-negative\n>category. It was impossible to fix the false-positives and\n>false-negatives without either (a) disallowing ever doing a merge with\n>a dirty working tree under any conditions (a backwards compatibility\n>break), or (b) waiting to do the notification of\n>dirty-files-in-the-way until after the merge tree has been computed.\n>I wasn't about to break that feature, so merge-ort had to delay error\n>notifications instead.\n\nCompletely understandable for the regular merge. In this case we are not \ntouching the index/working tree at all though so I don't quite see how this \nis comparable.\n\n>\n>Now, the dirty-file-in-the-way condition is for a very common case\n>(either for users who intentionally like keeping dirty changes around\n>and doing merges but the branch they are merging happens to touch a\n>file they didn't know about, or users who just forgot that they had\n>local modifications). In contrast, this case here is for when we\n>cannot open a file for writing -- with the filename explicitly just\n>specified by the user.\n>\n>\n>So, I'd rather keep the code nice and simple as it currently stands.\n\nEspecially since the extra work penalty is so miniscule i agree.\n\nThanks\n\n>\n>> Thanks for your work on this feature. I think this could open a lot of new\n>> possibilities.\n>\n>I hope people do interesting things with it, and with the server-side\n>commit replaying I'm working on as well.\n"},{"id":"445539","messageId":"pull.1114.v2.git.git.1641403655.gitgitgadget@gmail.com","threadId":"57169","inReplyTo":"pull.1114.git.git.1640927044.gitgitgadget@gmail.com","subject":"[PATCH v2 0/8] RFC: Server side merges (no ref updating, no commit creating, no touching worktree or index)","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-05T17:27:27Z","receivedAt":"2022-01-05T17:27:43Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"(NOTE for Junio: This series has a minor conflict with en/remerge-diff --\nthis series moves a code block into a new function, but en/remerge-diff adds\na BUG() message to that code block. But this series is just RFC, so you may\nwant to wait to pick it up.)\n\nUpdates since v1:\n\n * Fixed a bad patch splitting, and a style issue pointed out by Johannes\n Altimanninger\n * Fixed misleading commit messages in new test cases\n * Fixed my comments about how commit-tree could be used to correctly use\n two -p flags\n\nNOTE2: A preliminary version of this series was discussed here:\nhttps://lore.kernel.org/git/nycvar.QRO.7.76.6.2110211147490.56@tvgsbejvaqbjf.bet/\n\nNOTE3: An alternative has been implemented by Christian, over here:\nhttps://lore.kernel.org/git/20220105163324.73369-1-chriscool@tuxfamily.org/\n\nThis series introduces a new option to git-merge-tree: --real (best name I\ncould come up with). This new option is designed to allow a server-side\n\"real\" merge (or allow folks client-side to do merges with branches they\ndon't even have checked out). Real merges differ from trivial merges in that\nthey handle:\n\n * three way content merges\n * recursive ancestor consolidation\n * renames\n * proper directory/file conflict handling\n * etc.\n\nThe reason this is different from merge is that merge-tree does NOT:\n\n * Read/write/update any working tree (and assumes there probably isn't one)\n * Read/write/update any index (and assumes there probably isn't one)\n * Create a commit object\n * Update any refs\n\nThis series attempts to guess what kind of output would be wanted, basically\nchoosing:\n\n * clean merge or conflict signalled via exit status\n * stdout consists solely of printing the hash of the resulting tree (though\n that tree may include files that have conflict markers)\n * new optional --messages flag for specifying a file where informational\n messages (e.g. conflict notices and files involved in three-way-content\n merges) can be written; by default, this output is simply discarded\n * new optional --conflicted-list flag for specifying a file where the names\n of conflicted-files can be written in a NUL-character-separated list\n\nThis design means it's basically just a low-level tool that other scripts\nwould use and do additional work with. Perhaps something like this:\n\n NEWTREE=$(git merge-tree --real $BRANCH1 $BRANCH2)\n test $? -eq 0 || die \"There were conflicts...\"\n NEWCOMMIT=$(git commit-tree $NEWTREE -p $BRANCH1 -p $BRANCH2)\n git update-ref $BRANCH1 $NEWCOMMIT\n\n\nElijah Newren (8):\n merge-tree: rename merge_trees() to trivial_merge_trees()\n merge-tree: move logic for existing merge into new function\n merge-tree: add option parsing and initial shell for real merge\n function\n merge-tree: implement real merges\n merge-ort: split out a separate display_update_messages() function\n merge-ort: allow update messages to be written to different file\n stream\n merge-tree: support saving merge messages to a separate file\n merge-tree: provide an easy way to access which files have conflicts\n\n Documentation/git-merge-tree.txt | 32 +++++--\n builtin/merge-tree.c | 151 ++++++++++++++++++++++++++++---\n git.c | 2 +-\n merge-ort.c | 85 ++++++++++-------\n merge-ort.h | 12 +++\n t/t4301-merge-tree-real.sh | 108 ++++++++++++++++++++++\n 6 files changed, 333 insertions(+), 57 deletions(-)\n create mode 100755 t/t4301-merge-tree-real.sh\n\n\nbase-commit: 2ae0a9cb8298185a94e5998086f380a355dd8907\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1114%2Fnewren%2Fmerge-into-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1114/newren/merge-into-v2\nPull-Request: https://github.com/git/git/pull/1114\n\nRange-diff vs v1:\n\n 1: a7c7910d834 = 1: a7c7910d834 merge-tree: rename merge_trees() to trivial_merge_trees()\n 2: 9da8e77c1d7 ! 2: aafe67d7c69 merge-tree: move logic for existing merge into new function\n @@ builtin/merge-tree.c: static void *get_tree_descriptor(struct repository *r,\n }\n \n -int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n --{\n -+static int trivial_merge(int argc, const char **argv) {\n ++static int trivial_merge(int argc, const char **argv)\n + {\n \tstruct repository *r = the_repository;\n \tstruct tree_desc t[3];\n \tvoid *buf1, *buf2, *buf3;\n 3: 9d03d3f56ab ! 3: ee21aed0115 merge-tree: add option parsing and initial shell for real merge function\n @@ builtin/merge-tree.c: static void *get_tree_descriptor(struct repository *r,\n \treturn buf;\n }\n \n --static int trivial_merge(int argc, const char **argv) {\n +-static int trivial_merge(int argc, const char **argv)\n +static int trivial_merge(const char *base,\n +\t\t\t const char *branch1,\n -+\t\t\t const char *branch2) {\n ++\t\t\t const char *branch2)\n + {\n \tstruct repository *r = the_repository;\n \tstruct tree_desc t[3];\n \tvoid *buf1, *buf2, *buf3;\n @@ builtin/merge-tree.c: static void *get_tree_descriptor(struct repository *r,\n \ttrivial_merge_trees(t, \"\");\n \tfree(buf1);\n \tfree(buf2);\n -@@ builtin/merge-tree.c: static int trivial_merge(int argc, const char **argv) {\n +@@ builtin/merge-tree.c: static int trivial_merge(int argc, const char **argv)\n \treturn 0;\n }\n \n 4: 9fc71f4511b ! 4: 1710ba4a9e4 merge-tree: implement real merges\n @@ Commit message\n \n NEWTREE=$(git merge-tree --real $BRANCH1 $BRANCH2)\n test $? -eq 0 || die \"There were conflicts...\"\n - NEWCOMMIT=$(git commit-tree $NEWTREE -p $BRANCH1 $BRANCH2)\n + NEWCOMMIT=$(git commit-tree $NEWTREE -p $BRANCH1 -p $BRANCH2)\n git update-ref $BRANCH1 $NEWCOMMIT\n \n Note that higher level scripts may also want to access the\n @@ t/t4301-merge-tree-real.sh (new)\n +\techo hi >greeting &&\n +\techo bar >whatever &&\n +\tgit add numbers greeting whatever &&\n -+\tgit commit -m rename-and-modify &&\n ++\tgit commit -m modify-stuff &&\n +\n +\tgit checkout side2 &&\n +\ttest_write_lines 0 1 2 3 4 5 >numbers &&\n @@ t/t4301-merge-tree-real.sh (new)\n +\tmkdir whatever &&\n +\t>whatever/empty &&\n +\tgit add numbers greeting whatever/empty &&\n -+\tgit commit -m remove-and-rename\n ++\tgit commit -m other-modifications\n +'\n +\n +test_expect_success 'Content merge and a few conflicts' '\n 5: aa816e766e9 ! 5: bc6d01f1a0e merge-ort: split out a separate display_update_messages() function\n @@ merge-ort.c: void merge_switch_to_result(struct merge_options *opt,\n -\n -\t\ttrace2_region_leave(\"merge\", \"display messages\", opt->repo);\n -\t}\n ++\tif (display_update_msgs)\n ++\t\tmerge_display_update_messages(opt, result);\n \n \tmerge_finalize(opt, result);\n }\n 6: 32ad5b5c10d ! 6: c9e95a70d19 merge-ort: allow update messages to be written to different file stream\n @@ merge-ort.c: void merge_display_update_messages(struct merge_options *opt,\n \tstring_list_clear(&olist, 0);\n \n @@ merge-ort.c: void merge_switch_to_result(struct merge_options *opt,\n - \t\ttrace2_region_leave(\"merge\", \"write_auto_merge\", opt->repo);\n \t}\n \n -+\tif (display_update_msgs)\n + \tif (display_update_msgs)\n +-\t\tmerge_display_update_messages(opt, result);\n +\t\tmerge_display_update_messages(opt, result, stdout);\n \n \tmerge_finalize(opt, result);\n 7: 777de92d9f1 = 7: 4b513a6d696 merge-tree: support saving merge messages to a separate file\n 8: 1d24a4f4070 = 8: 01364bb020e merge-tree: provide an easy way to access which files have conflicts\n\n-- \ngitgitgadget\n"},{"id":"445540","messageId":"a7c7910d834b2e46ee6c5063db48f9cf551d0a35.1641403655.git.gitgitgadget@gmail.com","threadId":"57169","inReplyTo":"pull.1114.v2.git.git.1641403655.gitgitgadget@gmail.com","subject":"[PATCH v2 1/8] merge-tree: rename merge_trees() to trivial_merge_trees()","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-05T17:27:28Z","receivedAt":"2022-01-05T17:27:46Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nmerge-recursive.h defined its own merge_trees() function, different than\nthe one found in builtin/merge-tree.c. That was okay in the past, but\nwe want merge-tree to be able to use the merge-ort functions, which will\nend up including merge-recursive.h. Rename the function found in\nbuiltin/merge-tree.c to avoid the conflict.\n\nSigned-off-by: Elijah Newren \n---\n builtin/merge-tree.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex 5dc94d6f880..06f9eee9f78 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -28,7 +28,7 @@ static void add_merge_entry(struct merge_list *entry)\n \tmerge_result_end = &entry->next;\n }\n \n-static void merge_trees(struct tree_desc t[3], const char *base);\n+static void trivial_merge_trees(struct tree_desc t[3], const char *base);\n \n static const char *explanation(struct merge_list *entry)\n {\n@@ -225,7 +225,7 @@ static void unresolved_directory(const struct traverse_info *info,\n \tbuf2 = fill_tree_descriptor(r, t + 2, ENTRY_OID(n + 2));\n #undef ENTRY_OID\n \n-\tmerge_trees(t, newbase);\n+\ttrivial_merge_trees(t, newbase);\n \n \tfree(buf0);\n \tfree(buf1);\n@@ -342,7 +342,7 @@ static int threeway_callback(int n, unsigned long mask, unsigned long dirmask, s\n \treturn mask;\n }\n \n-static void merge_trees(struct tree_desc t[3], const char *base)\n+static void trivial_merge_trees(struct tree_desc t[3], const char *base)\n {\n \tstruct traverse_info info;\n \n@@ -378,7 +378,7 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \tbuf1 = get_tree_descriptor(r, t+0, argv[1]);\n \tbuf2 = get_tree_descriptor(r, t+1, argv[2]);\n \tbuf3 = get_tree_descriptor(r, t+2, argv[3]);\n-\tmerge_trees(t, \"\");\n+\ttrivial_merge_trees(t, \"\");\n \tfree(buf1);\n \tfree(buf2);\n \tfree(buf3);\n-- \ngitgitgadget\n\n"},{"id":"445541","messageId":"aafe67d7c6979aac5764e300ebbe3033fe7103d7.1641403655.git.gitgitgadget@gmail.com","threadId":"57169","inReplyTo":"pull.1114.v2.git.git.1641403655.gitgitgadget@gmail.com","subject":"[PATCH v2 2/8] merge-tree: move logic for existing merge into new function","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-05T17:27:29Z","receivedAt":"2022-01-05T17:27:46Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nIn preparation for adding a non-trivial merge capability to merge-tree,\nmove the existing merge logic for trivial merges into a new function.\n\nSigned-off-by: Elijah Newren \n---\n builtin/merge-tree.c | 12 ++++++++----\n 1 file changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex 06f9eee9f78..914ec960b7e 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -366,15 +366,12 @@ static void *get_tree_descriptor(struct repository *r,\n \treturn buf;\n }\n \n-int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n+static int trivial_merge(int argc, const char **argv)\n {\n \tstruct repository *r = the_repository;\n \tstruct tree_desc t[3];\n \tvoid *buf1, *buf2, *buf3;\n \n-\tif (argc != 4)\n-\t\tusage(merge_tree_usage);\n-\n \tbuf1 = get_tree_descriptor(r, t+0, argv[1]);\n \tbuf2 = get_tree_descriptor(r, t+1, argv[2]);\n \tbuf3 = get_tree_descriptor(r, t+2, argv[3]);\n@@ -386,3 +383,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \tshow_result();\n \treturn 0;\n }\n+\n+int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n+{\n+\tif (argc != 4)\n+\t\tusage(merge_tree_usage);\n+\treturn trivial_merge(argc, argv);\n+}\n-- \ngitgitgadget\n\n"},{"id":"445542","messageId":"ee21aed01153aff39eef2398d32722c3e8cc426d.1641403655.git.gitgitgadget@gmail.com","threadId":"57169","inReplyTo":"pull.1114.v2.git.git.1641403655.gitgitgadget@gmail.com","subject":"[PATCH v2 3/8] merge-tree: add option parsing and initial shell for real merge function","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-05T17:27:30Z","receivedAt":"2022-01-05T17:27:46Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nLet merge-tree accept a `--real` parameter for choosing real merges\ninstead of trivial merges. Note that real merges differ from trivial\nmerges in that they handle:\n - three way content merges\n - recursive ancestor consolidation\n - renames\n - proper directory/file conflict handling\n - etc.\nBasically all the stuff you'd expect from `git merge`, just without\nupdating the index and working tree. The initial shell added here does\nnothing more than die with \"real merges are not yet implemented\", but\nthat will be fixed in subsequent commits.\n\nSigned-off-by: Elijah Newren \n---\n builtin/merge-tree.c | 56 +++++++++++++++++++++++++++++++++++++-------\n git.c | 2 +-\n 2 files changed, 48 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex 914ec960b7e..e1d2832c809 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -3,13 +3,12 @@\n #include \"tree-walk.h\"\n #include \"xdiff-interface.h\"\n #include \"object-store.h\"\n+#include \"parse-options.h\"\n #include \"repository.h\"\n #include \"blob.h\"\n #include \"exec-cmd.h\"\n #include \"merge-blobs.h\"\n \n-static const char merge_tree_usage[] = \"git merge-tree \";\n-\n struct merge_list {\n \tstruct merge_list *next;\n \tstruct merge_list *link;\t/* other stages for this object */\n@@ -366,15 +365,17 @@ static void *get_tree_descriptor(struct repository *r,\n \treturn buf;\n }\n \n-static int trivial_merge(int argc, const char **argv)\n+static int trivial_merge(const char *base,\n+\t\t\t const char *branch1,\n+\t\t\t const char *branch2)\n {\n \tstruct repository *r = the_repository;\n \tstruct tree_desc t[3];\n \tvoid *buf1, *buf2, *buf3;\n \n-\tbuf1 = get_tree_descriptor(r, t+0, argv[1]);\n-\tbuf2 = get_tree_descriptor(r, t+1, argv[2]);\n-\tbuf3 = get_tree_descriptor(r, t+2, argv[3]);\n+\tbuf1 = get_tree_descriptor(r, t+0, base);\n+\tbuf2 = get_tree_descriptor(r, t+1, branch1);\n+\tbuf3 = get_tree_descriptor(r, t+2, branch2);\n \ttrivial_merge_trees(t, \"\");\n \tfree(buf1);\n \tfree(buf2);\n@@ -384,9 +385,46 @@ static int trivial_merge(int argc, const char **argv)\n \treturn 0;\n }\n \n+struct merge_tree_options {\n+\tint real;\n+};\n+\n+static int real_merge(struct merge_tree_options *o,\n+\t\t const char *branch1, const char *branch2)\n+{\n+\tdie(_(\"real merges are not yet implemented\"));\n+}\n+\n int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n {\n-\tif (argc != 4)\n-\t\tusage(merge_tree_usage);\n-\treturn trivial_merge(argc, argv);\n+\tstruct merge_tree_options o = { 0 };\n+\tint expected_remaining_argc;\n+\n+\tconst char * const merge_tree_usage[] = {\n+\t\tN_(\"git merge-tree --real \"),\n+\t\tN_(\"git merge-tree \"),\n+\t\tNULL\n+\t};\n+\tstruct option mt_options[] = {\n+\t\tOPT_BOOL(0, \"real\", &o.real,\n+\t\t\t N_(\"do a real merge instead of a trivial merge\")),\n+\t\tOPT_END()\n+\t};\n+\n+\t/* Check for a request for basic help */\n+\tif (argc == 2 && !strcmp(argv[1], \"-h\"))\n+\t\tusage_with_options(merge_tree_usage, mt_options);\n+\n+\t/* Parse arguments */\n+\targc = parse_options(argc, argv, prefix, mt_options,\n+\t\t\t merge_tree_usage, 0);\n+\texpected_remaining_argc = (o.real ? 2 : 3);\n+\tif (argc != expected_remaining_argc)\n+\t\tusage_with_options(merge_tree_usage, mt_options);\n+\n+\t/* Do the relevant type of merge */\n+\tif (o.real)\n+\t\treturn real_merge(&o, argv[0], argv[1]);\n+\telse\n+\t\treturn trivial_merge(argv[0], argv[1], argv[2]);\n }\ndiff --git a/git.c b/git.c\nindex 7edafd8ecff..0124c053878 100644\n--- a/git.c\n+++ b/git.c\n@@ -561,7 +561,7 @@ static struct cmd_struct commands[] = {\n \t{ \"merge-recursive-ours\", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT },\n \t{ \"merge-recursive-theirs\", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT },\n \t{ \"merge-subtree\", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT },\n-\t{ \"merge-tree\", cmd_merge_tree, RUN_SETUP | NO_PARSEOPT },\n+\t{ \"merge-tree\", cmd_merge_tree, RUN_SETUP },\n \t{ \"mktag\", cmd_mktag, RUN_SETUP | NO_PARSEOPT },\n \t{ \"mktree\", cmd_mktree, RUN_SETUP },\n \t{ \"multi-pack-index\", cmd_multi_pack_index, RUN_SETUP },\n-- \ngitgitgadget\n\n"},{"id":"445543","messageId":"bc6d01f1a0e735dcf23e7ca3ed0672bd3c4d7993.1641403655.git.gitgitgadget@gmail.com","threadId":"57169","inReplyTo":"pull.1114.v2.git.git.1641403655.gitgitgadget@gmail.com","subject":"[PATCH v2 5/8] merge-ort: split out a separate display_update_messages() function","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-05T17:27:32Z","receivedAt":"2022-01-05T17:27:48Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nNo functional changes included in this patch; it's just a preparatory\nstep in anticipation of wanting to handle the printed messages\ndifferently in `git merge-tree --real`.\n\nSigned-off-by: Elijah Newren \n---\n merge-ort.c | 71 ++++++++++++++++++++++++++++-------------------------\n merge-ort.h | 8 ++++++\n 2 files changed, 46 insertions(+), 33 deletions(-)\n\ndiff --git a/merge-ort.c b/merge-ort.c\nindex 0342f104836..3cdef173cd7 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -4197,6 +4197,42 @@ static int record_conflicted_index_entries(struct merge_options *opt)\n \treturn errs;\n }\n \n+void merge_display_update_messages(struct merge_options *opt,\n+\t\t\t\t struct merge_result *result)\n+{\n+\tstruct merge_options_internal *opti = result->priv;\n+\tstruct hashmap_iter iter;\n+\tstruct strmap_entry *e;\n+\tstruct string_list olist = STRING_LIST_INIT_NODUP;\n+\tint i;\n+\n+\ttrace2_region_enter(\"merge\", \"display messages\", opt->repo);\n+\n+\t/* Hack to pre-allocate olist to the desired size */\n+\tALLOC_GROW(olist.items, strmap_get_size(&opti->output),\n+\t\t olist.alloc);\n+\n+\t/* Put every entry from output into olist, then sort */\n+\tstrmap_for_each_entry(&opti->output, &iter, e) {\n+\t\tstring_list_append(&olist, e->key)->util = e->value;\n+\t}\n+\tstring_list_sort(&olist);\n+\n+\t/* Iterate over the items, printing them */\n+\tfor (i = 0; i < olist.nr; ++i) {\n+\t\tstruct strbuf *sb = olist.items[i].util;\n+\n+\t\tprintf(\"%s\", sb->buf);\n+\t}\n+\tstring_list_clear(&olist, 0);\n+\n+\t/* Also include needed rename limit adjustment now */\n+\tdiff_warn_rename_limit(\"merge.renamelimit\",\n+\t\t\t opti->renames.needed_limit, 0);\n+\n+\ttrace2_region_leave(\"merge\", \"display messages\", opt->repo);\n+}\n+\n void merge_switch_to_result(struct merge_options *opt,\n \t\t\t struct tree *head,\n \t\t\t struct merge_result *result,\n@@ -4235,39 +4271,8 @@ void merge_switch_to_result(struct merge_options *opt,\n \t\ttrace2_region_leave(\"merge\", \"write_auto_merge\", opt->repo);\n \t}\n \n-\tif (display_update_msgs) {\n-\t\tstruct merge_options_internal *opti = result->priv;\n-\t\tstruct hashmap_iter iter;\n-\t\tstruct strmap_entry *e;\n-\t\tstruct string_list olist = STRING_LIST_INIT_NODUP;\n-\t\tint i;\n-\n-\t\ttrace2_region_enter(\"merge\", \"display messages\", opt->repo);\n-\n-\t\t/* Hack to pre-allocate olist to the desired size */\n-\t\tALLOC_GROW(olist.items, strmap_get_size(&opti->output),\n-\t\t\t olist.alloc);\n-\n-\t\t/* Put every entry from output into olist, then sort */\n-\t\tstrmap_for_each_entry(&opti->output, &iter, e) {\n-\t\t\tstring_list_append(&olist, e->key)->util = e->value;\n-\t\t}\n-\t\tstring_list_sort(&olist);\n-\n-\t\t/* Iterate over the items, printing them */\n-\t\tfor (i = 0; i < olist.nr; ++i) {\n-\t\t\tstruct strbuf *sb = olist.items[i].util;\n-\n-\t\t\tprintf(\"%s\", sb->buf);\n-\t\t}\n-\t\tstring_list_clear(&olist, 0);\n-\n-\t\t/* Also include needed rename limit adjustment now */\n-\t\tdiff_warn_rename_limit(\"merge.renamelimit\",\n-\t\t\t\t opti->renames.needed_limit, 0);\n-\n-\t\ttrace2_region_leave(\"merge\", \"display messages\", opt->repo);\n-\t}\n+\tif (display_update_msgs)\n+\t\tmerge_display_update_messages(opt, result);\n \n \tmerge_finalize(opt, result);\n }\ndiff --git a/merge-ort.h b/merge-ort.h\nindex c011864ffeb..1b93555a60b 100644\n--- a/merge-ort.h\n+++ b/merge-ort.h\n@@ -70,6 +70,14 @@ void merge_switch_to_result(struct merge_options *opt,\n \t\t\t int update_worktree_and_index,\n \t\t\t int display_update_msgs);\n \n+/*\n+ * Display messages about conflicts and which files were 3-way merged.\n+ * Automatically called by merge_switch_to_result() with stream == stdout,\n+ * so only call this when bypassing merge_switch_to_result().\n+ */\n+void merge_display_update_messages(struct merge_options *opt,\n+\t\t\t\t struct merge_result *result);\n+\n /* Do needed cleanup when not calling merge_switch_to_result() */\n void merge_finalize(struct merge_options *opt,\n \t\t struct merge_result *result);\n-- \ngitgitgadget\n\n"},{"id":"445544","messageId":"1710ba4a9e432e2a854579c4c929e7f2cfc92211.1641403655.git.gitgitgadget@gmail.com","threadId":"57169","inReplyTo":"pull.1114.v2.git.git.1641403655.gitgitgadget@gmail.com","subject":"[PATCH v2 4/8] merge-tree: implement real merges","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-05T17:27:31Z","receivedAt":"2022-01-05T17:27:48Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nThis adds the ability to perform real merges rather than just trivial\nmerges (meaning handling three way content merges, recursive ancestor\nconsolidation, renames, proper directory/file conflict handling, and so\nforth). However, unlike `git merge`, the working tree and index are\nleft alone and no branch is updated.\n\nThe only output is:\n - the toplevel resulting tree printed on stdout\n - exit status of 0 (clean) or 1 (conflicts present)\n\nThis output is mean to be used by some higher level script, perhaps in a\nsequence of steps like this:\n\n NEWTREE=$(git merge-tree --real $BRANCH1 $BRANCH2)\n test $? -eq 0 || die \"There were conflicts...\"\n NEWCOMMIT=$(git commit-tree $NEWTREE -p $BRANCH1 -p $BRANCH2)\n git update-ref $BRANCH1 $NEWCOMMIT\n\nNote that higher level scripts may also want to access the\nconflict/warning messages normally output during a merge, or have quick\naccess to a list of files with conflicts. That is not available in this\npreliminary implementation, but subsequent commits will add that\nability.\n\nSigned-off-by: Elijah Newren \n---\n Documentation/git-merge-tree.txt | 28 +++++++----\n builtin/merge-tree.c | 55 +++++++++++++++++++++-\n t/t4301-merge-tree-real.sh | 81 ++++++++++++++++++++++++++++++++\n 3 files changed, 153 insertions(+), 11 deletions(-)\n create mode 100755 t/t4301-merge-tree-real.sh\n\ndiff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\nindex 58731c19422..5823938937f 100644\n--- a/Documentation/git-merge-tree.txt\n+++ b/Documentation/git-merge-tree.txt\n@@ -3,26 +3,34 @@ git-merge-tree(1)\n \n NAME\n ----\n-git-merge-tree - Show three-way merge without touching index\n+git-merge-tree - Perform merge without touching index or working tree\n \n \n SYNOPSIS\n --------\n [verse]\n+'git merge-tree' --real \n 'git merge-tree' \n \n DESCRIPTION\n -----------\n-Reads three tree-ish, and output trivial merge results and\n-conflicting stages to the standard output. This is similar to\n-what three-way 'git read-tree -m' does, but instead of storing the\n-results in the index, the command outputs the entries to the\n-standard output.\n+Performs a merge, but does not make any new commits and does not read\n+from or write to either the working tree or index.\n \n-This is meant to be used by higher level scripts to compute\n-merge results outside of the index, and stuff the results back into the\n-index. For this reason, the output from the command omits\n-entries that match the tree.\n+The first form will merge the two branches, doing a full recursive\n+merge with rename detection. If the merge is clean, the exit status\n+will be `0`, and if the merge has conflicts, the exit status will be\n+`1`. The output will consist solely of the resulting toplevel tree\n+(which may have files including conflict markers).\n+\n+The second form is meant for backward compatibility and will only do a\n+trival merge. It reads three tree-ish, and outputs trivial merge\n+results and conflicting stages to the standard output in a semi-diff\n+format. Since this was designed for higher level scripts to consume\n+and merge the results back into the index, it omits entries that match\n+. The result of this second form is is similar to what\n+three-way 'git read-tree -m' does, but instead of storing the results\n+in the index, the command outputs the entries to the standard output.\n \n GIT\n ---\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex e1d2832c809..ac50f3d108b 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -2,6 +2,9 @@\n #include \"builtin.h\"\n #include \"tree-walk.h\"\n #include \"xdiff-interface.h\"\n+#include \"help.h\"\n+#include \"commit-reach.h\"\n+#include \"merge-ort.h\"\n #include \"object-store.h\"\n #include \"parse-options.h\"\n #include \"repository.h\"\n@@ -392,7 +395,57 @@ struct merge_tree_options {\n static int real_merge(struct merge_tree_options *o,\n \t\t const char *branch1, const char *branch2)\n {\n-\tdie(_(\"real merges are not yet implemented\"));\n+\tstruct commit *parent1, *parent2;\n+\tstruct commit_list *common;\n+\tstruct commit_list *merge_bases = NULL;\n+\tstruct commit_list *j;\n+\tstruct merge_options opt;\n+\tstruct merge_result result = { 0 };\n+\n+\tparent1 = get_merge_parent(branch1);\n+\tif (!parent1)\n+\t\thelp_unknown_ref(branch1, \"merge\",\n+\t\t\t\t _(\"not something we can merge\"));\n+\n+\tparent2 = get_merge_parent(branch2);\n+\tif (!parent2)\n+\t\thelp_unknown_ref(branch2, \"merge\",\n+\t\t\t\t _(\"not something we can merge\"));\n+\n+\tinit_merge_options(&opt, the_repository);\n+\t/*\n+\t * TODO: Support subtree and other -X options?\n+\tif (use_strategies_nr == 1 &&\n+\t !strcmp(use_strategies[0]->name, \"subtree\"))\n+\t\topt.subtree_shift = \"\";\n+\tfor (x = 0; x < xopts_nr; x++)\n+\t\tif (parse_merge_opt(&opt, xopts[x]))\n+\t\t\tdie(_(\"Unknown strategy option: -X%s\"), xopts[x]);\n+\t*/\n+\n+\topt.show_rename_progress = 0;\n+\n+\topt.branch1 = merge_remote_util(parent1)->name; /* or just branch1? */\n+\topt.branch2 = merge_remote_util(parent2)->name; /* or just 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+\tcommon = get_merge_bases(parent1, parent2);\n+\tfor (j = common; j; j = j->next)\n+\t\tcommit_list_insert(j->item, &merge_bases);\n+\n+\t/*\n+\t * TODO: notify if merging unrelated histories?\n+\tif (!common)\n+\t\tfprintf(stderr, _(\"merging unrelated histories\"));\n+\t */\n+\n+\tmerge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);\n+\tprintf(\"%s\\n\", oid_to_hex(&result.tree->object.oid));\n+\tmerge_switch_to_result(&opt, NULL, &result, 0, 0);\n+\treturn result.clean ? 0 : 1;\n }\n \n int cmd_merge_tree(int argc, const char **argv, const char *prefix)\ndiff --git a/t/t4301-merge-tree-real.sh b/t/t4301-merge-tree-real.sh\nnew file mode 100755\nindex 00000000000..f7aa310f8c1\n--- /dev/null\n+++ b/t/t4301-merge-tree-real.sh\n@@ -0,0 +1,81 @@\n+#!/bin/sh\n+\n+test_description='git merge-tree --real'\n+\n+. ./test-lib.sh\n+\n+# This test is ort-specific\n+GIT_TEST_MERGE_ALGORITHM=ort\n+export GIT_TEST_MERGE_ALGORITHM\n+\n+test_expect_success setup '\n+\ttest_write_lines 1 2 3 4 5 >numbers &&\n+\techo hello >greeting &&\n+\techo foo >whatever &&\n+\tgit add numbers greeting whatever &&\n+\tgit commit -m initial &&\n+\n+\tgit branch side1 &&\n+\tgit branch side2 &&\n+\n+\tgit checkout side1 &&\n+\ttest_write_lines 1 2 3 4 5 6 >numbers &&\n+\techo hi >greeting &&\n+\techo bar >whatever &&\n+\tgit add numbers greeting whatever &&\n+\tgit commit -m modify-stuff &&\n+\n+\tgit checkout side2 &&\n+\ttest_write_lines 0 1 2 3 4 5 >numbers &&\n+\techo yo >greeting &&\n+\tgit rm whatever &&\n+\tmkdir whatever &&\n+\t>whatever/empty &&\n+\tgit add numbers greeting whatever/empty &&\n+\tgit commit -m other-modifications\n+'\n+\n+test_expect_success 'Content merge and a few conflicts' '\n+\tgit checkout side1^0 &&\n+\ttest_must_fail git merge side2 &&\n+\tcp .git/AUTO_MERGE EXPECT &&\n+\tE_TREE=$(cat EXPECT) &&\n+\n+\tgit reset --hard &&\n+\ttest_must_fail git merge-tree --real side1 side2 >RESULT &&\n+\tR_TREE=$(cat RESULT) &&\n+\n+\t# Due to differences of e.g. \"HEAD\" vs \"side1\", the results will not\n+\t# exactly match. Dig into individual files.\n+\n+\t# Numbers should have three-way merged cleanly\n+\ttest_write_lines 0 1 2 3 4 5 6 >expect &&\n+\tgit show ${R_TREE}:numbers >actual &&\n+\ttest_cmp expect actual &&\n+\n+\t# whatever and whatever~ should have same HASHES\n+\tgit rev-parse ${E_TREE}:whatever ${E_TREE}:whatever~HEAD >expect &&\n+\tgit rev-parse ${R_TREE}:whatever ${R_TREE}:whatever~side1 >actual &&\n+\ttest_cmp expect actual &&\n+\n+\t# greeting should have a merge conflict\n+\tgit show ${E_TREE}:greeting >tmp &&\n+\tcat tmp | sed -e s/HEAD/side1/ >expect &&\n+\tgit show ${R_TREE}:greeting >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'Barf on misspelled option' '\n+\t# Mis-spell with single \"s\" instead of double \"s\"\n+\ttest_expect_code 129 git merge-tree --real --mesages FOOBAR side1 side2 2>expect &&\n+\n+\tgrep \"error: unknown option.*mesages\" expect\n+'\n+\n+test_expect_success 'Barf on too many arguments' '\n+\ttest_expect_code 129 git merge-tree --real side1 side2 side3 2>expect &&\n+\n+\tgrep \"^usage: git merge-tree\" expect\n+'\n+\n+test_done\n-- \ngitgitgadget\n\n"},{"id":"445545","messageId":"c9e95a70d198f5e1c3c02fed1c3c51185a37b1ce.1641403655.git.gitgitgadget@gmail.com","threadId":"57169","inReplyTo":"pull.1114.v2.git.git.1641403655.gitgitgadget@gmail.com","subject":"[PATCH v2 6/8] merge-ort: allow update messages to be written to different file stream","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-05T17:27:33Z","receivedAt":"2022-01-05T17:27:53Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nThis modifies the new display_update_messages() function to allow\nprinting to somewhere other than stdout.\n\nSigned-off-by: Elijah Newren \n---\n merge-ort.c | 7 ++++---\n merge-ort.h | 3 ++-\n 2 files changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/merge-ort.c b/merge-ort.c\nindex 3cdef173cd7..86eebf39166 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -4198,7 +4198,8 @@ static int record_conflicted_index_entries(struct merge_options *opt)\n }\n \n void merge_display_update_messages(struct merge_options *opt,\n-\t\t\t\t struct merge_result *result)\n+\t\t\t\t struct merge_result *result,\n+\t\t\t\t FILE *stream)\n {\n \tstruct merge_options_internal *opti = result->priv;\n \tstruct hashmap_iter iter;\n@@ -4222,7 +4223,7 @@ void merge_display_update_messages(struct merge_options *opt,\n \tfor (i = 0; i < olist.nr; ++i) {\n \t\tstruct strbuf *sb = olist.items[i].util;\n \n-\t\tprintf(\"%s\", sb->buf);\n+\t\tfprintf(stream, \"%s\", sb->buf);\n \t}\n \tstring_list_clear(&olist, 0);\n \n@@ -4272,7 +4273,7 @@ void merge_switch_to_result(struct merge_options *opt,\n \t}\n \n \tif (display_update_msgs)\n-\t\tmerge_display_update_messages(opt, result);\n+\t\tmerge_display_update_messages(opt, result, stdout);\n \n \tmerge_finalize(opt, result);\n }\ndiff --git a/merge-ort.h b/merge-ort.h\nindex 1b93555a60b..55819a57da8 100644\n--- a/merge-ort.h\n+++ b/merge-ort.h\n@@ -76,7 +76,8 @@ void merge_switch_to_result(struct merge_options *opt,\n * so only call this when bypassing merge_switch_to_result().\n */\n void merge_display_update_messages(struct merge_options *opt,\n-\t\t\t\t struct merge_result *result);\n+\t\t\t\t struct merge_result *result,\n+\t\t\t\t FILE *stream);\n \n /* Do needed cleanup when not calling merge_switch_to_result() */\n void merge_finalize(struct merge_options *opt,\n-- \ngitgitgadget\n\n"},{"id":"445546","messageId":"4b513a6d696b8e6ff2c1b669059fcd8747bfa10d.1641403655.git.gitgitgadget@gmail.com","threadId":"57169","inReplyTo":"pull.1114.v2.git.git.1641403655.gitgitgadget@gmail.com","subject":"[PATCH v2 7/8] merge-tree: support saving merge messages to a separate file","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-05T17:27:34Z","receivedAt":"2022-01-05T17:27:54Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nWhen running `git merge-tree --real`, we previously would only return an\nexit status reflecting the cleanness of a merge, and print out the\ntoplevel tree of the resulting merge. Merges also have informational\nmessages, (\"Auto-merging \", \"CONFLICT (content): ...\", \"CONFLICT\n(file/directory)\", etc.) In fact, when non-content conflicts occur\n(such as file/directory, modify/delete, add/add with differing modes,\nrename/rename (1to2), etc.), these informational messages are often the\nonly notification since these conflicts are not representable in the\ncontents of the file.\n\nAdd a --messages option which names a file so that callers can request\nthese messages be recorded somewhere.\n\nSigned-off-by: Elijah Newren \n---\n Documentation/git-merge-tree.txt | 6 ++++--\n builtin/merge-tree.c | 18 ++++++++++++++++--\n t/t4301-merge-tree-real.sh | 18 ++++++++++++++++++\n 3 files changed, 38 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\nindex 5823938937f..4d5857b390b 100644\n--- a/Documentation/git-merge-tree.txt\n+++ b/Documentation/git-merge-tree.txt\n@@ -9,7 +9,7 @@ git-merge-tree - Perform merge without touching index or working tree\n SYNOPSIS\n --------\n [verse]\n-'git merge-tree' --real \n+'git merge-tree' --real [--messages=] \n 'git merge-tree' \n \n DESCRIPTION\n@@ -21,7 +21,9 @@ The first form will merge the two branches, doing a full recursive\n merge with rename detection. If the merge is clean, the exit status\n will be `0`, and if the merge has conflicts, the exit status will be\n `1`. The output will consist solely of the resulting toplevel tree\n-(which may have files including conflict markers).\n+(which may have files including conflict markers). With `--messages`,\n+it will write any informational messages (such as \"Auto-merging\n+\" and conflict notices) to the given file.\n \n The second form is meant for backward compatibility and will only do a\n trival merge. It reads three tree-ish, and outputs trivial merge\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex ac50f3d108b..46b746b6b7c 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -390,6 +390,7 @@ static int trivial_merge(const char *base,\n \n struct merge_tree_options {\n \tint real;\n+\tchar *messages_file;\n };\n \n static int real_merge(struct merge_tree_options *o,\n@@ -443,8 +444,15 @@ static int real_merge(struct merge_tree_options *o,\n \t */\n \n \tmerge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);\n+\n+\tif (o->messages_file) {\n+\t\tFILE *fp = xfopen(o->messages_file, \"w\");\n+\t\tmerge_display_update_messages(&opt, &result, fp);\n+\t\tfclose(fp);\n+\t}\n \tprintf(\"%s\\n\", oid_to_hex(&result.tree->object.oid));\n-\tmerge_switch_to_result(&opt, NULL, &result, 0, 0);\n+\n+\tmerge_finalize(&opt, &result);\n \treturn result.clean ? 0 : 1;\n }\n \n@@ -452,15 +460,18 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n {\n \tstruct merge_tree_options o = { 0 };\n \tint expected_remaining_argc;\n+\tint original_argc;\n \n \tconst char * const merge_tree_usage[] = {\n-\t\tN_(\"git merge-tree --real \"),\n+\t\tN_(\"git merge-tree --real [] \"),\n \t\tN_(\"git merge-tree \"),\n \t\tNULL\n \t};\n \tstruct option mt_options[] = {\n \t\tOPT_BOOL(0, \"real\", &o.real,\n \t\t\t N_(\"do a real merge instead of a trivial merge\")),\n+\t\tOPT_STRING(0, \"messages\", &o.messages_file, N_(\"file\"),\n+\t\t\t N_(\"filename to write informational/conflict messages to\")),\n \t\tOPT_END()\n \t};\n \n@@ -469,8 +480,11 @@ 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/* Parse arguments */\n+\toriginal_argc = argc;\n \targc = parse_options(argc, argv, prefix, mt_options,\n \t\t\t merge_tree_usage, 0);\n+\tif (!o.real && original_argc < argc)\n+\t\tdie(_(\"--real must be specified if any other options are\"));\n \texpected_remaining_argc = (o.real ? 2 : 3);\n \tif (argc != expected_remaining_argc)\n \t\tusage_with_options(merge_tree_usage, mt_options);\ndiff --git a/t/t4301-merge-tree-real.sh b/t/t4301-merge-tree-real.sh\nindex f7aa310f8c1..5f3f27f504d 100755\n--- a/t/t4301-merge-tree-real.sh\n+++ b/t/t4301-merge-tree-real.sh\n@@ -78,4 +78,22 @@ test_expect_success 'Barf on too many arguments' '\n \tgrep \"^usage: git merge-tree\" expect\n '\n \n+test_expect_success '--messages gives us the conflict notices and such' '\n+\ttest_must_fail git merge-tree --real --messages=MSG_FILE side1 side2 &&\n+\n+\t# Expected results:\n+\t# \"greeting\" should merge with conflicts\n+\t# \"numbers\" should merge cleanly\n+\t# \"whatever\" has *both* a modify/delete and a file/directory conflict\n+\tcat <<-EOF >expect &&\n+\tAuto-merging greeting\n+\tCONFLICT (content): Merge conflict in greeting\n+\tAuto-merging numbers\n+\tCONFLICT (file/directory): directory in the way of whatever from side1; moving it to whatever~side1 instead.\n+\tCONFLICT (modify/delete): whatever~side1 deleted in side2 and modified in side1. Version side1 of whatever~side1 left in tree.\n+\tEOF\n+\n+\ttest_cmp expect MSG_FILE\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"445547","messageId":"01364bb020ee2836016ec0e8eafa2261fb7800ab.1641403655.git.gitgitgadget@gmail.com","threadId":"57169","inReplyTo":"pull.1114.v2.git.git.1641403655.gitgitgadget@gmail.com","subject":"[PATCH v2 8/8] merge-tree: provide an easy way to access which files have conflicts","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-05T17:27:35Z","receivedAt":"2022-01-05T17:27:59Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nCallers of `git merge-tree --real` might want an easy way to determine\nwhich files conflicted. While they could potentially use the --messages\noption and parse the resulting messages written to that file, those\nmessages are not meant to be machine readable. Provide a simpler\nmechanism of having the user specify --unmerged-list=$FILENAME, and then\nwrite a NUL-separated list of unmerged filenames to the specified file.\n\nSigned-off-by: Elijah Newren \n---\n Documentation/git-merge-tree.txt | 6 ++++--\n builtin/merge-tree.c | 16 ++++++++++++++++\n merge-ort.c | 13 +++++++++++++\n merge-ort.h | 3 +++\n t/t4301-merge-tree-real.sh | 9 +++++++++\n 5 files changed, 45 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\nindex 4d5857b390b..542cea1a1a8 100644\n--- a/Documentation/git-merge-tree.txt\n+++ b/Documentation/git-merge-tree.txt\n@@ -9,7 +9,7 @@ git-merge-tree - Perform merge without touching index or working tree\n SYNOPSIS\n --------\n [verse]\n-'git merge-tree' --real [--messages=] \n+'git merge-tree' --real [--messages=] [--conflicted-list=] \n 'git merge-tree' \n \n DESCRIPTION\n@@ -23,7 +23,9 @@ will be `0`, and if the merge has conflicts, the exit status will be\n `1`. The output will consist solely of the resulting toplevel tree\n (which may have files including conflict markers). With `--messages`,\n it will write any informational messages (such as \"Auto-merging\n-\" and conflict notices) to the given file.\n+\" and conflict notices) to the given file. With\n+`--conflicted-list`, it will write a list of unmerged files, one per\n+line, to the given file.\n \n The second form is meant for backward compatibility and will only do a\n trival merge. It reads three tree-ish, and outputs trivial merge\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex 46b746b6b7c..4ae34da98b1 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -391,6 +391,7 @@ static int trivial_merge(const char *base,\n struct merge_tree_options {\n \tint real;\n \tchar *messages_file;\n+\tchar *conflicted_file;\n };\n \n static int real_merge(struct merge_tree_options *o,\n@@ -450,6 +451,19 @@ static int real_merge(struct merge_tree_options *o,\n \t\tmerge_display_update_messages(&opt, &result, fp);\n \t\tfclose(fp);\n \t}\n+\tif (o->conflicted_file) {\n+\t\tstruct string_list conflicted_files = STRING_LIST_INIT_NODUP;\n+\t\tFILE *fp = xfopen(o->conflicted_file, \"w\");\n+\t\tint i;\n+\n+\t\tmerge_get_conflicted_files(&result, &conflicted_files);\n+\t\tfor (i = 0; i < conflicted_files.nr; i++) {\n+\t\t\tfprintf(fp, \"%s\", conflicted_files.items[i].string);\n+\t\t\tfputc('\\0', fp);\n+\t\t}\n+\t\tfclose(fp);\n+\t\tstring_list_clear(&conflicted_files, 0);\n+\t}\n \tprintf(\"%s\\n\", oid_to_hex(&result.tree->object.oid));\n \n \tmerge_finalize(&opt, &result);\n@@ -472,6 +486,8 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"do a real merge instead of a trivial merge\")),\n \t\tOPT_STRING(0, \"messages\", &o.messages_file, N_(\"file\"),\n \t\t\t N_(\"filename to write informational/conflict messages to\")),\n+\t\tOPT_STRING(0, \"conflicted-list\", &o.conflicted_file, N_(\"file\"),\n+\t\t\t N_(\"filename to write list of unmerged files\")),\n \t\tOPT_END()\n \t};\n \ndiff --git a/merge-ort.c b/merge-ort.c\nindex 86eebf39166..3d6dd1b234c 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -4234,6 +4234,19 @@ void merge_display_update_messages(struct merge_options *opt,\n \ttrace2_region_leave(\"merge\", \"display messages\", opt->repo);\n }\n \n+void merge_get_conflicted_files(struct merge_result *result,\n+\t\t\t\tstruct string_list *conflicted_files)\n+{\n+\tstruct hashmap_iter iter;\n+\tstruct strmap_entry *e;\n+\tstruct merge_options_internal *opti = result->priv;\n+\n+\tstrmap_for_each_entry(&opti->conflicted, &iter, e) {\n+\t\tstring_list_append(conflicted_files, e->key);\n+\t}\n+\tstring_list_sort(conflicted_files);\n+}\n+\n void merge_switch_to_result(struct merge_options *opt,\n \t\t\t struct tree *head,\n \t\t\t struct merge_result *result,\ndiff --git a/merge-ort.h b/merge-ort.h\nindex 55819a57da8..165cef6616f 100644\n--- a/merge-ort.h\n+++ b/merge-ort.h\n@@ -79,6 +79,9 @@ void merge_display_update_messages(struct merge_options *opt,\n \t\t\t\t struct merge_result *result,\n \t\t\t\t FILE *stream);\n \n+void merge_get_conflicted_files(struct merge_result *result,\n+\t\t\t\tstruct string_list *conflicted_files);\n+\n /* Do needed cleanup when not calling merge_switch_to_result() */\n void merge_finalize(struct merge_options *opt,\n \t\t struct merge_result *result);\ndiff --git a/t/t4301-merge-tree-real.sh b/t/t4301-merge-tree-real.sh\nindex 5f3f27f504d..ec7bd8efd06 100755\n--- a/t/t4301-merge-tree-real.sh\n+++ b/t/t4301-merge-tree-real.sh\n@@ -96,4 +96,13 @@ test_expect_success '--messages gives us the conflict notices and such' '\n \ttest_cmp expect MSG_FILE\n '\n \n+test_expect_success '--messages gives us the conflict notices and such' '\n+\ttest_must_fail git merge-tree --real --conflicted-list=UNMERGED side1 side2 &&\n+\n+\tcat UNMERGED | tr \"\\0\" \"\\n\" >actual &&\n+\ttest_write_lines greeting whatever~side1 >expect &&\n+\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \ngitgitgadget\n"},{"id":"445553","messageId":"ed528125-a2bb-9445-80c5-8c2994ef0d56@ramsayjones.plus.com","threadId":"57169","inReplyTo":"01364bb020ee2836016ec0e8eafa2261fb7800ab.1641403655.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 8/8] merge-tree: provide an easy way to access which files have conflicts","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2022-01-05T19:09:18Z","receivedAt":"2022-01-05T19:09:23Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 05/01/2022 17:27, Elijah Newren via GitGitGadget wrote:\n> From: Elijah Newren \n> \n> Callers of `git merge-tree --real` might want an easy way to determine\n> which files conflicted. While they could potentially use the --messages\n> option and parse the resulting messages written to that file, those\n> messages are not meant to be machine readable. Provide a simpler\n> mechanism of having the user specify --unmerged-list=$FILENAME, and then\n\ns/unmerged-list/conflicted-list/\n\nATB,\nRamsay Jones\n\n\n"},{"id":"445554","messageId":"CABPp-BGL8OeGY_tDXYbMELBnvzZgR445xiK8nGnn_Fo2cn3AAw@mail.gmail.com","threadId":"57169","inReplyTo":"ed528125-a2bb-9445-80c5-8c2994ef0d56@ramsayjones.plus.com","subject":"Re: [PATCH v2 8/8] merge-tree: provide an easy way to access which files have conflicts","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-01-05T19:17:38Z","receivedAt":"2022-01-05T19:17:51Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Wed, Jan 5, 2022 at 11:09 AM Ramsay Jones\n wrote:\n>\n> On 05/01/2022 17:27, Elijah Newren via GitGitGadget wrote:\n> > From: Elijah Newren \n> >\n> > Callers of `git merge-tree --real` might want an easy way to determine\n> > which files conflicted. While they could potentially use the --messages\n> > option and parse the resulting messages written to that file, those\n> > messages are not meant to be machine readable. Provide a simpler\n> > mechanism of having the user specify --unmerged-list=$FILENAME, and then\n>\n> s/unmerged-list/conflicted-list/\n\nIndeed. I had noticed after v1 that I had a mixture of using both\n\"unmerged\" and \"conflicted\" to refer to the same thing, and tried to\nfix it by always using the latter term. Unfortunately, I missed\nupdating this commit message. Thanks for pointing it out; will fix.\n"},{"id":"445573","messageId":"xmqq8rvuhr6q.fsf@gitster.g","threadId":"57169","inReplyTo":"pull.1114.v2.git.git.1641403655.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/8] RFC: Server side merges (no ref updating, no commit creating, no touching worktree or index)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-01-05T20:18:37Z","receivedAt":"2022-01-05T20:18:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Elijah Newren via GitGitGadget\" writes:\n\n> This series introduces a new option to git-merge-tree: --real (best name I\n> could come up with). This new option is designed to allow a server-side\n> \"real\" merge (or allow folks client-side to do merges with branches they\n> don't even have checked out).\n\nFinally. merge-tree was added by Linus mostly as a demonstration of\nidea to trick other developers into enhancing it to implement a full\nmerge that does not need to touch the index or the working tree, but\neverybody failed to be enticed by it so far. It is true that it can\nbe used server-side, but I do not think that is what we want to sell\nit as (after all, receiving a push, merging it to the history in the\ncentral repository, and checking the result out to the working tree,\nwould be a good \"server-side\" operation to have, but it can be done\ntoday without this series). The selling point would rather be it is\ndone mostly in-core, without touching working tree or the index file,\nno?\n\nExciting ;-).\n\n\n\n"},{"id":"445584","messageId":"CABPp-BFHqu6J2TFAwVzBBznhWBi0ESq+hYoytCPXhrYw1d0JAg@mail.gmail.com","threadId":"57169","inReplyTo":"xmqq8rvuhr6q.fsf@gitster.g","subject":"Re: [PATCH v2 0/8] RFC: Server side merges (no ref updating, no commit creating, no touching worktree or index)","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-01-05T22:35:41Z","receivedAt":"2022-01-05T22:36:00Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Wed, Jan 5, 2022 at 12:18 PM Junio C Hamano wrote:\n>\n> \"Elijah Newren via GitGitGadget\" writes:\n>\n> > This series introduces a new option to git-merge-tree: --real (best name I\n> > could come up with). This new option is designed to allow a server-side\n> > \"real\" merge (or allow folks client-side to do merges with branches they\n> > don't even have checked out).\n>\n> Finally. merge-tree was added by Linus mostly as a demonstration of\n> idea to trick other developers into enhancing it to implement a full\n> merge that does not need to touch the index or the working tree, but\n> everybody failed to be enticed by it so far. It is true that it can\n> be used server-side, but I do not think that is what we want to sell\n> it as (after all, receiving a push, merging it to the history in the\n> central repository, and checking the result out to the working tree,\n> would be a good \"server-side\" operation to have, but it can be done\n> today without this series). The selling point would rather be it is\n> done mostly in-core, without touching working tree or the index file,\n> no?\n\nYou're probably right about how we try to sell it as a project to\nexternal folks, but I was focused instead on selling it to reviewers\nwithin the project.\n\n\"Server side merge\" was the name of the topic at the Git Summit and\nlots of folks had interesting comments back then, so I was hoping to\ngrab people's attention with a phrase they would have seen previously\nand commented on.\n\nFurther, the folks I know of who have experience trying to do an\nin-core merge are folks who operate on the server side (using libgit2\ninstead of git, which they have some gripes with). I wanted their\nexperience and views in the review and wanted to make sure it met\ntheir needs, and tried to highlight that to lure them into responding\nand reviewing.\n\n> Exciting ;-).\n\nThanks. :-)\n"},{"id":"445721","messageId":"nycvar.QRO.7.76.6.2201071602110.339@tvgsbejvaqbjf.bet","threadId":"57169","inReplyTo":"1710ba4a9e432e2a854579c4c929e7f2cfc92211.1641403655.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 4/8] merge-tree: implement real merges","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-01-07T15:30:07Z","receivedAt":"2022-01-07T15:30:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Elijah,\n\nOn Wed, 5 Jan 2022, Elijah Newren via GitGitGadget wrote:\n\n> From: Elijah Newren \n>\n> This adds the ability to perform real merges rather than just trivial\n> merges (meaning handling three way content merges, recursive ancestor\n> consolidation, renames, proper directory/file conflict handling, and so\n> forth). However, unlike `git merge`, the working tree and index are\n> left alone and no branch is updated.\n>\n> The only output is:\n> - the toplevel resulting tree printed on stdout\n> - exit status of 0 (clean) or 1 (conflicts present)\n>\n> This output is mean to be used by some higher level script, perhaps in a\n ^^^^\n\nMy apologies for pointing out a grammar issue: This probably intended to\nsay \"meant\", as the word \"mean\" changes the sense of the sentence.\n\nIn my defense, I have more substantial suggestions below.\n\n> sequence of steps like this:\n>\n> NEWTREE=$(git merge-tree --real $BRANCH1 $BRANCH2)\n> test $? -eq 0 || die \"There were conflicts...\"\n> NEWCOMMIT=$(git commit-tree $NEWTREE -p $BRANCH1 -p $BRANCH2)\n> git update-ref $BRANCH1 $NEWCOMMIT\n>\n> Note that higher level scripts may also want to access the\n> conflict/warning messages normally output during a merge, or have quick\n> access to a list of files with conflicts. That is not available in this\n> preliminary implementation, but subsequent commits will add that\n> ability.\n>\n> Signed-off-by: Elijah Newren \n> ---\n> Documentation/git-merge-tree.txt | 28 +++++++----\n> builtin/merge-tree.c | 55 +++++++++++++++++++++-\n> t/t4301-merge-tree-real.sh | 81 ++++++++++++++++++++++++++++++++\n> 3 files changed, 153 insertions(+), 11 deletions(-)\n> create mode 100755 t/t4301-merge-tree-real.sh\n>\n> diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\n> index 58731c19422..5823938937f 100644\n> --- a/Documentation/git-merge-tree.txt\n> +++ b/Documentation/git-merge-tree.txt\n> @@ -3,26 +3,34 @@ git-merge-tree(1)\n>\n> NAME\n> ----\n> -git-merge-tree - Show three-way merge without touching index\n> +git-merge-tree - Perform merge without touching index or working tree\n>\n>\n> SYNOPSIS\n> --------\n> [verse]\n> +'git merge-tree' --real \n> 'git merge-tree' \n\nHere is an idea: How about aiming for this synopsis instead, exploiting\nthe fact that the \"real\" mode takes a different amount of arguments?\n\n 'git merge-tree' [--write-tree] \n 'git merge-tree' [--demo-trivial-merge] \n\nThat way, the old mode can still function, and can even at some stage be\ndeprecated and eventually removed.\n\n>\n> DESCRIPTION\n> -----------\n> -Reads three tree-ish, and output trivial merge results and\n> -conflicting stages to the standard output. This is similar to\n> -what three-way 'git read-tree -m' does, but instead of storing the\n> -results in the index, the command outputs the entries to the\n> -standard output.\n> +Performs a merge, but does not make any new commits and does not read\n> +from or write to either the working tree or index.\n>\n> -This is meant to be used by higher level scripts to compute\n> -merge results outside of the index, and stuff the results back into the\n> -index. For this reason, the output from the command omits\n> -entries that match the tree.\n> +The first form will merge the two branches, doing a full recursive\n> +merge with rename detection. If the merge is clean, the exit status\n> +will be `0`, and if the merge has conflicts, the exit status will be\n> +`1`. The output will consist solely of the resulting toplevel tree\n> +(which may have files including conflict markers).\n> +\n> +The second form is meant for backward compatibility and will only do a\n> +trival merge. It reads three tree-ish, and outputs trivial merge\n> +results and conflicting stages to the standard output in a semi-diff\n> +format. Since this was designed for higher level scripts to consume\n> +and merge the results back into the index, it omits entries that match\n> +. The result of this second form is is similar to what\n> +three-way 'git read-tree -m' does, but instead of storing the results\n> +in the index, the command outputs the entries to the standard output.\n>\n> GIT\n> ---\n> diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\n> index e1d2832c809..ac50f3d108b 100644\n> --- a/builtin/merge-tree.c\n> +++ b/builtin/merge-tree.c\n> @@ -2,6 +2,9 @@\n> #include \"builtin.h\"\n> #include \"tree-walk.h\"\n> #include \"xdiff-interface.h\"\n> +#include \"help.h\"\n> +#include \"commit-reach.h\"\n> +#include \"merge-ort.h\"\n> #include \"object-store.h\"\n> #include \"parse-options.h\"\n> #include \"repository.h\"\n> @@ -392,7 +395,57 @@ struct merge_tree_options {\n> static int real_merge(struct merge_tree_options *o,\n> \t\t const char *branch1, const char *branch2)\n> {\n> -\tdie(_(\"real merges are not yet implemented\"));\n> +\tstruct commit *parent1, *parent2;\n> +\tstruct commit_list *common;\n> +\tstruct commit_list *merge_bases = NULL;\n> +\tstruct commit_list *j;\n> +\tstruct merge_options opt;\n> +\tstruct merge_result result = { 0 };\n> +\n> +\tparent1 = get_merge_parent(branch1);\n> +\tif (!parent1)\n> +\t\thelp_unknown_ref(branch1, \"merge\",\n> +\t\t\t\t _(\"not something we can merge\"));\n> +\n> +\tparent2 = get_merge_parent(branch2);\n> +\tif (!parent2)\n> +\t\thelp_unknown_ref(branch2, \"merge\",\n> +\t\t\t\t _(\"not something we can merge\"));\n> +\n> +\tinit_merge_options(&opt, the_repository);\n> +\t/*\n> +\t * TODO: Support subtree and other -X options?\n> +\tif (use_strategies_nr == 1 &&\n> +\t !strcmp(use_strategies[0]->name, \"subtree\"))\n> +\t\topt.subtree_shift = \"\";\n> +\tfor (x = 0; x < xopts_nr; x++)\n> +\t\tif (parse_merge_opt(&opt, xopts[x]))\n> +\t\t\tdie(_(\"Unknown strategy option: -X%s\"), xopts[x]);\n> +\t*/\n> +\n> +\topt.show_rename_progress = 0;\n> +\n> +\topt.branch1 = merge_remote_util(parent1)->name; /* or just branch1? */\n> +\topt.branch2 = merge_remote_util(parent2)->name; /* or just 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> +\tcommon = get_merge_bases(parent1, parent2);\n> +\tfor (j = common; j; j = j->next)\n> +\t\tcommit_list_insert(j->item, &merge_bases);\n> +\n> +\t/*\n> +\t * TODO: notify if merging unrelated histories?\n\nI guess that it would make most sense to add a flag whether this is\nallowed or not, and I would suggest the default to be `off`.\n\n> +\tif (!common)\n> +\t\tfprintf(stderr, _(\"merging unrelated histories\"));\n> +\t */\n> +\n> +\tmerge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);\n> +\tprintf(\"%s\\n\", oid_to_hex(&result.tree->object.oid));\n> +\tmerge_switch_to_result(&opt, NULL, &result, 0, 0);\n\nThis looks to be idempotent to `merge_finalize(&opt, &result)`, so maybe\nuse that instead?\n\n> +\treturn result.clean ? 0 : 1;\n> }\n>\n> int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> diff --git a/t/t4301-merge-tree-real.sh b/t/t4301-merge-tree-real.sh\n> new file mode 100755\n> index 00000000000..f7aa310f8c1\n> --- /dev/null\n> +++ b/t/t4301-merge-tree-real.sh\n> @@ -0,0 +1,81 @@\n> +#!/bin/sh\n> +\n> +test_description='git merge-tree --real'\n> +\n> +. ./test-lib.sh\n> +\n> +# This test is ort-specific\n> +GIT_TEST_MERGE_ALGORITHM=ort\n> +export GIT_TEST_MERGE_ALGORITHM\n\nIt might make sense to skip the entire test if the user asked for\n`recursive` to be tested:\n\n\ttest \"${GIT_TEST_MERGE_ALGORITHM:-ort}\" = ort ||\n\t\tskip_all=\"GIT_TEST_MERGE_ALGORITHM != ort\"\n\t\ttest_done\n\t}\n\n> +\n> +test_expect_success setup '\n> +\ttest_write_lines 1 2 3 4 5 >numbers &&\n> +\techo hello >greeting &&\n> +\techo foo >whatever &&\n> +\tgit add numbers greeting whatever &&\n> +\tgit commit -m initial &&\n\nI would really like to encourage the use of `test_tick`. It makes the\ncommit consistent, just in case you run into an issue that depends on some\nhash order.\n\n> +\n> +\tgit branch side1 &&\n> +\tgit branch side2 &&\n> +\n> +\tgit checkout side1 &&\n\nPlease use `git switch -c side1` or `git checkout -b side1`: it is more\ncompact than `git branch ... && git checkout ...`.\n\n> +\ttest_write_lines 1 2 3 4 5 6 >numbers &&\n> +\techo hi >greeting &&\n> +\techo bar >whatever &&\n> +\tgit add numbers greeting whatever &&\n> +\tgit commit -m modify-stuff &&\n> +\n> +\tgit checkout side2 &&\n\nThis could be written as `git checkout -b side2 HEAD^`, to make the setup\nmore succinct.\n\n> +\ttest_write_lines 0 1 2 3 4 5 >numbers &&\n> +\techo yo >greeting &&\n> +\tgit rm whatever &&\n> +\tmkdir whatever &&\n> +\t>whatever/empty &&\n> +\tgit add numbers greeting whatever/empty &&\n> +\tgit commit -m other-modifications\n> +'\n> +\n> +test_expect_success 'Content merge and a few conflicts' '\n> +\tgit checkout side1^0 &&\n> +\ttest_must_fail git merge side2 &&\n> +\tcp .git/AUTO_MERGE EXPECT &&\n> +\tE_TREE=$(cat EXPECT) &&\n\nThe file `EXPECT` is not used below. And can we use a more obvious name?\nSOmething like:\n\n\texpected_tree=$(cat .git/AUTO_MERGE)\n\n> +\tgit reset --hard &&\n\nFor an extra bonus, we could delay this via `test_when_finished`, to prove\nthat `git merge-tree --real` works even in a dirty worktree _with\nconflicts_.\n\n> +\ttest_must_fail git merge-tree --real side1 side2 >RESULT &&\n> +\tR_TREE=$(cat RESULT) &&\n\nHow about `actual_tree` instead?\n\n> +\n> +\t# Due to differences of e.g. \"HEAD\" vs \"side1\", the results will not\n> +\t# exactly match. Dig into individual files.\n> +\n> +\t# Numbers should have three-way merged cleanly\n> +\ttest_write_lines 0 1 2 3 4 5 6 >expect &&\n> +\tgit show ${R_TREE}:numbers >actual &&\n> +\ttest_cmp expect actual &&\n> +\n> +\t# whatever and whatever~ should have same HASHES\n> +\tgit rev-parse ${E_TREE}:whatever ${E_TREE}:whatever~HEAD >expect &&\n> +\tgit rev-parse ${R_TREE}:whatever ${R_TREE}:whatever~side1 >actual &&\n> +\ttest_cmp expect actual &&\n> +\n> +\t# greeting should have a merge conflict\n> +\tgit show ${E_TREE}:greeting >tmp &&\n> +\tcat tmp | sed -e s/HEAD/side1/ >expect &&\n> +\tgit show ${R_TREE}:greeting >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'Barf on misspelled option' '\n> +\t# Mis-spell with single \"s\" instead of double \"s\"\n> +\ttest_expect_code 129 git merge-tree --real --mesages FOOBAR side1 side2 2>expect &&\n> +\n> +\tgrep \"error: unknown option.*mesages\" expect\n> +'\n\nI do not think that this test case adds much, and we already test the\n`parse_options()` machinery elsewhere.\n\n> +\n> +test_expect_success 'Barf on too many arguments' '\n> +\ttest_expect_code 129 git merge-tree --real side1 side2 side3 2>expect &&\n> +\n> +\tgrep \"^usage: git merge-tree\" expect\n> +'\n> +\n> +test_done\n\nThe rest looks awesome. Thank you for working on it! I will definitely\ncome back to review the rest (have to take a break now), and then probably\nadd quite a bit of food for thought based on my experience _actually_\nusing `merge-ort` on the server-side. Stay tuned.\n\nThank you,\nDscho\n"},{"id":"445726","messageId":"CABPp-BFUJ6pU_CKM7ccnFvi0nkeeGfd2GETdksKLaz=B_=BZAQ@mail.gmail.com","threadId":"57169","inReplyTo":"nycvar.QRO.7.76.6.2201071602110.339@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2 4/8] merge-tree: implement real merges","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-01-07T17:26:31Z","receivedAt":"2022-01-07T17:26:47Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, Jan 7, 2022 at 7:30 AM Johannes Schindelin\n wrote:\n>\n> Hi Elijah,\n>\n> On Wed, 5 Jan 2022, Elijah Newren via GitGitGadget wrote:\n>\n> > From: Elijah Newren \n> >\n> > This adds the ability to perform real merges rather than just trivial\n> > merges (meaning handling three way content merges, recursive ancestor\n> > consolidation, renames, proper directory/file conflict handling, and so\n> > forth). However, unlike `git merge`, the working tree and index are\n> > left alone and no branch is updated.\n> >\n> > The only output is:\n> > - the toplevel resulting tree printed on stdout\n> > - exit status of 0 (clean) or 1 (conflicts present)\n> >\n> > This output is mean to be used by some higher level script, perhaps in a\n> ^^^^\n>\n> My apologies for pointing out a grammar issue: This probably intended to\n> say \"meant\", as the word \"mean\" changes the sense of the sentence.\n\nOops. Yeah, I'll correct that; thanks for pointing it out.\n\n> In my defense, I have more substantial suggestions below.\n>\n> > sequence of steps like this:\n> >\n> > NEWTREE=$(git merge-tree --real $BRANCH1 $BRANCH2)\n> > test $? -eq 0 || die \"There were conflicts...\"\n> > NEWCOMMIT=$(git commit-tree $NEWTREE -p $BRANCH1 -p $BRANCH2)\n> > git update-ref $BRANCH1 $NEWCOMMIT\n> >\n> > Note that higher level scripts may also want to access the\n> > conflict/warning messages normally output during a merge, or have quick\n> > access to a list of files with conflicts. That is not available in this\n> > preliminary implementation, but subsequent commits will add that\n> > ability.\n> >\n> > Signed-off-by: Elijah Newren \n> > ---\n> > Documentation/git-merge-tree.txt | 28 +++++++----\n> > builtin/merge-tree.c | 55 +++++++++++++++++++++-\n> > t/t4301-merge-tree-real.sh | 81 ++++++++++++++++++++++++++++++++\n> > 3 files changed, 153 insertions(+), 11 deletions(-)\n> > create mode 100755 t/t4301-merge-tree-real.sh\n> >\n> > diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt\n> > index 58731c19422..5823938937f 100644\n> > --- a/Documentation/git-merge-tree.txt\n> > +++ b/Documentation/git-merge-tree.txt\n> > @@ -3,26 +3,34 @@ git-merge-tree(1)\n> >\n> > NAME\n> > ----\n> > -git-merge-tree - Show three-way merge without touching index\n> > +git-merge-tree - Perform merge without touching index or working tree\n> >\n> >\n> > SYNOPSIS\n> > --------\n> > [verse]\n> > +'git merge-tree' --real \n> > 'git merge-tree' \n>\n> Here is an idea: How about aiming for this synopsis instead, exploiting\n> the fact that the \"real\" mode takes a different amount of arguments?\n\nMy turn on the grammar thing: s/amount/number/. :-)\n\n>\n> 'git merge-tree' [--write-tree] \n> 'git merge-tree' [--demo-trivial-merge] \n>\n> That way, the old mode can still function, and can even at some stage be\n> deprecated and eventually removed.\n\nOoh, interesting.\n\n> >\n> > DESCRIPTION\n> > -----------\n> > -Reads three tree-ish, and output trivial merge results and\n> > -conflicting stages to the standard output. This is similar to\n> > -what three-way 'git read-tree -m' does, but instead of storing the\n> > -results in the index, the command outputs the entries to the\n> > -standard output.\n> > +Performs a merge, but does not make any new commits and does not read\n> > +from or write to either the working tree or index.\n> >\n> > -This is meant to be used by higher level scripts to compute\n> > -merge results outside of the index, and stuff the results back into the\n> > -index. For this reason, the output from the command omits\n> > -entries that match the tree.\n> > +The first form will merge the two branches, doing a full recursive\n> > +merge with rename detection. If the merge is clean, the exit status\n> > +will be `0`, and if the merge has conflicts, the exit status will be\n> > +`1`. The output will consist solely of the resulting toplevel tree\n> > +(which may have files including conflict markers).\n> > +\n> > +The second form is meant for backward compatibility and will only do a\n> > +trival merge. It reads three tree-ish, and outputs trivial merge\n> > +results and conflicting stages to the standard output in a semi-diff\n> > +format. Since this was designed for higher level scripts to consume\n> > +and merge the results back into the index, it omits entries that match\n> > +. The result of this second form is is similar to what\n> > +three-way 'git read-tree -m' does, but instead of storing the results\n> > +in the index, the command outputs the entries to the standard output.\n> >\n> > GIT\n> > ---\n> > diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\n> > index e1d2832c809..ac50f3d108b 100644\n> > --- a/builtin/merge-tree.c\n> > +++ b/builtin/merge-tree.c\n> > @@ -2,6 +2,9 @@\n> > #include \"builtin.h\"\n> > #include \"tree-walk.h\"\n> > #include \"xdiff-interface.h\"\n> > +#include \"help.h\"\n> > +#include \"commit-reach.h\"\n> > +#include \"merge-ort.h\"\n> > #include \"object-store.h\"\n> > #include \"parse-options.h\"\n> > #include \"repository.h\"\n> > @@ -392,7 +395,57 @@ struct merge_tree_options {\n> > static int real_merge(struct merge_tree_options *o,\n> > const char *branch1, const char *branch2)\n> > {\n> > - die(_(\"real merges are not yet implemented\"));\n> > + struct commit *parent1, *parent2;\n> > + struct commit_list *common;\n> > + struct commit_list *merge_bases = NULL;\n> > + struct commit_list *j;\n> > + struct merge_options opt;\n> > + struct merge_result result = { 0 };\n> > +\n> > + parent1 = get_merge_parent(branch1);\n> > + if (!parent1)\n> > + help_unknown_ref(branch1, \"merge\",\n> > + _(\"not something we can merge\"));\n> > +\n> > + parent2 = get_merge_parent(branch2);\n> > + if (!parent2)\n> > + help_unknown_ref(branch2, \"merge\",\n> > + _(\"not something we can merge\"));\n> > +\n> > + init_merge_options(&opt, the_repository);\n> > + /*\n> > + * TODO: Support subtree and other -X options?\n> > + if (use_strategies_nr == 1 &&\n> > + !strcmp(use_strategies[0]->name, \"subtree\"))\n> > + opt.subtree_shift = \"\";\n> > + for (x = 0; x < xopts_nr; x++)\n> > + if (parse_merge_opt(&opt, xopts[x]))\n> > + die(_(\"Unknown strategy option: -X%s\"), xopts[x]);\n> > + */\n> > +\n> > + opt.show_rename_progress = 0;\n> > +\n> > + opt.branch1 = merge_remote_util(parent1)->name; /* or just branch1? */\n> > + opt.branch2 = merge_remote_util(parent2)->name; /* or just branch2? */\n> > +\n> > + /*\n> > + * Get the merge bases, in reverse order; see comment above\n> > + * merge_incore_recursive in merge-ort.h\n> > + */\n> > + common = get_merge_bases(parent1, parent2);\n> > + for (j = common; j; j = j->next)\n> > + commit_list_insert(j->item, &merge_bases);\n> > +\n> > + /*\n> > + * TODO: notify if merging unrelated histories?\n>\n> I guess that it would make most sense to add a flag whether this is\n> allowed or not, and I would suggest the default to be `off`.\n\nSounds fair. Thanks for commenting on one of the TODOs that I was unsure about.\n\n> > + if (!common)\n> > + fprintf(stderr, _(\"merging unrelated histories\"));\n> > + */\n> > +\n> > + merge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);\n> > + printf(\"%s\\n\", oid_to_hex(&result.tree->object.oid));\n> > + merge_switch_to_result(&opt, NULL, &result, 0, 0);\n>\n> This looks to be idempotent to `merge_finalize(&opt, &result)`, so maybe\n> use that instead?\n\nYeah, and add a TODO about the display messages (that'll be addressed\nin a later patch, unlike the above TODOs).\n\n>\n> > + return result.clean ? 0 : 1;\n> > }\n> >\n> > int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> > diff --git a/t/t4301-merge-tree-real.sh b/t/t4301-merge-tree-real.sh\n> > new file mode 100755\n> > index 00000000000..f7aa310f8c1\n> > --- /dev/null\n> > +++ b/t/t4301-merge-tree-real.sh\n> > @@ -0,0 +1,81 @@\n> > +#!/bin/sh\n> > +\n> > +test_description='git merge-tree --real'\n> > +\n> > +. ./test-lib.sh\n> > +\n> > +# This test is ort-specific\n> > +GIT_TEST_MERGE_ALGORITHM=ort\n> > +export GIT_TEST_MERGE_ALGORITHM\n>\n> It might make sense to skip the entire test if the user asked for\n> `recursive` to be tested:\n>\n> test \"${GIT_TEST_MERGE_ALGORITHM:-ort}\" = ort ||\n> skip_all=\"GIT_TEST_MERGE_ALGORITHM != ort\"\n> test_done\n> }\n\nThe idea makes sense, but it took me a bit to understand this code\nblock. I think you're just missing an opening left curly brace right\nafter the '||'?\n\n> > +\n> > +test_expect_success setup '\n> > + test_write_lines 1 2 3 4 5 >numbers &&\n> > + echo hello >greeting &&\n> > + echo foo >whatever &&\n> > + git add numbers greeting whatever &&\n> > + git commit -m initial &&\n>\n> I would really like to encourage the use of `test_tick`. It makes the\n> commit consistent, just in case you run into an issue that depends on some\n> hash order.\n\nI've used test_tick before, but I already know this test can't depend\non hash order. Further, the hashes in the output are also replaced\nbefore comparing in order to make the tests also work as-is under\nsha256. So the tests are explicitly ignoring precise hashes. As\nsuch, I'm not sure I see the value of test_tick here.\n\n> > +\n> > + git branch side1 &&\n> > + git branch side2 &&\n> > +\n> > + git checkout side1 &&\n>\n> Please use `git switch -c side1` or `git checkout -b side1`: it is more\n> compact than `git branch ... && git checkout ...`.\n\nYes, but less forgiving to later modification where I go and add\nadditional commits on one of the sides, because...\n\n>\n> > + test_write_lines 1 2 3 4 5 6 >numbers &&\n> > + echo hi >greeting &&\n> > + echo bar >whatever &&\n> > + git add numbers greeting whatever &&\n> > + git commit -m modify-stuff &&\n> > +\n> > + git checkout side2 &&\n>\n> This could be written as `git checkout -b side2 HEAD^`, to make the setup\n> more succinct.\n\n...the presumption of HEAD^ is hardcoded and has to be parsed by\nreaders to understand the test. It felt like more cognitive overhead\nto me, in addition to being less malleable.\n\n> > + test_write_lines 0 1 2 3 4 5 >numbers &&\n> > + echo yo >greeting &&\n> > + git rm whatever &&\n> > + mkdir whatever &&\n> > + >whatever/empty &&\n> > + git add numbers greeting whatever/empty &&\n> > + git commit -m other-modifications\n> > +'\n> > +\n> > +test_expect_success 'Content merge and a few conflicts' '\n> > + git checkout side1^0 &&\n> > + test_must_fail git merge side2 &&\n> > + cp .git/AUTO_MERGE EXPECT &&\n> > + E_TREE=$(cat EXPECT) &&\n>\n> The file `EXPECT` is not used below. And can we use a more obvious name?\n> SOmething like:\n>\n> expected_tree=$(cat .git/AUTO_MERGE)\n\nThere go my beautiful <80 character lines below. :-(\n\nBut on a more serious note, yeah this is probably better. I'll change it. :-)\n\n>\n> > + git reset --hard &&\n>\n> For an extra bonus, we could delay this via `test_when_finished`, to prove\n> that `git merge-tree --real` works even in a dirty worktree _with\n> conflicts_.\n\nOoh, good thought. I like that.\n\n>\n> > + test_must_fail git merge-tree --real side1 side2 >RESULT &&\n> > + R_TREE=$(cat RESULT) &&\n>\n> How about `actual_tree` instead?\n\nBut my 80-characters rev-parse lines....waaah. Just kidding, yeah\nthis would be better.\n\n> > +\n> > + # Due to differences of e.g. \"HEAD\" vs \"side1\", the results will not\n> > + # exactly match. Dig into individual files.\n> > +\n> > + # Numbers should have three-way merged cleanly\n> > + test_write_lines 0 1 2 3 4 5 6 >expect &&\n> > + git show ${R_TREE}:numbers >actual &&\n> > + test_cmp expect actual &&\n> > +\n> > + # whatever and whatever~ should have same HASHES\n> > + git rev-parse ${E_TREE}:whatever ${E_TREE}:whatever~HEAD >expect &&\n> > + git rev-parse ${R_TREE}:whatever ${R_TREE}:whatever~side1 >actual &&\n> > + test_cmp expect actual &&\n> > +\n> > + # greeting should have a merge conflict\n> > + git show ${E_TREE}:greeting >tmp &&\n> > + cat tmp | sed -e s/HEAD/side1/ >expect &&\n> > + git show ${R_TREE}:greeting >actual &&\n> > + test_cmp expect actual\n> > +'\n> > +\n> > +test_expect_success 'Barf on misspelled option' '\n> > + # Mis-spell with single \"s\" instead of double \"s\"\n> > + test_expect_code 129 git merge-tree --real --mesages FOOBAR side1 side2 2>expect &&\n> > +\n> > + grep \"error: unknown option.*mesages\" expect\n> > +'\n>\n> I do not think that this test case adds much, and we already test the\n> `parse_options()` machinery elsewhere.\n\nIt's more about verifying that exit codes of 0 & 1 are reserved for\n\"completed with no conflicts\" and \"completed with conflicts\". The 129\nbit in this test is the important bit (and perhaps is well-known to\nlots of other folks, but I thought it was worth highlighting). That\nsaid, I did a bad job mentioning that in the test description; I'll\nfix it up.\n\n> > +\n> > +test_expect_success 'Barf on too many arguments' '\n> > + test_expect_code 129 git merge-tree --real side1 side2 side3 2>expect &&\n> > +\n> > + grep \"^usage: git merge-tree\" expect\n> > +'\n> > +\n> > +test_done\n>\n> The rest looks awesome. Thank you for working on it! I will definitely\n> come back to review the rest (have to take a break now), and then probably\n> add quite a bit of food for thought based on my experience _actually_\n> using `merge-ort` on the server-side. Stay tuned.\n\nOoh, I'm intrigued. And thanks for reviewing!\n"},{"id":"445729","messageId":"nycvar.QRO.7.76.6.2201071906050.339@tvgsbejvaqbjf.bet","threadId":"57169","inReplyTo":"4b513a6d696b8e6ff2c1b669059fcd8747bfa10d.1641403655.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 7/8] merge-tree: support saving merge messages to a separate file","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-01-07T18:07:43Z","receivedAt":"2022-01-07T18:07:54Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Elijah,\n\nOn Wed, 5 Jan 2022, Elijah Newren via GitGitGadget wrote:\n\n> diff --git a/t/t4301-merge-tree-real.sh b/t/t4301-merge-tree-real.sh\n> index f7aa310f8c1..5f3f27f504d 100755\n> --- a/t/t4301-merge-tree-real.sh\n> +++ b/t/t4301-merge-tree-real.sh\n> @@ -78,4 +78,22 @@ test_expect_success 'Barf on too many arguments' '\n> \tgrep \"^usage: git merge-tree\" expect\n> '\n>\n> +test_expect_success '--messages gives us the conflict notices and such' '\n> +\ttest_must_fail git merge-tree --real --messages=MSG_FILE side1 side2 &&\n\nSince we discern between exit status 1 (= merge conflict) and >1 (fatal\nerror), we should probably use `test_expect_code` here.\n\nOther than that, this patch looks good.\n\nThank you,\nDscho\n\n> +\n> +\t# Expected results:\n> +\t# \"greeting\" should merge with conflicts\n> +\t# \"numbers\" should merge cleanly\n> +\t# \"whatever\" has *both* a modify/delete and a file/directory conflict\n> +\tcat <<-EOF >expect &&\n> +\tAuto-merging greeting\n> +\tCONFLICT (content): Merge conflict in greeting\n> +\tAuto-merging numbers\n> +\tCONFLICT (file/directory): directory in the way of whatever from side1; moving it to whatever~side1 instead.\n> +\tCONFLICT (modify/delete): whatever~side1 deleted in side2 and modified in side1. Version side1 of whatever~side1 left in tree.\n> +\tEOF\n> +\n> +\ttest_cmp expect MSG_FILE\n> +'\n> +\n> test_done\n> --\n> gitgitgadget\n>\n>\n"},{"id":"445730","messageId":"CAP8UFD1jTgxCc-r8vzBGUt8SRS=h5jA1KSz6Fw1XpKXB5-XtoQ@mail.gmail.com","threadId":"57169","inReplyTo":"1710ba4a9e432e2a854579c4c929e7f2cfc92211.1641403655.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 4/8] merge-tree: implement real merges","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2022-01-07T18:12:00Z","receivedAt":"2022-01-07T18:12:13Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wed, Jan 5, 2022 at 6:27 PM Elijah Newren via GitGitGadget\n wrote:\n\n> The only output is:\n> - the toplevel resulting tree printed on stdout\n> - exit status of 0 (clean) or 1 (conflicts present)\n\nI thought that the merge-ort API could (at least theoretically\naccording to merge-ort.h) return something < 0 in case of internal\nerror. In this case I would be interested in knowing what's the output\nof the command.\n\n> +The first form will merge the two branches, doing a full recursive\n> +merge with rename detection. If the merge is clean, the exit status\n> +will be `0`, and if the merge has conflicts, the exit status will be\n> +`1`.\n\nNo mention of what happens in case of an internal error in the merge-ort API.\n\n> + merge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);\n> + printf(\"%s\\n\", oid_to_hex(&result.tree->object.oid));\n> + merge_switch_to_result(&opt, NULL, &result, 0, 0);\n> + return result.clean ? 0 : 1;\n\nIf result.clean can be < 0, this might pretend that the merge was clean.\n"},{"id":"445731","messageId":"nycvar.QRO.7.76.6.2201071915290.339@tvgsbejvaqbjf.bet","threadId":"57169","inReplyTo":"CABPp-BFUJ6pU_CKM7ccnFvi0nkeeGfd2GETdksKLaz=B_=BZAQ@mail.gmail.com","subject":"Re: [PATCH v2 4/8] merge-tree: implement real merges","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-01-07T18:22:57Z","receivedAt":"2022-01-07T18:23:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Elijah,\n\n\nOn Fri, 7 Jan 2022, Elijah Newren wrote:\n\n> On Fri, Jan 7, 2022 at 7:30 AM Johannes Schindelin\n> wrote:\n> >\n> > On Wed, 5 Jan 2022, Elijah Newren via GitGitGadget wrote:\n> >\n> > > SYNOPSIS\n> > > --------\n> > > [verse]\n> > > +'git merge-tree' --real \n> > > 'git merge-tree' \n> >\n> > Here is an idea: How about aiming for this synopsis instead, exploiting\n> > the fact that the \"real\" mode takes a different amount of arguments?\n>\n> My turn on the grammar thing: s/amount/number/. :-)\n\nSee? I know why I'm refraining from nitpicking. It's just not good for\nanyone involved.\n\n> > > diff --git a/t/t4301-merge-tree-real.sh b/t/t4301-merge-tree-real.sh\n> > > new file mode 100755\n> > > index 00000000000..f7aa310f8c1\n> > > --- /dev/null\n> > > +++ b/t/t4301-merge-tree-real.sh\n> > > @@ -0,0 +1,81 @@\n> > > +#!/bin/sh\n> > > +\n> > > +test_description='git merge-tree --real'\n> > > +\n> > > +. ./test-lib.sh\n> > > +\n> > > +# This test is ort-specific\n> > > +GIT_TEST_MERGE_ALGORITHM=ort\n> > > +export GIT_TEST_MERGE_ALGORITHM\n> >\n> > It might make sense to skip the entire test if the user asked for\n> > `recursive` to be tested:\n> >\n> > test \"${GIT_TEST_MERGE_ALGORITHM:-ort}\" = ort ||\n> > skip_all=\"GIT_TEST_MERGE_ALGORITHM != ort\"\n> > test_done\n> > }\n>\n> The idea makes sense, but it took me a bit to understand this code\n> block. I think you're just missing an opening left curly brace right\n> after the '||'?\n\nYes. Sorry.\n\n> > > +test_expect_success setup '\n> > > + test_write_lines 1 2 3 4 5 >numbers &&\n> > > + echo hello >greeting &&\n> > > + echo foo >whatever &&\n> > > + git add numbers greeting whatever &&\n> > > + git commit -m initial &&\n> >\n> > I would really like to encourage the use of `test_tick`. It makes the\n> > commit consistent, just in case you run into an issue that depends on some\n> > hash order.\n>\n> I've used test_tick before, but I already know this test can't depend\n> on hash order. Further, the hashes in the output are also replaced\n> before comparing in order to make the tests also work as-is under\n> sha256. So the tests are explicitly ignoring precise hashes. As\n> such, I'm not sure I see the value of test_tick here.\n\nNevertheless. To make comparing logs of two different test runs easier, it\nmakes more sense to insist on consistency.\n\n> > > +\n> > > + git branch side1 &&\n> > > + git branch side2 &&\n> > > +\n> > > + git checkout side1 &&\n> >\n> > Please use `git switch -c side1` or `git checkout -b side1`: it is more\n> > compact than `git branch ... && git checkout ...`.\n>\n> Yes, but less forgiving to later modification where I go and add\n> additional commits on one of the sides, because...\n>\n> >\n> > > + test_write_lines 1 2 3 4 5 6 >numbers &&\n> > > + echo hi >greeting &&\n> > > + echo bar >whatever &&\n> > > + git add numbers greeting whatever &&\n> > > + git commit -m modify-stuff &&\n> > > +\n> > > + git checkout side2 &&\n> >\n> > This could be written as `git checkout -b side2 HEAD^`, to make the setup\n> > more succinct.\n>\n> ...the presumption of HEAD^ is hardcoded and has to be parsed by\n> readers to understand the test. It felt like more cognitive overhead\n> to me, in addition to being less malleable.\n\nRight. Different developers, different preferences. I wish we had a\nstandard way in the test suite to initialize a test setup that _everybody_\ncould agree to be succinct and helpful. So far, we use shell scripted Git\ncommands to recreate an initial commit topology, but especially when\ncomparing to existing test suites with fixtures that are not only\nwell-documented but also easy to wrap your head around, I find Git's test\nsuite awfully lacking. Mind you, the code _I_ introduced isn't stellar in\nthis respect, either, not by a very far stretch.\n\n> > > +test_expect_success 'Barf on misspelled option' '\n> > > + # Mis-spell with single \"s\" instead of double \"s\"\n> > > + test_expect_code 129 git merge-tree --real --mesages FOOBAR side1 side2 2>expect &&\n> > > +\n> > > + grep \"error: unknown option.*mesages\" expect\n> > > +'\n> >\n> > I do not think that this test case adds much, and we already test the\n> > `parse_options()` machinery elsewhere.\n>\n> It's more about verifying that exit codes of 0 & 1 are reserved for\n> \"completed with no conflicts\" and \"completed with conflicts\". The 129\n> bit in this test is the important bit (and perhaps is well-known to\n> lots of other folks, but I thought it was worth highlighting).\n\nFair enough.\n\nCiao,\nDscho\n"},{"id":"445732","messageId":"CAP8UFD1Z74yuUmzPCr6X8-i2B1zaiT8kPxNDHxK5MeHw8OcnRg@mail.gmail.com","threadId":"57169","inReplyTo":"pull.1114.v2.git.git.1641403655.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/8] RFC: Server side merges (no ref updating, no commit creating, no touching worktree or index)","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2022-01-07T18:46:17Z","receivedAt":"2022-01-07T18:46:41Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wed, Jan 5, 2022 at 6:27 PM Elijah Newren via GitGitGadget\n wrote:\n\n> This series attempts to guess what kind of output would be wanted, basically\n> choosing:\n>\n> * clean merge or conflict signalled via exit status\n\n(Maybe s/signalled/signaled/)\n\nNot sure that's the best way by default. I think it's very likely that\nmany users will be interested in parsing the command ouput, and they\nmight prefer that merge related errors be signaled in a different way\nthan other errors.\n\n> * stdout consists solely of printing the hash of the resulting tree (though\n> that tree may include files that have conflict markers)\n\nMaybe users will want diffs, the conflicted list and other things on\nstdout, as they might want to parse it anyway, and it would be a\nburden to have to perform diffs, or get other interesting info in a\ndifferent way or using a different process or call.\n\n> * new optional --messages flag for specifying a file where informational\n> messages (e.g. conflict notices and files involved in three-way-content\n> merges) can be written; by default, this output is simply discarded\n> * new optional --conflicted-list flag for specifying a file where the names\n> of conflicted-files can be written in a NUL-character-separated list\n\nIt would be nice if output was printed on stdout when the above flags\nare used without argument.\n\nThanks for working on this!\n"},{"id":"445734","messageId":"CABPp-BFph66FFH26gHTHnMW1OQpZQ7nUJBfr7vWJ_ZmXRG+DcA@mail.gmail.com","threadId":"57169","inReplyTo":"CAP8UFD1jTgxCc-r8vzBGUt8SRS=h5jA1KSz6Fw1XpKXB5-XtoQ@mail.gmail.com","subject":"Re: [PATCH v2 4/8] merge-tree: implement real merges","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-01-07T19:09:56Z","receivedAt":"2022-01-07T19:10:11Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, Jan 7, 2022 at 10:12 AM Christian Couder\n wrote:\n>\n> On Wed, Jan 5, 2022 at 6:27 PM Elijah Newren via GitGitGadget\n> wrote:\n>\n> > The only output is:\n> > - the toplevel resulting tree printed on stdout\n> > - exit status of 0 (clean) or 1 (conflicts present)\n>\n> I thought that the merge-ort API could (at least theoretically\n> according to merge-ort.h) return something < 0 in case of internal\n> error. In this case I would be interested in knowing what's the output\n> of the command.\n>\n> > +The first form will merge the two branches, doing a full recursive\n> > +merge with rename detection. If the merge is clean, the exit status\n> > +will be `0`, and if the merge has conflicts, the exit status will be\n> > +`1`.\n>\n> No mention of what happens in case of an internal error in the merge-ort API.\n>\n> > + merge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);\n> > + printf(\"%s\\n\", oid_to_hex(&result.tree->object.oid));\n> > + merge_switch_to_result(&opt, NULL, &result, 0, 0);\n> > + return result.clean ? 0 : 1;\n>\n> If result.clean can be < 0, this might pretend that the merge was clean.\n\nOoh, these are very good points. Thanks for bringing them up; I'll\ntry to address them in a re-roll.\n"},{"id":"445735","messageId":"CABPp-BHKxOodfVsoH9sS1FkS4=zL4Oh0NCfRExyZtJz_5YvFUw@mail.gmail.com","threadId":"57169","inReplyTo":"nycvar.QRO.7.76.6.2201071915290.339@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2 4/8] merge-tree: implement real merges","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-01-07T19:15:54Z","receivedAt":"2022-01-07T19:16:09Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi Dscho,\n\nOn Fri, Jan 7, 2022 at 10:23 AM Johannes Schindelin\n wrote:\n>\n> Hi Elijah,\n>\n> On Fri, 7 Jan 2022, Elijah Newren wrote:\n>\n> > On Fri, Jan 7, 2022 at 7:30 AM Johannes Schindelin\n> > wrote:\n> > >\n> > > On Wed, 5 Jan 2022, Elijah Newren via GitGitGadget wrote:\n> > >\n> > > > SYNOPSIS\n> > > > --------\n> > > > [verse]\n> > > > +'git merge-tree' --real \n> > > > 'git merge-tree' \n> > >\n> > > Here is an idea: How about aiming for this synopsis instead, exploiting\n> > > the fact that the \"real\" mode takes a different amount of arguments?\n> >\n> > My turn on the grammar thing: s/amount/number/. :-)\n>\n> See? I know why I'm refraining from nitpicking. It's just not good for\n> anyone involved.\n\nWell, in your case, the point you brought up will improve the commit\nmessage for future readers, and so it was totally justified (and I'm\nglad you brought it up). My comment is useful for nothing more than a\nbit of good-natured ribbing. But I'm not sure it was taken that way,\nso I'm sorry if my comment had any effect other than making you smile.\n\n> > > > diff --git a/t/t4301-merge-tree-real.sh b/t/t4301-merge-tree-real.sh\n> > > > new file mode 100755\n> > > > index 00000000000..f7aa310f8c1\n> > > > --- /dev/null\n> > > > +++ b/t/t4301-merge-tree-real.sh\n> > > > @@ -0,0 +1,81 @@\n> > > > +#!/bin/sh\n> > > > +\n> > > > +test_description='git merge-tree --real'\n> > > > +\n> > > > +. ./test-lib.sh\n> > > > +\n> > > > +# This test is ort-specific\n> > > > +GIT_TEST_MERGE_ALGORITHM=ort\n> > > > +export GIT_TEST_MERGE_ALGORITHM\n> > >\n> > > It might make sense to skip the entire test if the user asked for\n> > > `recursive` to be tested:\n> > >\n> > > test \"${GIT_TEST_MERGE_ALGORITHM:-ort}\" = ort ||\n> > > skip_all=\"GIT_TEST_MERGE_ALGORITHM != ort\"\n> > > test_done\n> > > }\n> >\n> > The idea makes sense, but it took me a bit to understand this code\n> > block. I think you're just missing an opening left curly brace right\n> > after the '||'?\n>\n> Yes. Sorry.\n>\n> > > > +test_expect_success setup '\n> > > > + test_write_lines 1 2 3 4 5 >numbers &&\n> > > > + echo hello >greeting &&\n> > > > + echo foo >whatever &&\n> > > > + git add numbers greeting whatever &&\n> > > > + git commit -m initial &&\n> > >\n> > > I would really like to encourage the use of `test_tick`. It makes the\n> > > commit consistent, just in case you run into an issue that depends on some\n> > > hash order.\n> >\n> > I've used test_tick before, but I already know this test can't depend\n> > on hash order. Further, the hashes in the output are also replaced\n> > before comparing in order to make the tests also work as-is under\n> > sha256. So the tests are explicitly ignoring precise hashes. As\n> > such, I'm not sure I see the value of test_tick here.\n>\n> Nevertheless. To make comparing logs of two different test runs easier, it\n> makes more sense to insist on consistency.\n\nAh...comparing logs between two different test runs; that sounds like\na reasonable justification. I'll add the test_tick's.\n\n> > > > +\n> > > > + git branch side1 &&\n> > > > + git branch side2 &&\n> > > > +\n> > > > + git checkout side1 &&\n> > >\n> > > Please use `git switch -c side1` or `git checkout -b side1`: it is more\n> > > compact than `git branch ... && git checkout ...`.\n> >\n> > Yes, but less forgiving to later modification where I go and add\n> > additional commits on one of the sides, because...\n> >\n> > >\n> > > > + test_write_lines 1 2 3 4 5 6 >numbers &&\n> > > > + echo hi >greeting &&\n> > > > + echo bar >whatever &&\n> > > > + git add numbers greeting whatever &&\n> > > > + git commit -m modify-stuff &&\n> > > > +\n> > > > + git checkout side2 &&\n> > >\n> > > This could be written as `git checkout -b side2 HEAD^`, to make the setup\n> > > more succinct.\n> >\n> > ...the presumption of HEAD^ is hardcoded and has to be parsed by\n> > readers to understand the test. It felt like more cognitive overhead\n> > to me, in addition to being less malleable.\n>\n> Right. Different developers, different preferences. I wish we had a\n> standard way in the test suite to initialize a test setup that _everybody_\n> could agree to be succinct and helpful. So far, we use shell scripted Git\n> commands to recreate an initial commit topology, but especially when\n> comparing to existing test suites with fixtures that are not only\n> well-documented but also easy to wrap your head around, I find Git's test\n> suite awfully lacking. Mind you, the code _I_ introduced isn't stellar in\n> this respect, either, not by a very far stretch.\n>\n> > > > +test_expect_success 'Barf on misspelled option' '\n> > > > + # Mis-spell with single \"s\" instead of double \"s\"\n> > > > + test_expect_code 129 git merge-tree --real --mesages FOOBAR side1 side2 2>expect &&\n> > > > +\n> > > > + grep \"error: unknown option.*mesages\" expect\n> > > > +'\n> > >\n> > > I do not think that this test case adds much, and we already test the\n> > > `parse_options()` machinery elsewhere.\n> >\n> > It's more about verifying that exit codes of 0 & 1 are reserved for\n> > \"completed with no conflicts\" and \"completed with conflicts\". The 129\n> > bit in this test is the important bit (and perhaps is well-known to\n> > lots of other folks, but I thought it was worth highlighting).\n>\n> Fair enough.\n>\n> Ciao,\n> Dscho\n"},{"id":"445739","messageId":"nycvar.QRO.7.76.6.2201071908580.339@tvgsbejvaqbjf.bet","threadId":"57169","inReplyTo":"01364bb020ee2836016ec0e8eafa2261fb7800ab.1641403655.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 8/8] merge-tree: provide an easy way to access which files have conflicts","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-01-07T19:36:18Z","receivedAt":"2022-01-07T19:36:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Elijah,\n\nOn Wed, 5 Jan 2022, Elijah Newren via GitGitGadget wrote:\n\n> From: Elijah Newren \n>\n> Callers of `git merge-tree --real` might want an easy way to determine\n> which files conflicted. While they could potentially use the --messages\n> option and parse the resulting messages written to that file, those\n> messages are not meant to be machine readable. Provide a simpler\n> mechanism of having the user specify --unmerged-list=$FILENAME, and then\n> write a NUL-separated list of unmerged filenames to the specified file.\n\nThis patch does what the commit message says, and it looks quite\nplausible. However, in practice it seems that you need either a tree (if\nthe merge succeeded) or the list of conflicted files (if the merge\nsucceeded).\n\nSo while it looks relatively clean from the implementation's point of\nview, the design itself could probably withstand a bit of consideration.\n\nAs I hinted earlier (to be precise, in\nhttps://lore.kernel.org/git/nycvar.QRO.7.76.6.2201071602110.339@tvgsbejvaqbjf.bet/),\nI had the chance in December to work on the server-side, using `merge-ort`\nfor a bit. In the following, I will talk about this a bit more than about\nthis particular patch, but I think it is highly relevant (not a tangent).\n\nOne of the things that became clear to me is that we really have an\neither/or situation here. Either the merge succeeds, and we _need_ that\ntree, or it fails, and we could not care less about the tree at all.\n\nIn fact, if the merge fails, we completely ignore the tree, and it would\nbe better if we would not even write out any Git objects in that case at\nall: even just writing the objects would be quite costly at the\nserver-side scale.\n\nSo my (somewhat hacky) patches for a proof-of-concept produced _either_\nthe hash of the tree on `stdout`, _or_ a header saying that there were\nconflicts followed by a NUL-separated list of file names.\n\nMind you, I did not even get to the point of analyzing things even more\ndeeply. My partner in crime and I only got to comparing the `merge-ort`\nway to the libgit2-based way, trying to get them to compare as much\napples-to-apples as possible [*1*], and we found that even the time to\nspawn the Git process (~1-3ms, with all overhead counted in) is _quite_\nnoticeable, at server-side scale.\n\nOf course, the `merge-ort` performance was _really_ nice when doing\nanything remotely complex, then `merge-ort` really blew the libgit2-based\nmerge out of the water. But that's not the common case. The common case\nare merges that involve very few modified files, a single merge base, and\nthey don't conflict. And those can be processed by the libgit2-based\nmethod within a fraction of the time it takes to even only so much as\nspawn `git` (libgit2-based merges can complete in less than a fifth\nmillisecond, that's at most a fifth of the time it takes to merely run\n`git merge-tree`).\n\nThe difference between 0.2-0.5ms for libgit2-based merges on the one hand,\nand 1-3ms for `merge-ort`-based merges on the other hand, might not seem\nlike much, but you have to multiply it by the times such a merge is\nperformed on the server. Which is a _lot_. Way more often than I thought.\n\nIn this particular instance, there is a silver-lining: the libgit2-based\nmerge is not actually recursive. It is a three-way merge. Which means that\nwe first have to determine a merge base. In our case, this is done by\nspawning a Git process anyway, so one of my ideas to move forward is to fold\nthat merge-base logic into `git merge-tree`, too.\n\nAnyway, the short short is: whenever we can avoid unnecessary work, we\nshould do so. In the context of this patch, I would say that we should\navoid writing out a tree (and avoid printing its hash to `stdout`) if\nthere are merge conflicts. And we should avoid writing (and later reading)\na file, if we can get away with avoiding it.\n\nAt least in the default case, that is. We still might need a flag to\nproduce some more information about those merge conflicts. But even in\nthat case, it would be better to have a list of file names with the three\nassociated stages than to output the hash of a tree that contains\nconflicts (and tons of files _without_ conflicts). The UI needs to\nre-generate those conflicts anyway. And remember: a tree can contain\nmillions of files even if there is but a single conflict. It makes more\nsense for `merge-tree --real` to output a concrete list of files that\nconflicted, rather than expecting the caller to discern between conflicts\nand non-conflicts by processing a tree object.\n\nMaybe you agree with this rationale and re-design the `--real` mode to try\nto avoid writing out files in the common case?\n\nAbout the form of the patch itself: I was tempted to go with the\nnitpicking spirit I see on the Git mailing list these days, especially\nabout the shell script code in the test scripts. But then I realized that\nI find such nitpicking pretty unhelpful, myself. The code is good as-is,\neven if I would write it differently. It is clear, and it does exactly\nwhat it is supposed to do.\n\nThank you,\nDscho\n\nFootnote *1*: I did not _quite_ get to the point of comparing the\n`merge-ort` merges to the libgit2 ones, unfortunately. I was on my way to\nadd code to respect `merge.renames = false` so that we could _truly_\ncompare the `merge-ort` merges to the libgit2 merges (we really will want\nto verify that the output is identical, before even considering to enable\nrecursive merges on the server side, and then only after studying the time\ndifference), and then had to take off due to the holidays. If you already\nhave that need to be able to turn off rename-detection on your radar, even\nif only for a transitional period, I would be _so_ delighted.\n"},{"id":"445744","messageId":"CABPp-BE5breKX5TciAwzKi+BQnqy1aKq_v4tjiqiX7swrZf=PA@mail.gmail.com","threadId":"57169","inReplyTo":"CAP8UFD1Z74yuUmzPCr6X8-i2B1zaiT8kPxNDHxK5MeHw8OcnRg@mail.gmail.com","subject":"Re: [PATCH v2 0/8] RFC: Server side merges (no ref updating, no commit creating, no touching worktree or index)","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-01-07T19:59:43Z","receivedAt":"2022-01-07T19:59:57Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, Jan 7, 2022 at 10:46 AM Christian Couder\n wrote:\n>\n> On Wed, Jan 5, 2022 at 6:27 PM Elijah Newren via GitGitGadget\n> wrote:\n>\n> > This series attempts to guess what kind of output would be wanted, basically\n> > choosing:\n> >\n> > * clean merge or conflict signalled via exit status\n>\n> (Maybe s/signalled/signaled/)\n\nI can't determine the difference after a few Google searches, and both\nseem to be in dictionaries with the same meaning so I'm having\ndifficulty figuring out which is preferred. Usually my searches will\neither suggest that one is a misspelling or at least bring up whether\none is a regional variance, but I'm not seeing anything of the sort.\n\nIt can't hurt to switch, though, so I'm happy to switch.\n\n> Not sure that's the best way by default. I think it's very likely that\n> many users will be interested in parsing the command ouput, and they\n> might prefer that merge related errors be signaled in a different way\n> than other errors.\n\nThat's fair.\n\nI was thinking in terms of various plumbing commands: hash-object,\nmktree, commit-tree, read-tree, write-tree and update-ref. Output\nfrom commands in that last can be fed as input to other commands and\nbe chained together to do various interesting and useful things. I\nhave done that at various times in the past. I thought merge-tree\nmight augment that category of commands (particularly since Peff\nsuggested to make the command be low-level at the summit), and thus\noutputting just a tree (at least by default) would make the command be\na useful building block within that context. That was part of my\nreason for including the code snippet\n\n NEWTREE=$(git merge-tree --real $BRANCH1 $BRANCH2)\n test $? -eq 0 || die \"There were conflicts...\"\n NEWCOMMIT=$(git commit-tree $NEWTREE -p $BRANCH1 -p $BRANCH2)\n git update-ref $BRANCH1 $NEWCOMMIT\n\nin the cover letter.\n\nBut merge-tree is much more likely to run into problems (i.e. into\nmerge conflicts), so maybe it doesn't belong in the same set, and the\nNEWTREE definition perhaps deserves to have additional special case\ncommand-line parsing that the user needs to do.\n\nI'm curious about others' thoughts on this matter too.\n\n> > * stdout consists solely of printing the hash of the resulting tree (though\n> > that tree may include files that have conflict markers)\n>\n> Maybe users will want diffs, the conflicted list and other things on\n> stdout, as they might want to parse it anyway, and it would be a\n> burden to have to perform diffs, or get other interesting info in a\n> different way or using a different process or call.\n\nYou mention the stdout thing both above and below, so I'll concentrate\nhere on the diffs part.\n\nDo you have a specific usecase you have in mind where diffs are\nwanted, separate from the two examples you gave in the other thread\n(namely Ævar's misguided hack for looking for whether there were\nconflicts, and a desire to just follow merge-tree's convoluted\nprecedent)? I'd rather not add diffs pre-emptively on the basis that\nusers \"might\" want them, especially if they come with the huge gamut\nof options Ævar was spitballing in [1] (some of which appeared to have\nmisguided assumptions relative to the possibility of renames and might\nintroduce edge and corner case bugs that'd be with us forever). If we\ndon't have concrete usecases yet, I'd rather avoid adding such options\nuntil we do have concrete usecases so we don't paint ourselves into a\ncorner.\n\n[1] https://lore.kernel.org/git/211109.861r3qdpt8.gmgdl@evledraar.gmail.com/\n\n> > * new optional --messages flag for specifying a file where informational\n> > messages (e.g. conflict notices and files involved in three-way-content\n> > merges) can be written; by default, this output is simply discarded\n> > * new optional --conflicted-list flag for specifying a file where the names\n> > of conflicted-files can be written in a NUL-character-separated list\n>\n> It would be nice if output was printed on stdout when the above flags\n> are used without argument.\n\nOh, that's an interesting idea. The --conflicted-list flag, though,\nseparates filenames by NUL characters, for simplicity of parsing. If\nI'm printing them to stdout, would that be problematic? (If so, should\nit instead print them in e.g. ls-tree format, where it escapes\nfilenames only when necessary)?\n\n> Thanks for working on this!\n"},{"id":"445745","messageId":"xmqqmtk78dty.fsf@gitster.g","threadId":"57169","inReplyTo":"CABPp-BFUJ6pU_CKM7ccnFvi0nkeeGfd2GETdksKLaz=B_=BZAQ@mail.gmail.com","subject":"Re: [PATCH v2 4/8] merge-tree: implement real merges","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-01-07T20:56:25Z","receivedAt":"2022-01-07T20:56:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n>> 'git merge-tree' [--write-tree] \n>> 'git merge-tree' [--demo-trivial-merge] \n>>\n>> That way, the old mode can still function, and can even at some stage be\n>> deprecated and eventually removed.\n>\n> Ooh, interesting.\n\nI wondered if we can _also_ extend the trivial-merge mode so that we\ndo not have to call it \"demo\".\n\nThe internal result is expressed in this way:\n\n struct merge_list {\n struct merge_list *next;\n struct merge_list *link;\t/* other stages for this object */\n\n unsigned int stage : 2;\n unsigned int mode;\n const char *path;\n struct blob *blob;\n };\n\nbecause the command was not designed to resolve content level\nmerges, but show the half-resolved state with the \"stage\" number.\nThe \"explanation\" the command gives on the result is truly trivial,\nbut there is no reason for it to stay that way.\n"},{"id":"445748","messageId":"96a83437-1858-bbe4-5218-2d8defcd5fe4@web.de","threadId":"57169","inReplyTo":"CABPp-BE5breKX5TciAwzKi+BQnqy1aKq_v4tjiqiX7swrZf=PA@mail.gmail.com","subject":"Re: [PATCH v2 0/8] RFC: Server side merges (no ref updating, no commit creating, no touching worktree or index)","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2022-01-07T21:26:05Z","receivedAt":"2022-01-07T21:26:11Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 07.01.22 um 20:59 schrieb Elijah Newren:\n> On Fri, Jan 7, 2022 at 10:46 AM Christian Couder\n> wrote:\n>>\n>> On Wed, Jan 5, 2022 at 6:27 PM Elijah Newren via GitGitGadget\n>> wrote:\n>>\n>>> This series attempts to guess what kind of output would be wanted, basically\n>>> choosing:\n>>>\n>>> * clean merge or conflict signalled via exit status\n>>\n>> (Maybe s/signalled/signaled/)\n>\n> I can't determine the difference after a few Google searches, and both\n> seem to be in dictionaries with the same meaning so I'm having\n> difficulty figuring out which is preferred. Usually my searches will\n> either suggest that one is a misspelling or at least bring up whether\n> one is a regional variance, but I'm not seeing anything of the sort.\n>\n> It can't hurt to switch, though, so I'm happy to switch.\n\nhttps://en.wiktionary.org/wiki/signal#Verb says: \"present participle\n(UK) signalling or (US) signaling\". And Documentation/SubmittingPatches\nsays: \"We prefer to gradually reconcile the inconsistencies in favor of\nUS English\". So this seems to go in the right direction.\n\nRené\n"},{"id":"445756","messageId":"CABPp-BHvXrP0sTTmuTYfACoJTCcm9+wk_f441nj4TstrmQdqMQ@mail.gmail.com","threadId":"57169","inReplyTo":"nycvar.QRO.7.76.6.2201071908580.339@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2 8/8] merge-tree: provide an easy way to access which files have conflicts","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-01-07T22:12:56Z","receivedAt":"2022-01-07T22:13:11Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, Jan 7, 2022 at 11:36 AM Johannes Schindelin\n wrote:\n>\n> Hi Elijah,\n>\n> On Wed, 5 Jan 2022, Elijah Newren via GitGitGadget wrote:\n>\n> > From: Elijah Newren \n> >\n> > Callers of `git merge-tree --real` might want an easy way to determine\n> > which files conflicted. While they could potentially use the --messages\n> > option and parse the resulting messages written to that file, those\n> > messages are not meant to be machine readable. Provide a simpler\n> > mechanism of having the user specify --unmerged-list=$FILENAME, and then\n> > write a NUL-separated list of unmerged filenames to the specified file.\n>\n> This patch does what the commit message says, and it looks quite\n> plausible. However, in practice it seems that you need either a tree (if\n> the merge succeeded) or the list of conflicted files (if the merge\n> succeeded).\n>\n> So while it looks relatively clean from the implementation's point of\n> view, the design itself could probably withstand a bit of consideration.\n>\n> As I hinted earlier (to be precise, in\n> https://lore.kernel.org/git/nycvar.QRO.7.76.6.2201071602110.339@tvgsbejvaqbjf.bet/),\n> I had the chance in December to work on the server-side, using `merge-ort`\n> for a bit. In the following, I will talk about this a bit more than about\n> this particular patch, but I think it is highly relevant (not a tangent).\n>\n> One of the things that became clear to me is that we really have an\n> either/or situation here. Either the merge succeeds, and we _need_ that\n> tree, or it fails, and we could not care less about the tree at all.\n>\n> In fact, if the merge fails, we completely ignore the tree, and it would\n> be better if we would not even write out any Git objects in that case at\n> all: even just writing the objects would be quite costly at the\n> server-side scale.\n>\n> So my (somewhat hacky) patches for a proof-of-concept produced _either_\n> the hash of the tree on `stdout`, _or_ a header saying that there were\n> conflicts followed by a NUL-separated list of file names.\n\nDid you really check that it only produced one of these? If you were\nusing ort, you wrote blob and tree objects to disk, even if you didn't\nprint their hash on stdout.\n\n> Mind you, I did not even get to the point of analyzing things even more\n> deeply. My partner in crime and I only got to comparing the `merge-ort`\n> way to the libgit2-based way, trying to get them to compare as much\n> apples-to-apples as possible [*1*], and we found that even the time to\n> spawn the Git process (~1-3ms, with all overhead counted in) is _quite_\n> noticeable, at server-side scale.\n\n1-3ms? I thought it was a good bit more than that.\n\n> Of course, the `merge-ort` performance was _really_ nice when doing\n> anything remotely complex, then `merge-ort` really blew the libgit2-based\n> merge out of the water. But that's not the common case. The common case\n> are merges that involve very few modified files, a single merge base, and\n> they don't conflict. And those can be processed by the libgit2-based\n> method within a fraction of the time it takes to even only so much as\n> spawn `git` (libgit2-based merges can complete in less than a fifth\n> millisecond, that's at most a fifth of the time it takes to merely run\n> `git merge-tree`).\n>\n> The difference between 0.2-0.5ms for libgit2-based merges on the one hand,\n> and 1-3ms for `merge-ort`-based merges on the other hand, might not seem\n> like much, but you have to multiply it by the times such a merge is\n> performed on the server. Which is a _lot_. Way more often than I thought.\n\nAh, I had been wondering a bit about process overhead. Having\nsomething that avoids that is definitely helpful. I tried to design\nort such that its API is relatively clean and easy-to-use, and I\ncarefully tested and fixed it until it ran memory-leak free. If\nprocess execution overhead is such a big problem, perhaps you could\nuse these new functions via libgit.a instead of invoking a git process\nand get the best of both worlds?\n\nUnfortunately, in-process would run into problems with finding merge\nbases, because the revision walking machinery is definitely not\nleak-free -- an annoyance I had to deal with while attempting to clean\nup merge-ort since the code I was using always did the merge-base\nfinding as well as the calls into merge-ort. (I hear Ævar has some\nnot-yet-submitted patches that might help with memory leaks in the\nrevision walking machinery.)\n\n> In this particular instance, there is a silver-lining: the libgit2-based\n> merge is not actually recursive. It is a three-way merge. Which means that\n> we first have to determine a merge base. In our case, this is done by\n> spawning a Git process anyway, so one of my ideas to move forward is to fold\n> that merge-base logic into `git merge-tree`, too.\n\nThe merge-base logic is already part of merge-tree in my patches.\nWhat exactly did you do with merge-tree? Were you using the existing\none and feeding it with a merge-base as an input, or did you write\nyour own that was more like Christian's that expected a merge base?\n\n> Anyway, the short short is: whenever we can avoid unnecessary work, we\n> should do so. In the context of this patch, I would say that we should\n> avoid writing out a tree (and avoid printing its hash to `stdout`) if\n> there are merge conflicts.\n\nWe can avoid printing its hash to `stdout`, but it'd take significant\nwork to avoid writing the tree to an object store, and it cannot be\ndone at the merge-tree level, it'd require replumbing some bits of\nmerge-ort (and making some already complex codepaths a bit more\ncomplex, but that's a price I'm willing to pay for significant\nperformance wins).\n\nCan I first suggest a simpler alternative that may give some\nperformance wins despite keeping the object writes:\n\nWe could make use of the tmp_objdir API that recently merged (see\nb3cecf49ea (\"tmp-objdir: new API for creating temporary writable\ndatabases\", 2021-12-06)). We could put that tmp-objdir on /dev/shm or\nother ramdisk, and if the merge is clean, migrate the contents into\nthe real object store. Perhaps we could even pack those objects first\nif there are a large number of them, but If it's not clean, we can\njust discard the tmp-objdir. Also, as a further variant on this\nalternative... packing these objects before migrating if there are a\nsufficient number of them. Now, this is rather unlikely to be needed\nin general by merge-tree, because you only need to write new objects\n(thus representing files modified on both sides, or whatever leading\ntrees are needed to reference the updated paths). However, it might\nmatter for big enough repos with large enough numbers of changes on\nboth sides. And it'd align nicely with my idea for server-side\nrebases (where implementing this is on my TODO list), because\nserver-side rebases are much more likely to generate a large number of\nobjects.\n\nBut if you really want to learn about avoiding object writes...\n\nIf you really want to only write tree and blob objects when the merge\nis clean, then as far as I can tell you have two options in regards to\nthe blobs: (1) you'll need to keep all files from three-way content\nmerges simultaneously in memory until you've determined if the result\nis clean, so that you can then write the merged contents out as blobs\nat the end. Or (2) doing all the three-way content merges and keeping\ntrack of whether the result for each is clean, and if they all turn\nout to be clean, then redo every single one of those three-way content\nmerges afterwards so that you can actually write out the merged-result\nto disk that time.\n\nI think (2) would cost you a lot more work than you'd save, and I\nworry that (1) might risk using large amounts of memory in the big\nrepositories if there are lots of changes on both sides. While that\nmay be uncommon, I've seen folks try to merge things with lots of\nchanges on both sides, and you do have the server side to worry about\nafter all.\n\nThere are similar issues with the fact that trees are written as they\nare processed as well. Those would also require re-running afterwards\nto re-generate the trees from the list of relevant-files-and-trees we\noperate on.\n\nHowever, if you are really curious about trying this out despite the\nfact that I think you might be causing more work than you're avoiding\n(or potentially requiring a lot more RAM), look for calls to\nwrite_tree() (there are precisely two in merge-ort.c, one for\nintermediate trees and one for the toplevel tree) and\nwrite_object_file() (there are precisely two in merge-ort.c, one\nwithin write_tree() for writing tree objects, and one in\nhandle_content_merge() for writing blob objects).\n\n> And we should avoid writing (and later reading)\n> a file, if we can get away with avoiding it.\n\nSure, we can do that. Christian had a suggestion that if the\n--conflicted-list didn't have an associated filename, then we just\nprint those to stdout. Would that be to your liking? And would you\nprefer NUL-separated, or ls-tree style escaping of filenames?\n\n> At least in the default case, that is. We still might need a flag to\n> produce some more information about those merge conflicts. But even in\n> that case, it would be better to have a list of file names with the three\n> associated stages than to output the hash of a tree that contains\n> conflicts (and tons of files _without_ conflicts). The UI needs to\n> re-generate those conflicts anyway. And remember: a tree can contain\n> millions of files even if there is but a single conflict. It makes more\n> sense for `merge-tree --real` to output a concrete list of files that\n> conflicted, rather than expecting the caller to discern between conflicts\n> and non-conflicts by processing a tree object.\n\n???\n\nWhere did I suggest that we discern between conflicts and\nnon-conflicts by processing a tree object? That's not merely\nsomething with atrocious performance, it's also utterly crazy from a\nUI perspective, and is downright *impossible* to achieve in general\nanyway. (Failure to merge binary files. modify/delete conflicts.\nmode conflicts. file/directory conflicts. Various rename\npermutatations. There's all kinds of non-content conflicts that are\nnot representable in a tree, and which folks will miss if they attempt\nto parse a tree and surmise what conflicts there were.) Whatever I\nwrote that might have suggested such a course of action needs some\nserious rewording or clarification; I consider attempting that to be a\nhorrible idea.\n\nThe tree exists so that if people want to get extra information (and\npresumably in a format similar to what they would find in their\nworking directory if they had asked `git merge` to merge those same\ntwo branches on their laptop), then they can do so. For example,\nperhaps in the list of --conflicted-files they notice a file of\ninterest and want to ask, \"What's found in this particular file?\".\nThey can use the tree together with the filename to get that kind of\ninfo.\n\n> Maybe you agree with this rationale and re-design the `--real` mode to try\n> to avoid writing out files in the common case?\n\nI'm totally open to something like having --conflicted-list be given\nwithout a filename (or maybe a '-') and write to stdout instead.\n\nI'm amenable to various tradeoffs to improve performance, but I'm\nworried that \"not-writing-tree-and-blob object files\" sounds like a\ngoal that might hurt performance rather than help it (either worse\nperformance in general due to the need to do things twice when the\nmerge is clean, or else a peak memory usage that is unmanageable and\ncauses problems). Perhaps there's a smarter solution I'm not seeing,\nor my worries about maximum memory usage are overblown. However, if\nyou still want to pursue such core changes, I would also like to see\nmore performance measuring work to justify them -- especially since in\nthe common case merge-ort won't recurse into most directories and will\nend up only writing a few object files (since only new blob or tree\nobjects need to be written). In particular, I think we'd need to do\nthe following first:\n\n * Do a real comparison; libgit2 + the separate find-merge-base git\nprocess, vs. the single `git merge-tree --real` call that handles\nboth. Excluding the merge-base computation makes it apples to oranges\nin my opinion.\n\n * While measuring the performance of the above, specifically use\ntrace2 to measure time spent in write_loose_object() in object-file.c.\nSee how much time object writing actually takes vs. everything else.\n\n * Try the tmp-objdir changes I suggested (examples of using the API\ncan be found by searching for \"tmp_objdir\" at\nhttps://lore.kernel.org/git/d57ae218cf9eaee0b66db299ee1bba9b488b69b1.1640907369.git.gitgitgadget@gmail.com/\nand https://lore.kernel.org/git/e1747ce00af7ab3170a69955b07d995d5321d6f3.1637020263.git.gitgitgadget@gmail.com/),\nand then re-measure the performance particular in respect to\nwrite_loose_object() as a percentage of overall time.\n\n * Get some kind of measure of the maximum number of three-way\ncontent merges needed in merges and the overall size of holding all\nthe results in memory simultaneously. If repositories have a million\nfiles and there are merges involving a high enough percentage of files\nchanges on both sides, then that could certainly theoretically be\nquite large.\n\n\nI also think that doing something in-process (via linking against\nlibgit.a?) rather than forking `git merge-tree --real` would probably\nnet you _much_ bigger wins than avoiding writing these object files,\nthough it'd be unsafe without memory leak fixes for all the merge-base\ncomputations. That, of course, might not be the only reason you're\nleery of such an approach.\n\n> About the form of the patch itself: I was tempted to go with the\n> nitpicking spirit I see on the Git mailing list these days, especially\n> about the shell script code in the test scripts. But then I realized that\n> I find such nitpicking pretty unhelpful, myself. The code is good as-is,\n> even if I would write it differently. It is clear, and it does exactly\n> what it is supposed to do.\n>\n> Thank you,\n> Dscho\n>\n> Footnote *1*: I did not _quite_ get to the point of comparing the\n> `merge-ort` merges to the libgit2 ones, unfortunately. I was on my way to\n> add code to respect `merge.renames = false` so that we could _truly_\n> compare the `merge-ort` merges to the libgit2 merges (we really will want\n> to verify that the output is identical, before even considering to enable\n> recursive merges on the server side, and then only after studying the time\n> difference), and then had to take off due to the holidays. If you already\n> have that need to be able to turn off rename-detection on your radar, even\n> if only for a transitional period, I would be _so_ delighted.\n\nWell, I had no intention of submitting it (and still don't), but I did\nimplement it a while back for folks who have needs for it in a\ntransitional period. It's a pretty simple change.\n\nhttps://github.com/newren/git/commit/5330a9de77f56f20e546acc65c924fc783f092e6\nhttps://github.com/newren/git/commit/2e7e8d79b0995d352558608b6308060fbc055fd1\n\n:-)\n"},{"id":"445771","messageId":"CABPp-BFQTTakkeDkwbiq41ie++dKeje3nGKWMp1Gm-YmvrevJA@mail.gmail.com","threadId":"57169","inReplyTo":"nycvar.QRO.7.76.6.2201071906050.339@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2 7/8] merge-tree: support saving merge messages to a separate file","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-01-08T01:02:25Z","receivedAt":"2022-01-08T01:02:39Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, Jan 7, 2022 at 10:07 AM Johannes Schindelin\n wrote:\n>\n> Hi Elijah,\n>\n> On Wed, 5 Jan 2022, Elijah Newren via GitGitGadget wrote:\n>\n> > diff --git a/t/t4301-merge-tree-real.sh b/t/t4301-merge-tree-real.sh\n> > index f7aa310f8c1..5f3f27f504d 100755\n> > --- a/t/t4301-merge-tree-real.sh\n> > +++ b/t/t4301-merge-tree-real.sh\n> > @@ -78,4 +78,22 @@ test_expect_success 'Barf on too many arguments' '\n> > grep \"^usage: git merge-tree\" expect\n> > '\n> >\n> > +test_expect_success '--messages gives us the conflict notices and such' '\n> > + test_must_fail git merge-tree --real --messages=MSG_FILE side1 side2 &&\n>\n> Since we discern between exit status 1 (= merge conflict) and >1 (fatal\n> error), we should probably use `test_expect_code` here.\n\nGood point; will do.\n"},{"id":"445773","messageId":"CABPp-BFCvOkC1KSVm3qKUcaBFV0pUg4MJf5h+shj=TFZzWscUA@mail.gmail.com","threadId":"57169","inReplyTo":"nycvar.QRO.7.76.6.2201071908580.339@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2 8/8] merge-tree: provide an easy way to access which files have conflicts","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-01-08T01:28:04Z","receivedAt":"2022-01-08T01:28:22Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi Dscho,\n\nOne more thing I forgot to ask...\n\nOn Fri, Jan 7, 2022 at 11:36 AM Johannes Schindelin\n wrote:\n>\n...\n> Mind you, I did not even get to the point of analyzing things even more\n> deeply. My partner in crime and I only got to comparing the `merge-ort`\n> way to the libgit2-based way, trying to get them to compare as much\n> apples-to-apples as possible [*1*], and we found that even the time to\n> spawn the Git process (~1-3ms, with all overhead counted in) is _quite_\n> noticeable, at server-side scale.\n>\n> Of course, the `merge-ort` performance was _really_ nice when doing\n> anything remotely complex, then `merge-ort` really blew the libgit2-based\n> merge out of the water. But that's not the common case. The common case\n> are merges that involve very few modified files, a single merge base, and\n> they don't conflict. And those can be processed by the libgit2-based\n> method within a fraction of the time it takes to even only so much as\n> spawn `git` (libgit2-based merges can complete in less than a fifth\n> millisecond, that's at most a fifth of the time it takes to merely run\n> `git merge-tree`).\n\nOut of curiosity, are you only doing merges, or are you also\nattempting server-side rebases in some fashion?\n"},{"id":"445921","messageId":"nycvar.QRO.7.76.6.2201111435430.1081@tvgsbejvaqbjf.bet","threadId":"57169","inReplyTo":"xmqqmtk78dty.fsf@gitster.g","subject":"Re: [PATCH v2 4/8] merge-tree: implement real merges","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-01-11T13:39:04Z","receivedAt":"2022-01-11T13:39:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Fri, 7 Jan 2022, Junio C Hamano wrote:\n\n> Elijah Newren writes:\n>\n> >> 'git merge-tree' [--write-tree] \n> >> 'git merge-tree' [--demo-trivial-merge] \n> >>\n> >> That way, the old mode can still function, and can even at some stage be\n> >> deprecated and eventually removed.\n> >\n> > Ooh, interesting.\n>\n> I wondered if we can _also_ extend the trivial-merge mode so that we\n> do not have to call it \"demo\".\n>\n> The internal result is expressed in this way:\n>\n> struct merge_list {\n> struct merge_list *next;\n> struct merge_list *link;\t/* other stages for this object */\n>\n> unsigned int stage : 2;\n> unsigned int mode;\n> const char *path;\n> struct blob *blob;\n> };\n>\n> because the command was not designed to resolve content level\n> merges, but show the half-resolved state with the \"stage\" number.\n> The \"explanation\" the command gives on the result is truly trivial,\n> but there is no reason for it to stay that way.\n\nThe original `merge-tree` code outputs a diff, which I think has now been\nfirmly established as something a low-level merge tool should not do at\nall.\n\nSo I am not sure how necessary it is to maintain the original UI. I don't\nthink it is a good UI. In fact, I am rather certain that we will want to\nget rid of it.\n\nWe can keep it for backwards-compatibility for now, keeping it working for\nexisting users (if any!) by that 3-arg vs 2-arg trick, eventually\ndeprecate and then remove it.\n\nCiao,\nDscho\n"},{"id":"449121","messageId":"nycvar.QRO.7.76.6.2202221345510.11118@tvgsbejvaqbjf.bet","threadId":"57169","inReplyTo":"CABPp-BHvXrP0sTTmuTYfACoJTCcm9+wk_f441nj4TstrmQdqMQ@mail.gmail.com","subject":"Re: [PATCH v2 8/8] merge-tree: provide an easy way to access which files have conflicts","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-02-22T13:03:46Z","receivedAt":"2022-02-22T13:03:54Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Elijah,\n\nI meant to answer this mail much earlier, oh well. Sorry. I will only\nanswer the still open questions below, clipping the quoted text to save\nevery reader some time.\n\nOn Fri, 7 Jan 2022, Elijah Newren wrote:\n\n> On Fri, Jan 7, 2022 at 11:36 AM Johannes Schindelin\n> wrote:\n>\n> > So my (somewhat hacky) patches for a proof-of-concept produced\n> > _either_ the hash of the tree on `stdout`, _or_ a header saying that\n> > there were conflicts followed by a NUL-separated list of file names.\n>\n> Did you really check that it only produced one of these? If you were\n> using ort, you wrote blob and tree objects to disk, even if you didn't\n> print their hash on stdout.\n\nD'oh. No, I had not checked that.\n\n> > Mind you, I did not even get to the point of analyzing things even more\n> > deeply. My partner in crime and I only got to comparing the `merge-ort`\n> > way to the libgit2-based way, trying to get them to compare as much\n> > apples-to-apples as possible [*1*], and we found that even the time to\n> > spawn the Git process (~1-3ms, with all overhead counted in) is _quite_\n> > noticeable, at server-side scale.\n>\n> 1-3ms? I thought it was a good bit more than that.\n\nThe absolute number really depends on a lot of factors, i.e. what is\nrelevant is the relative difference between in-process vs spawning a new\nprocess.\n\n> If process execution overhead is such a big problem, perhaps you could\n> use these new functions via libgit.a instead of invoking a git process\n> and get the best of both worlds?\n\nFor now, I solved this by combining multiple Git process invocations into\na single one, as described here:\n\n> > In this particular instance, there is a silver-lining: the\n> > libgit2-based merge is not actually recursive. It is a three-way\n> > merge. Which means that we first have to determine a merge base. In\n> > our case, this is done by spawning a Git process anyway, so one of my\n> > ideas to move forward is to fold that merge-base logic into `git\n> > merge-tree`, too.\n>\n> The merge-base logic is already part of merge-tree in my patches.\n> What exactly did you do with merge-tree? Were you using the existing\n> one and feeding it with a merge-base as an input, or did you write\n> your own that was more like Christian's that expected a merge base?\n\nFor an apples-to-apples comparison, I had to stay with the very same merge\nbase logic as before, i.e. originally I added a new option to `merge-tree`\nthat would take a merge base and then take a non-recursive path. In my\ncurrent version, it is merely an option that even determines said merge\nbase in the same process (and yes, it leaks memory, but that does not\nmatter because the process is short-lived anyway).\n\n> > Anyway, the short short is: whenever we can avoid unnecessary work, we\n> > should do so. In the context of this patch, I would say that we should\n> > avoid writing out a tree (and avoid printing its hash to `stdout`) if\n> > there are merge conflicts.\n>\n> We can avoid printing its hash to `stdout`, but it'd take significant\n> work to avoid writing the tree to an object store, and it cannot be\n> done at the merge-tree level, it'd require replumbing some bits of\n> merge-ort (and making some already complex codepaths a bit more\n> complex, but that's a price I'm willing to pay for significant\n> performance wins).\n\nYes, let's leave things as-are. It is not worth the trouble.\n\n> We could make use of the tmp_objdir API that recently merged (see\n> b3cecf49ea (\"tmp-objdir: new API for creating temporary writable\n> databases\", 2021-12-06)). We could put that tmp-objdir on /dev/shm or\n> other ramdisk, and if the merge is clean, migrate the contents into\n> the real object store. Perhaps we could even pack those objects first\n> if there are a large number of them, but If it's not clean, we can\n> just discard the tmp-objdir. Also, as a further variant on this\n> alternative... packing these objects before migrating if there are a\n> sufficient number of them. Now, this is rather unlikely to be needed\n> in general by merge-tree, because you only need to write new objects\n> (thus representing files modified on both sides, or whatever leading\n> trees are needed to reference the updated paths). However, it might\n> matter for big enough repos with large enough numbers of changes on\n> both sides. And it'd align nicely with my idea for server-side\n> rebases (where implementing this is on my TODO list), because\n> server-side rebases are much more likely to generate a large number of\n> objects.\n>\n> But if you really want to learn about avoiding object writes...\n>\n> If you really want to only write tree and blob objects when the merge\n> is clean, then as far as I can tell you have two options in regards to\n> the blobs: (1) you'll need to keep all files from three-way content\n> merges simultaneously in memory until you've determined if the result\n> is clean, so that you can then write the merged contents out as blobs\n> at the end. Or (2) doing all the three-way content merges and keeping\n> track of whether the result for each is clean, and if they all turn\n> out to be clean, then redo every single one of those three-way content\n> merges afterwards so that you can actually write out the merged-result\n> to disk that time.\n>\n> I think (2) would cost you a lot more work than you'd save, and I\n> worry that (1) might risk using large amounts of memory in the big\n> repositories if there are lots of changes on both sides. While that\n> may be uncommon, I've seen folks try to merge things with lots of\n> changes on both sides, and you do have the server side to worry about\n> after all.\n>\n> There are similar issues with the fact that trees are written as they\n> are processed as well. Those would also require re-running afterwards\n> to re-generate the trees from the list of relevant-files-and-trees we\n> operate on.\n>\n> However, if you are really curious about trying this out despite the\n> fact that I think you might be causing more work than you're avoiding\n> (or potentially requiring a lot more RAM), look for calls to\n> write_tree() (there are precisely two in merge-ort.c, one for\n> intermediate trees and one for the toplevel tree) and\n> write_object_file() (there are precisely two in merge-ort.c, one\n> within write_tree() for writing tree objects, and one in\n> handle_content_merge() for writing blob objects).\n\nThank you for this thorough analysis.\n\nI am a big fan of crossing bridges when they are reached, and not miles\nbefore that. So _iff_ it turns out that the speed, or the potential\ncluttering with objects, should present a problem in the future, I am\ninclined to follow the tmp-objdir route you described above (thank you for\npointing it out, I had not made the connection to this here scenario).\n\n> > Footnote *1*: I did not _quite_ get to the point of comparing the\n> > `merge-ort` merges to the libgit2 ones, unfortunately. I was on my way to\n> > add code to respect `merge.renames = false` so that we could _truly_\n> > compare the `merge-ort` merges to the libgit2 merges (we really will want\n> > to verify that the output is identical, before even considering to enable\n> > recursive merges on the server side, and then only after studying the time\n> > difference), and then had to take off due to the holidays. If you already\n> > have that need to be able to turn off rename-detection on your radar, even\n> > if only for a transitional period, I would be _so_ delighted.\n>\n> Well, I had no intention of submitting it (and still don't), but I did\n> implement it a while back for folks who have needs for it in a\n> transitional period. It's a pretty simple change.\n>\n> https://github.com/newren/git/commit/5330a9de77f56f20e546acc65c924fc783f092e6\n> https://github.com/newren/git/commit/2e7e8d79b0995d352558608b6308060fbc055fd1\n\nThank you _so_ much for that. It was super helpful, and it allowed me to\ngather enough evidence to justify continuing to work on this code.\n\nCiao,\nDscho\n"},{"id":"449123","messageId":"nycvar.QRO.7.76.6.2202221403590.11118@tvgsbejvaqbjf.bet","threadId":"57169","inReplyTo":"CABPp-BFCvOkC1KSVm3qKUcaBFV0pUg4MJf5h+shj=TFZzWscUA@mail.gmail.com","subject":"Re: [PATCH v2 8/8] merge-tree: provide an easy way to access which files have conflicts","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-02-22T13:05:31Z","receivedAt":"2022-02-22T13:05:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Elijah,\n\nOn Fri, 7 Jan 2022, Elijah Newren wrote:\n\n> On Fri, Jan 7, 2022 at 11:36 AM Johannes Schindelin\n> wrote:\n> >\n> ...\n> > Mind you, I did not even get to the point of analyzing things even more\n> > deeply. My partner in crime and I only got to comparing the `merge-ort`\n> > way to the libgit2-based way, trying to get them to compare as much\n> > apples-to-apples as possible [*1*], and we found that even the time to\n> > spawn the Git process (~1-3ms, with all overhead counted in) is _quite_\n> > noticeable, at server-side scale.\n> >\n> > Of course, the `merge-ort` performance was _really_ nice when doing\n> > anything remotely complex, then `merge-ort` really blew the libgit2-based\n> > merge out of the water. But that's not the common case. The common case\n> > are merges that involve very few modified files, a single merge base, and\n> > they don't conflict. And those can be processed by the libgit2-based\n> > method within a fraction of the time it takes to even only so much as\n> > spawn `git` (libgit2-based merges can complete in less than a fifth\n> > millisecond, that's at most a fifth of the time it takes to merely run\n> > `git merge-tree`).\n>\n> Out of curiosity, are you only doing merges, or are you also\n> attempting server-side rebases in some fashion?\n\nOne step after another. For now, I am focusing on merges.\n\nBut yes, rebases are on my radar, too, and I am very grateful for the\nhead-start you provided in `t/helper/test-fast-rebase.c` (and for the pun\ntherein).\n\nCiao,\nDscho\n"}]}