[PATCH v3 0/4] Generic conflict style fixes

[PATCH v3 0/4] Generic conflict style fixes

[Felipe Contreras]

I sent v2 of this series almost 2 years ago [1] and received zero
feedback. This is the same version except rebased on top of master.

In v1 the idea was to change the default conflictstyle, but that
proposal got stuck due to a variety of reasons (including unnecessary
personal drama).

v2 should not have been controversial, and neither is v3.

[1] https://lore.kernel.org/git/20210622002714.1720891-1-felipe.contreras@gmail.com/

Felipe Contreras (4):
  test: add merge style config test
  merge-tree: fix merge.conflictstyle handling
  notes: fix merge.conflictstyle handling
  test: document broken merge.conflictStyle handling

 builtin/merge-tree.c               |   4 +
 builtin/notes.c                    |   3 +-
 t/t6440-config-conflict-markers.sh | 116 +++++++++++++++++++++++++++++
 3 files changed, 122 insertions(+), 1 deletion(-)
 create mode 100755 t/t6440-config-conflict-markers.sh

Range-diff against v2:
1:  51351f1a77 = 1:  6867f22f49 test: add merge style config test
2:  1fccf561ed ! 2:  60bf233000 merge-tree: fix merge.conflictstyle handling
    @@ Commit message
     
      ## builtin/merge-tree.c ##
     @@
    - #include "blob.h"
    - #include "exec-cmd.h"
      #include "merge-blobs.h"
    + #include "quote.h"
    + #include "tree.h"
     +#include "config.h"
     +#include "xdiff-interface.h"
      
    - static const char merge_tree_usage[] = "git merge-tree <base-tree> <branch1> <branch2>";
    + static int line_termination = '\n';
      
     @@ builtin/merge-tree.c: int cmd_merge_tree(int argc, const char **argv, const char *prefix)
    - 	if (argc != 4)
    - 		usage(merge_tree_usage);
    + 	if (argc != expected_remaining_argc)
    + 		usage_with_options(merge_tree_usage, mt_options);
      
     +	git_config(git_xmerge_config, NULL);
     +
    - 	buf1 = get_tree_descriptor(r, t+0, argv[1]);
    - 	buf2 = get_tree_descriptor(r, t+1, argv[2]);
    - 	buf3 = get_tree_descriptor(r, t+2, argv[3]);
    + 	/* Do the relevant type of merge */
    + 	if (o.mode == MODE_REAL)
    + 		return real_merge(&o, merge_base, argv[0], argv[1], prefix);
     
      ## t/t6440-config-conflict-markers.sh ##
     @@ t/t6440-config-conflict-markers.sh: test_expect_success 'merge' '
3:  3bb872e3cd ! 3:  cf8cabaae3 notes: fix merge.conflictstyle handling
    @@ Commit message
     
      ## builtin/notes.c ##
     @@
    - #include "notes-merge.h"
      #include "notes-utils.h"
      #include "worktree.h"
    + #include "write-or-die.h"
     +#include "xdiff-interface.h"
      
      static const char * const git_notes_usage[] = {
    @@ builtin/notes.c: int cmd_notes(int argc, const char **argv, const char *prefix)
     -	git_config(git_default_config, NULL);
     +	git_config(git_xmerge_config, NULL);
      	argc = parse_options(argc, argv, prefix, options, git_notes_usage,
    - 			     PARSE_OPT_STOP_AT_NON_OPTION);
    - 
    + 			     PARSE_OPT_SUBCOMMAND_OPTIONAL);
    + 	if (!fn) {
     
      ## t/t6440-config-conflict-markers.sh ##
     @@ t/t6440-config-conflict-markers.sh: test_expect_success 'merge-tree' '
4:  a767bc68e6 = 4:  5959fea3c4 test: document broken merge.conflictStyle handling
-- 
2.40.0+fc1

[PATCH v3 1/4] test: add merge style config test

[Felipe Contreras]

We want to test different combinations of merge.conflictstyle, and a new
file is the best place to do that.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/t6440-config-conflict-markers.sh | 37 ++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100755 t/t6440-config-conflict-markers.sh

diff --git a/t/t6440-config-conflict-markers.sh b/t/t6440-config-conflict-markers.sh
new file mode 100755
index 0000000000..813d7dda9a
--- /dev/null
+++ b/t/t6440-config-conflict-markers.sh
@@ -0,0 +1,37 @@
+#!/bin/sh
+
+test_description='merge style conflict markers configurations'
+
+. ./test-lib.sh
+
+test_expect_success 'merge' '
+	test_create_repo merge &&
+	(
+		cd merge &&
+
+		test_write_lines 1 2 3 >content &&
+		git add content &&
+		git commit -m base &&
+
+		git checkout -b r &&
+		echo six >>content &&
+		git commit -a -m right &&
+
+		git checkout master &&
+		echo 7 >>content &&
+		git commit -a -m left &&
+
+		test_must_fail git merge r &&
+		! grep "^|||||||" content &&
+
+		git reset --hard &&
+		test_must_fail git -c merge.conflictstyle=diff3 merge r &&
+		grep "^|||||||" content &&
+
+		git reset --hard &&
+		test_must_fail git -c merge.conflictstyle=merge merge r &&
+		! grep "^|||||||" content
+	)
+'
+
+test_done
-- 
2.40.0+fc1

[PATCH v3 2/4] merge-tree: fix merge.conflictstyle handling

[Felipe Contreras]

Currently it's completely ignored.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/merge-tree.c               |  4 ++++
 t/t6440-config-conflict-markers.sh | 21 +++++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index aa8040c2a6..a98f84634e 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -17,6 +17,8 @@
 #include "merge-blobs.h"
 #include "quote.h"
 #include "tree.h"
+#include "config.h"
+#include "xdiff-interface.h"
 
 static int line_termination = '\n';
 
@@ -628,6 +630,8 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)
 	if (argc != expected_remaining_argc)
 		usage_with_options(merge_tree_usage, mt_options);
 
+	git_config(git_xmerge_config, NULL);
+
 	/* Do the relevant type of merge */
 	if (o.mode == MODE_REAL)
 		return real_merge(&o, merge_base, argv[0], argv[1], prefix);
diff --git a/t/t6440-config-conflict-markers.sh b/t/t6440-config-conflict-markers.sh
index 813d7dda9a..cb2ee3ad0a 100755
--- a/t/t6440-config-conflict-markers.sh
+++ b/t/t6440-config-conflict-markers.sh
@@ -34,4 +34,25 @@ test_expect_success 'merge' '
 	)
 '
 
+test_expect_success 'merge-tree' '
+	test_create_repo merge-tree &&
+	(
+		cd merge-tree &&
+
+		test_commit initial initial-file initial &&
+		test_commit r content r &&
+		git reset --hard initial &&
+		test_commit l content l &&
+
+		git merge-tree initial r l >actual &&
+		! grep "^+|||||||" content &&
+
+		git -c merge.conflictstyle=diff3 merge-tree initial r l >actual &&
+		grep "^+|||||||" actual &&
+
+		git -c merge.conflictstyle=merge merge-tree initial r l >actual &&
+		! grep "^+|||||||" content
+	)
+'
+
 test_done
-- 
2.40.0+fc1

[PATCH v3 3/4] notes: fix merge.conflictstyle handling

[Felipe Contreras]

Currently it's completely ignored.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/notes.c                    |  3 ++-
 t/t6440-config-conflict-markers.sh | 27 +++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/builtin/notes.c b/builtin/notes.c
index d5788352b6..9a9044b50b 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -28,6 +28,7 @@
 #include "notes-utils.h"
 #include "worktree.h"
 #include "write-or-die.h"
+#include "xdiff-interface.h"
 
 static const char * const git_notes_usage[] = {
 	N_("git notes [--ref <notes-ref>] [list [<object>]]"),
@@ -1020,7 +1021,7 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 		OPT_END()
 	};
 
-	git_config(git_default_config, NULL);
+	git_config(git_xmerge_config, NULL);
 	argc = parse_options(argc, argv, prefix, options, git_notes_usage,
 			     PARSE_OPT_SUBCOMMAND_OPTIONAL);
 	if (!fn) {
diff --git a/t/t6440-config-conflict-markers.sh b/t/t6440-config-conflict-markers.sh
index cb2ee3ad0a..c51512ced6 100755
--- a/t/t6440-config-conflict-markers.sh
+++ b/t/t6440-config-conflict-markers.sh
@@ -55,4 +55,31 @@ test_expect_success 'merge-tree' '
 	)
 '
 
+test_expect_success 'notes' '
+	test_create_repo notes &&
+	(
+		test_commit initial &&
+
+		git -c core.notesRef=refs/notes/b notes add -m b initial &&
+
+		git update-ref refs/notes/r refs/notes/b &&
+		git -c core.notesRef=refs/notes/r notes add -f -m r initial &&
+
+		git update-ref refs/notes/l refs/notes/b &&
+		git config core.notesRef refs/notes/l &&
+		git notes add -f -m l initial &&
+
+		test_must_fail git notes merge r &&
+		! grep "^|||||||" .git/NOTES_MERGE_WORKTREE/* &&
+
+		git notes merge --abort &&
+		test_must_fail git -c merge.conflictstyle=diff3 notes merge r &&
+		grep "^|||||||" .git/NOTES_MERGE_WORKTREE/* &&
+
+		git notes merge --abort &&
+		test_must_fail git -c merge.conflictstyle=merge notes merge r &&
+		! grep "^|||||||" .git/NOTES_MERGE_WORKTREE/*
+	)
+'
+
 test_done
-- 
2.40.0+fc1

[PATCH v3 4/4] test: document broken merge.conflictStyle handling

[Felipe Contreras]

Currently both merge.conflictStyle and --conflict=diff3 don't work
together for `git commit --merge`, since the former wrongly overrides
the later.

There is no easy way to fix this, so mark it as broken for now.

Signee-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/t6440-config-conflict-markers.sh | 31 ++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/t/t6440-config-conflict-markers.sh b/t/t6440-config-conflict-markers.sh
index c51512ced6..3ba993a6a8 100755
--- a/t/t6440-config-conflict-markers.sh
+++ b/t/t6440-config-conflict-markers.sh
@@ -82,4 +82,35 @@ test_expect_success 'notes' '
 	)
 '
 
+test_expect_failure 'checkout' '
+	test_create_repo checkout &&
+	(
+		test_commit checkout &&
+
+		test_write_lines a b c d e >content &&
+		git add content &&
+		git commit -m initial &&
+
+		git checkout -b simple master &&
+		test_write_lines a c e >content &&
+		git commit -a -m simple &&
+
+		test_write_lines b d >content &&
+		git checkout --merge master &&
+		! grep "^|||||||" content &&
+
+		git config merge.conflictstyle merge &&
+
+		git checkout -f simple &&
+		test_write_lines b d >content &&
+		git checkout --merge --conflict=diff3 master &&
+		grep "^|||||||" content &&
+
+		git checkout -f simple &&
+		test_write_lines b d >content &&
+		git checkout --merge --conflict=merge master &&
+		! grep "^|||||||" content
+	)
+'
+
 test_done
-- 
2.40.0+fc1