RevCommit: Remove RevCommitCG and move Commit Graph info to RevCommit * The graphPosition and generation fields were moved to RevCommit and the package-private RevCommitCG has been removed. * A new constructor allows creating RevCommit instances that know their position in the commit graph. * RevCommit.parseCanonical() will now parse the data from the commit graph, if the commit graph position is known. * RevCommit's implementations of getGeneration() and getChangedPathFilter() now also no longer return default values, if the position within the commit graph is known. This way, sub-types of RevCommit and RevWalk must only override the createCommit(AnyObjectId, int) method instead and add a new constructor. Existing sub-types of RevCommit within jgit have been extended with the new constructor. The respective RevWalk implementations within jgit have been adjusted to call these new constructors. Bug: jgit-250 Change-Id: I568df510784672c2c2b452b2030a916064ea173a
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkCommitGraphTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkCommitGraphTest.java index e47dd89..c390405 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkCommitGraphTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkCommitGraphTest.java
@@ -60,7 +60,6 @@ public void testParseHeaders() throws Exception { RevCommit notParseInGraph = rw.lookupCommit(c1); rw.parseHeaders(notParseInGraph); - assertFalse(notParseInGraph instanceof RevCommitCG); assertNotNull(notParseInGraph.getRawBuffer()); assertEquals(Constants.COMMIT_GENERATION_UNKNOWN, notParseInGraph.getGeneration()); @@ -71,7 +70,6 @@ public void testParseHeaders() throws Exception { RevCommit parseInGraph = rw.lookupCommit(c1); parseInGraph.parseHeaders(rw); - assertTrue(parseInGraph instanceof RevCommitCG); assertNotNull(parseInGraph.getRawBuffer()); assertEquals(1, parseInGraph.getGeneration()); assertEquals(notParseInGraph.getId(), parseInGraph.getId()); @@ -84,7 +82,6 @@ public void testParseHeaders() throws Exception { RevCommit noBody = rw.lookupCommit(c1); noBody.parseHeaders(rw); - assertTrue(noBody instanceof RevCommitCG); assertNull(noBody.getRawBuffer()); assertEquals(1, noBody.getGeneration()); assertEquals(notParseInGraph.getId(), noBody.getId()); @@ -105,7 +102,6 @@ public void testParseCanonical() throws Exception { RevCommit parseInGraph = rw.lookupCommit(c1); parseInGraph.parseCanonical(rw, rw.getCachedBytes(c1)); - assertTrue(parseInGraph instanceof RevCommitCG); assertNotNull(parseInGraph.getRawBuffer()); assertEquals(1, parseInGraph.getGeneration()); assertEquals(notParseInGraph.getId(), parseInGraph.getId()); @@ -120,7 +116,6 @@ public void testParseCanonical() throws Exception { RevCommit noBody = rw.lookupCommit(c1); noBody.parseCanonical(rw, rw.getCachedBytes(c1)); - assertTrue(noBody instanceof RevCommitCG); assertNull(noBody.getRawBuffer()); assertEquals(1, noBody.getGeneration()); assertEquals(notParseInGraph.getId(), noBody.getId()); @@ -140,7 +135,6 @@ public void testInitializeShallowCommits() throws Exception { RevCommit parseInGraph = rw.lookupCommit(c1); parseInGraph.parseHeaders(rw); - assertTrue(parseInGraph instanceof RevCommitCG); assertNotNull(parseInGraph.getRawBuffer()); assertEquals(2, parseInGraph.getGeneration()); assertEquals(0, parseInGraph.getParentCount());
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/NameRevCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/NameRevCommand.java index f72a451..83c063b 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/NameRevCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/NameRevCommand.java
@@ -51,8 +51,8 @@ private static class NameRevCommit extends RevCommit { private int distance; private long cost; - private NameRevCommit(AnyObjectId id) { - super(id); + private NameRevCommit(AnyObjectId id, int graphPosition) { + super(id, graphPosition); } private StringBuilder format() { @@ -97,7 +97,7 @@ protected NameRevCommand(Repository repo) { walk = new RevWalk(repo) { @Override protected RevCommit createCommit(AnyObjectId id, int graphPos) { - return new NameRevCommit(id); + return new NameRevCommit(id, graphPos); } }; }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/blame/ReverseWalk.java b/org.eclipse.jgit/src/org/eclipse/jgit/blame/ReverseWalk.java index 582313f..8d534e9 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/blame/ReverseWalk.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/blame/ReverseWalk.java
@@ -37,7 +37,7 @@ public ReverseCommit next() throws MissingObjectException, @Override protected RevCommit createCommit(AnyObjectId id, int graphPos) { - return new ReverseCommit(id); + return new ReverseCommit(id, graphPos); } static final class ReverseCommit extends RevCommit { @@ -45,8 +45,8 @@ static final class ReverseCommit extends RevCommit { private ReverseCommit[] children = NO_CHILDREN; - ReverseCommit(AnyObjectId id) { - super(id); + ReverseCommit(AnyObjectId id, int graphPosition) { + super(id, graphPosition); } void addChild(ReverseCommit c) {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revplot/PlotCommit.java b/org.eclipse.jgit/src/org/eclipse/jgit/revplot/PlotCommit.java index fa109e8..d301b47 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revplot/PlotCommit.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revplot/PlotCommit.java
@@ -47,7 +47,20 @@ public class PlotCommit<L extends PlotLane> extends RevCommit { * the identity of this commit. */ protected PlotCommit(AnyObjectId id) { - super(id); + this(id, -1); + } + + /** + * Create a new commit. + * + * @param id + * the identity of this commit. + * @param graphPosition + * the position of this commit in the commit graph + * @since 7.8 + */ + protected PlotCommit(AnyObjectId id, int graphPosition) { + super(id, graphPosition); forkingOffLanes = NO_LANES; passingLanes = NO_LANES; mergingLanes = NO_LANES;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revplot/PlotWalk.java b/org.eclipse.jgit/src/org/eclipse/jgit/revplot/PlotWalk.java index 99022f0..88c2d23 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revplot/PlotWalk.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revplot/PlotWalk.java
@@ -106,7 +106,7 @@ public void sort(RevSort s, boolean use) { @Override protected RevCommit createCommit(AnyObjectId id, int graphPos) { - return new PlotCommit(id); + return new PlotCommit(id, graphPos); } @Override
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/DepthWalk.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/DepthWalk.java index 442087d..cd0bd23 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/DepthWalk.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/DepthWalk.java
@@ -125,9 +125,23 @@ public boolean isBoundary() { * object name for the commit. */ protected Commit(AnyObjectId id) { - super(id); + this(id, -1); + } + + /** + * Initialize a new commit. + * + * @param id + * object name for the commit. + * @param graphPosition + * the position of this commit in the commit graph + * @since 7.8 + */ + protected Commit(AnyObjectId id, int graphPosition) { + super(id, graphPosition); depth = -1; } + } /** Subclass of RevWalk that performs depth filtering. */ @@ -197,7 +211,7 @@ public void markRoot(RevCommit c) throws MissingObjectException, @Override protected RevCommit createCommit(AnyObjectId id, int graphPos) { - return new Commit(id); + return new Commit(id, graphPos); } @Override @@ -368,7 +382,7 @@ public void markUnshallow(RevObject c) throws MissingObjectException, @Override protected RevCommit createCommit(AnyObjectId id, int graphPos) { - return new Commit(id); + return new Commit(id, graphPos); } @Override
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java index 871545f..3248fe1 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java
@@ -23,9 +23,11 @@ import org.eclipse.jgit.errors.IncorrectObjectTypeException; import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.internal.storage.commitgraph.ChangedPathFilter; +import org.eclipse.jgit.internal.storage.commitgraph.CommitGraph; import org.eclipse.jgit.lib.AnyObjectId; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.MutableObjectId; +import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectInserter; import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.PersonIdent; @@ -133,6 +135,10 @@ public static RevCommit parse(RevWalk rw, byte[] raw) throws IOException { */ protected byte[] buffer; + private final int graphPosition; + + private int generation = Constants.COMMIT_GENERATION_UNKNOWN; + /** * Create a new commit reference. * @@ -140,17 +146,31 @@ public static RevCommit parse(RevWalk rw, byte[] raw) throws IOException { * object name for the commit. */ protected RevCommit(AnyObjectId id) { + this(id, -1); + } + + /** + * Create a new commit reference. + * + * @param id + * object name for the commit. + * @param graphPosition + * the position of this commit in the commit graph + * @since 7.8 + */ + protected RevCommit(AnyObjectId id, int graphPosition) { super(id); + this.graphPosition = graphPosition; } @Override - void parseHeaders(RevWalk walk) throws MissingObjectException, + final void parseHeaders(RevWalk walk) throws MissingObjectException, IncorrectObjectTypeException, IOException { parseCanonical(walk, walk.getCachedBytes(this)); } @Override - void parseBody(RevWalk walk) throws MissingObjectException, + final void parseBody(RevWalk walk) throws MissingObjectException, IncorrectObjectTypeException, IOException { if (buffer == null) { buffer = walk.getCachedBytes(this); @@ -159,11 +179,25 @@ void parseBody(RevWalk walk) throws MissingObjectException, } } - void parseCanonical(RevWalk walk, byte[] raw) throws IOException { + final void parseCanonical(RevWalk walk, byte[] raw) throws IOException { if (!walk.shallowCommitsInitialized) { walk.initializeShallowCommits(this); } + if (graphPosition >= 0) { + if (walk.isRetainBody()) { + buffer = raw; + } + + parseInGraph(walk); + } else { + parseFromObject(walk, raw); + } + + flags |= PARSED; + } + + final void parseFromObject(RevWalk walk, byte[] raw) { final MutableObjectId idBuffer = walk.idBuffer; idBuffer.fromString(raw, 5); tree = walk.lookupTree(idBuffer); @@ -217,7 +251,36 @@ void parseCanonical(RevWalk walk, byte[] raw) throws IOException { if (walk.isRetainBody()) { buffer = raw; } - flags |= PARSED; + } + + final void parseInGraph(RevWalk walk) { + CommitGraph graph = walk.commitGraph(); + CommitGraph.CommitData data = graph.getCommitData(graphPosition); + if (data == null) { + // parseInGraph was called because we know this commit's + // position in the commit graph. If now the commit-graph doesn't + // know about it, something went wrong. + throw new IllegalStateException(); + } + + this.tree = walk.lookupTree(data.getTree()); + this.commitTime = (int) data.getCommitTime(); + this.generation = data.getGeneration(); + + if (getParents() == null) { + int[] pGraphList = data.getParents(); + if (pGraphList.length == 0) { + this.parents = RevCommit.NO_PARENTS; + } else { + RevCommit[] pList = new RevCommit[pGraphList.length]; + for (int i = 0; i < pList.length; i++) { + int graphPos = pGraphList[i]; + ObjectId objId = graph.getObjectId(graphPos); + pList[i] = walk.lookupCommit(objId, graphPos); + } + this.parents = pList; + } + } } @Override @@ -655,8 +718,8 @@ public final List<String> getFooterLines(FooterKey key) { * @return the generation number * @since 6.5 */ - int getGeneration() { - return Constants.COMMIT_GENERATION_UNKNOWN; + final int getGeneration() { + return generation; } /** @@ -671,6 +734,10 @@ int getGeneration() { * @since 6.7 */ public ChangedPathFilter getChangedPathFilter(RevWalk rw) { + if (graphPosition >= 0) { + return rw.commitGraph().getChangedPathFilter(graphPosition); + } + return null; }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommitCG.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommitCG.java deleted file mode 100644 index c7a0399..0000000 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommitCG.java +++ /dev/null
@@ -1,110 +0,0 @@ -/* - * Copyright (C) 2023, Tencent. - * - * 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 java.io.IOException; - -import org.eclipse.jgit.errors.IncorrectObjectTypeException; -import org.eclipse.jgit.errors.MissingObjectException; -import org.eclipse.jgit.internal.storage.commitgraph.ChangedPathFilter; -import org.eclipse.jgit.internal.storage.commitgraph.CommitGraph; -import org.eclipse.jgit.lib.AnyObjectId; -import org.eclipse.jgit.lib.Constants; -import org.eclipse.jgit.lib.ObjectId; - -/** - * RevCommit parsed from - * {@link org.eclipse.jgit.internal.storage.commitgraph.CommitGraph}. - * - * @since 6.5 - */ -class RevCommitCG extends RevCommit { - - private final int graphPosition; - - private int generation = Constants.COMMIT_GENERATION_UNKNOWN; - - /** - * Create a new commit reference. - * - * @param id - * object name for the commit. - * @param graphPosition - * the position in the commit-graph of the object. - */ - protected RevCommitCG(AnyObjectId id, int graphPosition) { - super(id); - this.graphPosition = graphPosition; - } - - @Override - void parseCanonical(RevWalk walk, byte[] raw) throws IOException { - if (walk.isRetainBody()) { - buffer = raw; - } - parseInGraph(walk); - } - - @Override - void parseHeaders(RevWalk walk) throws MissingObjectException, - IncorrectObjectTypeException, IOException { - if (walk.isRetainBody()) { - super.parseBody(walk); // This parses header and body - return; - } - parseInGraph(walk); - } - - private void parseInGraph(RevWalk walk) throws IOException { - CommitGraph graph = walk.commitGraph(); - CommitGraph.CommitData data = graph.getCommitData(graphPosition); - if (data == null) { - // RevCommitCG was created because we got its graphPosition from - // commit-graph. If now the commit-graph doesn't know about it, - // something went wrong. - throw new IllegalStateException(); - } - if (!walk.shallowCommitsInitialized) { - walk.initializeShallowCommits(this); - } - - this.tree = walk.lookupTree(data.getTree()); - this.commitTime = (int) data.getCommitTime(); - this.generation = data.getGeneration(); - - if (getParents() == null) { - int[] pGraphList = data.getParents(); - if (pGraphList.length == 0) { - this.parents = RevCommit.NO_PARENTS; - } else { - RevCommit[] pList = new RevCommit[pGraphList.length]; - for (int i = 0; i < pList.length; i++) { - int graphPos = pGraphList[i]; - ObjectId objId = graph.getObjectId(graphPos); - pList[i] = walk.lookupCommit(objId, graphPos); - } - this.parents = pList; - } - } - flags |= PARSED; - } - - @Override - int getGeneration() { - return generation; - } - - /** {@inheritDoc} */ - @Override - public ChangedPathFilter getChangedPathFilter(RevWalk rw) { - return rw.commitGraph().getChangedPathFilter(graphPosition); - } -}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java index 4f558bc..4b28045 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java
@@ -971,10 +971,10 @@ public RevCommit lookupCommit(AnyObjectId id) { } /** - * This method is intended to be invoked only by {@link RevCommitCG}, in - * order to give commit the correct graphPosition before accessing the - * commit-graph. In this way, the headers of the commit can be obtained in - * constant time. + * This method is intended to be invoked only by + * {@link RevCommit#parseInGraph(RevWalk)}, in order to give commit the + * correct graphPosition before accessing the commit-graph. In this way, the + * headers of the commit can be obtained in constant time. * * @param id * name of the commit object. @@ -1785,10 +1785,7 @@ protected RevCommit createCommit(AnyObjectId id) { * @since 7.8 */ protected RevCommit createCommit(AnyObjectId id, int graphPos) { - if (graphPos >= 0) { - return new RevCommitCG(id, graphPos); - } - return new RevCommit(id); + return new RevCommit(id, graphPos); } void carryFlagsImpl(RevCommit c) {