StartGenerator: Fix parent rewrite with non-default RevFilter

StartGenerator is responsible for propagating the RevWalk's
parent rewrite setting, but it currently only does so when a
non-default TreeFilter is set, when it should also do so if
the default TreeFilter is used with a non-default RevFilter.

Adding a new if condition within StartGenerator to enable parent
rewrite with non-default RevFilter.

TreeRevFilter relied on the old buggy functionality and has
been modified to explicitly refrain from rewriting parents.

Change-Id: I4e4ff67fb279edbcc3461496b132cea774fb742f

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 298facfd15f92d2e313d8c029342b99b91640584..f67a623ff6e8c73d36dcd2d727100dea7017670a 100644 (file)
--- 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 (file)
index 0000000..100f2e4
--- /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 a79901ca10e8f67861fe27c1d122ccf5d47e57da..2f7c4d579874706674174a313a5f2882cc1871ad 100644 (file)
--- 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 43571a686868e24d1cac355f5df6a7797a137243..4085954638d8c3468f55ac7a36ab4b247b309978 100644 (file)
--- 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