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
This commit is contained in:
parent
db0eb9f8ae
commit
a80df5380f
|
@ -360,6 +360,21 @@ public RevObject get(RevTree tree, String path)
|
||||||
return null; // never reached.
|
return null; // never reached.
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Create a new, unparsed commit.
|
||||||
|
* <p>
|
||||||
|
* See {@link #unparsedCommit(int, RevTree, ObjectId...)}. The tree is the
|
||||||
|
* empty tree (no files or subdirectories).
|
||||||
|
*
|
||||||
|
* @param parents
|
||||||
|
* zero or more IDs of the commit's parents.
|
||||||
|
* @return the ID of the new commit.
|
||||||
|
* @throws Exception
|
||||||
|
*/
|
||||||
|
public ObjectId unparsedCommit(ObjectId... parents) throws Exception {
|
||||||
|
return unparsedCommit(1, tree(), parents);
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Create a new commit.
|
* Create a new commit.
|
||||||
* <p>
|
* <p>
|
||||||
|
@ -428,6 +443,28 @@ public RevCommit commit(int secDelta, RevCommit... parents)
|
||||||
*/
|
*/
|
||||||
public RevCommit commit(final int secDelta, final RevTree tree,
|
public RevCommit commit(final int secDelta, final RevTree tree,
|
||||||
final RevCommit... parents) throws Exception {
|
final RevCommit... parents) throws Exception {
|
||||||
|
ObjectId id = unparsedCommit(secDelta, tree, parents);
|
||||||
|
return pool.parseCommit(id);
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Create a new, unparsed commit.
|
||||||
|
* <p>
|
||||||
|
* The author and committer identities are stored using the current
|
||||||
|
* timestamp, after being incremented by {@code secDelta}. The message body
|
||||||
|
* is empty.
|
||||||
|
*
|
||||||
|
* @param secDelta
|
||||||
|
* number of seconds to advance {@link #tick(int)} by.
|
||||||
|
* @param tree
|
||||||
|
* the root tree for the commit.
|
||||||
|
* @param parents
|
||||||
|
* zero or more IDs of the commit's parents.
|
||||||
|
* @return the ID of the new commit.
|
||||||
|
* @throws Exception
|
||||||
|
*/
|
||||||
|
public ObjectId unparsedCommit(final int secDelta, final RevTree tree,
|
||||||
|
final ObjectId... parents) throws Exception {
|
||||||
tick(secDelta);
|
tick(secDelta);
|
||||||
|
|
||||||
final org.eclipse.jgit.lib.CommitBuilder c;
|
final org.eclipse.jgit.lib.CommitBuilder c;
|
||||||
|
@ -443,7 +480,7 @@ public RevCommit commit(final int secDelta, final RevTree tree,
|
||||||
id = ins.insert(c);
|
id = ins.insert(c);
|
||||||
ins.flush();
|
ins.flush();
|
||||||
}
|
}
|
||||||
return pool.parseCommit(id);
|
return id;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
|
@ -46,6 +46,7 @@
|
||||||
import static org.junit.Assert.assertEquals;
|
import static org.junit.Assert.assertEquals;
|
||||||
import static org.junit.Assert.assertNull;
|
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.MessageRevFilter;
|
||||||
import org.eclipse.jgit.revwalk.filter.RevFilter;
|
import org.eclipse.jgit.revwalk.filter.RevFilter;
|
||||||
import org.junit.Test;
|
import org.junit.Test;
|
||||||
|
@ -267,6 +268,26 @@ public void testFirstParentOfFirstParentMarkedUninteresting()
|
||||||
assertNull(rw.next());
|
assertNull(rw.next());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@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
|
@Test
|
||||||
public void testFirstParentMarkedUninteresting() throws Exception {
|
public void testFirstParentMarkedUninteresting() throws Exception {
|
||||||
RevCommit a = commit();
|
RevCommit a = commit();
|
||||||
|
@ -282,6 +303,75 @@ public void testFirstParentMarkedUninteresting() throws Exception {
|
||||||
assertNull(rw.next());
|
assertNull(rw.next());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@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
|
@Test
|
||||||
public void testDepthWalk() throws Exception {
|
public void testDepthWalk() throws Exception {
|
||||||
RevCommit a = commit();
|
RevCommit a = commit();
|
||||||
|
|
|
@ -51,6 +51,7 @@
|
||||||
import org.eclipse.jgit.junit.RepositoryTestCase;
|
import org.eclipse.jgit.junit.RepositoryTestCase;
|
||||||
import org.eclipse.jgit.junit.TestRepository;
|
import org.eclipse.jgit.junit.TestRepository;
|
||||||
import org.eclipse.jgit.junit.TestRepository.CommitBuilder;
|
import org.eclipse.jgit.junit.TestRepository.CommitBuilder;
|
||||||
|
import org.eclipse.jgit.lib.ObjectId;
|
||||||
import org.eclipse.jgit.lib.Repository;
|
import org.eclipse.jgit.lib.Repository;
|
||||||
|
|
||||||
/** Support for tests of the {@link RevWalk} class. */
|
/** Support for tests of the {@link RevWalk} class. */
|
||||||
|
@ -96,6 +97,10 @@ protected RevObject get(RevTree tree, String path)
|
||||||
return util.get(tree, path);
|
return util.get(tree, path);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
protected ObjectId unparsedCommit(ObjectId... parents) throws Exception {
|
||||||
|
return util.unparsedCommit(parents);
|
||||||
|
}
|
||||||
|
|
||||||
protected RevCommit commit(RevCommit... parents) throws Exception {
|
protected RevCommit commit(RevCommit... parents) throws Exception {
|
||||||
return util.commit(parents);
|
return util.commit(parents);
|
||||||
}
|
}
|
||||||
|
|
|
@ -143,7 +143,9 @@ RevCommit next() throws MissingObjectException,
|
||||||
|
|
||||||
for (int i = 0; i < c.parents.length; i++) {
|
for (int i = 0; i < c.parents.length; i++) {
|
||||||
RevCommit p = c.parents[i];
|
RevCommit p = c.parents[i];
|
||||||
if (firstParent && i > 0) {
|
// If the commit is uninteresting, don't try to prune
|
||||||
|
// parents because we want the maximal uninteresting set.
|
||||||
|
if (firstParent && i > 0 && (c.flags & UNINTERESTING) == 0) {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
if ((p.flags & SEEN) != 0)
|
if ((p.flags & SEEN) != 0)
|
||||||
|
|
Loading…
Reference in New Issue