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

Commit Message

Elijah Newren Jan. 5, 2022, 5:27 p.m. UTC

From: Elijah Newren <[e-mail]>

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 <[e-mail]>
---
 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(-)

Comments

Johannes Schindelin Jan. 7, 2022, 6:07 p.m. UTC | #1

Hi Elijah,

On Wed, 5 Jan 2022, Elijah Newren via GitGitGadget wrote:

> diff --git a/t/t4301-merge-tree-real.sh b/t/t4301-merge-tree-real.sh
> index f7aa310f8c1..5f3f27f504d 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 &&

Since we discern between exit status 1 (= merge conflict) and >1 (fatal
error), we should probably use `test_expect_code` here.

Other than that, this patch looks good.

Thank you,
Dscho

> +
> +	# 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
>
>

Elijah Newren Jan. 8, 2022, 1:02 a.m. UTC | #2

On Fri, Jan 7, 2022 at 10:07 AM Johannes Schindelin
<[e-mail]> wrote:
>
> Hi Elijah,
>
> On Wed, 5 Jan 2022, Elijah Newren via GitGitGadget wrote:
>
> > diff --git a/t/t4301-merge-tree-real.sh b/t/t4301-merge-tree-real.sh
> > index f7aa310f8c1..5f3f27f504d 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 &&
>
> Since we discern between exit status 1 (= merge conflict) and >1 (fatal
> error), we should probably use `test_expect_code` here.

Good point; will do.

Patch

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 ac50f3d108b..46b746b6b7c 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -390,6 +390,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,
@@ -443,8 +444,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);
+	}
 	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;
 }
 
@@ -452,15 +460,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()
 	};
 
@@ -469,8 +480,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 f7aa310f8c1..5f3f27f504d 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