Re: [PATCH 7/8] merge-tree: support saving merge messages to a separate file

[Fabian Stelzer <fs@gigacodes.de>]

On 31.12.2021 05:04, Elijah Newren via GitGitGadget wrote:
>From: Elijah Newren <newren@gmail.com>
>
>When running `git merge-tree --real`, we previously would only return an
>exit status reflecting the cleanness of a merge, and print out the
>toplevel tree of the resulting merge.  Merges also have informational
>messages, ("Auto-merging <PATH>", "CONFLICT (content): ...", "CONFLICT
>(file/directory)", etc.)  In fact, when non-content conflicts occur
>(such as file/directory, modify/delete, add/add with differing modes,
>rename/rename (1to2), etc.), these informational messages are often the
>only notification since these conflicts are not representable in the
>contents of the file.
>
>Add a --messages option which names a file so that callers can request
>these messages be recorded somewhere.
>
>Signed-off-by: Elijah Newren <newren@gmail.com>
>---
> Documentation/git-merge-tree.txt |  6 ++++--
> builtin/merge-tree.c             | 18 ++++++++++++++++--
> t/t4301-merge-tree-real.sh       | 18 ++++++++++++++++++
> 3 files changed, 38 insertions(+), 4 deletions(-)
>
>diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt
>index 5823938937f..4d5857b390b 100644
>--- a/Documentation/git-merge-tree.txt
>+++ b/Documentation/git-merge-tree.txt
>@@ -9,7 +9,7 @@ git-merge-tree - Perform merge without touching index or working tree
> SYNOPSIS
> --------
> [verse]
>-'git merge-tree' --real <branch1> <branch2>
>+'git merge-tree' --real [--messages=<file>] <branch1> <branch2>
> 'git merge-tree' <base-tree> <branch1> <branch2>
>
> DESCRIPTION
>@@ -21,7 +21,9 @@ The first form will merge the two branches, doing a full recursive
> merge with rename detection.  If the merge is clean, the exit status
> will be `0`, and if the merge has conflicts, the exit status will be
> `1`.  The output will consist solely of the resulting toplevel tree
>-(which may have files including conflict markers).
>+(which may have files including conflict markers).  With `--messages`,
>+it will write any informational messages (such as "Auto-merging
>+<path>" and conflict notices) to the given file.
>
> The second form is meant for backward compatibility and will only do a
> trival merge.  It reads three tree-ish, and outputs trivial merge
>diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
>index c5757bed5bb..47deef0b199 100644
>--- a/builtin/merge-tree.c
>+++ b/builtin/merge-tree.c
>@@ -389,6 +389,7 @@ static int trivial_merge(const char *base,
>
> struct merge_tree_options {
> 	int real;
>+	char *messages_file;
> };
>
> static int real_merge(struct merge_tree_options *o,
>@@ -442,8 +443,15 @@ static int real_merge(struct merge_tree_options *o,
> 	 */
>
> 	merge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);
>+
>+	if (o->messages_file) {
>+		FILE *fp = xfopen(o->messages_file, "w");
>+		merge_display_update_messages(&opt, &result, fp);
>+		fclose(fp);

I don't know enough about how merge-ort works internally, but it looks to me
like at this point the merge already happened and we just didn't clean up 
(finalize) yet. It feels wrong to die() at this point just because we can't 
open messages_file.

>+	}
> 	printf("%s\n", oid_to_hex(&result.tree->object.oid));
>-	merge_switch_to_result(&opt, NULL, &result, 0, 0);
>+
>+	merge_finalize(&opt, &result);
> 	return result.clean ? 0 : 1;
> }
>
>@@ -451,15 +459,18 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)
> {
> 	struct merge_tree_options o = { 0 };
> 	int expected_remaining_argc;
>+	int original_argc;
>
> 	const char * const merge_tree_usage[] = {
>-		N_("git merge-tree --real <branch1> <branch2>"),
>+		N_("git merge-tree --real [<options>] <branch1> <branch2>"),
> 		N_("git merge-tree <base-tree> <branch1> <branch2>"),
> 		NULL
> 	};
> 	struct option mt_options[] = {
> 		OPT_BOOL(0, "real", &o.real,
> 			 N_("do a real merge instead of a trivial merge")),
>+		OPT_STRING(0, "messages", &o.messages_file, N_("file"),
>+			   N_("filename to write informational/conflict messages to")),
> 		OPT_END()
> 	};
>
>@@ -468,8 +479,11 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)
> 		usage_with_options(merge_tree_usage, mt_options);
>
> 	/* Parse arguments */
>+	original_argc = argc;
> 	argc = parse_options(argc, argv, prefix, mt_options,
> 			     merge_tree_usage, 0);
>+	if (!o.real && original_argc < argc)
>+		die(_("--real must be specified if any other options are"));
> 	expected_remaining_argc = (o.real ? 2 : 3);
> 	if (argc != expected_remaining_argc)
> 		usage_with_options(merge_tree_usage, mt_options);
>diff --git a/t/t4301-merge-tree-real.sh b/t/t4301-merge-tree-real.sh
>index 9fb617ccc7f..42218cdc019 100755
>--- a/t/t4301-merge-tree-real.sh
>+++ b/t/t4301-merge-tree-real.sh
>@@ -78,4 +78,22 @@ test_expect_success 'Barf on too many arguments' '
> 	grep "^usage: git merge-tree" expect
> '
>
>+test_expect_success '--messages gives us the conflict notices and such' '
>+	test_must_fail git merge-tree --real --messages=MSG_FILE side1 side2 &&
>+
>+	# Expected results:
>+	#   "greeting" should merge with conflicts
>+	#   "numbers" should merge cleanly
>+	#   "whatever" has *both* a modify/delete and a file/directory conflict
>+	cat <<-EOF >expect &&
>+	Auto-merging greeting
>+	CONFLICT (content): Merge conflict in greeting
>+	Auto-merging numbers
>+	CONFLICT (file/directory): directory in the way of whatever from side1; moving it to whatever~side1 instead.
>+	CONFLICT (modify/delete): whatever~side1 deleted in side2 and modified in side1.  Version side1 of whatever~side1 left in tree.
>+	EOF
>+
>+	test_cmp expect MSG_FILE
>+'
>+
> test_done
>-- 
>gitgitgadget
>