From 344a107b557604d3f958e7bf7ebf0901290c50d5 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Tue, 18 Feb 2025 16:24:35 +0000 Subject: [PATCH 1/5] merge-tree --stdin: flush stdout to avoid deadlock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If a process tries to read the output from "git merge-tree --stdin" before it closes merge-tree's stdin then it deadlocks. This happens because merge-tree does not flush its output before trying to read another line of input and means that it is not possible to cherry-pick a sequence of commits using "git merge-tree --stdin". Fix this by calling maybe_flush_or_die() before trying to read the next line of input. Flushing the output after each merge does not seem to affect the performance, any difference is lost in the noise even after increasing the number of runs. $ git rev-list --merges --parents -n100 origin/master | sed 's/^[^ ]* //' >/tmp/merges $ hyperfine -L flush 0,1 --warmup 1 --runs 30 \ 'GIT_FLUSH={flush} ./git merge-tree --stdin Acked-by: Elijah Newren Signed-off-by: Junio C Hamano --- builtin/merge-tree.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c index 9a6c8b4e4c..57f4340fab 100644 --- a/builtin/merge-tree.c +++ b/builtin/merge-tree.c @@ -18,6 +18,7 @@ #include "tree.h" #include "config.h" #include "strvec.h" +#include "write-or-die.h" static int line_termination = '\n'; @@ -623,6 +624,7 @@ int cmd_merge_tree(int argc, } else { die(_("malformed input line: '%s'."), buf.buf); } + maybe_flush_or_die(stdout, "stdout"); if (result < 0) die(_("merging cannot continue; got unclean result of %d"), result); From 1b0e5f4499a0c099d99b00a2a6a3edb45ae98660 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Tue, 18 Feb 2025 16:24:36 +0000 Subject: [PATCH 2/5] merge-tree: remove redundant code real_merge() only ever returns "0" or "1" as it dies if the merge status is less than zero. Therefore the check for "result < 0" is redundant and the result variable is not needed. The return value of real_merge() is ignored because exit status of "git merge-tree --stdin" is "0" for both successful and conflicted merges (the status of each merge is written to stdout). The return type of real_merge() is not changed as it is used for the program's exit status when "--stdin" is not given. Signed-off-by: Phillip Wood Acked-by: Elijah Newren Signed-off-by: Junio C Hamano --- builtin/merge-tree.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c index 57f4340fab..3c73482f2b 100644 --- a/builtin/merge-tree.c +++ b/builtin/merge-tree.c @@ -601,7 +601,6 @@ int cmd_merge_tree(int argc, line_termination = '\0'; while (strbuf_getline_lf(&buf, stdin) != EOF) { struct strbuf **split; - int result; const char *input_merge_base = NULL; split = strbuf_split(&buf, ' '); @@ -618,16 +617,14 @@ int cmd_merge_tree(int argc, if (input_merge_base && split[2] && split[3] && !split[4]) { strbuf_rtrim(split[2]); strbuf_rtrim(split[3]); - result = real_merge(&o, input_merge_base, split[2]->buf, split[3]->buf, prefix); + real_merge(&o, input_merge_base, split[2]->buf, split[3]->buf, prefix); } else if (!input_merge_base && !split[2]) { - result = real_merge(&o, NULL, split[0]->buf, split[1]->buf, prefix); + real_merge(&o, NULL, split[0]->buf, split[1]->buf, prefix); } else { die(_("malformed input line: '%s'."), buf.buf); } maybe_flush_or_die(stdout, "stdout"); - if (result < 0) - die(_("merging cannot continue; got unclean result of %d"), result); strbuf_list_free(split); } strbuf_release(&buf); From 54cf5d2da897d4ca2ed4872bd9b7e48851e5573e Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Tue, 18 Feb 2025 16:24:37 +0000 Subject: [PATCH 3/5] merge-tree: only use basic merge config Commit 9c93ba4d0ae (merge-recursive: honor diff.algorithm, 2024-07-13) replaced init_merge_options() with init_basic_merge_config() for use in plumbing commands and init_ui_merge_config() for use in porcelain commands. As "git merge-tree" is a plumbing command it should call init_basic_merge_config() rather than init_ui_merge_config(). The merge ort machinery ignores "diff.algorithm" so the behavior is unchanged by this commit but it future proofs us against any future changes to init_ui_merge_config(). Signed-off-by: Phillip Wood Acked-by: Elijah Newren Signed-off-by: Junio C Hamano --- builtin/merge-tree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c index 3c73482f2b..3ec7127b3a 100644 --- a/builtin/merge-tree.c +++ b/builtin/merge-tree.c @@ -576,7 +576,7 @@ int cmd_merge_tree(int argc, }; /* Init merge options */ - init_ui_merge_options(&o.merge_options, the_repository); + init_basic_merge_options(&o.merge_options, the_repository); /* Parse arguments */ original_argc = argc - 1; /* ignoring argv[0] */ From 3e681a7ccc97b81f9c93e5b4ca6d3a85d9817285 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Tue, 18 Feb 2025 16:24:38 +0000 Subject: [PATCH 4/5] merge-tree: improve docs for --stdin Add a section for --stdin in the list of options and document that it implies -z so readers know how to parse the output. Also correct the merge status documentation for --stdin as if the status is less than zero "git merge-tree" dies before printing it. Signed-off-by: Phillip Wood Acked-by: Elijah Newren Signed-off-by: Junio C Hamano --- Documentation/git-merge-tree.txt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt index 0b6a8a19b1..efb16b4f27 100644 --- a/Documentation/git-merge-tree.txt +++ b/Documentation/git-merge-tree.txt @@ -40,6 +40,11 @@ After the merge completes, a new toplevel tree object is created. See OPTIONS ------- +--stdin:: + Read the commits to merge from the standard input rather than + the command-line. See <> below for more + information. Implies `-z`. + -z:: Do not quote filenames in the section, and end each filename with a NUL character rather than @@ -116,8 +121,6 @@ This is an integer status followed by a NUL character. The integer status is: 0: merge had conflicts 1: merge was clean - <0: something prevented the merge from running (e.g. access to repository - objects denied by filesystem) [[OIDTLT]] OID of toplevel tree @@ -235,6 +238,7 @@ with linkgit:git-merge[1]: * any messages that would have been printed to stdout (the <>) +[[INPUT]] INPUT FORMAT ------------ 'git merge-tree --stdin' input format is fully text based. Each line From 6a9ae8101525360b8f79ed20d2f483616bd39c90 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Tue, 18 Feb 2025 16:24:39 +0000 Subject: [PATCH 5/5] merge-tree: fix link formatting in html docs In the html documentation the link to the "OUTPUT" section is surrounded by square brackets. Fix this by adding explicit link text to the cross reference. Signed-off-by: Phillip Wood Acked-by: Elijah Newren Signed-off-by: Junio C Hamano --- Documentation/git-merge-tree.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt index efb16b4f27..cf0578f9b5 100644 --- a/Documentation/git-merge-tree.txt +++ b/Documentation/git-merge-tree.txt @@ -49,7 +49,8 @@ OPTIONS Do not quote filenames in the section, and end each filename with a NUL character rather than newline. Also begin the messages section with a NUL character - instead of a newline. See <> below for more information. + instead of a newline. See <> below for more + information. --name-only:: In the Conflicted file info section, instead of writing a list