RevolveMerger: honor ignoreConflicts also for binary files

Currently difference in binary files during merge will cause them to be
added to unmergedPaths regardless of whether ignoreConflicts is true.

This creates an issue during merging with strategy "RECURSIVE", as it
makes it impossible to create a virtual commit if there is a difference
in a binary file. Resulting in the
CONFLICTS_DURING_MERGE_BASE_CALCULATION error being thrown.

This is especially problematic, since JGit has a
rather simplistic rules for considering file binary, which easily leads
to false positives.

What we should do instead is keep OURS. This will not lead to silently
ignoring difference in the final result. It will allow creation of
virtual merge-base commit, and then the difference would be presented
again in the final merge results. In essense it only affects what's
shown as BASE in 3-way merge.

Additionally, this is correct because
- It's consistent with treatment of other unmergeable entities, for
  example Gitlinks
- It's consistent with behaviour of CGit:
  - https://git-scm.com/docs/gitattributes#Documentation/gitattributes.txt-binary
    states on diffs in binary OURS is picked by default.
  - In code: https://git.kernel.org/pub/scm/git/git.git/tree/merge-ll.c#n81
- ignoreConflicts in CGit afterwards ignores all issues with content
  merging https://git.kernel.org/pub/scm/git/git.git/tree/merge-ort.c#n5201

We also adjust the behaviour when .gitattributes tell us to treat the
file as binary for the purpose of the merge.

We only change the behaviour when ignoreConlicts = true, as otherwise
the current behaviour works as intended.


Change-Id: I2b69f80a13d250aad3fe12dd438b2763f3022270

1 files changed, 70 insertions, 0 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/MergerTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/MergerTest.java
index 3a036acaca..c6a6321cf8 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/MergerTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/MergerTest.java
@@ -1792,7 +1792,77 @@ public class MergerTest extends RepositoryTestCase {
 		// children
 		mergeResult = git.merge().include(commitC3S).call();
 		assertEquals(mergeResult.getMergeStatus(), MergeStatus.MERGED);
+	}
+
+	/**
+	 * Merging two commits when binary files have equal content, but conflicting content in the
+	 * virtual ancestor.
+	 *
+	 * <p>
+	 * This test has the same set up as
+	 * {@code checkFileDirMergeConflictInVirtualAncestor_NoConflictInChildren}, only
+	 * with the content conflict in A1 and A2.
+	 */
+	@Theory
+	public void checkBinaryMergeConflictInVirtualAncestor(MergeStrategy strategy) throws Exception {
+		if (!strategy.equals(MergeStrategy.RECURSIVE)) {
+			return;
+		}
+
+		Git git = Git.wrap(db);
+
+		// master
+		writeTrashFile("c", "initial file");
+		git.add().addFilepattern("c").call();
+		RevCommit commitI = git.commit().setMessage("Initial commit").call();
+
+		writeTrashFile("a", "\0\1\1\1\1\0");  // content in Ancestor 1
+		git.add().addFilepattern("a").call();
+		RevCommit commitA1 = git.commit().setMessage("Ancestor 1").call();
+
+		writeTrashFile("a", "\0\1\2\3\4\5\0");  // content in Child 1 (commited on master)
+		git.add().addFilepattern("a").call();
+		// commit C1M
+		git.commit().setMessage("Child 1 on master").call();
+
+		git.checkout().setCreateBranch(true).setStartPoint(commitI).setName("branch-to-merge").call();
+		writeTrashFile("a", "\0\2\2\2\2\0");  // content in Ancestor 1
+		git.add().addFilepattern("a").call();
+		RevCommit commitA2 = git.commit().setMessage("Ancestor 2").call();
+
+		// second branch
+		git.checkout().setCreateBranch(true).setStartPoint(commitA1).setName("second-branch").call();
+		writeTrashFile("a", "\0\5\4\3\2\1\0");  // content in Child 2 (commited on second-branch)
+		git.add().addFilepattern("a").call();
+		// commit C2S
+		git.commit().setMessage("Child 2 on second-branch").call();
 
+		// Merge branch-to-merge into second-branch
+		MergeResult mergeResult = git.merge().include(commitA2).setStrategy(strategy).call();
+		assertEquals(mergeResult.getNewHead(), null);
+		assertEquals(mergeResult.getMergeStatus(), MergeStatus.CONFLICTING);
+		// Resolve the conflict manually
+		writeTrashFile("a", "\0\3\3\3\3\0");  // merge conflict resolution
+		git.add().addFilepattern("a").call();
+		RevCommit commitC3S = git.commit().setMessage("Child 3 on second bug - resolve merge conflict").call();
+
+		// Merge branch-to-merge into master
+		git.checkout().setName("master").call();
+		mergeResult = git.merge().include(commitA2).setStrategy(strategy).call();
+		assertEquals(mergeResult.getNewHead(), null);
+		assertEquals(mergeResult.getMergeStatus(), MergeStatus.CONFLICTING);
+
+		// Resolve the conflict manually - set the same value as in resolution above
+		writeTrashFile("a", "\0\3\3\3\3\0");  // merge conflict resolution
+		git.add().addFilepattern("a").call();
+		// commit C4M
+		git.commit().setMessage("Child 4 on master - resolve merge conflict").call();
+
+		// Merge C4M (second-branch) into master (C3S)
+		// Conflict in virtual base should be here, but there are no conflicts in
+		// children
+		mergeResult = git.merge().include(commitC3S).call();
+		assertEquals(mergeResult.getMergeStatus(), MergeStatus.MERGED);
 	}
 
 	/**