ObjectWalk: make setRetainBody(false) the default

Despite being the primary author of RevWalk and ObjectWalk I still
fail to remember to setRetainBody(false) in application code using
an ObjectWalk to examine the graph.

Document the default for RevWalk is setRetainBody(true), where the
application usually wants the commit bodies to display or inspect.

Change the default for ObjectWalk to setRetainBody(false), as nearly
all callers want only the graph shape and do not need the larger text
inside a commit body. This allows some code in JGit to be simplified.

Change-Id: I367e42209e805bd5e1f41b4072aeb2fa98ec9d99
This commit is contained in:
Shawn Pearce 2015-05-09 10:47:13 -07:00
parent 3d06349ff2
commit 53e39094bf
10 changed files with 44 additions and 36 deletions

View File

@ -360,10 +360,10 @@ private RevCommit parse(final String msg) throws IOException {
buf.append("\n"); buf.append("\n");
buf.append(msg); buf.append(msg);
final RevWalk walk = new RevWalk(db); try (RevWalk walk = new RevWalk(db)) {
walk.setRetainBody(true); RevCommit c = new RevCommit(ObjectId.zeroId());
final RevCommit c = new RevCommit(ObjectId.zeroId()); c.parseCanonical(walk, Constants.encode(buf.toString()));
c.parseCanonical(walk, Constants.encode(buf.toString())); return c;
return c; }
} }
} }

View File

@ -188,7 +188,6 @@ public StashCreateCommand setIncludeUntracked(boolean includeUntracked) {
private RevCommit parseCommit(final ObjectReader reader, private RevCommit parseCommit(final ObjectReader reader,
final ObjectId headId) throws IOException { final ObjectId headId) throws IOException {
final RevWalk walk = new RevWalk(reader); final RevWalk walk = new RevWalk(reader);
walk.setRetainBody(true);
return walk.parseCommit(headId); return walk.parseCommit(headId);
} }

View File

@ -96,10 +96,8 @@ public Collection<RevCommit> call() throws GitAPIException,
final List<RevCommit> stashCommits = new ArrayList<RevCommit>( final List<RevCommit> stashCommits = new ArrayList<RevCommit>(
stashEntries.size()); stashEntries.size());
final RevWalk walk = new RevWalk(repo); try (RevWalk walk = new RevWalk(repo)) {
walk.setRetainBody(true); for (ReflogEntry entry : stashEntries) {
try {
for (ReflogEntry entry : stashEntries)
try { try {
stashCommits.add(walk.parseCommit(entry.getNewId())); stashCommits.add(walk.parseCommit(entry.getNewId()));
} catch (IOException e) { } catch (IOException e) {
@ -107,8 +105,7 @@ public Collection<RevCommit> call() throws GitAPIException,
JGitText.get().cannotReadCommit, entry.getNewId()), JGitText.get().cannotReadCommit, entry.getNewId()),
e); e);
} }
} finally { }
walk.dispose();
} }
return stashCommits; return stashCommits;
} }

View File

@ -179,7 +179,6 @@ private void initRevPool(boolean reverse) {
else else
revPool = new RevWalk(getRepository()); revPool = new RevWalk(getRepository());
revPool.setRetainBody(true);
SEEN = revPool.newFlag("SEEN"); //$NON-NLS-1$ SEEN = revPool.newFlag("SEEN"); //$NON-NLS-1$
reader = revPool.getObjectReader(); reader = revPool.getObjectReader();
treeWalk = new TreeWalk(reader); treeWalk = new TreeWalk(reader);

View File

@ -1587,8 +1587,6 @@ private void findObjectsToPack(final ProgressMonitor countingMonitor,
stats.interestingObjects = Collections.unmodifiableSet(new HashSet<ObjectId>(want)); stats.interestingObjects = Collections.unmodifiableSet(new HashSet<ObjectId>(want));
stats.uninterestingObjects = Collections.unmodifiableSet(new HashSet<ObjectId>(have)); stats.uninterestingObjects = Collections.unmodifiableSet(new HashSet<ObjectId>(have));
walker.setRetainBody(false);
canBuildBitmaps = config.isBuildBitmaps() canBuildBitmaps = config.isBuildBitmaps()
&& !shallowPack && !shallowPack
&& have.isEmpty() && have.isEmpty()

View File

@ -133,6 +133,7 @@ public ObjectWalk(final Repository repo) {
*/ */
public ObjectWalk(ObjectReader or) { public ObjectWalk(ObjectReader or) {
super(or); super(or);
setRetainBody(false);
rootObjects = new ArrayList<RevObject>(); rootObjects = new ArrayList<RevObject>();
pendingObjects = new BlockObjQueue(); pendingObjects = new BlockObjQueue();
pathBuf = new byte[256]; pathBuf = new byte[256];

View File

@ -92,29 +92,34 @@ public static RevCommit parse(byte[] raw) {
/** /**
* Parse a commit from its canonical format. * Parse a commit from its canonical format.
* * <p>
* This method inserts the commit directly into the caller supplied revision * This method inserts the commit directly into the caller supplied revision
* pool, making it appear as though the commit exists in the repository, * pool, making it appear as though the commit exists in the repository,
* even if it doesn't. The repository under the pool is not affected. * even if it doesn't. The repository under the pool is not affected.
* <p>
* The body of the commit (message, author, committer) is always retained in
* the returned {@code RevCommit}, even if the supplied {@code RevWalk} has
* been configured with {@code setRetainBody(false)}.
* *
* @param rw * @param rw
* the revision pool to allocate the commit within. The commit's * the revision pool to allocate the commit within. The commit's
* tree and parent pointers will be obtained from this pool. * tree and parent pointers will be obtained from this pool.
* @param raw * @param raw
* the canonical formatted commit to be parsed. * the canonical formatted commit to be parsed. This buffer will
* be retained by the returned {@code RevCommit} and must not be
* modified by the caller.
* @return the parsed commit, in an isolated revision pool that is not * @return the parsed commit, in an isolated revision pool that is not
* available to the caller. * available to the caller.
* @throws IOException * @throws IOException
* in case of RevWalk initialization fails * in case of RevWalk initialization fails
*/ */
public static RevCommit parse(RevWalk rw, byte[] raw) throws IOException { public static RevCommit parse(RevWalk rw, byte[] raw) throws IOException {
ObjectInserter.Formatter fmt = new ObjectInserter.Formatter(); try (ObjectInserter.Formatter fmt = new ObjectInserter.Formatter()) {
boolean retain = rw.isRetainBody(); RevCommit r = rw.lookupCommit(fmt.idFor(Constants.OBJ_COMMIT, raw));
rw.setRetainBody(true); r.parseCanonical(rw, raw);
RevCommit r = rw.lookupCommit(fmt.idFor(Constants.OBJ_COMMIT, raw)); r.buffer = raw;
r.parseCanonical(rw, raw); return r;
rw.setRetainBody(retain); }
return r;
} }
static final RevCommit[] NO_PARENTS = {}; static final RevCommit[] NO_PARENTS = {};

View File

@ -87,16 +87,22 @@ public static RevTag parse(byte[] raw) throws CorruptObjectException {
/** /**
* Parse an annotated tag from its canonical format. * Parse an annotated tag from its canonical format.
* * <p>
* This method inserts the tag directly into the caller supplied revision * This method inserts the tag directly into the caller supplied revision
* pool, making it appear as though the tag exists in the repository, even * pool, making it appear as though the tag exists in the repository, even
* if it doesn't. The repository under the pool is not affected. * if it doesn't. The repository under the pool is not affected.
* <p>
* The body of the tag (message, tagger, signature) is always retained in
* the returned {@code RevTag}, even if the supplied {@code RevWalk} has
* been configured with {@code setRetainBody(false)}.
* *
* @param rw * @param rw
* the revision pool to allocate the tag within. The tag's object * the revision pool to allocate the tag within. The tag's object
* pointer will be obtained from this pool. * pointer will be obtained from this pool.
* @param raw * @param raw
* the canonical formatted tag to be parsed. * the canonical formatted tag to be parsed. This buffer will be
* retained by the returned {@code RevTag} and must not be
* modified by the caller.
* @return the parsed tag, in an isolated revision pool that is not * @return the parsed tag, in an isolated revision pool that is not
* available to the caller. * available to the caller.
* @throws CorruptObjectException * @throws CorruptObjectException
@ -104,13 +110,12 @@ public static RevTag parse(byte[] raw) throws CorruptObjectException {
*/ */
public static RevTag parse(RevWalk rw, byte[] raw) public static RevTag parse(RevWalk rw, byte[] raw)
throws CorruptObjectException { throws CorruptObjectException {
ObjectInserter.Formatter fmt = new ObjectInserter.Formatter(); try (ObjectInserter.Formatter fmt = new ObjectInserter.Formatter()) {
boolean retain = rw.isRetainBody(); RevTag r = rw.lookupTag(fmt.idFor(Constants.OBJ_TAG, raw));
rw.setRetainBody(true); r.parseCanonical(rw, raw);
RevTag r = rw.lookupTag(fmt.idFor(Constants.OBJ_TAG, raw)); r.buffer = raw;
r.parseCanonical(rw, raw); return r;
rw.setRetainBody(retain); }
return r;
} }
private RevObject object; private RevObject object;

View File

@ -192,7 +192,7 @@ public class RevWalk implements Iterable<RevCommit>, AutoCloseable {
private TreeFilter treeFilter; private TreeFilter treeFilter;
private boolean retainBody; private boolean retainBody = true;
private boolean rewriteParents = true; private boolean rewriteParents = true;
@ -233,7 +233,6 @@ private RevWalk(ObjectReader or, boolean closeReader) {
sorting = EnumSet.of(RevSort.NONE); sorting = EnumSet.of(RevSort.NONE);
filter = RevFilter.ALL; filter = RevFilter.ALL;
treeFilter = TreeFilter.ALL; treeFilter = TreeFilter.ALL;
retainBody = true;
this.closeReader = closeReader; this.closeReader = closeReader;
} }
@ -607,6 +606,9 @@ boolean getRewriteParents() {
* Usually the body is always retained, but some application code might not * Usually the body is always retained, but some application code might not
* care and would prefer to discard the body of a commit as early as * care and would prefer to discard the body of a commit as early as
* possible, to reduce memory usage. * possible, to reduce memory usage.
* <p>
* True by default on {@link RevWalk} and false by default for
* {@link ObjectWalk}.
* *
* @return true if the body should be retained; false it is discarded. * @return true if the body should be retained; false it is discarded.
*/ */
@ -620,6 +622,9 @@ public boolean isRetainBody() {
* If a body of a commit or tag is not retained, the application must * If a body of a commit or tag is not retained, the application must
* call {@link #parseBody(RevObject)} before the body can be safely * call {@link #parseBody(RevObject)} before the body can be safely
* accessed through the type specific access methods. * accessed through the type specific access methods.
* <p>
* True by default on {@link RevWalk} and false by default for
* {@link ObjectWalk}.
* *
* @param retain true to retain bodies; false to discard them early. * @param retain true to retain bodies; false to discard them early.
*/ */

View File

@ -1143,7 +1143,6 @@ private void checkConnectivity() throws IOException {
parser = null; parser = null;
try (final ObjectWalk ow = new ObjectWalk(db)) { try (final ObjectWalk ow = new ObjectWalk(db)) {
ow.setRetainBody(false);
if (baseObjects != null) { if (baseObjects != null) {
ow.sort(RevSort.TOPO); ow.sort(RevSort.TOPO);
if (!baseObjects.isEmpty()) if (!baseObjects.isEmpty())