RevWalk: Traverse all parents of UNINTERESTING commits

When firstParent is set, RevWalk traverses only the first parent of a
commit, even though that commit is UNINTERESTING. Since we want the
maximal UNINTERESTING set, we shouldn't prune any parents here. This
issue is apparent only when some of the commits being traversed are
unparsed, since walker.carryFlagsImpl() propagates the UNINTERESTING
flag to all parsed ancestors, masking the issue.

Therefore teach RevWalk to traverse all parents when a commit is
UNINTERESTING and not only the first parent. Since this issue is
masked by commit parsing, also test situations when the commits
involved are unparsed.

Signed-off-by: Alex Spradlin <alexaspradlin@google.com>
Change-Id: I95e2ad9ae8f1f50fbecae674367ee7e0855519b1

2 files changed, 95 insertions, 0 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FirstParentRevWalkTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FirstParentRevWalkTest.java
index 474ff7a7c2..1fc7a55457 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FirstParentRevWalkTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FirstParentRevWalkTest.java
@@ -46,6 +46,7 @@ package org.eclipse.jgit.revwalk;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 
+import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.revwalk.filter.MessageRevFilter;
 import org.eclipse.jgit.revwalk.filter.RevFilter;
 import org.junit.Test;
@@ -268,6 +269,26 @@ public class FirstParentRevWalkTest extends RevWalkTestCase {
 	}
 
 	@Test
+	public void testUnparsedFirstParentOfFirstParentMarkedUninteresting()
+			throws Exception {
+		ObjectId a = unparsedCommit();
+		ObjectId b1 = unparsedCommit(a);
+		ObjectId b2 = unparsedCommit(a);
+		ObjectId c1 = unparsedCommit(b1);
+		ObjectId c2 = unparsedCommit(b2);
+		ObjectId d = unparsedCommit(c1, c2);
+
+		rw.reset();
+		rw.setFirstParent(true);
+		RevCommit parsedD = rw.parseCommit(d);
+		markStart(parsedD);
+		markUninteresting(rw.parseCommit(b1));
+		assertCommit(parsedD, rw.next());
+		assertCommit(rw.parseCommit(c1), rw.next());
+		assertNull(rw.next());
+	}
+
+	@Test
 	public void testFirstParentMarkedUninteresting() throws Exception {
 		RevCommit a = commit();
 		RevCommit b1 = commit(a);
@@ -283,6 +304,75 @@ public class FirstParentRevWalkTest extends RevWalkTestCase {
 	}
 
 	@Test
+	public void testUnparsedFirstParentMarkedUninteresting() throws Exception {
+		ObjectId a = unparsedCommit();
+		ObjectId b1 = unparsedCommit(a);
+		ObjectId b2 = unparsedCommit(a);
+		ObjectId c = unparsedCommit(b1, b2);
+
+		rw.reset();
+		rw.setFirstParent(true);
+		RevCommit parsedC = rw.parseCommit(c);
+		markStart(parsedC);
+		markUninteresting(rw.parseCommit(b1));
+		assertCommit(parsedC, rw.next());
+		assertNull(rw.next());
+	}
+
+	@Test
+	public void testUninterestingCommitWithTwoParents() throws Exception {
+		RevCommit a = commit();
+		RevCommit b = commit(a);
+		RevCommit c1 = commit(b);
+		RevCommit c2 = commit(b);
+		RevCommit d = commit(c1);
+		RevCommit e = commit(c1, c2);
+
+		RevCommit uA = commit(a, b);
+		RevCommit uB1 = commit(uA, c2);
+		RevCommit uB2 = commit(uA, d);
+		RevCommit uninteresting = commit(uB1, uB2);
+
+		rw.reset();
+		rw.setFirstParent(true);
+		markStart(e);
+		markUninteresting(uninteresting);
+
+		assertCommit(e, rw.next());
+		assertNull(rw.next());
+	}
+
+	/**
+	 * This fails if we try to propagate flags before parsing commits.
+	 *
+	 * @throws Exception
+	 */
+	@Test
+	public void testUnparsedUninterestingCommitWithTwoParents()
+			throws Exception {
+		ObjectId a = unparsedCommit();
+		ObjectId b = unparsedCommit(a);
+		ObjectId c1 = unparsedCommit(b);
+		ObjectId c2 = unparsedCommit(b);
+		ObjectId d = unparsedCommit(c1);
+		ObjectId e = unparsedCommit(c1, c2);
+
+		ObjectId uA = unparsedCommit(a, b);
+		ObjectId uB1 = unparsedCommit(uA, c2);
+		ObjectId uB2 = unparsedCommit(uA, d);
+		ObjectId uninteresting = unparsedCommit(uB1, uB2);
+
+		rw.reset();
+		rw.setFirstParent(true);
+		RevCommit parsedE = rw.parseCommit(e);
+		markStart(parsedE);
+		markUninteresting(rw.parseCommit(uninteresting));
+
+		assertCommit(parsedE, rw.next());
+		assertNull(rw.next());
+	}
+
+	@Test
 	public void testDepthWalk() throws Exception {
 		RevCommit a = commit();
 		RevCommit b1 = commit(a);
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkTestCase.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkTestCase.java
index 544398219f..a0056ae217 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkTestCase.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkTestCase.java
@@ -51,6 +51,7 @@ import org.eclipse.jgit.dircache.DirCacheEntry;
 import org.eclipse.jgit.junit.RepositoryTestCase;
 import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.junit.TestRepository.CommitBuilder;
+import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Repository;
 
 /** Support for tests of the {@link RevWalk} class. */
@@ -96,6 +97,10 @@ public abstract class RevWalkTestCase extends RepositoryTestCase {
 		return util.get(tree, path);
 	}
 
+	protected ObjectId unparsedCommit(ObjectId... parents) throws Exception {
+		return util.unparsedCommit(parents);
+	}
+
 	protected RevCommit commit(RevCommit... parents) throws Exception {
 		return util.commit(parents);
 	}