Re: [PATCH v2 8/8] show, log: include conflict/warning messages in --remerge-diff headers

> From: Elijah Newren <newren@gmail.com>
> 
> Conflicts such as modify/delete, rename/rename, or file/directory are
> not representable via content conflict markers, and the normal output
> messages notifying users about these were dropped with --remerge-diff.
> While we don't want these messages randomly shown before the commit
> and diff headers, we do want them to still be shown; include them as
> part of the diff headers instead.
> 
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  log-tree.c              |  3 ++
>  merge-ort.c             |  1 +
>  merge-ort.h             | 10 +++++
>  t/t4069-remerge-diff.sh | 86 +++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 100 insertions(+)
> 
> diff --git a/log-tree.c b/log-tree.c
> index 33c28f537a6..97fbb756d21 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -922,6 +922,7 @@ static int do_remerge_diff(struct rev_info *opt,
>  	/* Setup merge options */
>  	init_merge_options(&o, the_repository);
>  	o.show_rename_progress = 0;
> +	o.record_conflict_msgs_as_headers = 1;
>  
>  	ctx.abbrev = DEFAULT_ABBREV;
>  	format_commit_message(parent1, "%h (%s)", &parent1_desc, &ctx);
> @@ -938,10 +939,12 @@ static int do_remerge_diff(struct rev_info *opt,
>  	merge_incore_recursive(&o, bases, parent1, parent2, &res);
>  
>  	/* Show the diff */
> +	opt->diffopt.additional_path_headers = res.path_messages;
>  	diff_tree_oid(&res.tree->object.oid, oid, "", &opt->diffopt);
>  	log_tree_diff_flush(opt);
>  
>  	/* Cleanup */
> +	opt->diffopt.additional_path_headers = NULL;
>  	strbuf_release(&parent1_desc);
>  	strbuf_release(&parent2_desc);
>  	merge_finalize(&o, &res);
> diff --git a/merge-ort.c b/merge-ort.c
> index 9142d56e0ad..07e53083cbd 100644
> --- a/merge-ort.c
> +++ b/merge-ort.c
> @@ -4579,6 +4579,7 @@ redo:
>  	trace2_region_leave("merge", "process_entries", opt->repo);
>  
>  	/* Set return values */
> +	result->path_messages = &opt->priv->output;
>  	result->tree = parse_tree_indirect(&working_tree_oid);
>  	/* existence of conflicted entries implies unclean */
>  	result->clean &= strmap_empty(&opt->priv->conflicted);
> diff --git a/merge-ort.h b/merge-ort.h
> index c011864ffeb..fe599b87868 100644
> --- a/merge-ort.h
> +++ b/merge-ort.h
> @@ -5,6 +5,7 @@
>  
>  struct commit;
>  struct tree;
> +struct strmap;
>  
>  struct merge_result {
>  	/*
> @@ -23,6 +24,15 @@ struct merge_result {
>  	 */
>  	struct tree *tree;
>  
> +	/*
> +	 * Special messages and conflict notices for various paths
> +	 *
> +	 * This is a map of pathnames to strbufs.  It contains various
> +	 * warning/conflict/notice messages (possibly multiple per path)
> +	 * that callers may want to use.
> +	 */
> +	struct strmap *path_messages;
> +
>  	/*
>  	 * Additional metadata used by merge_switch_to_result() or future calls
>  	 * to merge_incore_*().  Includes data needed to update the index (if
> diff --git a/t/t4069-remerge-diff.sh b/t/t4069-remerge-diff.sh
> index 192dbce2bfe..a040d3bcd91 100755
> --- a/t/t4069-remerge-diff.sh
> +++ b/t/t4069-remerge-diff.sh
> @@ -4,6 +4,15 @@ test_description='remerge-diff handling'
>  
>  . ./test-lib.sh
>  
> +# --remerge-diff uses ort under the hood regardless of setting.  However,
> +# we set up a file/directory conflict beforehand, and the different backends
> +# handle the conflict differently, which would require separate code paths
> +# to resolve.  There's not much point in making the code uglier to do that,
> +# though, when the real thing we are testing (--remerge-diff) will hardcode
> +# calls directly into the merge-ort API anyway.  So just force the use of
> +# ort on the setup too.
> +GIT_TEST_MERGE_ALGORITHM=ort
> +
>  test_expect_success 'setup basic merges' '
>  	test_write_lines 1 2 3 4 5 6 7 8 9 >numbers &&
>  	git add numbers &&
> @@ -55,6 +64,7 @@ test_expect_success 'remerge-diff with both a resolved conflict and an unrelated
>  	git log -1 --oneline ab_resolution >tmp &&
>  	cat <<-EOF >>tmp &&
>  	diff --git a/numbers b/numbers
> +	CONFLICT (content): Merge conflict in numbers
>  	index a1fb731..6875544 100644
>  	--- a/numbers
>  	+++ b/numbers
> @@ -83,4 +93,80 @@ test_expect_success 'remerge-diff with both a resolved conflict and an unrelated
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'setup non-content conflicts' '
> +	git switch --orphan base &&
> +
> +	test_write_lines 1 2 3 4 5 6 7 8 9 >numbers &&
> +	test_write_lines a b c d e f g h i >letters &&
> +	test_write_lines in the way >content &&
> +	git add numbers letters content &&
> +	git commit -m base &&
> +
> +	git branch side1 &&
> +	git branch side2 &&
> +
> +	git checkout side1 &&
> +	test_write_lines 1 2 three 4 5 6 7 8 9 >numbers &&
> +	git mv letters letters_side1 &&
> +	git mv content file_or_directory &&
> +	git add numbers &&
> +	git commit -m side1 &&
> +
> +	git checkout side2 &&
> +	git rm numbers &&
> +	git mv letters letters_side2 &&
> +	mkdir file_or_directory &&
> +	echo hello >file_or_directory/world &&
> +	git add file_or_directory/world &&
> +	git commit -m side2 &&
> +
> +	git checkout -b resolution side1 &&
> +	test_must_fail git merge side2 &&
> +	test_write_lines 1 2 three 4 5 6 7 8 9 >numbers &&
> +	git add numbers &&
> +	git add letters_side1 &&
> +	git rm letters &&
> +	git rm letters_side2 &&
> +	git add file_or_directory~HEAD &&
> +	git mv file_or_directory~HEAD wanted_content &&
> +	git commit -m resolved
> +'
> +
> +test_expect_success 'remerge-diff with non-content conflicts' '
> +	git log -1 --oneline resolution >tmp &&
> +	cat <<-EOF >>tmp &&
> +	diff --git a/file_or_directory~HASH (side1) b/wanted_content

> +	diff --git a/letters b/letters
> +	CONFLICT (rename/rename): letters renamed to letters_side1 in HASH (side1) and to letters_side2 in HASH (side2).
> +	diff --git a/letters_side2 b/letters_side2
> +	deleted file mode 100644
> +	index b236ae5..0000000
> +	--- a/letters_side2
> +	+++ /dev/null
> +	@@ -1,9 +0,0 @@
> +	-a
> +	-b
> +	-c
> +	-d
> +	-e
> +	-f
> +	-g
> +	-h
> +	-i
> +	diff --git a/numbers b/numbers
> +	CONFLICT (modify/delete): numbers deleted in HASH (side2) and modified in HASH (side1).  Version HASH (side1) of numbers left in tree.
> +	EOF

> +	# We still have some sha1 hashes above; rip them out so test works
> +	# with sha256
> +	sed -e "s/[0-9a-f]\{7,\}/HASH/g" tmp >expect &&
> +
> +	git show --oneline --remerge-diff resolution >tmp &&
> +	sed -e "s/[0-9a-f]\{7,\}/HASH/g" tmp >actual &&
> +	test_cmp expect actual
> +'
> +
>  test_done
> -- 
> gitgitgadget