Merge "StartGenerator: Fix parent rewrite with non-default RevFilter"

4 files changed, 123 insertions, 24 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/TreeRevFilterTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/TreeRevFilterTest.java
index 298facfd15..f67a623ff6 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/TreeRevFilterTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/TreeRevFilterTest.java
@@ -21,6 +21,7 @@ import org.junit.Test;
 
 public class TreeRevFilterTest extends RevWalkTestCase {
 	private RevFilter treeRevFilter() {
+		rw.setRewriteParents(false);
 		return new TreeRevFilter(rw, TreeFilter.ANY_DIFF);
 	}
 
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/TreeRevFilterWithRewriteParentsTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/TreeRevFilterWithRewriteParentsTest.java
new file mode 100644
index 0000000000..100f2e4164
--- /dev/null
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/TreeRevFilterWithRewriteParentsTest.java
@@ -0,0 +1,112 @@
+/*
+ * Copyright (C) 2023, Google LLC and others
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Distribution License v. 1.0 which is available at
+ * https://www.eclipse.org/org/documents/edl-v10.php.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+package org.eclipse.jgit.revwalk;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+import org.eclipse.jgit.revwalk.filter.RevFilter;
+import org.eclipse.jgit.treewalk.filter.TreeFilter;
+import org.junit.Test;
+
+public class TreeRevFilterWithRewriteParentsTest extends RevWalkTestCase {
+	private RevFilter treeRevFilter() {
+		rw.setRewriteParents(true);
+		return new TreeRevFilter(rw, TreeFilter.ANY_DIFF);
+	}
+
+	@Test
+	public void testStringOfPearls_FilePath1()
+			throws Exception {
+		RevCommit a = commit(tree(file("d/f", blob("a"))));
+		RevCommit b = commit(tree(file("d/f", blob("a"))), a);
+		RevCommit c = commit(tree(file("d/f", blob("b"))), b);
+		rw.setRevFilter(treeRevFilter());
+		markStart(c);
+
+		assertCommit(c, rw.next());
+		assertEquals(1, c.getParentCount());
+		assertCommit(a, c.getParent(0));
+
+		assertCommit(a, rw.next()); // b was skipped
+		assertEquals(0, a.getParentCount());
+		assertNull(rw.next());
+	}
+
+	@Test
+	public void testStringOfPearls_FilePath2() throws Exception {
+		RevCommit a = commit(tree(file("d/f", blob("a"))));
+		RevCommit b = commit(tree(file("d/f", blob("a"))), a);
+		RevCommit c = commit(tree(file("d/f", blob("b"))), b);
+		RevCommit d = commit(tree(file("d/f", blob("b"))), c);
+		rw.setRevFilter(treeRevFilter());
+		markStart(d);
+
+		// d was skipped
+		assertCommit(c, rw.next());
+		assertEquals(1, c.getParentCount());
+		assertCommit(a, c.getParent(0));
+
+		// b was skipped
+		assertCommit(a, rw.next());
+		assertEquals(0, a.getParentCount());
+		assertNull(rw.next());
+	}
+
+	@Test
+	public void testStringOfPearls_DirPath2() throws Exception {
+		RevCommit a = commit(tree(file("d/f", blob("a"))));
+		RevCommit b = commit(tree(file("d/f", blob("a"))), a);
+		RevCommit c = commit(tree(file("d/f", blob("b"))), b);
+		RevCommit d = commit(tree(file("d/f", blob("b"))), c);
+		rw.setRevFilter(treeRevFilter());
+		markStart(d);
+
+		// d was skipped
+		assertCommit(c, rw.next());
+		assertEquals(1, c.getParentCount());
+		assertCommit(a, c.getParent(0));
+
+		// b was skipped
+		assertCommit(a, rw.next());
+		assertEquals(0, a.getParentCount());
+		assertNull(rw.next());
+	}
+
+	@Test
+	public void testStringOfPearls_FilePath3() throws Exception {
+		RevCommit a = commit(tree(file("d/f", blob("a"))));
+		RevCommit b = commit(tree(file("d/f", blob("a"))), a);
+		RevCommit c = commit(tree(file("d/f", blob("b"))), b);
+		RevCommit d = commit(tree(file("d/f", blob("b"))), c);
+		RevCommit e = commit(tree(file("d/f", blob("b"))), d);
+		RevCommit f = commit(tree(file("d/f", blob("b"))), e);
+		RevCommit g = commit(tree(file("d/f", blob("b"))), f);
+		RevCommit h = commit(tree(file("d/f", blob("b"))), g);
+		RevCommit i = commit(tree(file("d/f", blob("c"))), h);
+		rw.setRevFilter(treeRevFilter());
+		markStart(i);
+
+		assertCommit(i, rw.next());
+		assertEquals(1, i.getParentCount());
+		assertCommit(c, i.getParent(0));
+
+		// h..d was skipped
+		assertCommit(c, rw.next());
+		assertEquals(1, c.getParentCount());
+		assertCommit(a, c.getParent(0));
+
+		// b was skipped
+		assertCommit(a, rw.next());
+		assertEquals(0, a.getParentCount());
+		assertNull(rw.next());
+	}
+}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/StartGenerator.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/StartGenerator.java
index a79901ca10..2f7c4d5798 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/StartGenerator.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/StartGenerator.java
@@ -97,14 +97,14 @@ class StartGenerator extends Generator {
 			pending = (DateRevQueue)q;
 		else
 			pending = new DateRevQueue(q);
+		if (rf != RevFilter.ALL && w.getRewriteParents()) {
+			pendingOutputType |= HAS_REWRITE | NEEDS_REWRITE;
+		}
 		if (tf != TreeFilter.ALL) {
-			int rewriteFlag;
 			if (w.getRewriteParents()) {
 				pendingOutputType |= HAS_REWRITE | NEEDS_REWRITE;
-				rewriteFlag = RevWalk.REWRITE;
-			} else
-				rewriteFlag = 0;
-			rf = AndRevFilter.create(new TreeRevFilter(w, tf, rewriteFlag), rf);
+			}
+			rf = AndRevFilter.create(new TreeRevFilter(w, tf), rf);
 		}
 
 		walker.queue = q;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/TreeRevFilter.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/TreeRevFilter.java
index 43571a6868..4085954638 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/TreeRevFilter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/TreeRevFilter.java
@@ -60,21 +60,8 @@ public class TreeRevFilter extends RevFilter {
 	 * Create a {@link org.eclipse.jgit.revwalk.filter.RevFilter} from a
 	 * {@link org.eclipse.jgit.treewalk.filter.TreeFilter}.
 	 *
-	 * @param walker
-	 *            walker used for reading trees.
-	 * @param t
-	 *            filter to compare against any changed paths in each commit. If
-	 *            a {@link org.eclipse.jgit.revwalk.FollowFilter}, will be
-	 *            replaced with a new filter following new paths after a rename.
-	 * @since 3.5
-	 */
-	public TreeRevFilter(RevWalk walker, TreeFilter t) {
-		this(walker, t, 0);
-	}
-
-	/**
-	 * Create a filter for the first phase of a parent-rewriting limited
-	 * revision walk.
+	 * When revWalk's rewrite parent flag is set, it creates a filter for the
+	 * first phase of a parent-rewriting limited revision walk.
 	 * <p>
 	 * This filter is ANDed to evaluate before all other filters and ties the
 	 * configured {@link TreeFilter} into the revision walking process.
@@ -91,14 +78,13 @@ public class TreeRevFilter extends RevFilter {
 	 *            filter to compare against any changed paths in each commit. If
 	 *            a {@link FollowFilter}, will be replaced with a new filter
 	 *            following new paths after a rename.
-	 * @param rewriteFlag
-	 *            flag to color commits to be removed from the simplified DAT.
+	 * @since 3.5
 	 */
-	TreeRevFilter(RevWalk walker, TreeFilter t, int rewriteFlag) {
+	public TreeRevFilter(RevWalk walker, TreeFilter t) {
 		pathFilter = new TreeWalk(walker.reader);
 		pathFilter.setFilter(t);
 		pathFilter.setRecursive(t.shouldBeRecursive());
-		this.rewriteFlag = rewriteFlag;
+		this.rewriteFlag = walker.getRewriteParents() ? RevWalk.REWRITE : 0;
 	}
 
 	@Override