{"thread":{"id":"19304","subject":"[JGIT PATCH] Read symbolic refs too when reading all refs","startedAt":"2009-05-11T22:49:00Z","lastAt":"2009-05-12T02:22:34Z","messageCount":2,"participants":["Robin Rosenberg","Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"113601","messageId":"1242082140-740-1-git-send-email-robin.rosenberg@dewire.com","threadId":"19304","inReplyTo":null,"subject":"[JGIT PATCH] Read symbolic refs too when reading all refs","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2009-05-11T22:49:00Z","receivedAt":"2009-05-11T22:49:00Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"We failed to read symrefs when scanning for all refs. Single refs\nwere ok before too.\n\nSigned-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>\n---\n .../tst/org/spearce/jgit/lib/RefTest.java          |   71 ++++++++++++++++++++\n .../src/org/spearce/jgit/lib/RefDatabase.java      |   13 ++--\n 2 files changed, 77 insertions(+), 7 deletions(-)\n create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefTest.java\n\ndiff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefTest.java\nnew file mode 100644\nindex 0000000..db79a8a\n--- /dev/null\n+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefTest.java\n@@ -0,0 +1,71 @@\n+/*\n+ * Copyright (C) 2009, Robin Rosenberg\n+ *\n+ * All rights reserved.\n+ *\n+ * Redistribution and use in source and binary forms, with or\n+ * without modification, are permitted provided that the following\n+ * conditions are met:\n+ *\n+ * - Redistributions of source code must retain the above copyright\n+ *   notice, this list of conditions and the following disclaimer.\n+ *\n+ * - Redistributions in binary form must reproduce the above\n+ *   copyright notice, this list of conditions and the following\n+ *   disclaimer in the documentation and/or other materials provided\n+ *   with the distribution.\n+ *\n+ * - Neither the name of the Git Development Community nor the\n+ *   names of its contributors may be used to endorse or promote\n+ *   products derived from this software without specific prior\n+ *   written permission.\n+ *\n+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND\n+ * CONTRIBUTORS \"AS IS\" AND ANY EXPRESS OR IMPLIED WARRANTIES,\n+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES\n+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE\n+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR\n+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,\n+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT\n+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;\n+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER\n+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,\n+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)\n+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF\n+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.\n+ */\n+package org.spearce.jgit.lib;\n+\n+import java.util.Map;\n+\n+/**\n+ * Misc tests for refs. A lot of things are tested elsewhere so not having a\n+ * test for a ref related method, does not mean it is untested.\n+ */\n+public class RefTest extends RepositoryTestCase {\n+\n+\tpublic void testReadAllIncludingSymrefs() throws Exception {\n+\t\tObjectId masterId = db.resolve(\"refs/heads/master\");\n+\t\tRefUpdate updateRef = db.updateRef(\"refs/remotes/origin/master\");\n+\t\tupdateRef.setNewObjectId(masterId);\n+\t\tupdateRef.setForceUpdate(true);\n+\t\tupdateRef.update();\n+\t\tdb\n+\t\t\t\t.writeSymref(\"refs/remotes/origin/HEAD\",\n+\t\t\t\t\t\t\"refs/remotes/origin/master\");\n+\n+\t\tObjectId r = db.resolve(\"refs/remotes/origin/HEAD\");\n+\t\tassertEquals(masterId, r);\n+\n+\t\tMap<String, Ref> allRefs = db.getAllRefs();\n+\t\tRef refHEAD = allRefs.get(\"refs/remotes/origin/HEAD\");\n+\t\tassertEquals(masterId, refHEAD.getObjectId());\n+\t\tassertTrue(refHEAD.isPeeled());\n+\t\tassertNull(refHEAD.getPeeledObjectId());\n+\n+\t\tRef refmaster = allRefs.get(\"refs/remotes/origin/master\");\n+\t\tassertEquals(masterId, refmaster.getObjectId());\n+\t\tassertFalse(refmaster.isPeeled());\n+\t\tassertNull(refmaster.getPeeledObjectId());\n+\t}\n+}\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java\nindex 87f26bf..e364b15 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java\n@@ -265,23 +265,22 @@ private synchronized void readOneLooseRef(final Map<String, Ref> avail,\n \t\t\t\t\tfinal byte[] str = new byte[Constants.OBJECT_ID_LENGTH * 2];\n \t\t\t\t\tNB.readFully(in, str, 0, str.length);\n \t\t\t\t\tid = ObjectId.fromString(str, 0);\n+\t\t\t\t\tref = new Ref(Ref.Storage.LOOSE, origName, refName, id, null, false); // unpeeled\n \t\t\t\t} catch (EOFException tooShortToBeRef) {\n \t\t\t\t\t// Its below the minimum length needed. It could\n \t\t\t\t\t// be a symbolic reference.\n \t\t\t\t\t//\n-\t\t\t\t\treturn;\n+\t\t\t\t\tref = readRefBasic(refName, refName, 0); \n \t\t\t\t} catch (IllegalArgumentException notRef) {\n \t\t\t\t\t// It is not a well-formed ObjectId. It may be\n \t\t\t\t\t// a symbolic reference (\"ref: \").\n \t\t\t\t\t//\n-\t\t\t\t\treturn;\n+\t\t\t\t\tref = readRefBasic(refName, refName, 0); \n \t\t\t\t}\n \n-\t\t\t\tref = new Ref(Ref.Storage.LOOSE, origName, refName, id, null, false); // unpeeled\n-\n-\t\t\t\tlooseRefs.put(ref.getName(), ref);\n-\t\t\t\tlooseRefsMTime.put(ref.getName(), ent.lastModified());\n-\t\t\t\tavail.put(ref.getName(), ref);\n+\t\t\t\tlooseRefs.put(ref.getOrigName(), ref);\n+\t\t\t\tlooseRefsMTime.put(ref.getOrigName(), ent.lastModified());\n+\t\t\t\tavail.put(ref.getOrigName(), ref);\n \t\t\t} finally {\n \t\t\t\tin.close();\n \t\t\t}\n-- \n1.6.3.dirty\n"},{"id":"113608","messageId":"20090512022234.GQ30527@spearce.org","threadId":"19304","inReplyTo":"1242082140-740-1-git-send-email-robin.rosenberg@dewire.com","subject":"Re: [JGIT PATCH] Read symbolic refs too when reading all refs","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-05-12T02:22:34Z","receivedAt":"2009-05-12T02:22:34Z","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> We failed to read symrefs when scanning for all refs. Single refs\n> were ok before too.\n\nHmm, I think this is a better patch, given that readOneLooseRef and\nreadRefBasic are basically trying to do the same thing here anyway.\n\n\n--8<--\nFrom: Robin Rosenberg <robin.rosenberg@dewire.com>\nSubject: [PATCH] Read symbolic refs too when reading all refs\n\nWe failed to read symrefs when scanning for all refs. Single refs\nwere ok before too.\n\nSigned-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n .../tst/org/spearce/jgit/lib/RefTest.java          |   72 ++++++++++++++++++++\n .../src/org/spearce/jgit/lib/RefDatabase.java      |   70 ++-----------------\n 2 files changed, 80 insertions(+), 62 deletions(-)\n create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefTest.java\n\ndiff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefTest.java\nnew file mode 100644\nindex 0000000..cae5143\n--- /dev/null\n+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefTest.java\n@@ -0,0 +1,72 @@\n+/*\n+ * Copyright (C) 2009, Robin Rosenberg\n+ *\n+ * All rights reserved.\n+ *\n+ * Redistribution and use in source and binary forms, with or\n+ * without modification, are permitted provided that the following\n+ * conditions are met:\n+ *\n+ * - Redistributions of source code must retain the above copyright\n+ *   notice, this list of conditions and the following disclaimer.\n+ *\n+ * - Redistributions in binary form must reproduce the above\n+ *   copyright notice, this list of conditions and the following\n+ *   disclaimer in the documentation and/or other materials provided\n+ *   with the distribution.\n+ *\n+ * - Neither the name of the Git Development Community nor the\n+ *   names of its contributors may be used to endorse or promote\n+ *   products derived from this software without specific prior\n+ *   written permission.\n+ *\n+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND\n+ * CONTRIBUTORS \"AS IS\" AND ANY EXPRESS OR IMPLIED WARRANTIES,\n+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES\n+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE\n+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR\n+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,\n+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT\n+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;\n+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER\n+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,\n+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)\n+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF\n+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.\n+ */\n+package org.spearce.jgit.lib;\n+\n+import java.util.Map;\n+\n+/**\n+ * Misc tests for refs. A lot of things are tested elsewhere so not having a\n+ * test for a ref related method, does not mean it is untested.\n+ */\n+public class RefTest extends RepositoryTestCase {\n+\n+\tpublic void testReadAllIncludingSymrefs() throws Exception {\n+\t\tObjectId masterId = db.resolve(\"refs/heads/master\");\n+\t\tRefUpdate updateRef = db.updateRef(\"refs/remotes/origin/master\");\n+\t\tupdateRef.setNewObjectId(masterId);\n+\t\tupdateRef.setForceUpdate(true);\n+\t\tupdateRef.update();\n+\t\tdb\n+\t\t\t\t.writeSymref(\"refs/remotes/origin/HEAD\",\n+\t\t\t\t\t\t\"refs/remotes/origin/master\");\n+\n+\t\tObjectId r = db.resolve(\"refs/remotes/origin/HEAD\");\n+\t\tassertEquals(masterId, r);\n+\n+\t\tMap<String, Ref> allRefs = db.getAllRefs();\n+\t\tRef refHEAD = allRefs.get(\"refs/remotes/origin/HEAD\");\n+\t\tassertNotNull(refHEAD);\n+\t\tassertEquals(masterId, refHEAD.getObjectId());\n+\t\tassertTrue(refHEAD.isPeeled());\n+\t\tassertNull(refHEAD.getPeeledObjectId());\n+\n+\t\tRef refmaster = allRefs.get(\"refs/remotes/origin/master\");\n+\t\tassertEquals(masterId, refmaster.getObjectId());\n+\t\tassertFalse(refmaster.isPeeled());\n+\t\tassertNull(refmaster.getPeeledObjectId());\n+\t}\n+}\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java\nindex 87f26bf..155ed9a 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java\n@@ -41,7 +41,6 @@\n import static org.spearce.jgit.lib.Constants.R_TAGS;\n \n import java.io.BufferedReader;\n-import java.io.EOFException;\n import java.io.File;\n import java.io.FileInputStream;\n import java.io.FileNotFoundException;\n@@ -53,7 +52,6 @@\n import org.spearce.jgit.errors.ObjectWritingException;\n import org.spearce.jgit.lib.Ref.Storage;\n import org.spearce.jgit.util.FS;\n-import org.spearce.jgit.util.NB;\n \n class RefDatabase {\n \tprivate static final String REFS_SLASH = \"refs/\";\n@@ -229,69 +227,17 @@ private void readLooseRefs(final Map<String, Ref> avail,\n \t\t\tfinal String entName = ent.getName();\n \t\t\tif (\".\".equals(entName) || \"..\".equals(entName))\n \t\t\t\tcontinue;\n-\t\t\treadOneLooseRef(avail, prefix + entName, prefix + entName, ent);\n-\t\t}\n-\t}\n-\n-\tprivate synchronized void readOneLooseRef(final Map<String, Ref> avail,\n-\t\t\tfinal String origName, final String refName, final File ent) {\n-\t\t// Unchanged and cached? Don't read it again.\n-\t\t//\n-\t\tRef ref = looseRefs.get(refName);\n-\t\tif (ref != null) {\n-\t\t\tLong cachedlastModified = looseRefsMTime.get(refName);\n-\t\t\tif (cachedlastModified != null && cachedlastModified == ent.lastModified()) {\n-\t\t\t\tavail.put(ref.getName(), ref);\n-\t\t\t\treturn;\n-\t\t\t}\n-\t\t\tlooseRefs.remove(refName);\n-\t\t\tlooseRefsMTime.remove(refName);\n-\t\t}\n-\n-\t\t// Recurse into the directory.\n-\t\t//\n-\t\tif (ent.isDirectory()) {\n-\t\t\treadLooseRefs(avail, refName + \"/\", ent);\n-\t\t\treturn;\n-\t\t}\n-\n-\t\t// Assume its a valid loose reference we need to cache.\n-\t\t//\n-\t\ttry {\n-\t\t\tfinal FileInputStream in = new FileInputStream(ent);\n-\t\t\ttry {\n-\t\t\t\tfinal ObjectId id;\n+\t\t\tif (ent.isDirectory()) {\n+\t\t\t\treadLooseRefs(avail, prefix + entName + \"/\", ent);\n+\t\t\t} else {\n \t\t\t\ttry {\n-\t\t\t\t\tfinal byte[] str = new byte[Constants.OBJECT_ID_LENGTH * 2];\n-\t\t\t\t\tNB.readFully(in, str, 0, str.length);\n-\t\t\t\t\tid = ObjectId.fromString(str, 0);\n-\t\t\t\t} catch (EOFException tooShortToBeRef) {\n-\t\t\t\t\t// Its below the minimum length needed. It could\n-\t\t\t\t\t// be a symbolic reference.\n-\t\t\t\t\t//\n-\t\t\t\t\treturn;\n-\t\t\t\t} catch (IllegalArgumentException notRef) {\n-\t\t\t\t\t// It is not a well-formed ObjectId. It may be\n-\t\t\t\t\t// a symbolic reference (\"ref: \").\n-\t\t\t\t\t//\n-\t\t\t\t\treturn;\n+\t\t\t\t\tfinal Ref ref = readRefBasic(prefix + entName, 0);\n+\t\t\t\t\tif (ref != null)\n+\t\t\t\t\t\tavail.put(ref.getOrigName(), ref);\n+\t\t\t\t} catch (IOException e) {\n+\t\t\t\t\tcontinue;\n \t\t\t\t}\n-\n-\t\t\t\tref = new Ref(Ref.Storage.LOOSE, origName, refName, id, null, false); // unpeeled\n-\n-\t\t\t\tlooseRefs.put(ref.getName(), ref);\n-\t\t\t\tlooseRefsMTime.put(ref.getName(), ent.lastModified());\n-\t\t\t\tavail.put(ref.getName(), ref);\n-\t\t\t} finally {\n-\t\t\t\tin.close();\n \t\t\t}\n-\t\t} catch (FileNotFoundException noFile) {\n-\t\t\t// Deleted while we were reading? Its gone now!\n-\t\t\t//\n-\t\t} catch (IOException err) {\n-\t\t\t// Whoops.\n-\t\t\t//\n-\t\t\tthrow new RuntimeException(\"Cannot read ref \" + ent, err);\n \t\t}\n \t}\n \n-- \n1.6.3.48.g99c76\n"}]}