[JGIT PATCH (RESEND) 2/3] Work around Sun javac compiler error in RefUpdate
- From
Shawn O. Pearce <spearce@spearce.org>
- Date
- Sep 1, 2009, 23:16 UTC
- Message-ID
- <1251847010-9992-2-git-send-email-spearce@spearce.org>
- In-Reply-To
- <1251847010-9992-1-git-send-email-spearce@spearce.org>
Sun's javac, version 5 and 6, apparently miscompiles the for loop which is looking for a conflicting ref name in the existing set of refs for this repository.
Debugging this code showed the control flow to return LOCK_FAILURE when startsWith returned false, which is highly illogical and the exact opposite of what we have written here.
Sun's javap tool was unable to disassemble the compiled method. Instead it simply failed to produce anything about updateImpl. So my remark about the code being compiled wrong is only a guess based on how I observed the behavior, and not by actually studying the resulting instructions.
Eclipse's JDT appears to have compiled the updateImpl method correctly, and produces a working executable. But this is a much less common compiler to build Java libraries with.
This refactoring to extract the name conflicting test out into its own method appears to work around the Sun javac bug, and the resulting class works correctly with either compiler. The code is also more clear, so its a gain either way.
Signed-off-by: Shawn O. Pearce <sop@google.com> Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../src/org/spearce/jgit/lib/RefUpdate.java | 26 +++++++++++++------- 1 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java index 8dffed2..8226e10 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java @@ -449,15 +449,8 @@ private Result updateImpl(final RevWalk walk, final Store store) RevObject newObj; RevObject oldObj; - int lastSlash = getName().lastIndexOf('/'); - if (lastSlash > 0) - if (db.getRepository().getRef(getName().substring(0, lastSlash)) != null) - return Result.LOCK_FAILURE; - String rName = getName() + "/"; - for (Ref r : db.getAllRefs().values()) { - if (r.getName().startsWith(rName)) - return Result.LOCK_FAILURE; - } + if (isNameConflicting()) + return Result.LOCK_FAILURE; lock = new LockFile(looseFile); if (!lock.lock()) return Result.LOCK_FAILURE; @@ -490,6 +483,21 @@ private Result updateImpl(final RevWalk walk, final Store store) } } + private boolean isNameConflicting() throws IOException { + final String myName = getName(); + final int lastSlash = myName.lastIndexOf('/'); + if (lastSlash > 0) + if (db.getRepository().getRef(myName.substring(0, lastSlash)) != null) + return true; + + final String rName = myName + "/"; + for (Ref r : db.getAllRefs().values()) { + if (r.getName().startsWith(rName)) + return true; + } + return false; + } + private static RevObject safeParse(final RevWalk rw, final AnyObjectId id) throws IOException { try {
-- 1.6.4.1.341.gf2a44