Throwing ClassCastException here for non-commits is really difficult to work with at the caller level because we may catch the wrong sort of ClassCastException and may mask a bug deep inside of RevWalk's parsing code. It is cleaner to throw IncorrectObjectTypeException and catch that. Besides, the method javadoc says that is what gets thrown if either method is given the wrong type. Signed-off-by: Shawn O. Pearce <spearce@xxxxxxxxxxx> --- .../jgit/pgm/opt/AbstractTreeIteratorHandler.java | 2 -- .../org/spearce/jgit/pgm/opt/RevCommitHandler.java | 2 -- .../org/spearce/jgit/pgm/opt/RevTreeHandler.java | 2 -- .../src/org/spearce/jgit/revwalk/RevWalk.java | 7 ++++++- .../jgit/transport/BasePackFetchConnection.java | 2 -- 5 files changed, 6 insertions(+), 9 deletions(-) diff --git a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/AbstractTreeIteratorHandler.java b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/AbstractTreeIteratorHandler.java index e439c87..9432d5f 100644 --- a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/AbstractTreeIteratorHandler.java +++ b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/AbstractTreeIteratorHandler.java @@ -112,8 +112,6 @@ public int parseArguments(final Parameters params) throws CmdLineException { final CanonicalTreeParser p = new CanonicalTreeParser(); try { p.reset(clp.getRepository(), clp.getRevWalk().parseTree(id)); - } catch (ClassCastException e) { - throw new CmdLineException(name + " is not a tree"); } catch (MissingObjectException e) { throw new CmdLineException(name + " is not a tree"); } catch (IncorrectObjectTypeException e) { diff --git a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/RevCommitHandler.java b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/RevCommitHandler.java index 772a8d7..5bfc61e 100644 --- a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/RevCommitHandler.java +++ b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/RevCommitHandler.java @@ -115,8 +115,6 @@ private void addOne(final String name, final boolean interesting) final RevCommit c; try { c = clp.getRevWalk().parseCommit(id); - } catch (ClassCastException e) { - throw new CmdLineException(name + " is not a commit"); } catch (MissingObjectException e) { throw new CmdLineException(name + " is not a commit"); } catch (IncorrectObjectTypeException e) { diff --git a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/RevTreeHandler.java b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/RevTreeHandler.java index de64336..0a0c5d2 100644 --- a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/RevTreeHandler.java +++ b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/RevTreeHandler.java @@ -88,8 +88,6 @@ public int parseArguments(final Parameters params) throws CmdLineException { final RevTree c; try { c = clp.getRevWalk().parseTree(id); - } catch (ClassCastException e) { - throw new CmdLineException(name + " is not a tree"); } catch (MissingObjectException e) { throw new CmdLineException(name + " is not a tree"); } catch (IncorrectObjectTypeException e) { diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java index c90f2a3..243d9b3 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java +++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java @@ -608,6 +608,9 @@ public RevCommit parseCommit(final AnyObjectId id) c = ((RevTag) c).getObject(); parse(c); } + if (!(c instanceof RevCommit)) + throw new IncorrectObjectTypeException(id.toObjectId(), + Constants.TYPE_COMMIT); return (RevCommit) c; } @@ -639,7 +642,9 @@ public RevTree parseTree(final AnyObjectId id) if (c instanceof RevCommit) { c = ((RevCommit) c).getTree(); parse(c); - } + } else if (!(c instanceof RevTree)) + throw new IncorrectObjectTypeException(id.toObjectId(), + Constants.TYPE_TREE); final RevTree t = (RevTree) c; if (db.openObject(t).getType() != Constants.OBJ_TREE) throw new IncorrectObjectTypeException(t, Constants.TYPE_TREE); diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java index 39fa761..a22b33d 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java @@ -197,8 +197,6 @@ private void markReachable(final int maxTime) throws IOException { reachableCommits.add(o); } catch (IOException readError) { // If we cannot read the value of the ref skip it. - } catch (ClassCastException cce) { - // Not a commit type. } } -- 1.6.0.1.319.g9f32b -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html