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
This commit is contained in:
Ronald Bhuleskar 2023-10-25 13:10:33 -07:00
parent 6cdd04aa46
commit db5ce6b5c2
4 changed files with 123 additions and 24 deletions

View File

@ -21,6 +21,7 @@
public class TreeRevFilterTest extends RevWalkTestCase {
private RevFilter treeRevFilter() {
rw.setRewriteParents(false);
return new TreeRevFilter(rw, TreeFilter.ANY_DIFF);
}

View File

@ -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());
}
}

View File

@ -97,14 +97,14 @@ RevCommit next() throws MissingObjectException,
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;

View File

@ -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 TreeRevFilter(RevWalk walker, TreeFilter t) {
* 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