ResolveMerger: Fix the issue with binary modify-modify conflicts

1) If the file was marked as binary by git attributes, we should add the
path to conflicts if content differs in OURS and THEIRS
2) If the path is a file in OURS, THEIRS and BASE and if it is a binary
in any one of them, no content merge should be attempted and the file
content is kept as is in the work tree

Bug: jgit-14
Change-Id: I9201bdc53a55f8f40adade4b6a36ee8ae25f4db8

2 files changed, 187 insertions, 8 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/attributes/merge/MergeGitAttributeTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/attributes/merge/MergeGitAttributeTest.java
index 795029188d..6b23de3320 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/attributes/merge/MergeGitAttributeTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/attributes/merge/MergeGitAttributeTest.java
@@ -10,7 +10,6 @@
 package org.eclipse.jgit.attributes.merge;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
@@ -42,7 +41,6 @@ import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.treewalk.FileTreeIterator;
 import org.eclipse.jgit.treewalk.TreeWalk;
 import org.eclipse.jgit.treewalk.filter.PathFilter;
-import org.junit.Ignore;
 import org.junit.Test;
 
 public class MergeGitAttributeTest extends RepositoryTestCase {
@@ -268,12 +266,7 @@ public class MergeGitAttributeTest extends RepositoryTestCase {
 		}
 	}
 
-	/*
-	 * This test is commented because JGit add conflict markers in binary files.
-	 * cf. https://www.eclipse.org/forums/index.php/t/1086511/
-	 */
 	@Test
-	@Ignore
 	public void mergeBinaryFile_NoAttr_Conflict() throws IllegalStateException,
 			IOException, NoHeadException, ConcurrentRefUpdateException,
 			CheckoutConflictException, InvalidMergeHeadsException,
@@ -433,7 +426,7 @@ public class MergeGitAttributeTest extends RepositoryTestCase {
 			try (FileInputStream mergeResultFile = new FileInputStream(
 					db.getWorkTree().toPath().resolve(ENABLED_CHECKED_GIF)
 							.toFile())) {
-				assertFalse(contentEquals(
+				assertTrue(contentEquals(
 						getClass().getResourceAsStream(ENABLED_CHECKED_GIF),
 						mergeResultFile));
 			}
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 022e8cd55e..3f99fe2b26 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
@@ -22,9 +22,12 @@ import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
+import java.nio.file.Files;
 import java.time.Instant;
 import java.util.Arrays;
+import java.util.HashSet;
 import java.util.Map;
+import java.util.Set;
 
 import org.eclipse.jgit.api.Git;
 import org.eclipse.jgit.api.MergeResult;
@@ -51,6 +54,7 @@ import org.eclipse.jgit.lib.ObjectInserter;
 import org.eclipse.jgit.lib.ObjectLoader;
 import org.eclipse.jgit.lib.ObjectReader;
 import org.eclipse.jgit.lib.ObjectStream;
+import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.lib.StoredConfig;
 import org.eclipse.jgit.merge.ResolveMerger.MergeFailureReason;
 import org.eclipse.jgit.revwalk.RevCommit;
@@ -1789,6 +1793,188 @@ public class MergerTest extends RepositoryTestCase {
 
 	}
 
+	/**
+	 * File is binary in ours, theirs and base with different content in each of
+	 * them. Content of the file should not change after the merge conflict as
+	 * no conflict markers are added to the binary files
+	 */
+	@Theory
+	public void oursBinaryTheirsBinaryBaseBinary(MergeStrategy strategy)
+			throws Exception {
+		Git git = Git.wrap(db);
+		String binaryFile = "file";
+
+		writeTrashFile(binaryFile, "\u0000\u0001");
+		git.add().addFilepattern(binaryFile).call();
+		RevCommit parent = git.commit().setMessage("BASE COMMIT").call();
+		String fileHashInBase = getFileHashInWorkTree(git, binaryFile);
+
+		writeTrashFile(binaryFile, "\u0001\u0002");
+		git.add().addFilepattern(binaryFile).call();
+		RevCommit child1 = git.commit().setMessage("THEIRS COMMIT").call();
+		String fileHashInChild1 = getFileHashInWorkTree(git, binaryFile);
+
+		git.checkout().setCreateBranch(true).setStartPoint(parent)
+				.setName("side").call();
+
+		writeTrashFile(binaryFile, "\u0002\u0000");
+		git.add().addFilepattern(binaryFile).call();
+		git.commit().setMessage("OURS COMMIT").call();
+		String fileHashInChild2 = getFileHashInWorkTree(git, binaryFile);
+
+		MergeResult mergeResult = git.merge().setStrategy(strategy)
+				.include(child1).call();
+
+		// check if the merge caused a conflict
+		assertTrue(mergeResult.getConflicts() != null
+				&& !mergeResult.getConflicts().isEmpty());
+		String fileHashInChild2AfterMerge = getFileHashInWorkTree(git,
+				binaryFile);
+
+		// check if the file content changed during a conflicting merge
+		assertEquals(fileHashInChild2AfterMerge, fileHashInChild2);
+
+		Set<String> hashesInIndexFile = new HashSet<>();
+		DirCache indexContent = git.getRepository().readDirCache();
+		for (int i = 0; i < indexContent.getEntryCount(); ++i) {
+			DirCacheEntry indexEntry = indexContent.getEntry(i);
+			if (binaryFile.equals(indexEntry.getPathString())) {
+				hashesInIndexFile.add(indexEntry.getObjectId().name());
+			}
+		}
+
+		// check if all the three stages are added to index file
+		assertTrue(hashesInIndexFile.contains(fileHashInBase));
+		assertTrue(hashesInIndexFile.contains(fileHashInChild1));
+		assertTrue(hashesInIndexFile.contains(fileHashInChild2));
+	}
+
+	/**
+	 * File is text in ours and theirs with different content but binary in
+	 * base. Even in this case, file will be treated as a binary and no conflict
+	 * markers are added to it
+	 */
+	@Theory
+	public void oursAndTheirsDifferentTextBaseBinary(MergeStrategy strategy)
+			throws Exception {
+		Git git = Git.wrap(db);
+		String binaryFile = "file";
+
+		writeTrashFile(binaryFile, "\u0000\u0001");
+		git.add().addFilepattern(binaryFile).call();
+		RevCommit parent = git.commit().setMessage("BASE COMMIT").call();
+		String fileHashInBase = getFileHashInWorkTree(git, binaryFile);
+
+		writeTrashFile(binaryFile, "TEXT1");
+		git.add().addFilepattern(binaryFile).call();
+		RevCommit child1 = git.commit().setMessage("THEIRS COMMIT").call();
+		String fileHashInChild1 = getFileHashInWorkTree(git, binaryFile);
+
+		git.checkout().setCreateBranch(true).setStartPoint(parent)
+				.setName("side").call();
+
+		writeTrashFile(binaryFile, "TEXT2");
+		git.add().addFilepattern(binaryFile).call();
+		git.commit().setMessage("OURS COMMIT").call();
+		String fileHashInChild2 = getFileHashInWorkTree(git, binaryFile);
+
+		MergeResult mergeResult = git.merge().setStrategy(strategy)
+				.include(child1).call();
+
+		assertTrue(mergeResult.getConflicts() != null
+				&& !mergeResult.getConflicts().isEmpty());
+		String fileHashInChild2AfterMerge = getFileHashInWorkTree(git,
+				binaryFile);
+
+		assertEquals(fileHashInChild2AfterMerge, fileHashInChild2);
+
+		Set<String> hashesInIndexFile = new HashSet<>();
+		DirCache indexContent = git.getRepository().readDirCache();
+		for (int i = 0; i < indexContent.getEntryCount(); ++i) {
+			DirCacheEntry indexEntry = indexContent.getEntry(i);
+			if (binaryFile.equals(indexEntry.getPathString())) {
+				hashesInIndexFile.add(indexEntry.getObjectId().name());
+			}
+		}
+
+		assertTrue(hashesInIndexFile.contains(fileHashInBase));
+		assertTrue(hashesInIndexFile.contains(fileHashInChild1));
+		assertTrue(hashesInIndexFile.contains(fileHashInChild2));
+	}
+
+	/**
+	 * Tests the scenario where a file is expected to be treated as binary
+	 * according to Git attributes
+	 */
+	@Theory
+	public void fileInBinaryInAttribute(MergeStrategy strategy)
+			throws Exception {
+		Git git = Git.wrap(db);
+		String binaryFile = "file.bin";
+
+		writeTrashFile(".gitattributes", binaryFile + " binary");
+		git.add().addFilepattern(".gitattributes").call();
+		git.commit().setMessage("ADDING GITATTRIBUTES").call();
+
+		writeTrashFile(binaryFile, "\u0000\u0001");
+		git.add().addFilepattern(binaryFile).call();
+		RevCommit parent = git.commit().setMessage("BASE COMMIT").call();
+		String fileHashInBase = getFileHashInWorkTree(git, binaryFile);
+
+		writeTrashFile(binaryFile, "\u0001\u0002");
+		git.add().addFilepattern(binaryFile).call();
+		RevCommit child1 = git.commit().setMessage("THEIRS COMMIT").call();
+		String fileHashInChild1 = getFileHashInWorkTree(git, binaryFile);
+
+		git.checkout().setCreateBranch(true).setStartPoint(parent)
+				.setName("side").call();
+
+		writeTrashFile(binaryFile, "\u0002\u0000");
+		git.add().addFilepattern(binaryFile).call();
+		git.commit().setMessage("OURS COMMIT").call();
+		String fileHashInChild2 = getFileHashInWorkTree(git, binaryFile);
+
+		MergeResult mergeResult = git.merge().setStrategy(strategy)
+				.include(child1).call();
+
+		// check if the merge caused a conflict
+		assertTrue(mergeResult.getConflicts() != null
+				&& !mergeResult.getConflicts().isEmpty());
+		String fileHashInChild2AfterMerge = getFileHashInWorkTree(git,
+				binaryFile);
+
+		// check if the file content changed during a conflicting merge
+		assertEquals(fileHashInChild2AfterMerge, fileHashInChild2);
+
+		Set<String> hashesInIndexFile = new HashSet<>();
+		DirCache indexContent = git.getRepository().readDirCache();
+		for (int i = 0; i < indexContent.getEntryCount(); ++i) {
+			DirCacheEntry indexEntry = indexContent.getEntry(i);
+			if (binaryFile.equals(indexEntry.getPathString())) {
+				hashesInIndexFile.add(indexEntry.getObjectId().name());
+			}
+		}
+
+		// check if all the three stages are added to index file
+		assertTrue(hashesInIndexFile.contains(fileHashInBase));
+		assertTrue(hashesInIndexFile.contains(fileHashInChild1));
+		assertTrue(hashesInIndexFile.contains(fileHashInChild2));
+	}
+
+	private String getFileHashInWorkTree(Git git, String filePath)
+			throws IOException {
+		Repository repository = git.getRepository();
+		ObjectInserter objectInserter = repository.newObjectInserter();
+
+		File conflictingFile = new File(repository.getWorkTree(), filePath);
+		byte[] fileContent = Files.readAllBytes(conflictingFile.toPath());
+		ObjectId blobId = objectInserter.insert(Constants.OBJ_BLOB,
+				fileContent);
+		objectInserter.flush();
+
+		return blobId.name();
+	}
+
 	private void writeSubmodule(String path, ObjectId commit)
 			throws IOException, ConfigInvalidException {
 		addSubmoduleToIndex(path, commit);