Skip to content

Commit acd1eff

Browse files
committed
PendingGenerator: drop only buffers loaded in the generator
PendingGenerator dropped RevCommit buffers of "uninteresting" commits that it didn't produce. This led to the surprising behavior that a fully loaded commit that was fed into a RevWalk marked as uninteresting would no longer have its buffer afterwards and one needed to parse the commit afresh. Change the logic in PendingGenerator to drop such buffers only if the generator itself loaded it earlier (because a filter required the commit body and it wasn't present on the commit yet). This preserves buffers on commits that had their buffers before the RevWalk. Bug: jgit-203 Change-Id: I606a4c63a3a6a9cdcc619c200a2676dbeab03ac3
1 parent 4013b3e commit acd1eff

File tree

2 files changed

+60
-6
lines changed

2 files changed

+60
-6
lines changed

org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkSortTest.java

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,56 @@ public void testSort_TOPO() throws Exception {
127127
assertNull(rw.next());
128128
}
129129

130+
@Test
131+
public void testSort_TOPO_reachable() throws Exception {
132+
// c1 is back dated before its parent.
133+
//
134+
final RevCommit a = commit();
135+
final RevCommit b = commit(a);
136+
final RevCommit c1 = commit(-5, b);
137+
final RevCommit c2 = commit(10, b);
138+
final RevCommit d = commit(c1, c2);
139+
140+
assertEquals("", d.getShortMessage());
141+
assertEquals("", c2.getShortMessage());
142+
assertEquals("", c1.getShortMessage());
143+
assertEquals("", b.getShortMessage());
144+
rw.sort(RevSort.TOPO);
145+
markStart(d);
146+
markUninteresting(b);
147+
assertEquals("", d.getShortMessage());
148+
assertEquals("", c2.getShortMessage());
149+
assertEquals("", c1.getShortMessage());
150+
assertEquals("", b.getShortMessage());
151+
assertCommit(d, rw.next());
152+
assertCommit(c2, rw.next());
153+
assertCommit(c1, rw.next());
154+
assertNull(rw.next());
155+
assertEquals("", d.getShortMessage());
156+
assertEquals("", c2.getShortMessage());
157+
assertEquals("", c1.getShortMessage());
158+
assertEquals("", b.getShortMessage());
159+
}
160+
161+
@Test
162+
public void testSort_TOPO_reachable2() throws Exception {
163+
// c1 is back dated before its parent.
164+
//
165+
final RevCommit a = commit();
166+
final RevCommit b = commit(a);
167+
final RevCommit c1 = commit(-5, b);
168+
final RevCommit c2 = commit(10, b);
169+
final RevCommit d = commit(c1, c2);
170+
171+
assertEquals("", d.getShortMessage());
172+
rw.sort(RevSort.TOPO);
173+
markStart(d);
174+
markUninteresting(d);
175+
assertEquals("", d.getShortMessage());
176+
assertNull(rw.next());
177+
assertEquals("", d.getShortMessage());
178+
}
179+
130180
@Test
131181
public void testSort_TOPO_REVERSE() throws Exception {
132182
// c1 is back dated before its parent.

org.eclipse.jgit/src/org/eclipse/jgit/revwalk/PendingGenerator.java

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -100,11 +100,13 @@ RevCommit next() throws MissingObjectException,
100100
}
101101

102102
final boolean produce;
103-
if ((c.flags & UNINTERESTING) != 0)
103+
final boolean hadBody = c.getRawBuffer() != null;
104+
if ((c.flags & UNINTERESTING) != 0) {
104105
produce = false;
105-
else {
106-
if (filter.requiresCommitBody())
106+
} else {
107+
if (filter.requiresCommitBody() && !hadBody) {
107108
c.parseBody(walker);
109+
}
108110
produce = filter.include(walker, c);
109111
}
110112

@@ -140,15 +142,17 @@ RevCommit next() throws MissingObjectException,
140142
} else {
141143
overScan = OVER_SCAN;
142144
}
143-
if (canDispose)
145+
if (canDispose && !hadBody) {
144146
c.disposeBody();
147+
}
145148
continue;
146149
}
147150

148-
if (produce)
151+
if (produce) {
149152
return last = c;
150-
else if (canDispose)
153+
} else if (canDispose && !hadBody) {
151154
c.disposeBody();
155+
}
152156
}
153157
} catch (StopWalkException swe) {
154158
pending.clear();

0 commit comments

Comments
 (0)