RecursiveMerger should not fail on content-merge conflicts of parents

Previously when RecursiveMerger was trying to create a single virtual
common base for the merge it was failing when this lead to content-merge
conflicts. This is different from what native git is doing. When native
git's recursive merge algorithm creates a new common base it will merge
the multiple parents and simply take the merge result (potentially
including conflict markers) as common base. See my discussion with Shawn
here: http://www.spinics.net/lists/git/msg234959.html :

  > - How should workingtree, index (stage1,2,3) look like if during
that
  > merge of common ancestors a conflict occurs? Will I see in stage2
and
  > stage3 really see content of X1 and X2?

  Its done entirely in memory and never touches the working tree or
  index. When a conflict exists in the X1-X2 merge the conflict is
  preserved into the new virtual base.

There is still the possibility that the merge of parents lead to
conflicts. File/Folder conclicts, conflicts on filemodes. This commit
only fixes the situation for conflicts when merging content.

Bug: 438203
Change-Id: If45bc3d078b3d3de87b758e71d7379059d709603

org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/RecursiveMergerTest.java

@@ -343,6 +343,89 @@ public class RecursiveMergerTest extends RepositoryTestCase {
 		}
 	}
 
+	@Theory
+	/**
+	 * Merging m2,s2 from the following topology. m1 and s1 are not mergeable
+	 * without conflicts. The same file is modified in both branches. The
+	 * modifications should be mergeable but only if the merge result of
+	 * merging m1 and s1 is choosen as parent (including the conflict markers).
+	 *
+	 * <pre>
+	 * m0--m1--m2
+	 *   \   \/
+	 *    \  /\
+	 *     s1--s2
+	 * </pre>
+	 */
+	public void crissCrossMerge_ParentsNotMergeable(MergeStrategy strategy,
+			IndexState indexState, WorktreeState worktreeState)
+			throws Exception {
+		if (!validateStates(indexState, worktreeState))
+			return;
+
+		BranchBuilder master = db_t.branch("master");
+		RevCommit m0 = master.commit().add("f", "1\n2\n3\n").message("m0")
+				.create();
+		RevCommit m1 = master.commit().add("f", "1\nx(master)\n2\n3\n")
+				.message("m1").create();
+		db_t.getRevWalk().parseCommit(m1);
+
+		BranchBuilder side = db_t.branch("side");
+		RevCommit s1 = side.commit().parent(m0)
+				.add("f", "1\nx(side)\n2\n3\ny(side)\n")
+				.message("s1").create();
+		RevCommit s2 = side.commit().parent(m1)
+				.add("f", "1\nx(side)\n2\n3\ny(side-again)\n")
+				.message("s2(merge)")
+				.create();
+		RevCommit m2 = master.commit().parent(s1)
+				.add("f", "1\nx(side)\n2\n3\ny(side)\n").message("m2(merge)")
+				.create();
+
+		Git git = Git.wrap(db);
+		git.checkout().setName("master").call();
+		modifyWorktree(worktreeState, "f", "side");
+		modifyIndex(indexState, "f", "side");
+
+		ResolveMerger merger = (ResolveMerger) strategy.newMerger(db,
+				worktreeState == WorktreeState.Bare);
+		if (worktreeState != WorktreeState.Bare)
+			merger.setWorkingTreeIterator(new FileTreeIterator(db));
+		try {
+			boolean expectSuccess = true;
+			if (!(indexState == IndexState.Bare
+					|| indexState == IndexState.Missing || indexState == IndexState.SameAsHead))
+				// index is dirty
+				expectSuccess = false;
+			else if (worktreeState == WorktreeState.DifferentFromHeadAndOther
+					|| worktreeState == WorktreeState.SameAsOther)
+				expectSuccess = false;
+			assertEquals("Merge didn't return as expected: strategy:"
+					+ strategy.getName() + ", indexState:" + indexState
+					+ ", worktreeState:" + worktreeState + " . ",
+					Boolean.valueOf(expectSuccess),
+					Boolean.valueOf(merger.merge(new RevCommit[] { m2, s2 })));
+			assertEquals(MergeStrategy.RECURSIVE, strategy);
+			if (!expectSuccess)
+				// if the merge was not successful skip testing the state of
+				// index and workingtree
+				return;
+			assertEquals("1\nx(side)\n2\n3\ny(side-again)",
+					contentAsString(db, merger.getResultTreeId(), "f"));
+			if (indexState != IndexState.Bare)
+				assertEquals(
+						"[f, mode:100644, content:1\nx(side)\n2\n3\ny(side-again)\n]",
+						indexState(RepositoryTestCase.CONTENT));
+			if (worktreeState != WorktreeState.Bare
+					&& worktreeState != WorktreeState.Missing)
+				assertEquals("1\nx(side)\n2\n3\ny(side-again)\n", read("f"));
+		} catch (NoMergeBaseException e) {
+			assertEquals(MergeStrategy.RESOLVE, strategy);
+			assertEquals(e.getReason(),
+					MergeBaseFailureReason.MULTIPLE_MERGE_BASES_NOT_SUPPORTED);
+		}
+	}
+
 	@Theory
 	/**
 	 * Merging m2,s2 from the following topology. The same file is modified

org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeResult.java

@@ -161,4 +161,16 @@ public class MergeResult<S extends Sequence> implements Iterable<MergeChunk> {
 	public boolean containsConflicts() {
 		return containsConflicts;
 	}
+
+	/**
+	 * Sets explicitly whether this merge should be seen as containing a
+	 * conflict or not. Needed because during RecursiveMerger we want to do
+	 * content-merges and take the resulting content (even with conflict
+	 * markers!) as new conflict-free content
+	 *
+	 * @param containsConflicts
+	 */
+	protected void setContainsConflicts(boolean containsConflicts) {
+		this.containsConflicts = containsConflicts;
+	}
 }

org.eclipse.jgit/src/org/eclipse/jgit/merge/RecursiveMerger.java

@@ -196,8 +196,7 @@ public class RecursiveMerger extends ResolveMerger {
 				if (mergeTrees(
 						openTree(getBaseCommit(currentBase, nextBase,
 								callDepth + 1).getTree()),
-						currentBase.getTree(),
-						nextBase.getTree()))
+						currentBase.getTree(), nextBase.getTree(), true))
 					currentBase = createCommitForTree(resultTree, parents);
 				else
 					throw new NoMergeBaseException(

org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java

@@ -297,7 +297,8 @@ public class ResolveMerger extends ThreeWayMerger {
 			dircache = getRepository().lockDirCache();
 
 		try {
-			return mergeTrees(mergeBase(), sourceTrees[0], sourceTrees[1]);
+			return mergeTrees(mergeBase(), sourceTrees[0], sourceTrees[1],
+					false);
 		} finally {
 			if (implicitDirCache)
 				dircache.unlock();
@@ -457,6 +458,9 @@ public class ResolveMerger extends ThreeWayMerger {
 	 *            the index entry
 	 * @param work
 	 *            the file in the working tree
+	 * @param ignoreConflicts
+	 *            see
+	 *            {@link ResolveMerger#mergeTrees(AbstractTreeIterator, RevTree, RevTree, boolean)}
 	 * @return <code>false</code> if the merge will fail because the index entry
 	 *         didn't match ours or the working-dir file was dirty and a
 	 *         conflict occurred
@@ -468,7 +472,8 @@ public class ResolveMerger extends ThreeWayMerger {
 	 */
 	protected boolean processEntry(CanonicalTreeParser base,
 			CanonicalTreeParser ours, CanonicalTreeParser theirs,
-			DirCacheBuildIterator index, WorkingTreeIterator work)
+			DirCacheBuildIterator index, WorkingTreeIterator work,
+			boolean ignoreConflicts)
 			throws MissingObjectException, IncorrectObjectTypeException,
 			CorruptObjectException, IOException {
 		enterSubtree = true;
@@ -627,9 +632,11 @@ public class ResolveMerger extends ThreeWayMerger {
 			}
 
 			MergeResult<RawText> result = contentMerge(base, ours, theirs);
+			if (ignoreConflicts)
+				result.setContainsConflicts(false);
 			File of = writeMergedFile(result);
 			updateIndex(base, ours, theirs, result, of);
-			if (result.containsConflicts())
+			if (result.containsConflicts() && !ignoreConflicts)
 				unmergedPaths.add(tw.getPathString());
 			modifiedFiles.add(tw.getPathString());
 		} else if (modeO != modeT) {
@@ -993,12 +1000,32 @@ public class ResolveMerger extends ThreeWayMerger {
 	 * @param baseTree
 	 * @param headTree
 	 * @param mergeTree
+	 * @param ignoreConflicts
+	 *            Controls what to do in case a content-merge is done and a
+	 *            conflict is detected. The default setting for this should be
+	 *            <code>false</code>. In this case the working tree file is
+	 *            filled with new content (containing conflict markers) and the
+	 *            index is filled with multiple stages containing BASE, OURS and
+	 *            THEIRS content. Having such non-0 stages is the sign to git
+	 *            tools that there are still conflicts for that path.
+	 *            <p>
+	 *            If <code>true</code> is specified the behavior is different.
+	 *            In case a conflict is detected the working tree file is again
+	 *            filled with new content (containing conflict markers). But
+	 *            also stage 0 of the index is filled with that content. No
+	 *            other stages are filled. Means: there is no conflict on that
+	 *            path but the new content (including conflict markers) is
+	 *            stored as successful merge result. This is needed in the
+	 *            context of {@link RecursiveMerger} where when determining
+	 *            merge bases we don't want to deal with content-merge
+	 *            conflicts.
 	 * @return whether the trees merged cleanly
 	 * @throws IOException
 	 * @since 3.0
 	 */
 	protected boolean mergeTrees(AbstractTreeIterator baseTree,
-			RevTree headTree, RevTree mergeTree) throws IOException {
+			RevTree headTree, RevTree mergeTree, boolean ignoreConflicts)
+			throws IOException {
 
 		builder = dircache.builder();
 		DirCacheBuildIterator buildIt = new DirCacheBuildIterator(builder);
@@ -1011,7 +1038,7 @@ public class ResolveMerger extends ThreeWayMerger {
 		if (workingTreeIterator != null)
 			tw.addTree(workingTreeIterator);
 
-		if (!mergeTreeWalk(tw)) {
+		if (!mergeTreeWalk(tw, ignoreConflicts)) {
 			return false;
 		}
 
@@ -1050,11 +1077,15 @@ public class ResolveMerger extends ThreeWayMerger {
 	 *
 	 * @param treeWalk
 	 *            The walk to iterate over.
+	 * @param ignoreConflicts
+	 *            see
+	 *            {@link ResolveMerger#mergeTrees(AbstractTreeIterator, RevTree, RevTree, boolean)}
 	 * @return Whether the trees merged cleanly.
 	 * @throws IOException
 	 * @since 3.4
 	 */
-	protected boolean mergeTreeWalk(TreeWalk treeWalk) throws IOException {
+	protected boolean mergeTreeWalk(TreeWalk treeWalk, boolean ignoreConflicts)
+			throws IOException {
 		boolean hasWorkingTreeIterator = tw.getTreeCount() > T_FILE;
 		while (treeWalk.next()) {
 			if (!processEntry(
@@ -1063,7 +1094,7 @@ public class ResolveMerger extends ThreeWayMerger {
 					treeWalk.getTree(T_THEIRS, CanonicalTreeParser.class),
 					treeWalk.getTree(T_INDEX, DirCacheBuildIterator.class),
 					hasWorkingTreeIterator ? treeWalk.getTree(T_FILE,
-							WorkingTreeIterator.class) : null)) {
+							WorkingTreeIterator.class) : null, ignoreConflicts)) {
 				cleanUp();
 				return false;
 			}