In a new RevWalk, if the first object parsed is one of the shallow commits, the following happens: 1) RevCommit.parseCanonical() is called on a new "r1" RevCommit. 2) RevCommit.parseCanonical() immediately calls RevWalk.initializeShallowCommits(). 3) RevWalk.initializeShallowCommits() calls lookupCommit(id), creating and adding a new "r2" version of this same object and marking its parents empty. 4) RevCommit.parseCanonical() initializes the "r1" RevCommit's fields, including the parents. 5) RevCommit.parseCanonical()'s caller uses the "r1" commit that has parents, losing the fact that it is a shallow commit. This change passes the current RevCommit as an argument to RevWalk.initializeShallowCommits() so that method can set its parents empty rather than creating the duplicate "r2" commit. Change-Id: I67b79aa2927dd71ac7b0d8f8917f423dcaf08c8a Signed-off-by: Terry Parker <tparker@google.com>tags/v5.1.0.201808281540-m3
assertNull(rw.next()); | assertNull(rw.next()); | ||||
} | } | ||||
@Test | |||||
public void testShallowCommitParse() throws Exception { | |||||
RevCommit a = commit(); | |||||
RevCommit b = commit(a); | |||||
createShallowFile(b); | |||||
rw.close(); | |||||
rw = createRevWalk(); | |||||
b = rw.parseCommit(b); | |||||
markStart(b); | |||||
assertCommit(b, rw.next()); | |||||
assertNull(rw.next()); | |||||
} | |||||
private void createShallowFile(ObjectId... shallowCommits) | private void createShallowFile(ObjectId... shallowCommits) | ||||
throws IOException { | throws IOException { | ||||
final StringBuilder builder = new StringBuilder(); | final StringBuilder builder = new StringBuilder(); |
} | } | ||||
} | } | ||||
void parseCanonical(RevWalk walk, byte[] raw) | |||||
throws IOException { | |||||
if (!walk.shallowCommitsInitialized) | |||||
walk.initializeShallowCommits(); | |||||
void parseCanonical(RevWalk walk, byte[] raw) throws IOException { | |||||
if (!walk.shallowCommitsInitialized) { | |||||
walk.initializeShallowCommits(this); | |||||
} | |||||
final MutableObjectId idBuffer = walk.idBuffer; | final MutableObjectId idBuffer = walk.idBuffer; | ||||
idBuffer.fromString(raw, 5); | idBuffer.fromString(raw, 5); | ||||
RevCommit[] pList = new RevCommit[1]; | RevCommit[] pList = new RevCommit[1]; | ||||
int nParents = 0; | int nParents = 0; | ||||
for (;;) { | for (;;) { | ||||
if (raw[ptr] != 'p') | |||||
if (raw[ptr] != 'p') { | |||||
break; | break; | ||||
} | |||||
idBuffer.fromString(raw, ptr + 7); | idBuffer.fromString(raw, ptr + 7); | ||||
final RevCommit p = walk.lookupCommit(idBuffer); | final RevCommit p = walk.lookupCommit(idBuffer); | ||||
if (nParents == 0) | |||||
if (nParents == 0) { | |||||
pList[nParents++] = p; | pList[nParents++] = p; | ||||
else if (nParents == 1) { | |||||
} else if (nParents == 1) { | |||||
pList = new RevCommit[] { pList[0], p }; | pList = new RevCommit[] { pList[0], p }; | ||||
nParents = 2; | nParents = 2; | ||||
} else { | } else { | ||||
commitTime = RawParseUtils.parseBase10(raw, ptr, null); | commitTime = RawParseUtils.parseBase10(raw, ptr, null); | ||||
} | } | ||||
if (walk.isRetainBody()) | |||||
if (walk.isRetainBody()) { | |||||
buffer = raw; | buffer = raw; | ||||
} | |||||
flags |= PARSED; | flags |= PARSED; | ||||
} | } | ||||
lookupCommit(id).parents = RevCommit.NO_PARENTS; | lookupCommit(id).parents = RevCommit.NO_PARENTS; | ||||
} | } | ||||
void initializeShallowCommits() throws IOException { | |||||
if (shallowCommitsInitialized) | |||||
/** | |||||
* Reads the "shallow" file and applies it by setting the parents of shallow | |||||
* commits to an empty array. | |||||
* <p> | |||||
* There is a sequencing problem if the first commit being parsed is a | |||||
* shallow commit, since {@link RevCommit#parseCanonical(RevWalk, byte[])} | |||||
* calls this method before its callers add the new commit to the | |||||
* {@link RevWalk#objects} map. That means a call from this method to | |||||
* {@link #lookupCommit(AnyObjectId)} fails to find that commit and creates | |||||
* a new one, which is promptly discarded. | |||||
* <p> | |||||
* To avoid that, {@link RevCommit#parseCanonical(RevWalk, byte[])} passes | |||||
* its commit to this method, so that this method can apply the shallow | |||||
* state to it directly and avoid creating the duplicate commit object. | |||||
* | |||||
* @param rc | |||||
* the initial commit being parsed | |||||
* @throws IOException | |||||
* if the shallow commits file can't be read | |||||
*/ | |||||
void initializeShallowCommits(RevCommit rc) throws IOException { | |||||
if (shallowCommitsInitialized) { | |||||
throw new IllegalStateException( | throw new IllegalStateException( | ||||
JGitText.get().shallowCommitsAlreadyInitialized); | JGitText.get().shallowCommitsAlreadyInitialized); | ||||
} | |||||
shallowCommitsInitialized = true; | shallowCommitsInitialized = true; | ||||
if (reader == null) | |||||
if (reader == null) { | |||||
return; | return; | ||||
} | |||||
for (ObjectId id : reader.getShallowCommits()) | |||||
lookupCommit(id).parents = RevCommit.NO_PARENTS; | |||||
for (ObjectId id : reader.getShallowCommits()) { | |||||
if (id.equals(rc.getId())) { | |||||
rc.parents = RevCommit.NO_PARENTS; | |||||
} else { | |||||
lookupCommit(id).parents = RevCommit.NO_PARENTS; | |||||
} | |||||
} | |||||
} | } | ||||
} | } |