{"thread":{"id":"20817","subject":"[JGIT PATCH (RESEND) 1/3] Allow RefUpdate.setExpectedOldObjectId to accept RevCommit","startedAt":"2009-09-01T23:16:48Z","lastAt":"2009-09-04T00:06:39Z","messageCount":7,"participants":["Shawn O. Pearce","Robin Rosenberg","Nicolas Pitre"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"122269","messageId":"1251847010-9992-1-git-send-email-spearce@spearce.org","threadId":"20817","inReplyTo":null,"subject":"[JGIT PATCH (RESEND) 1/3] Allow RefUpdate.setExpectedOldObjectId to accept RevCommit","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-09-01T23:16:48Z","receivedAt":"2009-09-01T23:16:48Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"RevCommit overrides .equals() such that it only implements a\nreference equality test.  If the expected old ObjectId was set\nby the application to a RevCommit instance, it would always fail,\nresulting in LOCK_FAILURE.  Instead use AnyObject.equals() to compare\nthe value, ignoring the possibly overloaded equals in RevCommit.\n\nSigned-off-by: Shawn O. Pearce <sop@google.com>\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n .../tst/org/spearce/jgit/lib/RefUpdateTest.java    |   52 ++++++++++++++++++++\n .../src/org/spearce/jgit/lib/RefUpdate.java        |    2 +-\n 2 files changed, 53 insertions(+), 1 deletions(-)\n\ndiff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java\nindex 800c0a4..a8ccf43 100644\n--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java\n+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java\n@@ -45,6 +45,7 @@\n \n import org.spearce.jgit.lib.RefUpdate.Result;\n import org.spearce.jgit.revwalk.RevCommit;\n+import org.spearce.jgit.revwalk.RevWalk;\n \n public class RefUpdateTest extends RepositoryTestCase {\n \n@@ -397,6 +398,57 @@ public void testUpdateRefLockFailureWrongOldValue() throws IOException {\n \t}\n \n \t/**\n+\t * Try modify a ref forward, fast forward, checking old value first\n+\t *\n+\t * @throws IOException\n+\t */\n+\tpublic void testUpdateRefForwardWithCheck1() throws IOException {\n+\t\tObjectId ppid = db.resolve(\"refs/heads/master^\");\n+\t\tObjectId pid = db.resolve(\"refs/heads/master\");\n+\n+\t\tRefUpdate updateRef = db.updateRef(\"refs/heads/master\");\n+\t\tupdateRef.setNewObjectId(ppid);\n+\t\tupdateRef.setForceUpdate(true);\n+\t\tResult update = updateRef.update();\n+\t\tassertEquals(Result.FORCED, update);\n+\t\tassertEquals(ppid, db.resolve(\"refs/heads/master\"));\n+\n+\t\t// real test\n+\t\tRefUpdate updateRef2 = db.updateRef(\"refs/heads/master\");\n+\t\tupdateRef2.setExpectedOldObjectId(ppid);\n+\t\tupdateRef2.setNewObjectId(pid);\n+\t\tResult update2 = updateRef2.update();\n+\t\tassertEquals(Result.FAST_FORWARD, update2);\n+\t\tassertEquals(pid, db.resolve(\"refs/heads/master\"));\n+\t}\n+\n+\t/**\n+\t * Try modify a ref forward, fast forward, checking old commit first\n+\t *\n+\t * @throws IOException\n+\t */\n+\tpublic void testUpdateRefForwardWithCheck2() throws IOException {\n+\t\tObjectId ppid = db.resolve(\"refs/heads/master^\");\n+\t\tObjectId pid = db.resolve(\"refs/heads/master\");\n+\n+\t\tRefUpdate updateRef = db.updateRef(\"refs/heads/master\");\n+\t\tupdateRef.setNewObjectId(ppid);\n+\t\tupdateRef.setForceUpdate(true);\n+\t\tResult update = updateRef.update();\n+\t\tassertEquals(Result.FORCED, update);\n+\t\tassertEquals(ppid, db.resolve(\"refs/heads/master\"));\n+\n+\t\t// real test\n+\t\tRevCommit old = new RevWalk(db).parseCommit(ppid);\n+\t\tRefUpdate updateRef2 = db.updateRef(\"refs/heads/master\");\n+\t\tupdateRef2.setExpectedOldObjectId(old);\n+\t\tupdateRef2.setNewObjectId(pid);\n+\t\tResult update2 = updateRef2.update();\n+\t\tassertEquals(Result.FAST_FORWARD, update2);\n+\t\tassertEquals(pid, db.resolve(\"refs/heads/master\"));\n+\t}\n+\n+\t/**\n \t * Try modify a ref that is locked\n \t *\n \t * @throws IOException\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java\nindex 69399ec..8dffed2 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java\n@@ -466,7 +466,7 @@ private Result updateImpl(final RevWalk walk, final Store store)\n \t\t\tif (expValue != null) {\n \t\t\t\tfinal ObjectId o;\n \t\t\t\to = oldValue != null ? oldValue : ObjectId.zeroId();\n-\t\t\t\tif (!expValue.equals(o))\n+\t\t\t\tif (!AnyObjectId.equals(expValue, o))\n \t\t\t\t\treturn Result.LOCK_FAILURE;\n \t\t\t}\n \t\t\tif (oldValue == null)\n-- \n1.6.4.1.341.gf2a44\n"},{"id":"122270","messageId":"1251847010-9992-2-git-send-email-spearce@spearce.org","threadId":"20817","inReplyTo":"1251847010-9992-1-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH (RESEND) 2/3] Work around Sun javac compiler error in RefUpdate","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-09-01T23:16:49Z","receivedAt":"2009-09-01T23:16:49Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Sun's javac, version 5 and 6, apparently miscompiles the for loop\nwhich is looking for a conflicting ref name in the existing set of\nrefs for this repository.\n\nDebugging this code showed the control flow to return LOCK_FAILURE\nwhen startsWith returned false, which is highly illogical and the\nexact opposite of what we have written here.\n\nSun's javap tool was unable to disassemble the compiled method.\nInstead it simply failed to produce anything about updateImpl.\nSo my remark about the code being compiled wrong is only a guess\nbased on how I observed the behavior, and not by actually studying\nthe resulting instructions.\n\nEclipse's JDT appears to have compiled the updateImpl method\ncorrectly, and produces a working executable.  But this is a\nmuch less common compiler to build Java libraries with.\n\nThis refactoring to extract the name conflicting test out into\nits own method appears to work around the Sun javac bug, and the\nresulting class works correctly with either compiler.  The code is\nalso more clear, so its a gain either way.\n\nSigned-off-by: Shawn O. Pearce <sop@google.com>\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n .../src/org/spearce/jgit/lib/RefUpdate.java        |   26 +++++++++++++-------\n 1 files changed, 17 insertions(+), 9 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java\nindex 8dffed2..8226e10 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java\n@@ -449,15 +449,8 @@ private Result updateImpl(final RevWalk walk, final Store store)\n \t\tRevObject newObj;\n \t\tRevObject oldObj;\n \n-\t\tint lastSlash = getName().lastIndexOf('/');\n-\t\tif (lastSlash > 0)\n-\t\t\tif (db.getRepository().getRef(getName().substring(0, lastSlash)) != null)\n-\t\t\t\treturn Result.LOCK_FAILURE;\n-\t\tString rName = getName() + \"/\";\n-\t\tfor (Ref r : db.getAllRefs().values()) {\n-\t\t\tif (r.getName().startsWith(rName))\n-\t\t\t\treturn Result.LOCK_FAILURE;\n-\t\t}\n+\t\tif (isNameConflicting())\n+\t\t\treturn Result.LOCK_FAILURE;\n \t\tlock = new LockFile(looseFile);\n \t\tif (!lock.lock())\n \t\t\treturn Result.LOCK_FAILURE;\n@@ -490,6 +483,21 @@ private Result updateImpl(final RevWalk walk, final Store store)\n \t\t}\n \t}\n \n+\tprivate boolean isNameConflicting() throws IOException {\n+\t\tfinal String myName = getName();\n+\t\tfinal int lastSlash = myName.lastIndexOf('/');\n+\t\tif (lastSlash > 0)\n+\t\t\tif (db.getRepository().getRef(myName.substring(0, lastSlash)) != null)\n+\t\t\t\treturn true;\n+\n+\t\tfinal String rName = myName + \"/\";\n+\t\tfor (Ref r : db.getAllRefs().values()) {\n+\t\t\tif (r.getName().startsWith(rName))\n+\t\t\t\treturn true;\n+\t\t}\n+\t\treturn false;\n+\t}\n+\n \tprivate static RevObject safeParse(final RevWalk rw, final AnyObjectId id)\n \t\t\tthrows IOException {\n \t\ttry {\n-- \n1.6.4.1.341.gf2a44\n"},{"id":"122271","messageId":"1251847010-9992-3-git-send-email-spearce@spearce.org","threadId":"20817","inReplyTo":"1251847010-9992-2-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH (RESEND) 3/3] Fix DirCache.findEntry to work on an empty cache","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-09-01T23:16:50Z","receivedAt":"2009-09-01T23:16:50Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"If the cache has no entries, we want to return -1 rather than throw\nArrayIndexOutOfBoundsException.  This binary search loop was stolen\nfrom some other code which contained a test before the loop to see if\nthe collection was empty or not, but we failed to include that here.\n\nFlipping the loop around to a standard while loop ensures we test\nthe condition properly first.\n\nSigned-off-by: Shawn O. Pearce <sop@google.com>\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n .../spearce/jgit/dircache/DirCacheBasicTest.java   |    6 ++++++\n .../src/org/spearce/jgit/dircache/DirCache.java    |    6 ++----\n 2 files changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheBasicTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheBasicTest.java\nindex b3097ac..4d737c0 100644\n--- a/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheBasicTest.java\n+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheBasicTest.java\n@@ -39,6 +39,7 @@\n \n import java.io.File;\n \n+import org.spearce.jgit.lib.Constants;\n import org.spearce.jgit.lib.RepositoryTestCase;\n \n public class DirCacheBasicTest extends RepositoryTestCase {\n@@ -182,4 +183,9 @@ public void testBuildThenClear() throws Exception {\n \t\tassertEquals(0, dc.getEntryCount());\n \t}\n \n+\tpublic void testFindOnEmpty() throws Exception {\n+\t\tfinal DirCache dc = DirCache.newInCore();\n+\t\tfinal byte[] path = Constants.encode(\"a\");\n+\t\tassertEquals(-1, dc.findEntry(path, path.length));\n+\t}\n }\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCache.java b/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCache.java\nindex bfb7925..9f0810a 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCache.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCache.java\n@@ -583,8 +583,6 @@ public void unlock() {\n \t *         information. If < 0 the entry does not exist in the index.\n \t */\n \tpublic int findEntry(final String path) {\n-\t\tif (entryCnt == 0)\n-\t\t\treturn -1;\n \t\tfinal byte[] p = Constants.encode(path);\n \t\treturn findEntry(p, p.length);\n \t}\n@@ -592,7 +590,7 @@ public int findEntry(final String path) {\n \tint findEntry(final byte[] p, final int pLen) {\n \t\tint low = 0;\n \t\tint high = entryCnt;\n-\t\tdo {\n+\t\twhile (low < high) {\n \t\t\tint mid = (low + high) >>> 1;\n \t\t\tfinal int cmp = cmp(p, pLen, sortedEntries[mid]);\n \t\t\tif (cmp < 0)\n@@ -603,7 +601,7 @@ else if (cmp == 0) {\n \t\t\t\treturn mid;\n \t\t\t} else\n \t\t\t\tlow = mid + 1;\n-\t\t} while (low < high);\n+\t\t}\n \t\treturn -(low + 1);\n \t}\n \n-- \n1.6.4.1.341.gf2a44\n"},{"id":"122273","messageId":"20090901231812.GB1033@spearce.org","threadId":"20817","inReplyTo":"1251847010-9992-1-git-send-email-spearce@spearce.org","subject":"Re: [JGIT PATCH (RESEND) 1/3] Allow RefUpdate.setExpectedOldObjectId to accept RevCommit","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-09-01T23:18:12Z","receivedAt":"2009-09-01T23:18:12Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"\"Shawn O. Pearce\" <spearce@spearce.org> wrote:\n> Subject: Re: [JGIT PATCH (RESEND) 1/3] Allow RefUpdate.setExpectedOldObjectId to accept RevCommit\n\nSorry, this is not a resend, its the first time I've sent it...\n\n-- \nShawn.\n"},{"id":"122371","messageId":"200909040114.12980.robin.rosenberg@dewire.com","threadId":"20817","inReplyTo":"1251847010-9992-3-git-send-email-spearce@spearce.org","subject":"Re: [JGIT PATCH (RESEND) 3/3] Fix DirCache.findEntry to work on an empty cache","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2009-09-03T23:14:12Z","receivedAt":"2009-09-03T23:14:12Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"onsdag 02 september 2009 01:16:50 skrev \"Shawn O. Pearce\" <spearce@spearce.org>:\n> If the cache has no entries, we want to return -1 rather than throw\n> ArrayIndexOutOfBoundsException.  This binary search loop was stolen\n> from some other code which contained a test before the loop to see if\n> the collection was empty or not, but we failed to include that here.\n> \n> Flipping the loop around to a standard while loop ensures we test\n> the condition properly first.\n> \n> Signed-off-by: Shawn O. Pearce <sop@google.com>\n> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>\n\nIs this a new policy?\n\n-- robin\n"},{"id":"122372","messageId":"20090903231910.GN1033@spearce.org","threadId":"20817","inReplyTo":"200909040114.12980.robin.rosenberg@dewire.com","subject":"Re: [JGIT PATCH (RESEND) 3/3] Fix DirCache.findEntry to work on an empty cache","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-09-03T23:19:10Z","receivedAt":"2009-09-03T23:19:10Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Robin Rosenberg <robin.rosenberg@dewire.com> wrote:\n> > Signed-off-by: Shawn O. Pearce <sop@google.com>\n> > Signed-off-by: Shawn O. Pearce <spearce@spearce.org>\n> \n> Is this a new policy?\n\nCall it a new habit.  I send from my @spearce.org because that's my\n\"public identity\", but this was on work time, so I added an extra\nSOB line to indicate that yes, my employer is also OK with this.\nNot that anyone probably doubted that in the first place though...\nI work for a company that is pretty friendly towards open source.\n\n-- \nShawn.\n"},{"id":"122374","messageId":"alpine.LFD.2.00.0909032003490.6044@xanadu.home","threadId":"20817","inReplyTo":"20090903231910.GN1033@spearce.org","subject":"Re: [JGIT PATCH (RESEND) 3/3] Fix DirCache.findEntry to work on an empty cache","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2009-09-04T00:06:39Z","receivedAt":"2009-09-04T00:06:39Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Thu, 3 Sep 2009, Shawn O. Pearce wrote:\n\n> Robin Rosenberg <robin.rosenberg@dewire.com> wrote:\n> > > Signed-off-by: Shawn O. Pearce <sop@google.com>\n> > > Signed-off-by: Shawn O. Pearce <spearce@spearce.org>\n> > \n> > Is this a new policy?\n> \n> Call it a new habit.  I send from my @spearce.org because that's my\n> \"public identity\", but this was on work time, so I added an extra\n> SOB line to indicate that yes, my employer is also OK with this.\n\nYou might as well simply use your Google SOB alone, even if your From is \n@spearce.org.  That's what I do with my Linux patches done on $day_job \ntime.\n\n\nNicolas\n"}]}