{"thread":{"id":"18981","subject":"[JGIT PATCH 00/10] Object access improvements","startedAt":"2009-04-21T01:21:02Z","lastAt":"2009-04-22T23:02:20Z","messageCount":17,"participants":["Shawn O. Pearce","Robin Rosenberg"],"isPatch":true,"patchVersion":1,"patchTotal":10},"messages":[{"id":"111808","messageId":"1240276872-17893-1-git-send-email-spearce@spearce.org","threadId":"18981","inReplyTo":null,"subject":"[JGIT PATCH 00/10] Object access improvements","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-04-21T01:21:02Z","receivedAt":"2009-04-21T01:21:02Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"This series tries to address issue 79[1] and GERRIT-81[2] by\ndetecting when packs have been overwritten on disk, or when\nnew packs have appeared and old packs have disappeared.\n\nThe first 5 patches are quite trivial and are just things I\nfound while working on the real issue.\n\nPatch 6 is rather intrusive. It starts a refactoring I have\nalways wanted to do, which is to split the object database out\nof the Repository class, and also start to divorce it from the\nlocal filesystem. The library is in the same state after patch\n5 as it was before patch 5; that is issue 79 is still an issue,\nbut everything else is fine.\n\nPatch 7 and 8 fix issue 79.\n\nPatch 9 is a rewritten version of your 1/3 posted Sun 19 Apr,\nproviding test cases for issue 79 / GERRIT-81.\n\nPatch 10 is a broken test case. Basically as I explain in the\ncommit message of 7, PackWriter can still crash randomly.\n\nWith 1-9 applied, issue 79 can be marked fixed, but GERRIT-81 is\nstill a problem, as any concurrent PackWriter thread may randomly\ncrash during a repack.\n\n[1] http://code.google.com/p/egit/issues/detail?id=79\n[2] http://jira.source.android.com/jira/browse/GERRIT-81\n\nShawn O. Pearce (10):\n Safely handle closing an already closed WindowedFile\n Change empty tree test case to use a temporary repository\n Replace hand-coded read fully loop with NB.readFully\n Clear the reverse index when closing a PackFile\n Introduce a new exception type for an in-place pack modification\n Refactor object database access with new abstraction\n Rescan packs if a pack is modified in-place (part 1)\n Scan for new packs if GIT_DIR/objects/pack has been modified\n Add test cases for loading new (or replaced) pack files\n BROKEN TEST: ObjectLoader stays valid across repacks\n\n .../org/spearce/jgit/lib/ConcurrentRepackTest.java | 252 +++++++++++\n .../org/spearce/jgit/lib/RepositoryTestCase.java | 2 -\n .../tst/org/spearce/jgit/lib/T0003_Basic.java | 11 +-\n .../tst/org/spearce/jgit/lib/T0004_PackReader.java | 1 -\n .../spearce/jgit/errors/PackMismatchException.java | 55 +++\n .../src/org/spearce/jgit/lib/ObjectDatabase.java | 367 ++++++++++++++++\n .../src/org/spearce/jgit/lib/ObjectDirectory.java | 438 ++++++++++++++++++++\n .../src/org/spearce/jgit/lib/PackFile.java | 36 ++-\n .../src/org/spearce/jgit/lib/Repository.java | 174 +--------\n .../org/spearce/jgit/lib/UnpackedObjectLoader.java | 34 +-\n .../src/org/spearce/jgit/lib/WindowedFile.java | 16 +-\n 11 files changed, 1183 insertions(+), 203 deletions(-)\n create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/lib/ConcurrentRepackTest.java\n create mode 100644 org.spearce.jgit/src/org/spearce/jgit/errors/PackMismatchException.java\n create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java\n create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java\n"},{"id":"111809","messageId":"1240276872-17893-2-git-send-email-spearce@spearce.org","threadId":"18981","inReplyTo":"1240276872-17893-1-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH 01/10] Safely handle closing an already closed WindowedFile","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-04-21T01:21:03Z","receivedAt":"2009-04-21T01:21:03Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"If cacheOpen throws an exception (e.g. FileNotFoundException) we\nmay wind up in cacheClose, where the fd is null because we didn't\nsuccessfully construct the RandomAccessFile. In such cases there\nis nothing to attempt to close.\n\nSigned-off-by: Shawn O. Pearce \n---\n .../src/org/spearce/jgit/lib/WindowedFile.java | 16 +++++++++-------\n 1 files changed, 9 insertions(+), 7 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java\nindex 938f44c..9293eb9 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java\n@@ -297,14 +297,16 @@ void cacheOpen() throws IOException {\n \t}\n \n \tvoid cacheClose() {\n-\t\ttry {\n-\t\t\tfd.close();\n-\t\t} catch (IOException err) {\n-\t\t\t// Ignore a close event. We had it open only for reading.\n-\t\t\t// There should not be errors related to network buffers\n-\t\t\t// not flushed, etc.\n+\t\tif (fd != null) {\n+\t\t\ttry {\n+\t\t\t\tfd.close();\n+\t\t\t} catch (IOException err) {\n+\t\t\t\t// Ignore a close event. We had it open only for reading.\n+\t\t\t\t// There should not be errors related to network buffers\n+\t\t\t\t// not flushed, etc.\n+\t\t\t}\n+\t\t\tfd = null;\n \t\t}\n-\t\tfd = null;\n \t}\n \n \tvoid allocWindow(final WindowCursor curs, final int windowId,\n-- \n1.6.3.rc1.188.ga02b\n"},{"id":"111810","messageId":"1240276872-17893-3-git-send-email-spearce@spearce.org","threadId":"18981","inReplyTo":"1240276872-17893-2-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH 02/10] Change empty tree test case to use a temporary repository","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-04-21T01:21:04Z","receivedAt":"2009-04-21T01:21:04Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"This test case looks for a loose object of the empty tree, but our\nstock test repository contains the empty tree in a pack file. So\nthe only way we can ensure it will be written is if we write to an\nempty repository.\n\nSigned-off-by: Shawn O. Pearce \n---\n .../tst/org/spearce/jgit/lib/T0003_Basic.java | 11 +++++------\n 1 files changed, 5 insertions(+), 6 deletions(-)\n\ndiff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0003_Basic.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0003_Basic.java\nindex 6a296be..b9e8d1d 100644\n--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0003_Basic.java\n+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0003_Basic.java\n@@ -75,14 +75,13 @@ public void test002_WriteEmptyTree() throws IOException {\n \t\t// open when we create it we won't write the object file out as a loose\n \t\t// object (as it already exists in the pack).\n \t\t//\n-\t\tdb.closePacks();\n-\n-\t\tfinal Tree t = new Tree(db);\n-\t\tt.accept(new WriteTree(trash, db), TreeEntry.MODIFIED_ONLY);\n+\t\tfinal Repository newdb = createNewEmptyRepo();\n+\t\tfinal Tree t = new Tree(newdb);\n+\t\tt.accept(new WriteTree(trash, newdb), TreeEntry.MODIFIED_ONLY);\n \t\tassertEquals(\"4b825dc642cb6eb9a060e54bf8d69288fbee4904\", t.getId()\n \t\t\t\t.name());\n-\t\tfinal File o = new File(new File(new File(trash_git, \"objects\"), \"4b\"),\n-\t\t\t\t\"825dc642cb6eb9a060e54bf8d69288fbee4904\");\n+\t\tfinal File o = new File(new File(new File(newdb.getDirectory(),\n+\t\t\t\t\"objects\"), \"4b\"), \"825dc642cb6eb9a060e54bf8d69288fbee4904\");\n \t\tassertTrue(\"Exists \" + o, o.isFile());\n \t\tassertTrue(\"Read-only \" + o, !o.canWrite());\n \t}\n-- \n1.6.3.rc1.188.ga02b\n"},{"id":"111811","messageId":"1240276872-17893-4-git-send-email-spearce@spearce.org","threadId":"18981","inReplyTo":"1240276872-17893-3-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH 03/10] Replace hand-coded read fully loop with NB.readFully","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-04-21T01:21:05Z","receivedAt":"2009-04-21T01:21:05Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"This code predates the NB utility class. I'd prefer to reuse the\ncode over keeping this duplicate copy of the logic.\n\nSigned-off-by: Shawn O. Pearce \n---\n .../org/spearce/jgit/lib/UnpackedObjectLoader.java | 14 ++++++--------\n 1 files changed, 6 insertions(+), 8 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectLoader.java b/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectLoader.java\nindex f2cae87..b086821 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectLoader.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectLoader.java\n@@ -46,6 +46,7 @@\n \n import org.spearce.jgit.errors.CorruptObjectException;\n import org.spearce.jgit.util.MutableInteger;\n+import org.spearce.jgit.util.NB;\n import org.spearce.jgit.util.RawParseUtils;\n \n /**\n@@ -74,17 +75,14 @@ public UnpackedObjectLoader(final Repository db, final AnyObjectId id)\n \n \tprivate static byte[] readCompressed(final Repository db,\n \t\t\tfinal AnyObjectId id) throws FileNotFoundException, IOException {\n-\t\tfinal FileInputStream objStream = new FileInputStream(db.toFile(id));\n-\t\tfinal byte[] compressed;\n+\t\tfinal FileInputStream in = new FileInputStream(db.toFile(id));\n \t\ttry {\n-\t\t\tcompressed = new byte[objStream.available()];\n-\t\t\tint off = 0;\n-\t\t\twhile (off < compressed.length)\n-\t\t\t\toff += objStream.read(compressed, off, compressed.length - off);\n+\t\t\tfinal byte[] compressed = new byte[(int) in.getChannel().size()];\n+\t\t\tNB.readFully(in, compressed, 0, compressed.length);\n+\t\t\treturn compressed;\n \t\t} finally {\n-\t\t\tobjStream.close();\n+\t\t\tin.close();\n \t\t}\n-\t\treturn compressed;\n \t}\n \n \t/**\n-- \n1.6.3.rc1.188.ga02b\n"},{"id":"111814","messageId":"1240276872-17893-5-git-send-email-spearce@spearce.org","threadId":"18981","inReplyTo":"1240276872-17893-4-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH 04/10] Clear the reverse index when closing a PackFile","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-04-21T01:21:06Z","receivedAt":"2009-04-21T01:21:06Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"We clear the forward index (ObjectId -> offset) when we close the pack,\nso we might as well clear the reverse index (offset -> ObjectId).\n\nThe reverse index is protected by the lock on \"this\", just like the\nforward index is.\n\nSigned-off-by: Shawn O. Pearce \n---\n .../src/org/spearce/jgit/lib/PackFile.java | 1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java\nindex 1e6f170..b525a82 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java\n@@ -155,6 +155,7 @@ public void close() {\n \t\tpack.close();\n \t\tsynchronized (this) {\n \t\t\tloadedIdx = null;\n+\t\t\treverseIdx = null;\n \t\t}\n \t}\n \n-- \n1.6.3.rc1.188.ga02b\n"},{"id":"111815","messageId":"1240276872-17893-6-git-send-email-spearce@spearce.org","threadId":"18981","inReplyTo":"1240276872-17893-5-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH 05/10] Introduce a new exception type for an in-place pack modification","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-04-21T01:21:07Z","receivedAt":"2009-04-21T01:21:07Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"If a pack file is modified in place by \"git gc\" or \"git repack\"\nit means that the list of objects contained within the pack has\nnot changed, but it is possible for the layout of the pack to\nbe modified.\n\nThe layout can change when the compressed version of any object\nis changed, and usually happens because the packer has found a\nsmaller way to represent a contained object. It also can change\nif an object's delta base encoding format is switched, such as from\nOBJ_REF to OBJ_OFS.\n\nThis new exception type can be caught by callers to recognize that\nthe object contents of the pack is quite likely the same, but that\nany offsets and cached windows are wrong and need to be computed\nor loaded again.\n\nSigned-off-by: Shawn O. Pearce \n---\n .../spearce/jgit/errors/PackMismatchException.java | 55 ++++++++++++++++++++\n .../src/org/spearce/jgit/lib/PackFile.java | 15 +++--\n 2 files changed, 64 insertions(+), 6 deletions(-)\n create mode 100644 org.spearce.jgit/src/org/spearce/jgit/errors/PackMismatchException.java\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/errors/PackMismatchException.java b/org.spearce.jgit/src/org/spearce/jgit/errors/PackMismatchException.java\nnew file mode 100644\nindex 0000000..b96f9e8\n--- /dev/null\n+++ b/org.spearce.jgit/src/org/spearce/jgit/errors/PackMismatchException.java\n@@ -0,0 +1,55 @@\n+/*\n+ * Copyright (C) 2009, Google Inc.\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+\n+package org.spearce.jgit.errors;\n+\n+import java.io.IOException;\n+\n+/** Thrown when a PackFile no longer matches the PackIndex. */\n+public class PackMismatchException extends IOException {\n+\tprivate static final long serialVersionUID = 1L;\n+\n+\t/**\n+\t * Construct a pack modification error.\n+\t *\n+\t * @param why\n+\t * description of the type of error.\n+\t */\n+\tpublic PackMismatchException(final String why) {\n+\t\tsuper(why);\n+\t}\n+}\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java\nindex b525a82..bda4843 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java\n@@ -51,6 +51,7 @@\n import java.util.zip.DataFormatException;\n \n import org.spearce.jgit.errors.CorruptObjectException;\n+import org.spearce.jgit.errors.PackMismatchException;\n import org.spearce.jgit.util.NB;\n \n /**\n@@ -292,16 +293,18 @@ private void onOpenPack() throws IOException {\n \t\tfinal long packCnt = NB.decodeUInt32(intbuf, 0);\n \t\tfinal long idxCnt = idx.getObjectCount();\n \t\tif (idxCnt != packCnt)\n-\t\t\tthrow new IOException(\"Pack index\"\n-\t\t\t\t\t+ \" object count mismatch; expected \" + packCnt\n-\t\t\t\t\t+ \" found \" + idxCnt + \": \" + pack.getName());\n+\t\t\tthrow new PackMismatchException(\"Pack object count mismatch:\"\n+\t\t\t\t\t+ \" pack \" + packCnt\n+\t\t\t\t\t+ \" index \" + idxCnt\n+\t\t\t\t\t+ \": \" + pack.getName());\n \n \t\tfinal byte[] csumbuf = new byte[20];\n \t\tpack.readFully(pack.length() - 20, csumbuf, curs);\n \t\tif (!Arrays.equals(csumbuf, idx.packChecksum))\n-\t\t\tthrow new IOException(\"Pack index mismatch; pack SHA-1 is \"\n-\t\t\t\t\t+ ObjectId.fromRaw(csumbuf).name() + \", index expects \"\n-\t\t\t\t\t+ ObjectId.fromRaw(idx.packChecksum).name());\n+\t\t\tthrow new PackMismatchException(\"Pack checksum mismatch:\"\n+\t\t\t\t\t+ \" pack \" + ObjectId.fromRaw(csumbuf).name()\n+\t\t\t\t\t+ \" index \" + ObjectId.fromRaw(idx.packChecksum).name()\n+\t\t\t\t\t+ \": \" + pack.getName());\n \t}\n \n \tprivate PackedObjectLoader reader(final WindowCursor curs,\n-- \n1.6.3.rc1.188.ga02b\n"},{"id":"111813","messageId":"1240276872-17893-7-git-send-email-spearce@spearce.org","threadId":"18981","inReplyTo":"1240276872-17893-6-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH 06/10] Refactor object database access with new abstraction","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-04-21T01:21:08Z","receivedAt":"2009-04-21T01:21:08Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"By moving object database access out of the Repository class we\nstart to open the door for less-traditional databases, such as\nstoring objects in a distributed hash table or in-memory.\n\nThis change doesn't introduce any new functionality, nor does it\nfix any existing bugs. A Repository instance still assumes that\nits object database is in \"$GIT_DIR/objects\", and there is no way\nto override that assumption.\n\nUnlike the prior implementation, alternates are maintained as\ndiscrete instances, rather than being inlined into the same list\nas the repository's own object directory. This cleans up that\nsearch logic as its now more clear when we scan the alternate\nand when we scan the local repository. It also opens the door\nfor sharing shared repositories, e.g. if an IDE were to open\ntwo repositories both forked from the same reference location\nwe could reuse the same ObjectDatabase instance between them.\n\nSigned-off-by: Shawn O. Pearce \n---\n .../org/spearce/jgit/lib/RepositoryTestCase.java | 2 -\n .../tst/org/spearce/jgit/lib/T0004_PackReader.java | 1 -\n .../src/org/spearce/jgit/lib/ObjectDatabase.java | 354 +++++++++++++++++\n .../src/org/spearce/jgit/lib/ObjectDirectory.java | 401 ++++++++++++++++++++\n .../src/org/spearce/jgit/lib/Repository.java | 174 +--------\n .../org/spearce/jgit/lib/UnpackedObjectLoader.java | 22 +-\n 6 files changed, 779 insertions(+), 175 deletions(-)\n create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java\n create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java\n\ndiff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java\nindex 588daf4..b85d3eb 100644\n--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java\n+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java\n@@ -297,8 +297,6 @@ copyFile(JGitTestUtil.getTestResourceFile(packs[k] + \".idx\"), new File(packDir,\n \n \t\tcopyFile(JGitTestUtil.getTestResourceFile(\"packed-refs\"), new File(trash_git,\"packed-refs\"));\n \n-\t\tdb.scanForPacks();\n-\n \t\tfakeSystemReader.values.clear();\n \t\tfakeSystemReader.values.put(Constants.OS_USER_NAME_KEY, Constants.OS_USER_NAME_KEY);\n \t\tfakeSystemReader.values.put(Constants.GIT_AUTHOR_NAME_KEY, Constants.GIT_AUTHOR_NAME_KEY);\ndiff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0004_PackReader.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0004_PackReader.java\nindex b9e5b49..e35e623 100644\n--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0004_PackReader.java\n+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0004_PackReader.java\n@@ -89,7 +89,6 @@ copyFile(new File(todopack, packname + \".pack\"), new File(packDir,\n \t\t\t\tpackname + \".pack\"));\n \t\tcopyFile(new File(todopack, packname + \".idx\"), new File(packDir,\n \t\t\t\tpackname + \".idx\"));\n-\t\tdb.scanForPacks();\n \t\tTree t;\n \n \t\tt = db\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java\nnew file mode 100644\nindex 0000000..ed1290f\n--- /dev/null\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java\n@@ -0,0 +1,354 @@\n+/*\n+ * Copyright (C) 2009, Google Inc.\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+\n+package org.spearce.jgit.lib;\n+\n+import java.io.IOException;\n+import java.util.Collection;\n+import java.util.concurrent.atomic.AtomicReference;\n+\n+/**\n+ * Abstraction of arbitrary object storage.\n+ *

\n+ * An object database stores one or more Git objects, indexed by their unique\n+ * {@link ObjectId}. Optionally an object database can reference one or more\n+ * alternates; other ObjectDatabase instances that are searched in addition to\n+ * the current database.\n+ *

\n+ * Databases are usually divided into two halves: a half that is considered to\n+ * be fast to search, and a half that is considered to be slow to search. When\n+ * alternates are present the fast half is fully searched (recursively through\n+ * all alternates) before the slow half is considered.\n+ */\n+public abstract class ObjectDatabase {\n+\t/** Constant indicating no alternate databases exist. */\n+\tprotected static final ObjectDatabase[] NO_ALTERNATES = {};\n+\n+\tprivate final AtomicReference alternates;\n+\n+\t/** Initialize a new database instance for access. */\n+\tprotected ObjectDatabase() {\n+\t\talternates = new AtomicReference();\n+\t}\n+\n+\t/**\n+\t * Does this database exist yet?\n+\t * \n+\t * @return true if this database is already created; false if the caller\n+\t * should invoke {@link #create()} to create this database location.\n+\t */\n+\tpublic boolean exists() {\n+\t\treturn true;\n+\t}\n+\n+\t/**\n+\t * Initialize a new object database at this location.\n+\t * \n+\t * @throws IOException\n+\t * the database could not be created.\n+\t */\n+\tpublic void create() throws IOException {\n+\t\t// Assume no action is required.\n+\t}\n+\n+\t/**\n+\t * Close any resources held by this database and its active alternates.\n+\t */\n+\tpublic final void close() {\n+\t\tcloseSelf();\n+\t\tcloseAlternates();\n+\t}\n+\n+\t/**\n+\t * Close any resources held by this database only; ignoring alternates.\n+\t *

\n+\t * To fully close this database and its referenced alternates, the caller\n+\t * should instead invoke {@link #close()}.\n+\t */\n+\tpublic void closeSelf() {\n+\t\t// Assume no action is required.\n+\t}\n+\n+\t/** Fully close all loaded alternates and clear the alternate list. */\n+\tpublic final void closeAlternates() {\n+\t\tObjectDatabase[] alt = alternates.get();\n+\t\tif (alt != null) {\n+\t\t\talternates.set(null);\n+\t\t\tfor (final ObjectDatabase d : alt) {\n+\t\t\t\td.close();\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\t/**\n+\t * Does the requested object exist in this database?\n+\t *

\n+\t * Alternates (if present) are searched automatically.\n+\t * \n+\t * @param objectId\n+\t * identity of the object to test for existence of.\n+\t * @return true if the specified object is stored in this database, or any\n+\t * of the alternate databases.\n+\t */\n+\tpublic final boolean hasObject(final AnyObjectId objectId) {\n+\t\treturn hasObjectImpl1(objectId) || hasObjectImpl2(objectId.name());\n+\t}\n+\n+\tprivate final boolean hasObjectImpl1(final AnyObjectId objectId) {\n+\t\tif (hasObject1(objectId)) {\n+\t\t\treturn true;\n+\t\t}\n+\t\tfor (final ObjectDatabase alt : getAlternates()) {\n+\t\t\tif (alt.hasObjectImpl1(objectId)) {\n+\t\t\t\treturn true;\n+\t\t\t}\n+\t\t}\n+\t\treturn false;\n+\t}\n+\n+\tprivate final boolean hasObjectImpl2(final String objectId) {\n+\t\tif (hasObject2(objectId)) {\n+\t\t\treturn true;\n+\t\t}\n+\t\tfor (final ObjectDatabase alt : getAlternates()) {\n+\t\t\tif (alt.hasObjectImpl2(objectId)) {\n+\t\t\t\treturn true;\n+\t\t\t}\n+\t\t}\n+\t\treturn false;\n+\t}\n+\n+\t/**\n+\t * Fast half of {@link #hasObject(AnyObjectId)}.\n+\t * \n+\t * @param objectId\n+\t * identity of the object to test for existence of.\n+\t * @return true if the specified object is stored in this database.\n+\t */\n+\tprotected abstract boolean hasObject1(AnyObjectId objectId);\n+\n+\t/**\n+\t * Slow half of {@link #hasObject(AnyObjectId)}.\n+\t * \n+\t * @param objectName\n+\t * identity of the object to test for existence of.\n+\t * @return true if the specified object is stored in this database.\n+\t */\n+\tprotected boolean hasObject2(String objectName) {\n+\t\t// Assume the search took place during hasObject1.\n+\t\treturn false;\n+\t}\n+\n+\t/**\n+\t * Open an object from this database.\n+\t *

\n+\t * Alternates (if present) are searched automatically.\n+\t * \n+\t * @param curs\n+\t * temporary working space associated with the calling thread.\n+\t * @param objectId\n+\t * identity of the object to open.\n+\t * @return a {@link ObjectLoader} for accessing the data of the named\n+\t * object, or null if the object does not exist.\n+\t * @throws IOException\n+\t */\n+\tpublic final ObjectLoader openObject(final WindowCursor curs,\n+\t\t\tfinal AnyObjectId objectId) throws IOException {\n+\t\tObjectLoader ldr;\n+\n+\t\tldr = openObjectImpl1(curs, objectId);\n+\t\tif (ldr != null) {\n+\t\t\treturn ldr;\n+\t\t}\n+\n+\t\tldr = openObjectImpl2(curs, objectId.name(), objectId);\n+\t\tif (ldr != null) {\n+\t\t\treturn ldr;\n+\t\t}\n+\t\treturn null;\n+\t}\n+\n+\tprivate ObjectLoader openObjectImpl1(final WindowCursor curs,\n+\t\t\tfinal AnyObjectId objectId) throws IOException {\n+\t\tObjectLoader ldr;\n+\n+\t\tldr = openObject1(curs, objectId);\n+\t\tif (ldr != null) {\n+\t\t\treturn ldr;\n+\t\t}\n+\t\tfor (final ObjectDatabase alt : getAlternates()) {\n+\t\t\tldr = alt.openObjectImpl1(curs, objectId);\n+\t\t\tif (ldr != null) {\n+\t\t\t\treturn ldr;\n+\t\t\t}\n+\t\t}\n+\t\treturn null;\n+\t}\n+\n+\tprivate ObjectLoader openObjectImpl2(final WindowCursor curs,\n+\t\t\tfinal String objectName, final AnyObjectId objectId)\n+\t\t\tthrows IOException {\n+\t\tObjectLoader ldr;\n+\n+\t\tldr = openObject2(curs, objectName, objectId);\n+\t\tif (ldr != null) {\n+\t\t\treturn ldr;\n+\t\t}\n+\t\tfor (final ObjectDatabase alt : getAlternates()) {\n+\t\t\tldr = alt.openObjectImpl2(curs, objectName, objectId);\n+\t\t\tif (ldr != null) {\n+\t\t\t\treturn ldr;\n+\t\t\t}\n+\t\t}\n+\t\treturn null;\n+\t}\n+\n+\t/**\n+\t * Fast half of {@link #openObject(WindowCursor, AnyObjectId)}.\n+\t * \n+\t * @param curs\n+\t * temporary working space associated with the calling thread.\n+\t * @param objectId\n+\t * identity of the object to open.\n+\t * @return a {@link ObjectLoader} for accessing the data of the named\n+\t * object, or null if the object does not exist.\n+\t * @throws IOException\n+\t */\n+\tprotected abstract ObjectLoader openObject1(WindowCursor curs,\n+\t\t\tAnyObjectId objectId) throws IOException;\n+\n+\t/**\n+\t * Slow half of {@link #openObject(WindowCursor, AnyObjectId)}.\n+\t * \n+\t * @param curs\n+\t * temporary working space associated with the calling thread.\n+\t * @param objectName\n+\t * name of the object to open.\n+\t * @param objectId\n+\t * identity of the object to open.\n+\t * @return a {@link ObjectLoader} for accessing the data of the named\n+\t * object, or null if the object does not exist.\n+\t * @throws IOException\n+\t */\n+\tprotected ObjectLoader openObject2(WindowCursor curs, String objectName,\n+\t\t\tAnyObjectId objectId) throws IOException {\n+\t\t// Assume the search took place during openObject1.\n+\t\treturn null;\n+\t}\n+\n+\t/**\n+\t * Open the object from all packs containing it.\n+\t *

\n+\t * If any alternates are present, their packs are also considered.\n+\t * \n+\t * @param out\n+\t * result collection of loaders for this object, filled with\n+\t * loaders from all packs containing specified object\n+\t * @param curs\n+\t * temporary working space associated with the calling thread.\n+\t * @param objectId\n+\t * id of object to search for\n+\t * @throws IOException\n+\t */\n+\tfinal void openObjectInAllPacks(final Collection out,\n+\t\t\tfinal WindowCursor curs, final AnyObjectId objectId)\n+\t\t\tthrows IOException {\n+\t\topenObjectInAllPacks1(out, curs, objectId);\n+\t\tfor (final ObjectDatabase alt : getAlternates()) {\n+\t\t\talt.openObjectInAllPacks1(out, curs, objectId);\n+\t\t}\n+\t}\n+\n+\t/**\n+\t * Open the object from all packs containing it.\n+\t * \n+\t * @param out\n+\t * result collection of loaders for this object, filled with\n+\t * loaders from all packs containing specified object\n+\t * @param curs\n+\t * temporary working space associated with the calling thread.\n+\t * @param objectId\n+\t * id of object to search for\n+\t * @throws IOException\n+\t */\n+\tvoid openObjectInAllPacks1(Collection out,\n+\t\t\tWindowCursor curs, AnyObjectId objectId) throws IOException {\n+\t\t// Assume no pack support\n+\t}\n+\n+\t/**\n+\t * Get the alternate databases known to this database.\n+\t * \n+\t * @return the alternate list. Never null, but may be an empty array.\n+\t */\n+\tpublic final ObjectDatabase[] getAlternates() {\n+\t\tObjectDatabase[] r = alternates.get();\n+\t\tif (r == null) {\n+\t\t\tsynchronized (alternates) {\n+\t\t\t\tr = alternates.get();\n+\t\t\t\tif (r == null) {\n+\t\t\t\t\ttry {\n+\t\t\t\t\t\tr = loadAlternates();\n+\t\t\t\t\t} catch (IOException e) {\n+\t\t\t\t\t\tr = NO_ALTERNATES;\n+\t\t\t\t\t}\n+\t\t\t\t\talternates.set(r);\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\t\treturn r;\n+\t}\n+\n+\t/**\n+\t * Load the list of alternate databases into memory.\n+\t *

\n+\t * This method is invoked by {@link #getAlternates()} if the alternate list\n+\t * has not yet been populated, or if {@link #closeAlternates()} has been\n+\t * called on this instance and the alternate list is needed again.\n+\t *

\n+\t * If the alternate array is empty, implementors should consider using the\n+\t * constant {@link #NO_ALTERNATES}.\n+\t * \n+\t * @return the alternate list for this database.\n+\t * @throws IOException\n+\t * the alternate list could not be accessed. The empty alternate\n+\t * array {@link #NO_ALTERNATES} will be assumed by the caller.\n+\t */\n+\tprotected ObjectDatabase[] loadAlternates() throws IOException {\n+\t\treturn NO_ALTERNATES;\n+\t}\n+}\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java\nnew file mode 100644\nindex 0000000..e7156c4\n--- /dev/null\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java\n@@ -0,0 +1,401 @@\n+/*\n+ * Copyright (C) 2009, Google Inc.\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+\n+package org.spearce.jgit.lib;\n+\n+import java.io.BufferedReader;\n+import java.io.File;\n+import java.io.FileNotFoundException;\n+import java.io.FileReader;\n+import java.io.FilenameFilter;\n+import java.io.IOException;\n+import java.util.ArrayList;\n+import java.util.Arrays;\n+import java.util.Collection;\n+import java.util.HashMap;\n+import java.util.List;\n+import java.util.Map;\n+import java.util.concurrent.atomic.AtomicReference;\n+\n+import org.spearce.jgit.util.FS;\n+\n+/**\n+ * Traditional file system based {@link ObjectDatabase}.\n+ *

\n+ * This is the classical object database representation for a Git repository,\n+ * where objects are stored loose by hashing them into directories by their\n+ * {@link ObjectId}, or are stored in compressed containers known as\n+ * {@link PackFile}s.\n+ */\n+public class ObjectDirectory extends ObjectDatabase {\n+\tprivate static final PackFile[] NO_PACKS = {};\n+\n+\tprivate final File objects;\n+\n+\tprivate final File infoDirectory;\n+\n+\tprivate final File packDirectory;\n+\n+\tprivate final File alternatesFile;\n+\n+\tprivate final AtomicReference packList;\n+\n+\t/**\n+\t * Initialize a reference to an on-disk object directory.\n+\t * \n+\t * @param dir\n+\t * the location of the objects directory.\n+\t */\n+\tpublic ObjectDirectory(final File dir) {\n+\t\tobjects = dir;\n+\t\tinfoDirectory = new File(objects, \"info\");\n+\t\tpackDirectory = new File(objects, \"pack\");\n+\t\talternatesFile = new File(infoDirectory, \"alternates\");\n+\n+\t\tpackList = new AtomicReference();\n+\t}\n+\n+\t/**\n+\t * @return the location of the objects directory.\n+\t */\n+\tpublic final File getDirectory() {\n+\t\treturn objects;\n+\t}\n+\n+\t@Override\n+\tpublic boolean exists() {\n+\t\treturn objects.exists();\n+\t}\n+\n+\t@Override\n+\tpublic void create() throws IOException {\n+\t\tobjects.mkdirs();\n+\t\tinfoDirectory.mkdir();\n+\t\tpackDirectory.mkdir();\n+\t}\n+\n+\t@Override\n+\tpublic void closeSelf() {\n+\t\tPackFile[] packs = packList.get();\n+\t\tif (packs != null) {\n+\t\t\tpackList.set(null);\n+\t\t\tfor (final PackFile p : packs) {\n+\t\t\t\tp.close();\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\t/**\n+\t * Compute the location of a loose object file.\n+\t * \n+\t * @param objectId\n+\t * identity of the loose object to map to the directory.\n+\t * @return location of the object, if it were to exist as a loose object.\n+\t */\n+\tpublic File fileFor(final AnyObjectId objectId) {\n+\t\treturn fileFor(objectId.name());\n+\t}\n+\n+\tprivate File fileFor(final String objectName) {\n+\t\tfinal String d = objectName.substring(0, 2);\n+\t\tfinal String f = objectName.substring(2);\n+\t\treturn new File(new File(objects, d), f);\n+\t}\n+\n+\t/**\n+\t * Add a single existing pack to the list of available pack files.\n+\t * \n+\t * @param pack\n+\t * path of the pack file to open.\n+\t * @param idx\n+\t * path of the corresponding index file.\n+\t * @throws IOException\n+\t * index file could not be opened, read, or is not recognized as\n+\t * a Git pack file index.\n+\t */\n+\tpublic void openPack(final File pack, final File idx) throws IOException {\n+\t\tfinal String p = pack.getName();\n+\t\tfinal String i = idx.getName();\n+\n+\t\tif (p.length() != 50 || !p.startsWith(\"pack-\") || !p.endsWith(\".pack\"))\n+\t\t\tthrow new IOException(\"Not a valid pack \" + pack);\n+\n+\t\tif (i.length() != 49 || !i.startsWith(\"pack-\") || !i.endsWith(\".idx\"))\n+\t\t\tthrow new IOException(\"Not a valid pack \" + idx);\n+\n+\t\tif (!p.substring(0, 45).equals(i.substring(0, 45)))\n+\t\t\tthrow new IOException(\"Pack \" + pack + \"does not match index\");\n+\n+\t\tinsertPack(new PackFile(idx, pack));\n+\t}\n+\n+\t@Override\n+\tpublic String toString() {\n+\t\treturn \"ObjectDirectory[\" + getDirectory() + \"]\";\n+\t}\n+\n+\t@Override\n+\tprotected boolean hasObject1(final AnyObjectId objectId) {\n+\t\tfor (final PackFile p : packs()) {\n+\t\t\ttry {\n+\t\t\t\tif (p.hasObject(objectId)) {\n+\t\t\t\t\treturn true;\n+\t\t\t\t}\n+\t\t\t} catch (IOException e) {\n+\t\t\t\t// The hasObject call should have only touched the index,\n+\t\t\t\t// so any failure here indicates the index is unreadable\n+\t\t\t\t// by this process, and the pack is likewise not readable.\n+\t\t\t\t//\n+\t\t\t\tremovePack(p);\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t}\n+\t\treturn false;\n+\t}\n+\n+\t@Override\n+\tprotected ObjectLoader openObject1(final WindowCursor curs,\n+\t\t\tfinal AnyObjectId objectId) throws IOException {\n+\t\tfor (final PackFile p : packs()) {\n+\t\t\ttry {\n+\t\t\t\tfinal ObjectLoader ldr = p.get(curs, objectId);\n+\t\t\t\tif (ldr != null) {\n+\t\t\t\t\treturn ldr;\n+\t\t\t\t}\n+\t\t\t} catch (IOException e) {\n+\t\t\t\t// Assume the pack is corrupted.\n+\t\t\t\t//\n+\t\t\t\tremovePack(p);\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t}\n+\t\treturn null;\n+\t}\n+\n+\t@Override\n+\tvoid openObjectInAllPacks1(final Collection out,\n+\t\t\tfinal WindowCursor curs, final AnyObjectId objectId)\n+\t\t\tthrows IOException {\n+\t\tfor (final PackFile p : packs()) {\n+\t\t\ttry {\n+\t\t\t\tfinal PackedObjectLoader ldr = p.get(curs, objectId);\n+\t\t\t\tif (ldr != null) {\n+\t\t\t\t\tout.add(ldr);\n+\t\t\t\t}\n+\t\t\t} catch (IOException e) {\n+\t\t\t\t// Assume the pack is corrupted.\n+\t\t\t\t//\n+\t\t\t\tremovePack(p);\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\t@Override\n+\tprotected boolean hasObject2(final String objectName) {\n+\t\treturn fileFor(objectName).exists();\n+\t}\n+\n+\t@Override\n+\tprotected ObjectLoader openObject2(final WindowCursor curs,\n+\t\t\tfinal String objectName, final AnyObjectId objectId)\n+\t\t\tthrows IOException {\n+\t\ttry {\n+\t\t\treturn new UnpackedObjectLoader(fileFor(objectName), objectId);\n+\t\t} catch (FileNotFoundException noFile) {\n+\t\t\treturn null;\n+\t\t}\n+\t}\n+\n+\tprivate void insertPack(final PackFile pf) {\n+\t\tPackFile[] o, n;\n+\t\tdo {\n+\t\t\to = packs();\n+\t\t\tn = new PackFile[1 + o.length];\n+\t\t\tn[0] = pf;\n+\t\t\tSystem.arraycopy(o, 0, n, 1, o.length);\n+\t\t} while (!packList.compareAndSet(o, n));\n+\t}\n+\n+\tprivate void removePack(final PackFile deadPack) {\n+\t\tPackFile[] o, n;\n+\t\tdo {\n+\t\t\to = packList.get();\n+\t\t\tif (o == null || !inList(o, deadPack)) {\n+\t\t\t\tbreak;\n+\n+\t\t\t} else if (o.length == 1) {\n+\t\t\t\tn = NO_PACKS;\n+\n+\t\t\t} else {\n+\t\t\t\tn = new PackFile[o.length - 1];\n+\t\t\t\tint j = 0;\n+\t\t\t\tfor (final PackFile p : o) {\n+\t\t\t\t\tif (p != deadPack) {\n+\t\t\t\t\t\tn[j++] = p;\n+\t\t\t\t\t}\n+\t\t\t\t}\n+\t\t\t}\n+\t\t} while (!packList.compareAndSet(o, n));\n+\t\tdeadPack.close();\n+\t}\n+\n+\tprivate static boolean inList(final PackFile[] list, final PackFile pack) {\n+\t\tfor (final PackFile p : list) {\n+\t\t\tif (p == pack) {\n+\t\t\t\treturn true;\n+\t\t\t}\n+\t\t}\n+\t\treturn false;\n+\t}\n+\n+\tprivate PackFile[] packs() {\n+\t\tPackFile[] r = packList.get();\n+\t\tif (r == null) {\n+\t\t\tr = scanPacks(r);\n+\t\t}\n+\t\treturn r;\n+\t}\n+\n+\tprivate PackFile[] scanPacks(final PackFile[] original) {\n+\t\tsynchronized (packList) {\n+\t\t\tPackFile[] o, n;\n+\t\t\tdo {\n+\t\t\t\to = packList.get();\n+\t\t\t\tif (o != original) {\n+\t\t\t\t\t// Another thread did the scan for us, while we\n+\t\t\t\t\t// were blocked on the monitor above.\n+\t\t\t\t\t//\n+\t\t\t\t\treturn o;\n+\t\t\t\t}\n+\t\t\t\tn = scanPacksImpl(o != null ? o : NO_PACKS);\n+\t\t\t} while (!packList.compareAndSet(o, n));\n+\t\t\treturn n;\n+\t\t}\n+\t}\n+\n+\tprivate PackFile[] scanPacksImpl(final PackFile[] old) {\n+\t\tfinal Map forReuse = reuseMap(old);\n+\t\tfinal String[] idxList = listPackIdx();\n+\t\tfinal List list = new ArrayList(idxList.length);\n+\t\tfor (final String indexName : idxList) {\n+\t\t\tfinal String base = indexName.substring(0, indexName.length() - 4);\n+\t\t\tfinal String packName = base + \".pack\";\n+\n+\t\t\tfinal PackFile oldPack = forReuse.remove(packName);\n+\t\t\tif (oldPack != null) {\n+\t\t\t\tlist.add(oldPack);\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\n+\t\t\tfinal File packFile = new File(packDirectory, packName);\n+\t\t\tif (!packFile.isFile()) {\n+\t\t\t\t// Sometimes C Git's HTTP fetch transport leaves a\n+\t\t\t\t// .idx file behind and does not download the .pack.\n+\t\t\t\t// We have to skip over such useless indexes.\n+\t\t\t\t//\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\n+\t\t\tfinal File idxFile = new File(packDirectory, indexName);\n+\t\t\tlist.add(new PackFile(idxFile, packFile));\n+\t\t}\n+\n+\t\tfor (final PackFile p : forReuse.values()) {\n+\t\t\tp.close();\n+\t\t}\n+\n+\t\tif (list.isEmpty()) {\n+\t\t\treturn NO_PACKS;\n+\t\t}\n+\t\tfinal PackFile[] r = list.toArray(new PackFile[list.size()]);\n+\t\tArrays.sort(r, PackFile.SORT);\n+\t\treturn r;\n+\t}\n+\n+\tprivate static Map reuseMap(final PackFile[] old) {\n+\t\tfinal Map forReuse = new HashMap();\n+\t\tfor (final PackFile p : old) {\n+\t\t\tfinal PackFile prior = forReuse.put(p.getPackFile().getName(), p);\n+\t\t\tif (prior != null) {\n+\t\t\t\t// This should never occur. It should be impossible for us\n+\t\t\t\t// to have two pack files with the same name, as all of them\n+\t\t\t\t// came out of the same directory. If it does, we promised to\n+\t\t\t\t// close any PackFiles we did not reuse, so close the one we\n+\t\t\t\t// just evicted out of the reuse map.\n+\t\t\t\t//\n+\t\t\t\tprior.close();\n+\t\t\t}\n+\t\t}\n+\t\treturn forReuse;\n+\t}\n+\n+\tprivate String[] listPackIdx() {\n+\t\tfinal String[] idxList = packDirectory.list(new FilenameFilter() {\n+\t\t\tpublic boolean accept(final File baseDir, final String n) {\n+\t\t\t\t// Must match \"pack-[0-9a-f]{40}.idx\" to be an index.\n+\t\t\t\treturn n.length() == 49 && n.endsWith(\".idx\")\n+\t\t\t\t\t\t&& n.startsWith(\"pack-\");\n+\t\t\t}\n+\t\t});\n+\t\treturn idxList != null ? idxList : new String[0];\n+\t}\n+\n+\t@Override\n+\tprotected ObjectDatabase[] loadAlternates() throws IOException {\n+\t\tfinal BufferedReader br = open(alternatesFile);\n+\t\tfinal List l = new ArrayList(4);\n+\t\ttry {\n+\t\t\tString line;\n+\t\t\twhile ((line = br.readLine()) != null) {\n+\t\t\t\tl.add(new ObjectDirectory(FS.resolve(objects, line)));\n+\t\t\t}\n+\t\t} finally {\n+\t\t\tbr.close();\n+\t\t}\n+\n+\t\tif (l.isEmpty()) {\n+\t\t\treturn NO_ALTERNATES;\n+\t\t}\n+\t\treturn l.toArray(new ObjectDirectory[l.size()]);\n+\t}\n+\n+\tprivate static BufferedReader open(final File f)\n+\t\t\tthrows FileNotFoundException {\n+\t\treturn new BufferedReader(new FileReader(f));\n+\t}\n+}\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\nindex cfd92b8..f7bacf3 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\n@@ -43,10 +43,8 @@\n import java.io.File;\n import java.io.FileNotFoundException;\n import java.io.FileReader;\n-import java.io.FilenameFilter;\n import java.io.IOException;\n import java.util.ArrayList;\n-import java.util.Arrays;\n import java.util.Collection;\n import java.util.Collections;\n import java.util.HashMap;\n@@ -93,9 +91,7 @@\n \n \tprivate final RefDatabase refs;\n \n-\tprivate File[] objectDirectoryList;\n-\n-\tprivate PackFile[] packFileList;\n+\tprivate final ObjectDirectory objectDatabase;\n \n \tprivate GitIndex index;\n \n@@ -113,21 +109,11 @@\n \t */\n \tpublic Repository(final File d) throws IOException {\n \t\tgitDir = d.getAbsoluteFile();\n-\t\ttry {\n-\t\t\tobjectDirectoryList = readObjectsDirs(\n-\t\t\t\t\tFS.resolve(gitDir, \"objects\"), new ArrayList())\n-\t\t\t\t\t.toArray(new File[0]);\n-\t\t} catch (IOException e) {\n-\t\t\tIOException ex = new IOException(\"Cannot find all object dirs for \" + gitDir);\n-\t\t\tex.initCause(e);\n-\t\t\tthrow ex;\n-\t\t}\n \t\trefs = new RefDatabase(this);\n-\t\tpackFileList = new PackFile[0];\n+\t\tobjectDatabase = new ObjectDirectory(FS.resolve(gitDir, \"objects\"));\n \t\tconfig = new RepositoryConfig(this);\n \n-\t\tfinal boolean isExisting = objectDirectoryList[0].exists();\n-\t\tif (isExisting) {\n+\t\tif (objectDatabase.exists()) {\n \t\t\tgetConfig().load();\n \t\t\tfinal String repositoryFormatVersion = getConfig().getString(\n \t\t\t\t\t\"core\", null, \"repositoryFormatVersion\");\n@@ -138,25 +124,6 @@ public Repository(final File d) throws IOException {\n \t\t} else {\n \t\t\tgetConfig().create();\n \t\t}\n-\t\tif (isExisting)\n-\t\t\tscanForPacks();\n-\t}\n-\n-\tprivate static Collection readObjectsDirs(File objectsDir,\n-\t\t\tCollection ret) throws IOException {\n-\t\tret.add(objectsDir);\n-\t\tfinal File altFile = FS.resolve(objectsDir, \"info/alternates\");\n-\t\tif (altFile.exists()) {\n-\t\t\tBufferedReader ar = new BufferedReader(new FileReader(altFile));\n-\t\t\ttry {\n-\t\t\t\tfor (String alt=ar.readLine(); alt!=null; alt=ar.readLine()) {\n-\t\t\t\t\treadObjectsDirs(FS.resolve(objectsDir, alt), ret);\n-\t\t\t\t}\n-\t\t\t} finally {\n-\t\t\t\tar.close();\n-\t\t\t}\n-\t\t}\n-\t\treturn ret;\n \t}\n \n \t/**\n@@ -173,10 +140,7 @@ public synchronized void create() throws IOException {\n \n \t\tgitDir.mkdirs();\n \t\trefs.create();\n-\n-\t\tobjectDirectoryList[0].mkdirs();\n-\t\tnew File(objectDirectoryList[0], \"pack\").mkdir();\n-\t\tnew File(objectDirectoryList[0], \"info\").mkdir();\n+\t\tobjectDatabase.create();\n \n \t\tnew File(gitDir, \"branches\").mkdir();\n \t\tnew File(gitDir, \"remotes\").mkdir();\n@@ -187,14 +151,6 @@ public synchronized void create() throws IOException {\n \t\tgetConfig().save();\n \t}\n \n-\tprivate synchronized File[] objectsDirs(){\n-\t\treturn objectDirectoryList;\n-\t}\n-\n-\tprivate synchronized PackFile[] packs(){\n-\t\treturn packFileList;\n-\t}\n-\n \t/**\n \t * @return GIT_DIR\n \t */\n@@ -206,7 +162,7 @@ public File getDirectory() {\n \t * @return the directory containing the objects owned by this repository.\n \t */\n \tpublic File getObjectsDirectory() {\n-\t\treturn objectsDirs()[0];\n+\t\treturn objectDatabase.getDirectory();\n \t}\n \n \t/**\n@@ -227,16 +183,7 @@ public RepositoryConfig getConfig() {\n \t * @return suggested file name\n \t */\n \tpublic File toFile(final AnyObjectId objectId) {\n-\t\tfinal String n = objectId.name();\n-\t\tString d=n.substring(0, 2);\n-\t\tString f=n.substring(2);\n-\t\tfinal File[] objectsDirs = objectsDirs();\n-\t\tfor (File objectsDir : objectsDirs) {\n-\t\t\tFile ret = new File(new File(objectsDir, d), f);\n-\t\t\tif (ret.exists())\n-\t\t\t\treturn ret;\n-\t\t}\n-\t\treturn new File(new File(objectsDirs[0], d), f);\n+\t\treturn objectDatabase.fileFor(objectId);\n \t}\n \n \t/**\n@@ -245,20 +192,7 @@ public File toFile(final AnyObjectId objectId) {\n \t * known shared repositories.\n \t */\n \tpublic boolean hasObject(final AnyObjectId objectId) {\n-\t\tfinal PackFile[] packs = packs();\n-\t\tint k = packs.length;\n-\t\twhile (k > 0) {\n-\t\t\ttry {\n-\t\t\t\tif (packs[--k].hasObject(objectId))\n-\t\t\t\t\treturn true;\n-\t\t\t} catch (IOException e) {\n-\t\t\t\t// Assume that means the pack is invalid, and such\n-\t\t\t\t// packs are treated as though they are empty.\n-\t\t\t\t//\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t}\n-\t\treturn toFile(objectId).isFile();\n+\t\treturn objectDatabase.hasObject(objectId);\n \t}\n \n \t/**\n@@ -291,18 +225,7 @@ public ObjectLoader openObject(final AnyObjectId id)\n \t */\n \tpublic ObjectLoader openObject(final WindowCursor curs, final AnyObjectId id)\n \t\t\tthrows IOException {\n-\t\tfinal PackFile[] packs = packs();\n-\t\tint k = packs.length;\n-\t\twhile (k > 0) {\n-\t\t\tfinal ObjectLoader ol = packs[--k].get(curs, id);\n-\t\t\tif (ol != null)\n-\t\t\t\treturn ol;\n-\t\t}\n-\t\ttry {\n-\t\t\treturn new UnpackedObjectLoader(this, id);\n-\t\t} catch (FileNotFoundException fnfe) {\n-\t\t\treturn null;\n-\t\t}\n+\t\treturn objectDatabase.openObject(curs, id);\n \t}\n \n \t/**\n@@ -339,11 +262,7 @@ public ObjectLoader openObject(final WindowCursor curs, final AnyObjectId id)\n \tvoid openObjectInAllPacks(final AnyObjectId objectId,\n \t\t\tfinal Collection resultLoaders,\n \t\t\tfinal WindowCursor curs) throws IOException {\n-\t\tfor (PackFile pack : packs()) {\n-\t\t\tfinal PackedObjectLoader loader = pack.get(curs, objectId);\n-\t\t\tif (loader != null)\n-\t\t\t\tresultLoaders.add(loader);\n-\t\t}\n+\t\tobjectDatabase.openObjectInAllPacks(resultLoaders, curs, objectId);\n \t}\n \n \t/**\n@@ -772,13 +691,7 @@ private ObjectId resolveSimple(final String revstr) throws IOException {\n \t * Close all resources used by this repository\n \t */\n \tpublic void close() {\n-\t\tclosePacks();\n-\t}\n-\n-\tsynchronized void closePacks() {\n-\t\tfor (int k = packFileList.length - 1; k >= 0; k--)\n-\t\t\tpackFileList[k].close();\n-\t\tpackFileList = new PackFile[0];\n+\t\tobjectDatabase.close();\n \t}\n \n \t/**\n@@ -793,72 +706,7 @@ synchronized void closePacks() {\n \t * a Git pack file index.\n \t */\n \tpublic void openPack(final File pack, final File idx) throws IOException {\n-\t\tfinal String p = pack.getName();\n-\t\tfinal String i = idx.getName();\n-\t\tif (p.length() != 50 || !p.startsWith(\"pack-\") || !p.endsWith(\".pack\"))\n-\t\t throw new IllegalArgumentException(\"Not a valid pack \" + pack);\n-\t\tif (i.length() != 49 || !i.startsWith(\"pack-\") || !i.endsWith(\".idx\"))\n-\t\t throw new IllegalArgumentException(\"Not a valid pack \" + idx);\n-\t\tif (!p.substring(0,45).equals(i.substring(0,45)))\n-\t\t\tthrow new IllegalArgumentException(\"Pack \" + pack\n-\t\t\t\t\t+ \"does not match index \" + idx);\n-\n-\t\tsynchronized (this) {\n-\t\t\tfinal PackFile[] cur = packFileList;\n-\t\t\tfinal PackFile[] arr = new PackFile[cur.length + 1];\n-\t\t\tSystem.arraycopy(cur, 0, arr, 1, cur.length);\n-\t\t\tarr[0] = new PackFile(idx, pack);\n-\t\t\tpackFileList = arr;\n-\t\t}\n-\t}\n-\n-\t/**\n-\t * Scan the object dirs, including alternates for packs\n-\t * to use.\n-\t */\n-\tpublic void scanForPacks() {\n-\t\tfinal ArrayList p = new ArrayList();\n-\t\tp.addAll(Arrays.asList(packs()));\n-\t\tfor (final File d : objectsDirs())\n-\t\t\tscanForPacks(new File(d, \"pack\"), p);\n-\t\tfinal PackFile[] arr = new PackFile[p.size()];\n-\t\tp.toArray(arr);\n-\t\tArrays.sort(arr, PackFile.SORT);\n-\t\tsynchronized (this) {\n-\t\t\tpackFileList = arr;\n-\t\t}\n-\t}\n-\n-\tprivate void scanForPacks(final File packDir, Collection packList) {\n-\t\tfinal String[] idxList = packDir.list(new FilenameFilter() {\n-\t\t\tpublic boolean accept(final File baseDir, final String n) {\n-\t\t\t\t// Must match \"pack-[0-9a-f]{40}.idx\" to be an index.\n-\t\t\t\treturn n.length() == 49 && n.endsWith(\".idx\")\n-\t\t\t\t\t\t&& n.startsWith(\"pack-\");\n-\t\t\t}\n-\t\t});\n-\t\tif (idxList != null) {\n-\t\t\tSCAN: for (final String indexName : idxList) {\n-\t\t\t\tfinal String n = indexName.substring(0, indexName.length() - 4);\n-\t\t\t\tfinal File idxFile = new File(packDir, n + \".idx\");\n-\t\t\t\tfinal File packFile = new File(packDir, n + \".pack\");\n-\n-\t\t\t\tif (!packFile.isFile()) {\n-\t\t\t\t\t// Sometimes C Git's http fetch transport leaves a\n-\t\t\t\t\t// .idx file behind and does not download the .pack.\n-\t\t\t\t\t// We have to skip over such useless indexes.\n-\t\t\t\t\t//\n-\t\t\t\t\tcontinue;\n-\t\t\t\t}\n-\n-\t\t\t\tfor (final PackFile p : packList) {\n-\t\t\t\t\tif (packFile.equals(p.getPackFile()))\n-\t\t\t\t\t\tcontinue SCAN;\n-\t\t\t\t}\n-\n-\t\t\t\tpackList.add(new PackFile(idxFile, packFile));\n-\t\t\t}\n-\t\t}\n+\t\tobjectDatabase.openPack(pack, idx);\n \t}\n \n /**\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectLoader.java b/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectLoader.java\nindex b086821..7552b42 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectLoader.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectLoader.java\n@@ -38,6 +38,7 @@\n \n package org.spearce.jgit.lib;\n \n+import java.io.File;\n import java.io.FileInputStream;\n import java.io.FileNotFoundException;\n import java.io.IOException;\n@@ -60,22 +61,25 @@\n \tprivate final byte[] bytes;\n \n \t/**\n-\t * Construct an ObjectLoader for the specified SHA-1\n+\t * Construct an ObjectLoader to read from the file.\n \t *\n-\t * @param db\n-\t * repository\n+\t * @param path\n+\t * location of the loose object to read.\n \t * @param id\n-\t * SHA-1\n+\t * expected identity of the object being loaded, if known.\n+\t * @throws FileNotFoundException\n+\t * the loose object file does not exist.\n \t * @throws IOException\n+\t * the loose object file exists, but is corrupt.\n \t */\n-\tpublic UnpackedObjectLoader(final Repository db, final AnyObjectId id)\n+\tpublic UnpackedObjectLoader(final File path, final AnyObjectId id)\n \t\t\tthrows IOException {\n-\t\tthis(readCompressed(db, id), id);\n+\t\tthis(readCompressed(path), id);\n \t}\n \n-\tprivate static byte[] readCompressed(final Repository db,\n-\t\t\tfinal AnyObjectId id) throws FileNotFoundException, IOException {\n-\t\tfinal FileInputStream in = new FileInputStream(db.toFile(id));\n+\tprivate static byte[] readCompressed(final File path)\n+\t\t\tthrows FileNotFoundException, IOException {\n+\t\tfinal FileInputStream in = new FileInputStream(path);\n \t\ttry {\n \t\t\tfinal byte[] compressed = new byte[(int) in.getChannel().size()];\n \t\t\tNB.readFully(in, compressed, 0, compressed.length);\n-- \n1.6.3.rc1.188.ga02b\n"},{"id":"111818","messageId":"1240276872-17893-8-git-send-email-spearce@spearce.org","threadId":"18981","inReplyTo":"1240276872-17893-7-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH 07/10] Rescan packs if a pack is modified in-place (part 1)","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-04-21T01:21:09Z","receivedAt":"2009-04-21T01:21:09Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"If a pack file is modified in place it usually means that some other\nprocess has repacked this repository, but the contents of this one\npack remained the same, but the object offsets may be different due\nto different pack creation settings. In such cases we can no longer\nuse the existing PacKFile or PackIndex instances and instead we need\nto create new ones.\n\nThis isn't a full solution to the problem. It is possible for an\napplication to obtain a PackedObjectLoader, see the PackFile get\nclosed due to pressure on the WindowCache, then fail when the pack\nis reopened due to the pack being recreated on disk. PackWriter\nis very susceptible to this, as it caches PackedObjectLoaders for\na long time before it uses them to copy data out of an existing\npack file.\n\nThis particular solution only catches the failure where we had\nopened the index a long time ago, closed the pack, and are now\nopening it again in order to construct a PackedObjectLoader for\nthe caller.\n\nSigned-off-by: Shawn O. Pearce \n---\n .../src/org/spearce/jgit/lib/ObjectDirectory.java | 66 +++++++++++++------\n .../src/org/spearce/jgit/lib/PackFile.java | 20 +++++-\n 2 files changed, 63 insertions(+), 23 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java\nindex e7156c4..36f221e 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java\n@@ -51,6 +51,7 @@\n import java.util.Map;\n import java.util.concurrent.atomic.AtomicReference;\n \n+import org.spearce.jgit.errors.PackMismatchException;\n import org.spearce.jgit.util.FS;\n \n /**\n@@ -190,38 +191,53 @@ protected boolean hasObject1(final AnyObjectId objectId) {\n \t@Override\n \tprotected ObjectLoader openObject1(final WindowCursor curs,\n \t\t\tfinal AnyObjectId objectId) throws IOException {\n-\t\tfor (final PackFile p : packs()) {\n-\t\t\ttry {\n-\t\t\t\tfinal ObjectLoader ldr = p.get(curs, objectId);\n-\t\t\t\tif (ldr != null) {\n-\t\t\t\t\treturn ldr;\n+\t\tPackFile[] pList = packs();\n+\t\tSEARCH: for (;;) {\n+\t\t\tfor (final PackFile p : pList) {\n+\t\t\t\ttry {\n+\t\t\t\t\tfinal ObjectLoader ldr = p.get(curs, objectId);\n+\t\t\t\t\tif (ldr != null) {\n+\t\t\t\t\t\treturn ldr;\n+\t\t\t\t\t}\n+\t\t\t\t} catch (PackMismatchException e) {\n+\t\t\t\t\t// Pack was modified; refresh the entire pack list.\n+\t\t\t\t\t//\n+\t\t\t\t\tpList = scanPacks(pList);\n+\t\t\t\t\tcontinue SEARCH;\n+\t\t\t\t} catch (IOException e) {\n+\t\t\t\t\t// Assume the pack is corrupted.\n+\t\t\t\t\t//\n+\t\t\t\t\tremovePack(p);\n \t\t\t\t}\n-\t\t\t} catch (IOException e) {\n-\t\t\t\t// Assume the pack is corrupted.\n-\t\t\t\t//\n-\t\t\t\tremovePack(p);\n-\t\t\t\tcontinue;\n \t\t\t}\n+\t\t\treturn null;\n \t\t}\n-\t\treturn null;\n \t}\n \n \t@Override\n \tvoid openObjectInAllPacks1(final Collection out,\n \t\t\tfinal WindowCursor curs, final AnyObjectId objectId)\n \t\t\tthrows IOException {\n-\t\tfor (final PackFile p : packs()) {\n-\t\t\ttry {\n-\t\t\t\tfinal PackedObjectLoader ldr = p.get(curs, objectId);\n-\t\t\t\tif (ldr != null) {\n-\t\t\t\t\tout.add(ldr);\n+\t\tPackFile[] pList = packs();\n+\t\tSEARCH: for (;;) {\n+\t\t\tfor (final PackFile p : pList) {\n+\t\t\t\ttry {\n+\t\t\t\t\tfinal PackedObjectLoader ldr = p.get(curs, objectId);\n+\t\t\t\t\tif (ldr != null) {\n+\t\t\t\t\t\tout.add(ldr);\n+\t\t\t\t\t}\n+\t\t\t\t} catch (PackMismatchException e) {\n+\t\t\t\t\t// Pack was modified; refresh the entire pack list.\n+\t\t\t\t\t//\n+\t\t\t\t\tpList = scanPacks(pList);\n+\t\t\t\t\tcontinue SEARCH;\n+\t\t\t\t} catch (IOException e) {\n+\t\t\t\t\t// Assume the pack is corrupted.\n+\t\t\t\t\t//\n+\t\t\t\t\tremovePack(p);\n \t\t\t\t}\n-\t\t\t} catch (IOException e) {\n-\t\t\t\t// Assume the pack is corrupted.\n-\t\t\t\t//\n-\t\t\t\tremovePack(p);\n-\t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tbreak SEARCH;\n \t\t}\n \t}\n \n@@ -350,6 +366,14 @@ synchronized (packList) {\n \tprivate static Map reuseMap(final PackFile[] old) {\n \t\tfinal Map forReuse = new HashMap();\n \t\tfor (final PackFile p : old) {\n+\t\t\tif (p.invalid()) {\n+\t\t\t\t// The pack instance is corrupted, and cannot be safely used\n+\t\t\t\t// again. Do not include it in our reuse map.\n+\t\t\t\t//\n+\t\t\t\tp.close();\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\n \t\t\tfinal PackFile prior = forReuse.put(p.getPackFile().getName(), p);\n \t\t\tif (prior != null) {\n \t\t\t\t// This should never occur. It should be impossible for us\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java\nindex bda4843..74ffef8 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java\n@@ -73,6 +73,8 @@ public int compare(final PackFile a, final PackFile b) {\n \n \tprivate int packLastModified;\n \n+\tprivate volatile boolean invalid;\n+\n \tprivate PackIndex loadedIdx;\n \n \tprivate PackReverseIndex reverseIdx;\n@@ -91,14 +93,24 @@ public PackFile(final File idxFile, final File packFile) {\n \t\tpack = new WindowedFile(packFile) {\n \t\t\t@Override\n \t\t\tprotected void onOpen() throws IOException {\n-\t\t\t\tonOpenPack();\n+\t\t\t\ttry {\n+\t\t\t\t\tonOpenPack();\n+\t\t\t\t} catch (IOException e) {\n+\t\t\t\t\tinvalid = true;\n+\t\t\t\t\tthrow e;\n+\t\t\t\t}\n \t\t\t}\n \t\t};\n \t}\n \n \tprivate synchronized PackIndex idx() throws IOException {\n \t\tif (loadedIdx == null) {\n-\t\t\tloadedIdx = PackIndex.open(idxFile);\n+\t\t\ttry {\n+\t\t\t\tloadedIdx = PackIndex.open(idxFile);\n+\t\t\t} catch (IOException e) {\n+\t\t\t\tinvalid = true;\n+\t\t\t\tthrow e;\n+\t\t\t}\n \t\t}\n \t\treturn loadedIdx;\n \t}\n@@ -267,6 +279,10 @@ boolean supportsFastCopyRawData() throws IOException {\n \t\treturn idx().hasCRC32Support();\n \t}\n \n+\tboolean invalid() {\n+\t\treturn invalid;\n+\t}\n+\n \tprivate void onOpenPack() throws IOException {\n \t\tfinal PackIndex idx = idx();\n \t\tfinal WindowCursor curs = new WindowCursor();\n-- \n1.6.3.rc1.188.ga02b\n"},{"id":"111812","messageId":"1240276872-17893-9-git-send-email-spearce@spearce.org","threadId":"18981","inReplyTo":"1240276872-17893-8-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH 08/10] Scan for new packs if GIT_DIR/objects/pack has been modified","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-04-21T01:21:10Z","receivedAt":"2009-04-21T01:21:10Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"If GIT_DIR/objects/pack directory has changed then one or more packs\nmay have been added to the directory, or removed from it, due to a\nconcurrent application performing a repack. In such cases we need\nto reload the list of available packs before conducting a search.\n\nAs packs are infrequently created or modified relative to searches,\nwe only scan for new packs if our search through existing packs has\nturned up no results.\n\nTransports that create packs locally through IndexPack automatically\nregister their new PackFile, so even those cases will typically not\nrequire scanning the object directory again.\n\nAs misses are fairly common during IndexPack (due to its automatic\ncollision detection logic) we avoid scanning for packs unless the\nGIT_DIR/objects/pack directory has been modified since the last\nscan of it.\n\nSigned-off-by: Shawn O. Pearce \n---\n .../src/org/spearce/jgit/lib/ObjectDatabase.java | 15 ++++++++++++++-\n .../src/org/spearce/jgit/lib/ObjectDirectory.java | 13 +++++++++++++\n 2 files changed, 27 insertions(+), 1 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java\nindex ed1290f..ec228a1 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java\n@@ -137,7 +137,7 @@ private final boolean hasObjectImpl1(final AnyObjectId objectId) {\n \t\t\t\treturn true;\n \t\t\t}\n \t\t}\n-\t\treturn false;\n+\t\treturn tryAgain1() && hasObject1(objectId);\n \t}\n \n \tprivate final boolean hasObjectImpl2(final String objectId) {\n@@ -216,6 +216,12 @@ private ObjectLoader openObjectImpl1(final WindowCursor curs,\n \t\t\t\treturn ldr;\n \t\t\t}\n \t\t}\n+\t\tif (tryAgain1()) {\n+\t\t\tldr = openObject1(curs, objectId);\n+\t\t\tif (ldr != null) {\n+\t\t\t\treturn ldr;\n+\t\t\t}\n+\t\t}\n \t\treturn null;\n \t}\n \n@@ -311,6 +317,13 @@ void openObjectInAllPacks1(Collection out,\n \t}\n \n \t/**\n+\t * @return true if the fast-half search should be tried again.\n+\t */\n+\tprotected boolean tryAgain1() {\n+\t\treturn false;\n+\t}\n+\n+\t/**\n \t * Get the alternate databases known to this database.\n \t * \n \t * @return the alternate list. Never null, but may be an empty array.\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java\nindex 36f221e..d29c63a 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java\n@@ -75,6 +75,8 @@\n \n \tprivate final AtomicReference packList;\n \n+\tprivate volatile long packDirectoryLastModified;\n+\n \t/**\n \t * Initialize a reference to an on-disk object directory.\n \t * \n@@ -257,6 +259,16 @@ protected ObjectLoader openObject2(final WindowCursor curs,\n \t\t}\n \t}\n \n+\t@Override\n+\tprotected boolean tryAgain1() {\n+\t\tfinal PackFile[] old = packList.get();\n+\t\tif (packDirectoryLastModified <= packDirectory.lastModified()) {\n+\t\t\tscanPacks(old);\n+\t\t\treturn true;\n+\t\t}\n+\t\treturn false;\n+\t}\n+\n \tprivate void insertPack(final PackFile pf) {\n \t\tPackFile[] o, n;\n \t\tdo {\n@@ -389,6 +401,7 @@ synchronized (packList) {\n \t}\n \n \tprivate String[] listPackIdx() {\n+\t\tpackDirectoryLastModified = packDirectory.lastModified();\n \t\tfinal String[] idxList = packDirectory.list(new FilenameFilter() {\n \t\t\tpublic boolean accept(final File baseDir, final String n) {\n \t\t\t\t// Must match \"pack-[0-9a-f]{40}.idx\" to be an index.\n-- \n1.6.3.rc1.188.ga02b\n"},{"id":"111817","messageId":"1240276872-17893-10-git-send-email-spearce@spearce.org","threadId":"18981","inReplyTo":"1240276872-17893-9-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH 09/10] Add test cases for loading new (or replaced) pack files","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-04-21T01:21:11Z","receivedAt":"2009-04-21T01:21:11Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Originally-by: Robin Rosenberg \nSigned-off-by: Shawn O. Pearce \n---\n .../org/spearce/jgit/lib/ConcurrentRepackTest.java | 206 ++++++++++++++++++++\n 1 files changed, 206 insertions(+), 0 deletions(-)\n create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/lib/ConcurrentRepackTest.java\n\ndiff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ConcurrentRepackTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ConcurrentRepackTest.java\nnew file mode 100644\nindex 0000000..6eb368c\n--- /dev/null\n+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ConcurrentRepackTest.java\n@@ -0,0 +1,206 @@\n+/*\n+ * Copyright (C) 2009, Robin Rosenberg \n+ * Copyright (C) 2009, Google Inc.\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+\n+package org.spearce.jgit.lib;\n+\n+import java.io.File;\n+import java.io.FileOutputStream;\n+import java.io.IOException;\n+\n+import org.spearce.jgit.errors.IncorrectObjectTypeException;\n+import org.spearce.jgit.errors.MissingObjectException;\n+import org.spearce.jgit.revwalk.RevObject;\n+import org.spearce.jgit.revwalk.RevWalk;\n+\n+public class ConcurrentRepackTest extends RepositoryTestCase {\n+\tpublic void setUp() throws Exception {\n+\t\tWindowCacheConfig windowCacheConfig = new WindowCacheConfig();\n+\t\twindowCacheConfig.setPackedGitOpenFiles(1);\n+\t\tWindowCache.reconfigure(windowCacheConfig);\n+\t\tsuper.setUp();\n+\t}\n+\n+\tprotected void tearDown() throws Exception {\n+\t\tsuper.tearDown();\n+\t\tWindowCacheConfig windowCacheConfig = new WindowCacheConfig();\n+\t\tWindowCache.reconfigure(windowCacheConfig);\n+\t}\n+\n+\tpublic void testObjectInNewPack() throws IncorrectObjectTypeException,\n+\t\t\tIOException {\n+\t\t// Create a new object in a new pack, and test that it is present.\n+\t\t//\n+\t\tfinal Repository eden = createNewEmptyRepo();\n+\t\tfinal RevObject o1 = writeBlob(eden, \"o1\");\n+\t\tpack(eden, o1);\n+\t\tassertEquals(o1.name(), parse(o1).name());\n+\t}\n+\n+\tpublic void testObjectMovedToNewPack1()\n+\t\t\tthrows IncorrectObjectTypeException, IOException {\n+\t\t// Create an object and pack it. Then remove that pack and put the\n+\t\t// object into a different pack file, with some other object. We\n+\t\t// still should be able to access the objects.\n+\t\t//\n+\t\tfinal Repository eden = createNewEmptyRepo();\n+\t\tfinal RevObject o1 = writeBlob(eden, \"o1\");\n+\t\tfinal File[] out1 = pack(eden, o1);\n+\t\tassertEquals(o1.name(), parse(o1).name());\n+\n+\t\tfinal RevObject o2 = writeBlob(eden, \"o2\");\n+\t\tpack(eden, o2, o1);\n+\n+\t\t// Force close, and then delete, the old pack.\n+\t\t//\n+\t\twhackCache();\n+\t\tdelete(out1);\n+\n+\t\t// Now here is the interesting thing. Will git figure the new\n+\t\t// object exists in the new pack, and not the old one.\n+\t\t//\n+\t\tassertEquals(o2.name(), parse(o2).name());\n+\t\tassertEquals(o1.name(), parse(o1).name());\n+\t}\n+\n+\tpublic void testObjectMovedWithinPack()\n+\t\t\tthrows IncorrectObjectTypeException, IOException {\n+\t\t// Create an object and pack it.\n+\t\t//\n+\t\tfinal Repository eden = createNewEmptyRepo();\n+\t\tfinal RevObject o1 = writeBlob(eden, \"o1\");\n+\t\tfinal File[] out1 = pack(eden, o1);\n+\t\tassertEquals(o1.name(), parse(o1).name());\n+\n+\t\t// Force close the old pack.\n+\t\t//\n+\t\twhackCache();\n+\n+\t\t// Now overwrite the old pack in place. This method of creating a\n+\t\t// different pack under the same file name is partially broken. We\n+\t\t// should also have a different file name because the list of objects\n+\t\t// within the pack has been modified.\n+\t\t//\n+\t\tfinal RevObject o2 = writeBlob(eden, \"o2\");\n+\t\tfinal PackWriter pw = new PackWriter(eden, NullProgressMonitor.INSTANCE);\n+\t\tpw.addObject(o2);\n+\t\tpw.addObject(o1);\n+\t\twrite(out1, pw);\n+\n+\t\t// Try the old name, then the new name. The old name should cause the\n+\t\t// pack to reload when it opens and the index and pack mismatch.\n+\t\t//\n+\t\tassertEquals(o1.name(), parse(o1).name());\n+\t\tassertEquals(o2.name(), parse(o2).name());\n+\t}\n+\n+\tprivate static void whackCache() {\n+\t\tfinal WindowCacheConfig config = new WindowCacheConfig();\n+\n+\t\tconfig.setPackedGitOpenFiles(0);\n+\t\tWindowCache.reconfigure(config);\n+\n+\t\tconfig.setPackedGitOpenFiles(1);\n+\t\tWindowCache.reconfigure(config);\n+\t}\n+\n+\tprivate RevObject parse(final AnyObjectId id)\n+\t\t\tthrows MissingObjectException, IOException {\n+\t\treturn new RevWalk(db).parseAny(id);\n+\t}\n+\n+\tprivate File[] pack(final Repository src, final RevObject... list)\n+\t\t\tthrows IOException {\n+\t\tfinal PackWriter pw = new PackWriter(src, NullProgressMonitor.INSTANCE);\n+\t\tfor (final RevObject o : list) {\n+\t\t\tpw.addObject(o);\n+\t\t}\n+\n+\t\tfinal ObjectId name = pw.computeName();\n+\t\tfinal File packFile = fullPackFileName(name, \".pack\");\n+\t\tfinal File idxFile = fullPackFileName(name, \".idx\");\n+\t\tfinal File[] files = new File[] { packFile, idxFile };\n+\t\twrite(files, pw);\n+\t\treturn files;\n+\t}\n+\n+\tprivate static void write(final File[] files, final PackWriter pw)\n+\t\t\tthrows IOException {\n+\t\tFileOutputStream out;\n+\n+\t\tout = new FileOutputStream(files[0]);\n+\t\ttry {\n+\t\t\tpw.writePack(out);\n+\t\t} finally {\n+\t\t\tout.close();\n+\t\t}\n+\n+\t\tout = new FileOutputStream(files[1]);\n+\t\ttry {\n+\t\t\tpw.writeIndex(out);\n+\t\t} finally {\n+\t\t\tout.close();\n+\t\t}\n+\t}\n+\n+\tprivate static void delete(final File[] list) {\n+\t\tfor (final File f : list) {\n+\t\t\tf.delete();\n+\t\t\tassertFalse(f + \" was removed\", f.exists());\n+\t\t}\n+\t}\n+\n+\tprivate File fullPackFileName(final ObjectId name, final String suffix) {\n+\t\tfinal File packdir = new File(db.getObjectsDirectory(), \"pack\");\n+\t\treturn new File(packdir, \"pack-\" + name.name() + suffix);\n+\t}\n+\n+\tprivate RevObject writeBlob(final Repository repo, final String data)\n+\t\t\tthrows IOException {\n+\t\tfinal RevWalk revWalk = new RevWalk(repo);\n+\t\tfinal byte[] bytes = data.getBytes(Constants.CHARACTER_ENCODING);\n+\t\tfinal ObjectWriter ow = new ObjectWriter(repo);\n+\t\tfinal ObjectId id = ow.writeBlob(bytes);\n+\t\ttry {\n+\t\t\tparse(id);\n+\t\t\tfail(\"Object \" + id.name() + \" should not exist in test repository\");\n+\t\t} catch (MissingObjectException e) {\n+\t\t\t// Ok\n+\t\t}\n+\t\treturn revWalk.lookupBlob(id);\n+\t}\n+}\n-- \n1.6.3.rc1.188.ga02b\n"},{"id":"111816","messageId":"1240276872-17893-11-git-send-email-spearce@spearce.org","threadId":"18981","inReplyTo":"1240276872-17893-10-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH 10/10] BROKEN TEST: ObjectLoader stays valid across repacks","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-04-21T01:21:12Z","receivedAt":"2009-04-21T01:21:12Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"This doesn't doesn't work.\n\nWhat we are trying to verify is that an ObjectLoader remains valid\nif the underlying storage for the object has moved, such as when a\nrepository is repacked, the old pack was deleted, and the object is\nnow in the new pack.\n---\n .../org/spearce/jgit/lib/ConcurrentRepackTest.java | 46 ++++++++++++++++++++\n 1 files changed, 46 insertions(+), 0 deletions(-)\n\ndiff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ConcurrentRepackTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ConcurrentRepackTest.java\nindex 6eb368c..b1b1af2 100644\n--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ConcurrentRepackTest.java\n+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ConcurrentRepackTest.java\n@@ -41,6 +41,7 @@\n import java.io.File;\n import java.io.FileOutputStream;\n import java.io.IOException;\n+import java.util.Arrays;\n \n import org.spearce.jgit.errors.IncorrectObjectTypeException;\n import org.spearce.jgit.errors.MissingObjectException;\n@@ -128,6 +129,51 @@ public void testObjectMovedWithinPack()\n \t\tassertEquals(o2.name(), parse(o2).name());\n \t}\n \n+\tpublic void testObjectMovedToNewPack2()\n+\t\t\tthrows IncorrectObjectTypeException, IOException {\n+\t\t// Create an object and pack it. Then remove that pack and put the\n+\t\t// object into a different pack file, with some other object. We\n+\t\t// still should be able to access the objects.\n+\t\t//\n+\t\tfinal Repository eden = createNewEmptyRepo();\n+\t\tfinal RevObject o1 = writeBlob(eden, \"o1\");\n+\t\tfinal File[] out1 = pack(eden, o1);\n+\t\tassertEquals(o1.name(), parse(o1).name());\n+\n+\t\tfinal ObjectLoader load1 = db.openBlob(o1);\n+\t\tassertNotNull(load1);\n+\n+\t\tfinal RevObject o2 = writeBlob(eden, \"o2\");\n+\t\tpack(eden, o2, o1);\n+\n+\t\t// Force close, and then delete, the old pack.\n+\t\t//\n+\t\twhackCache();\n+\t\tdelete(out1);\n+\n+\t\t// Now here is the interesting thing... can the loader we made\n+\t\t// earlier still resolve the object, even though its underlying\n+\t\t// pack is gone, but the object still exists.\n+\t\t//\n+\t\tfinal ObjectLoader load2 = db.openBlob(o1);\n+\t\tassertNotNull(load2);\n+\t\tassertNotSame(load1, load2);\n+\n+\t\t// Currently load1.getCachedBytes NPEs due to the openCount == 1,\n+\t\t// but fd == null. Aside from the underlying pack being gone, the\n+\t\t// WindowCache didn't reset the openCount to 0 when it closed the\n+\t\t// file during the mass cache eviction. Thus we never tried to do\n+\t\t// an subsequent open.\n+\t\t//\n+\t\tfinal byte[] data2 = load2.getCachedBytes();\n+\t\tfinal byte[] data1 = load1.getCachedBytes();\n+\t\tassertNotNull(data2);\n+\t\tassertNotNull(data1);\n+\t\tassertNotSame(data1, data2); // cache should be per-pack, not per object\n+\t\tassertTrue(Arrays.equals(data1, data2));\n+\t\tassertEquals(load2.getType(), load1.getType());\n+\t}\n+\n \tprivate static void whackCache() {\n \t\tfinal WindowCacheConfig config = new WindowCacheConfig();\n \n-- \n1.6.3.rc1.188.ga02b\n"},{"id":"111820","messageId":"20090421013911.GZ23604@spearce.org","threadId":"18981","inReplyTo":"1240276872-17893-9-git-send-email-spearce@spearce.org","subject":"Re: [JGIT PATCH 08/10] Scan for new packs if GIT_DIR/objects/pack has been modified","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-04-21T01:39:11Z","receivedAt":"2009-04-21T01:39:11Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"\"Shawn O. Pearce\" wrote:\n> If GIT_DIR/objects/pack directory has changed then one or more packs\n...\n> +\t@Override\n> +\tprotected boolean tryAgain1() {\n> +\t\tfinal PackFile[] old = packList.get();\n> +\t\tif (packDirectoryLastModified <= packDirectory.lastModified()) {\n\nSadly we needed to use <= here to avoid racy-git cases. Worse,\nunlike with the DirCache we don't have another file system inode\nwe can use to compare timestamps against.\n\nConsequently, I just realized my commit message isn't entirely\naccurate. In reality we rescan the #F*@!*! directory every time\nwe have a miss, because the time stamp hasn't changed.\n\nI can't think of another method to use here. I'd love to avoid\nthe rescan on a miss if the aren't any new packs in the directory,\nbut I just don't see how we can do that and avoid the racy-git case\nat the same time.\n\nWe could try to fudge it by saying < here, but the test cases in\n9/10 would then require a Thread.sleep(2) or something to force\nenough time to pass for the directory modification time to advance.\n\nI think in real applications, its \"good enough\" to allow this amount\nof fudging. We shouldn't see more than one repack per repository\nper FS clock tick. And if we do, the admin should be shot on sight.\n\ngit-repack is careful to ensure the new pack is linked into the\ndirectory before the old packs are unlinked, so we really shouldn't\never scan and miss a pack.\n\n-- \nShawn.\n"},{"id":"111822","messageId":"20090421020807.GA23604@spearce.org","threadId":"18981","inReplyTo":"20090421013911.GZ23604@spearce.org","subject":"[JGIT PATCH v2 08/10] Scan for new packs if GIT_DIR/objects/pack has been modified","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-04-21T02:08:07Z","receivedAt":"2009-04-21T02:08:07Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"If GIT_DIR/objects/pack directory has changed then one or more packs\nmay have been added to the directory, or removed from it, due to a\nconcurrent application performing a repack. In such cases we need\nto reload the list of available packs before conducting a search.\n\nAs packs are infrequently created or modified relative to searches,\nwe only scan for new packs if our search through existing packs has\nturned up no results.\n\nTransports that create packs locally through IndexPack automatically\nregister their new PackFile, so even those cases will typically not\nrequire scanning the object directory again.\n\nAs misses are fairly common during IndexPack (due to its automatic\ncollision detection logic) we avoid scanning for packs unless the\nGIT_DIR/objects/pack directory has been modified since the last\nscan of it.\n\nSigned-off-by: Shawn O. Pearce \n---\n\n \"Shawn O. Pearce\" wrote:\n > \n > We could try to fudge it by saying < here, [...]\n > I think in real applications, its \"good enough\" to allow this amount\n > of fudging.\n\n And this version makes it so. The interdiff is just <= changed to <.\n\n .../src/org/spearce/jgit/lib/ObjectDatabase.java | 15 ++++++++++++++-\n .../src/org/spearce/jgit/lib/ObjectDirectory.java | 13 +++++++++++++\n 2 files changed, 27 insertions(+), 1 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java\nindex ed1290f..ec228a1 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java\n@@ -137,7 +137,7 @@ private final boolean hasObjectImpl1(final AnyObjectId objectId) {\n \t\t\t\treturn true;\n \t\t\t}\n \t\t}\n-\t\treturn false;\n+\t\treturn tryAgain1() && hasObject1(objectId);\n \t}\n \n \tprivate final boolean hasObjectImpl2(final String objectId) {\n@@ -216,6 +216,12 @@ private ObjectLoader openObjectImpl1(final WindowCursor curs,\n \t\t\t\treturn ldr;\n \t\t\t}\n \t\t}\n+\t\tif (tryAgain1()) {\n+\t\t\tldr = openObject1(curs, objectId);\n+\t\t\tif (ldr != null) {\n+\t\t\t\treturn ldr;\n+\t\t\t}\n+\t\t}\n \t\treturn null;\n \t}\n \n@@ -311,6 +317,13 @@ void openObjectInAllPacks1(Collection out,\n \t}\n \n \t/**\n+\t * @return true if the fast-half search should be tried again.\n+\t */\n+\tprotected boolean tryAgain1() {\n+\t\treturn false;\n+\t}\n+\n+\t/**\n \t * Get the alternate databases known to this database.\n \t * \n \t * @return the alternate list. Never null, but may be an empty array.\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java\nindex 36f221e..6ba0180 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java\n@@ -75,6 +75,8 @@\n \n \tprivate final AtomicReference packList;\n \n+\tprivate volatile long packDirectoryLastModified;\n+\n \t/**\n \t * Initialize a reference to an on-disk object directory.\n \t * \n@@ -257,6 +259,16 @@ protected ObjectLoader openObject2(final WindowCursor curs,\n \t\t}\n \t}\n \n+\t@Override\n+\tprotected boolean tryAgain1() {\n+\t\tfinal PackFile[] old = packList.get();\n+\t\tif (packDirectoryLastModified < packDirectory.lastModified()) {\n+\t\t\tscanPacks(old);\n+\t\t\treturn true;\n+\t\t}\n+\t\treturn false;\n+\t}\n+\n \tprivate void insertPack(final PackFile pf) {\n \t\tPackFile[] o, n;\n \t\tdo {\n@@ -389,6 +401,7 @@ synchronized (packList) {\n \t}\n \n \tprivate String[] listPackIdx() {\n+\t\tpackDirectoryLastModified = packDirectory.lastModified();\n \t\tfinal String[] idxList = packDirectory.list(new FilenameFilter() {\n \t\t\tpublic boolean accept(final File baseDir, final String n) {\n \t\t\t\t// Must match \"pack-[0-9a-f]{40}.idx\" to be an index.\n-- \n1.6.3.rc1.188.ga02b\n"},{"id":"111823","messageId":"20090421021035.GB23604@spearce.org","threadId":"18981","inReplyTo":"1240276872-17893-10-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH v2 09/10] Add test cases for loading new (or replaced) pack files","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-04-21T02:10:35Z","receivedAt":"2009-04-21T02:10:35Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Originally-by: Robin Rosenberg \nSigned-off-by: Shawn O. Pearce \n---\n\n Matches with v2 08/10. Tests take a few seconds longer now as\n we have to delay at least 1 second on most filesystems; longer if\n their modification timestamp granularity is larger than 1 second.\n\n .../org/spearce/jgit/lib/ConcurrentRepackTest.java | 222 ++++++++++++++++++++\n 1 files changed, 222 insertions(+), 0 deletions(-)\n create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/lib/ConcurrentRepackTest.java\n\ndiff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ConcurrentRepackTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ConcurrentRepackTest.java\nnew file mode 100644\nindex 0000000..825fbb8\n--- /dev/null\n+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ConcurrentRepackTest.java\n@@ -0,0 +1,222 @@\n+/*\n+ * Copyright (C) 2009, Robin Rosenberg \n+ * Copyright (C) 2009, Google Inc.\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+\n+package org.spearce.jgit.lib;\n+\n+import java.io.File;\n+import java.io.FileOutputStream;\n+import java.io.IOException;\n+\n+import org.spearce.jgit.errors.IncorrectObjectTypeException;\n+import org.spearce.jgit.errors.MissingObjectException;\n+import org.spearce.jgit.revwalk.RevObject;\n+import org.spearce.jgit.revwalk.RevWalk;\n+\n+public class ConcurrentRepackTest extends RepositoryTestCase {\n+\tpublic void setUp() throws Exception {\n+\t\tWindowCacheConfig windowCacheConfig = new WindowCacheConfig();\n+\t\twindowCacheConfig.setPackedGitOpenFiles(1);\n+\t\tWindowCache.reconfigure(windowCacheConfig);\n+\t\tsuper.setUp();\n+\t}\n+\n+\tprotected void tearDown() throws Exception {\n+\t\tsuper.tearDown();\n+\t\tWindowCacheConfig windowCacheConfig = new WindowCacheConfig();\n+\t\tWindowCache.reconfigure(windowCacheConfig);\n+\t}\n+\n+\tpublic void testObjectInNewPack() throws IncorrectObjectTypeException,\n+\t\t\tIOException {\n+\t\t// Create a new object in a new pack, and test that it is present.\n+\t\t//\n+\t\tfinal Repository eden = createNewEmptyRepo();\n+\t\tfinal RevObject o1 = writeBlob(eden, \"o1\");\n+\t\tpack(eden, o1);\n+\t\tassertEquals(o1.name(), parse(o1).name());\n+\t}\n+\n+\tpublic void testObjectMovedToNewPack1()\n+\t\t\tthrows IncorrectObjectTypeException, IOException {\n+\t\t// Create an object and pack it. Then remove that pack and put the\n+\t\t// object into a different pack file, with some other object. We\n+\t\t// still should be able to access the objects.\n+\t\t//\n+\t\tfinal Repository eden = createNewEmptyRepo();\n+\t\tfinal RevObject o1 = writeBlob(eden, \"o1\");\n+\t\tfinal File[] out1 = pack(eden, o1);\n+\t\tassertEquals(o1.name(), parse(o1).name());\n+\n+\t\tfinal RevObject o2 = writeBlob(eden, \"o2\");\n+\t\tpack(eden, o2, o1);\n+\n+\t\t// Force close, and then delete, the old pack.\n+\t\t//\n+\t\twhackCache();\n+\t\tdelete(out1);\n+\n+\t\t// Now here is the interesting thing. Will git figure the new\n+\t\t// object exists in the new pack, and not the old one.\n+\t\t//\n+\t\tassertEquals(o2.name(), parse(o2).name());\n+\t\tassertEquals(o1.name(), parse(o1).name());\n+\t}\n+\n+\tpublic void testObjectMovedWithinPack()\n+\t\t\tthrows IncorrectObjectTypeException, IOException {\n+\t\t// Create an object and pack it.\n+\t\t//\n+\t\tfinal Repository eden = createNewEmptyRepo();\n+\t\tfinal RevObject o1 = writeBlob(eden, \"o1\");\n+\t\tfinal File[] out1 = pack(eden, o1);\n+\t\tassertEquals(o1.name(), parse(o1).name());\n+\n+\t\t// Force close the old pack.\n+\t\t//\n+\t\twhackCache();\n+\n+\t\t// Now overwrite the old pack in place. This method of creating a\n+\t\t// different pack under the same file name is partially broken. We\n+\t\t// should also have a different file name because the list of objects\n+\t\t// within the pack has been modified.\n+\t\t//\n+\t\tfinal RevObject o2 = writeBlob(eden, \"o2\");\n+\t\tfinal PackWriter pw = new PackWriter(eden, NullProgressMonitor.INSTANCE);\n+\t\tpw.addObject(o2);\n+\t\tpw.addObject(o1);\n+\t\twrite(out1, pw);\n+\n+\t\t// Try the old name, then the new name. The old name should cause the\n+\t\t// pack to reload when it opens and the index and pack mismatch.\n+\t\t//\n+\t\tassertEquals(o1.name(), parse(o1).name());\n+\t\tassertEquals(o2.name(), parse(o2).name());\n+\t}\n+\n+\tprivate static void whackCache() {\n+\t\tfinal WindowCacheConfig config = new WindowCacheConfig();\n+\n+\t\tconfig.setPackedGitOpenFiles(0);\n+\t\tWindowCache.reconfigure(config);\n+\n+\t\tconfig.setPackedGitOpenFiles(1);\n+\t\tWindowCache.reconfigure(config);\n+\t}\n+\n+\tprivate RevObject parse(final AnyObjectId id)\n+\t\t\tthrows MissingObjectException, IOException {\n+\t\treturn new RevWalk(db).parseAny(id);\n+\t}\n+\n+\tprivate File[] pack(final Repository src, final RevObject... list)\n+\t\t\tthrows IOException {\n+\t\tfinal PackWriter pw = new PackWriter(src, NullProgressMonitor.INSTANCE);\n+\t\tfor (final RevObject o : list) {\n+\t\t\tpw.addObject(o);\n+\t\t}\n+\n+\t\tfinal ObjectId name = pw.computeName();\n+\t\tfinal File packFile = fullPackFileName(name, \".pack\");\n+\t\tfinal File idxFile = fullPackFileName(name, \".idx\");\n+\t\tfinal File[] files = new File[] { packFile, idxFile };\n+\t\twrite(files, pw);\n+\t\treturn files;\n+\t}\n+\n+\tprivate static void write(final File[] files, final PackWriter pw)\n+\t\t\tthrows IOException {\n+\t\tfinal long begin = files[0].getParentFile().lastModified();\n+\t\tFileOutputStream out;\n+\n+\t\tout = new FileOutputStream(files[0]);\n+\t\ttry {\n+\t\t\tpw.writePack(out);\n+\t\t} finally {\n+\t\t\tout.close();\n+\t\t}\n+\n+\t\tout = new FileOutputStream(files[1]);\n+\t\ttry {\n+\t\t\tpw.writeIndex(out);\n+\t\t} finally {\n+\t\t\tout.close();\n+\t\t}\n+\n+\t\ttouch(begin, files[0].getParentFile());\n+\t}\n+\n+\tprivate static void delete(final File[] list) {\n+\t\tfinal long begin = list[0].getParentFile().lastModified();\n+\t\tfor (final File f : list) {\n+\t\t\tf.delete();\n+\t\t\tassertFalse(f + \" was removed\", f.exists());\n+\t\t}\n+\t\ttouch(begin, list[0].getParentFile());\n+\t}\n+\n+\tprivate static void touch(final long begin, final File dir) {\n+\t\twhile (begin >= dir.lastModified()) {\n+\t\t\ttry {\n+\t\t\t\tThread.sleep(25);\n+\t\t\t} catch (InterruptedException ie) {\n+\t\t\t\t//\n+\t\t\t}\n+\t\t\tdir.setLastModified(System.currentTimeMillis());\n+\t\t}\n+\t}\n+\n+\tprivate File fullPackFileName(final ObjectId name, final String suffix) {\n+\t\tfinal File packdir = new File(db.getObjectsDirectory(), \"pack\");\n+\t\treturn new File(packdir, \"pack-\" + name.name() + suffix);\n+\t}\n+\n+\tprivate RevObject writeBlob(final Repository repo, final String data)\n+\t\t\tthrows IOException {\n+\t\tfinal RevWalk revWalk = new RevWalk(repo);\n+\t\tfinal byte[] bytes = data.getBytes(Constants.CHARACTER_ENCODING);\n+\t\tfinal ObjectWriter ow = new ObjectWriter(repo);\n+\t\tfinal ObjectId id = ow.writeBlob(bytes);\n+\t\ttry {\n+\t\t\tparse(id);\n+\t\t\tfail(\"Object \" + id.name() + \" should not exist in test repository\");\n+\t\t} catch (MissingObjectException e) {\n+\t\t\t// Ok\n+\t\t}\n+\t\treturn revWalk.lookupBlob(id);\n+\t}\n+}\n-- \n1.6.3.rc1.188.ga02b\n"},{"id":"111910","messageId":"200904220116.51076.robin.rosenberg.lists@dewire.com","threadId":"18981","inReplyTo":"1240276872-17893-11-git-send-email-spearce@spearce.org","subject":"Re: [JGIT PATCH 10/10] BROKEN TEST: ObjectLoader stays valid across repacks","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg.lists@dewire.com","sentAt":"2009-04-21T23:16:50Z","receivedAt":"2009-04-21T23:16:50Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"tisdag 21 april 2009 03:21:12 skrev \"Shawn O. Pearce\" :\n> This doesn't doesn't work.\n> \n> What we are trying to verify is that an ObjectLoader remains valid\n> if the underlying storage for the object has moved, such as when a\n> repository is repacked, the old pack was deleted, and the object is\n> now in the new pack.\n\nSo, I had an idea and started hacking and suddenly the supposedly ok cases\nstarted crashing like this.\n\norg.spearce.jgit.errors.MissingObjectException: Missing unknown 4a75554761c96be80602c05145d1ef41c77e1b72\n\tat org.spearce.jgit.revwalk.RevWalk.parseAny(RevWalk.java:704)\n\tat org.spearce.jgit.lib.ConcurrentRepackTest.parse(ConcurrentRepackTest.java:189)\n\tat org.spearce.jgit.lib.ConcurrentRepackTest.testObjectMovedWithinPack(ConcurrentRepackTest.java:129)\n\tat sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)\n\tat sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)\n\tat sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)\n\tat java.lang.reflect.Method.invoke(Method.java:597)\n\tat junit.framework.TestCase.runTest(TestCase.java:164)\n\tat junit.framework.TestCase.runBare(TestCase.java:130)\n\tat junit.framework.TestResult$1.protect(TestResult.java:106)\n\tat junit.framework.TestResult.runProtected(TestResult.java:124)\n\tat junit.framework.TestResult.run(TestResult.java:109)\n\tat junit.framework.TestCase.run(TestCase.java:120)\n\tat org.eclipse.jdt.internal.junit.runner.junit3.JUnit3TestReference.run(JUnit3TestReference.java:130)\n\tat org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)\n\tat org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:460)\n\tat org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:673)\n\tat org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:386)\n\tat org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:196)\n\nAfter pruning the other changes, it still crashes , even with mvn clean test\n\nHashCode /home/me/SW/EGIT.contrib/org.spearce.jgit.test/trash/trash1240354367548.0/.git/objects/pack/pack-34be9032ac282b11fa9babdc2b2a93ca996c9c2f.pack = 327983069\nHashCode /home/me/SW/EGIT.contrib/org.spearce.jgit.test/trash/trash1240354367548.0/.git/objects/pack/pack-df2982f284bbabb6bdb59ee3fcc6eb0983e20371.pack = 458811222\nHashCode /home/me/SW/EGIT.contrib/org.spearce.jgit.test/trash/trash1240354367548.0/.git/objects/pack/pack-9fb5b411fe6dfa89cc2e6b89d2bd8e5de02b5745.pack = 331392325\nHashCode /home/me/SW/EGIT.contrib/org.spearce.jgit.test/trash/trash1240354367548.0/.git/objects/pack/pack-546ff360fe3488adb20860ce3436a2d6373d2796.pack = 547821429\nHashCode /home/me/SW/EGIT.contrib/org.spearce.jgit.test/trash/trash1240354367548.0/.git/objects/pack/pack-e6d07037cbcf13376308a0a995d1fa48f8f76aaa.pack = 536578194\nHashCode /home/me/SW/EGIT.contrib/org.spearce.jgit.test/trash/trash1240354367548.0/.git/objects/pack/pack-3280af9c07ee18a87705ef50b0cc4cd20266cf12.pack = 885722359\nHashCode /home/me/SW/EGIT.contrib/org.spearce.jgit.test/trash/trash1240354367548.0/.git/objects/pack/pack-146aac1f6a11aed8e06af7d9f5cc1b4b8beedb36.pack = 830203467\n\nThe hash codes printed are the same everytime it crashes, removing the invalid flags will create these codes and the test succeeds.\n\nHashCode /home/me/SW/EGIT.contrib/org.spearce.jgit.test/trash/trash1240354568171.0/.git/objects/pack/pack-34be9032ac282b11fa9babdc2b2a93ca996c9c2f.pack = 327983069\nHashCode /home/me/SW/EGIT.contrib/org.spearce.jgit.test/trash/trash1240354568171.0/.git/objects/pack/pack-df2982f284bbabb6bdb59ee3fcc6eb0983e20371.pack = 458811222\nHashCode /home/me/SW/EGIT.contrib/org.spearce.jgit.test/trash/trash1240354568171.0/.git/objects/pack/pack-9fb5b411fe6dfa89cc2e6b89d2bd8e5de02b5745.pack = 331392325\nHashCode /home/me/SW/EGIT.contrib/org.spearce.jgit.test/trash/trash1240354568171.0/.git/objects/pack/pack-546ff360fe3488adb20860ce3436a2d6373d2796.pack = 547821429\nHashCode /home/me/SW/EGIT.contrib/org.spearce.jgit.test/trash/trash1240354568171.0/.git/objects/pack/pack-e6d07037cbcf13376308a0a995d1fa48f8f76aaa.pack = 536578194\nHashCode /home/me/SW/EGIT.contrib/org.spearce.jgit.test/trash/trash1240354568171.0/.git/objects/pack/pack-3280af9c07ee18a87705ef50b0cc4cd20266cf12.pack = 885722359\nHashCode /home/me/SW/EGIT.contrib/org.spearce.jgit.test/trash/trash1240354568171.0/.git/objects/pack/pack-146aac1f6a11aed8e06af7d9f5cc1b4b8beedb36.pack = 830203467\n\nOne cannot obviously assume they are the same, but the numbers might be a lead to why it crashes here. Looks\nlike as hash collision and a failure of the \"equals\" part to distinguish different WindowedFile's.\n\n-- robin\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java\nindex 9293eb9..f9e1991 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java\n@@ -80,6 +80,8 @@\n \t/** Total number of windows actively in the associated cache. */\n \tint openCount;\n \n+\tboolean invalid;\n+\n \t/**\n \t * Open a file for reading through window caching.\n \t * \n@@ -93,6 +95,10 @@ public WindowedFile(final File file) {\n \t\t// value in WindowCache.hash(), without doing the multiply there.\n \t\t//\n \t\thash = System.identityHashCode(this) * 31;\n+\t\tSystem.out.print(\"HashCode \");\n+\t\tSystem.out.print(file.getPath());\n+\t\tSystem.out.print(\" = \");\n+\t\tSystem.out.println(hash);\n \t\tlength = Long.MAX_VALUE;\n \t}\n \n"},{"id":"111963","messageId":"20090422163349.GN23604@spearce.org","threadId":"18981","inReplyTo":"200904220116.51076.robin.rosenberg.lists@dewire.com","subject":"Re: [JGIT PATCH 10/10] BROKEN TEST: ObjectLoader stays valid across repacks","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-04-22T16:33:50Z","receivedAt":"2009-04-22T16:33:50Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Robin Rosenberg wrote:\n> So, I had an idea and started hacking and suddenly the supposedly ok cases\n> started crashing like this.\n> \n> org.spearce.jgit.errors.MissingObjectException: Missing unknown 4a75554761c96be80602c05145d1ef41c77e1b72\n\nYou broke it! :-)\n\n> The hash codes printed are the same everytime it crashes, removing\n> the invalid flags will create these codes and the test succeeds.\n\nI am not surprised. The hash codes are derived from the system\nidentity hash code for the object, which are assigned using a rather\ndeterministic algorithm. Given the same sequence of tests through\nthe JVM and the same heap state, you are likely going to have the\nsame hash code for the same objects every run.\n\nActually, I think you missed it, but the two sets of hash codes\nyou printed here in the email are identical.\n \n> One cannot obviously assume they are the same, but the numbers might be a lead to why it crashes here. Looks\n> like as hash collision and a failure of the \"equals\" part to distinguish different WindowedFile's.\n\nNope, that's not it. We never use .equals() on WindowedFile,\nand we never use that hash field you were printing as any part\nof an equality computation. That hash field is mixed with the\noffset of a window to hash it into the WindowCache. We *always*\nuse reference equality (\"foo.wp == wp\") to test a WindowedFile.\n\n> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java\n> index 9293eb9..f9e1991 100644\n> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java\n> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java\n> @@ -80,6 +80,8 @@\n> \t/** Total number of windows actively in the associated cache. */\n> \tint openCount;\n> \n> +\tboolean invalid;\n\nThis shadowed the \"invalid\" field in PackFile, causing the anonymous\ninner class on l.93-103 that subclasses WindowedFile to write to\nthis new field, while PackFile still read from its own field.\n\nIf you had called this \"invalid2\", or marked it private, you\nwouldn't have seen the test failure.\n\n\nFWIW, I've decided to merge PackFile and WindowedFile together.\nThere is no point in keeping them separate anymore. We used to\nuse WindowedFile for both the .pack and the .idx, but long ago\nyou showed it was faster to load the entire .idx into memory in\nthe PackIndex structure. And this split is causing confusion,\nlike the one you just stepped in.\n\n-- \nShawn.\n"},{"id":"112027","messageId":"20090422230220.GQ23604@spearce.org","threadId":"18981","inReplyTo":"200904220116.51076.robin.rosenberg.lists@dewire.com","subject":"Re: [JGIT PATCH 10/10] BROKEN TEST: ObjectLoader stays valid across repacks","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-04-22T23:02:20Z","receivedAt":"2009-04-22T23:02:20Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Robin Rosenberg wrote:\n> tisdag 21 april 2009 03:21:12 skrev \"Shawn O. Pearce\" :\n> > This doesn't doesn't work.\n> > \n> > What we are trying to verify is that an ObjectLoader remains valid\n> > if the underlying storage for the object has moved, such as when a\n> > repository is repacked, the old pack was deleted, and the object is\n> > now in the new pack.\n> \n> So, I had an idea and started hacking [...]\n\nYea, I also have an idea. I'm working up a patch series that should\nfix this bug. I'll post it later this evening once its complete\nand passes tests.\n\n-- \nShawn.\n"}]}