Fix RecursiveMerger in case of multiple, independent base commits

When RecursiveMerger found that there are multiple base-commits for the
commits to be merged it tries to temporarily merge the base commits. But
if these base commits have no common predecessor there was a bug in JGit
leading to a NPE. This commit fixes this by enforcing that an empty tree
is used as base when merging two unrelated base commits.

This logic was already there when merging two commits which have no
common predecessor (ThreeWayMerger.mergeBase()). But the code which was
computing a new temporary base commit in case of criss-cross merges
didn't take care to pick an empty tree when no common predecessor can be
found.

Bug: 462671
Change-Id: Ibd96302f5f81383f36d3b1e3edcbf5822147b1a4

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/RecursiveMergerTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/RecursiveMergerTest.java
index 37cc88be1166c065d0e625d7ea0c7947fe0b3054..19e495b40809ea763c9073c689ea887aec403a83 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/RecursiveMergerTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/RecursiveMergerTest.java
@@ -175,6 +175,69 @@ public class RecursiveMergerTest extends RepositoryTestCase {
                }
        }
 
+       @Theory
+       /**
+        * Merging m2,s2 from the following topology. m1 and s1 are the two root
+        * commits of the repo. In master and side different files are touched.
+        * No need to do a real content merge.
+        *
+        * <pre>
+        * m1--m2
+        *   \/
+        *   /\
+        * s1--s2
+        * </pre>
+        */
+       public void crissCrossMerge_twoRoots(MergeStrategy strategy,
+                       IndexState indexState, WorktreeState worktreeState)
+                       throws Exception {
+               if (!validateStates(indexState, worktreeState))
+                       return;
+               // fill the repo
+               BranchBuilder master = db_t.branch("master");
+               BranchBuilder side = db_t.branch("side");
+               RevCommit m1 = master.commit().add("m", "m1").message("m1").create();
+               db_t.getRevWalk().parseCommit(m1);
+
+               RevCommit s1 = side.commit().add("s", "s1").message("s1").create();
+               RevCommit s2 = side.commit().parent(m1).add("m", "m1")
+                               .message("s2(merge)").create();
+               RevCommit m2 = master.commit().parent(s1).add("s", "s1")
+                               .message("m2(merge)").create();
+
+               Git git = Git.wrap(db);
+               git.checkout().setName("master").call();
+               modifyWorktree(worktreeState, "m", "side");
+               modifyWorktree(worktreeState, "s", "side");
+               modifyIndex(indexState, "m", "side");
+               modifyIndex(indexState, "s", "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 || indexState == IndexState.SameAsOther))
+                               // index is dirty
+                               expectSuccess = false;
+
+                       assertEquals(Boolean.valueOf(expectSuccess),
+                                       Boolean.valueOf(merger.merge(new RevCommit[] { m2, s2 })));
+                       assertEquals(MergeStrategy.RECURSIVE, strategy);
+                       assertEquals("m1",
+                                       contentAsString(db, merger.getResultTreeId(), "m"));
+                       assertEquals("s1",
+                                       contentAsString(db, merger.getResultTreeId(), "s"));
+               } 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

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/merge/RecursiveMerger.java b/org.eclipse.jgit/src/org/eclipse/jgit/merge/RecursiveMerger.java
index 85cdb76847b67061367d62d5745a286e6f914bbf..36ffe7a632c8a613ad0bd57cc13ea74348e2dcd3 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/merge/RecursiveMerger.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/merge/RecursiveMerger.java
@@ -68,6 +68,8 @@ import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.filter.RevFilter;
+import org.eclipse.jgit.treewalk.AbstractTreeIterator;
+import org.eclipse.jgit.treewalk.EmptyTreeIterator;
 import org.eclipse.jgit.treewalk.TreeWalk;
 import org.eclipse.jgit.treewalk.WorkingTreeIterator;
 
@@ -194,10 +196,12 @@ public class RecursiveMerger extends ResolveMerger {
                                                        Integer.valueOf(MAX_BASES), a.name(), b.name(),
                                                                        Integer.valueOf(baseCommits.size())));
                                parents.add(nextBase);
-                               if (mergeTrees(
-                                               openTree(getBaseCommit(currentBase, nextBase,
-                                                               callDepth + 1).getTree()),
-                                               currentBase.getTree(), nextBase.getTree(), true))
+                               RevCommit bc = getBaseCommit(currentBase, nextBase,
+                                               callDepth + 1);
+                               AbstractTreeIterator bcTree = (bc == null) ? new EmptyTreeIterator()
+                                               : openTree(bc.getTree());
+                               if (mergeTrees(bcTree, currentBase.getTree(),
+                                               nextBase.getTree(), true))
                                        currentBase = createCommitForTree(resultTree, parents);
                                else
                                        throw new NoMergeBaseException(