From 2bd3556e7e1f07c4bb06282ead95a936758e388e Mon Sep 17 00:00:00 2001 From: Christian Halstrick Date: Wed, 25 Mar 2015 10:48:03 +0100 Subject: [PATCH] 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 --- .../jgit/merge/RecursiveMergerTest.java | 63 +++++++++++++++++++ .../eclipse/jgit/merge/RecursiveMerger.java | 12 ++-- 2 files changed, 71 insertions(+), 4 deletions(-) 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 37cc88be1..19e495b40 100644 --- 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. + * + *
+	 * m1--m2
+	 *   \/
+	 *   /\
+	 * s1--s2
+	 * 
+ */ + 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 85cdb7684..36ffe7a63 100644 --- 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(