[v2,10/13] merge-tree: provide a list of which files have conflicts

Commit Message

Elijah Newren Jan. 29, 2022, 6:07 p.m. UTC

From: Elijah Newren <[e-mail]>

Callers of `git merge-tree --write-tree` will often want to know which
files had conflicts.  While they could potentially attempt to parse the
CONFLICT notices printed, those messages are not meant to be machine
readable.  Provide a simpler mechanism of just printing the files (in
the same format as `git ls-files` with quoting, but restricted to
unmerged files) in the output before the free-form messages.

Signed-off-by: Elijah Newren <[e-mail]>
---
 Documentation/git-merge-tree.txt |  8 ++++++++
 builtin/merge-tree.c             | 24 ++++++++++++++++++++++--
 t/t4301-merge-tree-write-tree.sh | 11 +++++++++++
 3 files changed, 41 insertions(+), 2 deletions(-)

Comments

Junio C Hamano Feb. 2, 2022, 9:32 p.m. UTC | #1

"Elijah Newren via GitGitGadget" <[e-mail]> writes:

> +Conflicted file list
> +~~~~~~~~~~~~~~~~~~~~
> +
> +This is a sequence of lines containing a filename on each line, quoted
> +as explained for the configuration variable `core.quotePath` (see
> +linkgit:git-config[1]).

Makes sense.  Ideally things like this should be discoverable by
inspecting the tree object shown as the result of the (conflicted)
merge, but since the design of the output is to show only a single
tree, there is nowhere to store such an extra piece of information
per path (grepping for markers in blobs of course does not count).

I guess an alternative to show four trees when conflicted instead of
one (i.e. the primary tree may either contain only the cleanly
merged paths _or_ also blobs with conflict markers for conflicted
paths; the three other trees record three stages that would be in
the index, if we were performing the same merge using the index),
but a machine-parseable list of paths is fine.

> +		merge_get_conflicted_files(&result, &conflicted_files);
> +		for (i = 0; i < conflicted_files.nr; i++) {
> +			const char *name = conflicted_files.items[i].string;
> +			if (last && !strcmp(last, name))
> +				continue;
> +			write_name_quoted_relative(
> +				name, prefix, stdout, line_termination);
> +			last = name;

OK.  The iteration used here makes casual readers wonder why the
helper doesn't make paths unique, but the string list item holds
in its util pointer a pointer to a structure with <stage, mode, oid>
tuple, so it is natural to make the consumer, who wants uniquified
list, responsible for deduping, like this loop.

> +		}
> +		string_list_clear(&conflicted_files, 1);

And the stage-info structure associated with these paths are
deallocated with this call.  Good.

> +	}

Junio C Hamano Feb. 2, 2022, 9:32 p.m. UTC | #2

"Elijah Newren via GitGitGadget" <[e-mail]> writes:

> +Conflicted file list
> +~~~~~~~~~~~~~~~~~~~~
> +
> +This is a sequence of lines containing a filename on each line, quoted
> +as explained for the configuration variable `core.quotePath` (see
> +linkgit:git-config[1]).

Makes sense.  Ideally things like this should be discoverable by
inspecting the tree object shown as the result of the (conflicted)
merge, but since the design of the output is to show only a single
tree, there is nowhere to store such an extra piece of information
per path (grepping for markers in blobs of course does not count).

I guess an alternative to show four trees when conflicted instead of
one (i.e. the primary tree may either contain only the cleanly
merged paths _or_ also blobs with conflict markers for conflicted
paths; the three other trees record three stages that would be in
the index, if we were performing the same merge using the index),
but a machine-parseable list of paths is fine.

> +		merge_get_conflicted_files(&result, &conflicted_files);
> +		for (i = 0; i < conflicted_files.nr; i++) {
> +			const char *name = conflicted_files.items[i].string;
> +			if (last && !strcmp(last, name))
> +				continue;
> +			write_name_quoted_relative(
> +				name, prefix, stdout, line_termination);
> +			last = name;

OK.  The iteration used here makes casual readers wonder why the
helper doesn't make paths unique, but the string list item holds
in its util pointer a pointer to a structure with <stage, mode, oid>
tuple, so it is natural to make the consumer, who wants uniquified
list, responsible for deduping, like this loop.

> +		}
> +		string_list_clear(&conflicted_files, 1);

And the stage-info structure associated with these paths are
deallocated with this call.  Good.

> +	}

Junio C Hamano Feb. 3, 2022, 11:55 p.m. UTC | #3

"Elijah Newren via GitGitGadget" <[e-mail]> writes:

> +Conflicted file list
> +~~~~~~~~~~~~~~~~~~~~
> +
> +This is a sequence of lines containing a filename on each line, quoted
> +as explained for the configuration variable `core.quotePath` (see
> +linkgit:git-config[1]).

Makes sense.  Ideally things like this should be discoverable by
inspecting the tree object shown as the result of the (conflicted)
merge, but since the design of the output is to show only a single
tree, there is nowhere to store such an extra piece of information
per path (grepping for markers in blobs of course does not count).

I guess an alternative to show four trees when conflicted instead of
one (i.e. the primary tree may either contain only the cleanly
merged paths _or_ also blobs with conflict markers for conflicted
paths; the three other trees record three stages that would be in
the index, if we were performing the same merge using the index),
but a machine-parseable list of paths is fine.

> +		merge_get_conflicted_files(&result, &conflicted_files);
> +		for (i = 0; i < conflicted_files.nr; i++) {
> +			const char *name = conflicted_files.items[i].string;
> +			if (last && !strcmp(last, name))
> +				continue;
> +			write_name_quoted_relative(
> +				name, prefix, stdout, line_termination);
> +			last = name;

OK.  The iteration used here makes casual readers wonder why the
helper doesn't make paths unique, but the string list item holds
in its util pointer a pointer to a structure with <stage, mode, oid>
tuple, so it is natural to make the consumer, who wants uniquified
list, responsible for deduping, like this loop.

> +		}
> +		string_list_clear(&conflicted_files, 1);

And the stage-info structure associated with these paths are
deallocated with this call.  Good.

> +	}

Patch

diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt
index 42e0f8f6183..160e8f44b62 100644
--- a/Documentation/git-merge-tree.txt
+++ b/Documentation/git-merge-tree.txt
@@ -55,6 +55,7 @@  simply one line:
 Whereas for a conflicted merge, the output is by default of the form:
 
 	<OID of toplevel tree>
+	<Conflicted file list>
 	<Informational messages>
 
 These are discussed individually below.
@@ -66,6 +67,13 @@  This is a tree object that represents what would be checked out in the
 working tree at the end of `git merge`.  If there were conflicts, then
 files within this tree may have embedded conflict markers.
 
+Conflicted file list
+~~~~~~~~~~~~~~~~~~~~
+
+This is a sequence of lines containing a filename on each line, quoted
+as explained for the configuration variable `core.quotePath` (see
+linkgit:git-config[1]).
+
 Informational messages
 ~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index 6a556ab1c9c..54dae018203 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -11,6 +11,9 @@ 
 #include "blob.h"
 #include "exec-cmd.h"
 #include "merge-blobs.h"
+#include "quote.h"
+
+static int line_termination = '\n';
 
 struct merge_list {
 	struct merge_list *next;
@@ -394,7 +397,8 @@  struct merge_tree_options {
 };
 
 static int real_merge(struct merge_tree_options *o,
-		      const char *branch1, const char *branch2)
+		      const char *branch1, const char *branch2,
+		      const char *prefix)
 {
 	struct commit *parent1, *parent2;
 	struct commit_list *common;
@@ -438,6 +442,22 @@  static int real_merge(struct merge_tree_options *o,
 		o->show_messages = !result.clean;
 
 	puts(oid_to_hex(&result.tree->object.oid));
+	if (!result.clean) {
+		struct string_list conflicted_files = STRING_LIST_INIT_NODUP;
+		const char *last = NULL;
+		int i;
+
+		merge_get_conflicted_files(&result, &conflicted_files);
+		for (i = 0; i < conflicted_files.nr; i++) {
+			const char *name = conflicted_files.items[i].string;
+			if (last && !strcmp(last, name))
+				continue;
+			write_name_quoted_relative(
+				name, prefix, stdout, line_termination);
+			last = name;
+		}
+		string_list_clear(&conflicted_files, 1);
+	}
 	if (o->show_messages) {
 		printf("\n");
 		merge_display_update_messages(&opt, &result, stdout);
@@ -486,7 +506,7 @@  int cmd_merge_tree(int argc, const char **argv, const char *prefix)
 
 	/* Do the relevant type of merge */
 	if (o.mode == 'w')
-		return real_merge(&o, argv[0], argv[1]);
+		return real_merge(&o, argv[0], argv[1], prefix);
 	else
 		return trivial_merge(argv[0], argv[1], argv[2]);
 }
diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh
index e2255711f9c..7113d060bc5 100755
--- a/t/t4301-merge-tree-write-tree.sh
+++ b/t/t4301-merge-tree-write-tree.sh
@@ -95,6 +95,8 @@  test_expect_success 'test conflict notices and such' '
 	#   "whatever" has *both* a modify/delete and a file/directory conflict
 	cat <<-EOF >expect &&
 	HASH
+	greeting
+	whatever~side1
 
 	Auto-merging greeting
 	CONFLICT (content): Merge conflict in greeting
@@ -106,4 +108,13 @@  test_expect_success 'test conflict notices and such' '
 	test_cmp expect actual
 '
 
+test_expect_success 'Just the conflicted files without the messages' '
+	test_expect_code 1 git merge-tree --write-tree --no-messages side1 side2 >out &&
+	sed -e "s/[0-9a-f]\{40,\}/HASH/g" out >actual &&
+
+	test_write_lines HASH greeting whatever~side1 >expect &&
+
+	test_cmp expect actual
+'
+
 test_done