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@xxxxxxxxxx>
Signed-off-by: Shawn O. Pearce <spearce@xxxxxxxxxxx>
---
.../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
--
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