Fix cloning of repositories with big objects

When running IndexPack we use a CachedObjectDirectory, which
knows what objects are loose and tries to avoid stat(2) calls for
objects that do not exist in the repository, as stat(2) on Win32
is very slow.

However large delta objects found in a pack file are expanded into
a loose object, in order to avoid costly delta chain processing
when that object is used as a base for another delta.

If this expand occurs while working with the CachedObjectDirectory,
we need to update the cached directory data to include this new
object, otherwise it won't be available when we try to open it
during the object verify phase.

Bug: 324868
Change-Id: Idf0c76d4849d69aa415ead32e46a435622395d68
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/CachedObjectDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/CachedObjectDirectory.java
index f0159f626fc629e809bcc269f2163ae62df7d742..a5762b61ee8c0303a599f5ef29bd14e861d1f5ee 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/CachedObjectDirectory.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/CachedObjectDirectory.java
@@ -213,8 +213,22 @@ class CachedObjectDirectory extends FileObjectDatabase {
        }
 
        @Override
-       boolean insertUnpackedObject(File tmp, ObjectId objectId, boolean force) {
-               return wrapped.insertUnpackedObject(tmp, objectId, force);
+       InsertLooseObjectResult insertUnpackedObject(File tmp, ObjectId objectId,
+                       boolean createDuplicate) {
+               InsertLooseObjectResult result = wrapped.insertUnpackedObject(tmp,
+                               objectId, createDuplicate);
+               switch (result) {
+               case INSERTED:
+               case EXISTS_LOOSE:
+                       if (!unpackedObjects.contains(objectId))
+                               unpackedObjects.add(objectId);
+                       break;
+
+               case EXISTS_PACKED:
+               case FAILURE:
+                       break;
+               }
+               return result;
        }
 
        @Override

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileObjectDatabase.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileObjectDatabase.java
index 29c7a25312b6231d4184caba5f86567d0bbe0fb3..8bd3751010e76e746b6206a579a683c40da14bde 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileObjectDatabase.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileObjectDatabase.java
@@ -57,6 +57,10 @@ import org.eclipse.jgit.storage.pack.ObjectToPack;
 import org.eclipse.jgit.storage.pack.PackWriter;
 
 abstract class FileObjectDatabase extends ObjectDatabase {
+       static enum InsertLooseObjectResult {
+               INSERTED, EXISTS_PACKED, EXISTS_LOOSE, FAILURE;
+       }
+
        @Override
        public ObjectReader newReader() {
                return new WindowCursor(this);
@@ -249,7 +253,8 @@ abstract class FileObjectDatabase extends ObjectDatabase {
        abstract long getObjectSize2(WindowCursor curs, String objectName,
                        AnyObjectId objectId) throws IOException;
 
-       abstract boolean insertUnpackedObject(File tmp, ObjectId id, boolean force);
+       abstract InsertLooseObjectResult insertUnpackedObject(File tmp,
+                       ObjectId id, boolean createDuplicate);
 
        abstract FileObjectDatabase newCachedFileObjectDatabase();
 

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/ObjectDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/ObjectDirectory.java
index 372a97813e3889498cc7981403f8f5c3fca5bab1..e7ccba0820fb38344c23bc16f259dccef1049c17 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/ObjectDirectory.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/ObjectDirectory.java
@@ -455,23 +455,33 @@ public class ObjectDirectory extends FileObjectDatabase {
        }
 
        @Override
-       boolean insertUnpackedObject(File tmp, ObjectId id, boolean force) {
-               if (!force && has(id)) {
-                       // Object is already in the repository, remove temporary file.
-                       //
+       InsertLooseObjectResult insertUnpackedObject(File tmp, ObjectId id,
+                       boolean createDuplicate) {
+               // If the object is already in the repository, remove temporary file.
+               //
+               if (unpackedObjectCache.isUnpacked(id)) {
                        tmp.delete();
-                       return true;
+                       return InsertLooseObjectResult.EXISTS_LOOSE;
                }
+               if (!createDuplicate && has(id)) {
+                       tmp.delete();
+                       return InsertLooseObjectResult.EXISTS_PACKED;
+               }
+
                tmp.setReadOnly();
 
                final File dst = fileFor(id);
-               if (force && dst.exists()) {
+               if (dst.exists()) {
+                       // We want to be extra careful and avoid replacing an object
+                       // that already exists. We can't be sure renameTo() would
+                       // fail on all platforms if dst exists, so we check first.
+                       //
                        tmp.delete();
-                       return true;
+                       return InsertLooseObjectResult.EXISTS_LOOSE;
                }
                if (tmp.renameTo(dst)) {
                        unpackedObjectCache.add(id);
-                       return true;
+                       return InsertLooseObjectResult.INSERTED;
                }
 
                // Maybe the directory doesn't exist yet as the object
@@ -481,12 +491,12 @@ public class ObjectDirectory extends FileObjectDatabase {
                dst.getParentFile().mkdir();
                if (tmp.renameTo(dst)) {
                        unpackedObjectCache.add(id);
-                       return true;
+                       return InsertLooseObjectResult.INSERTED;
                }
 
-               if (!force && has(id)) {
+               if (!createDuplicate && has(id)) {
                        tmp.delete();
-                       return true;
+                       return InsertLooseObjectResult.EXISTS_PACKED;
                }
 
                // The object failed to be renamed into its proper
@@ -495,7 +505,7 @@ public class ObjectDirectory extends FileObjectDatabase {
                // fail.
                //
                tmp.delete();
-               return false;
+               return InsertLooseObjectResult.FAILURE;
        }
 
        boolean tryAgain1() {

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/ObjectDirectoryInserter.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/ObjectDirectoryInserter.java
index d92285de8cee8fd3e0ba6e4d04fd99670820a322..074ebb9617486c90590670ee122142b6574d47c8 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/ObjectDirectoryInserter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/ObjectDirectoryInserter.java
@@ -83,9 +83,18 @@ class ObjectDirectoryInserter extends ObjectInserter {
                final MessageDigest md = digest();
                final File tmp = toTemp(md, type, len, is);
                final ObjectId id = ObjectId.fromRaw(md.digest());
-               if (db.insertUnpackedObject(tmp, id, false /* no duplicate */))
+
+               switch (db.insertUnpackedObject(tmp, id, false /* no duplicate */)) {
+               case INSERTED:
+               case EXISTS_PACKED:
+               case EXISTS_LOOSE:
                        return id;
 
+               case FAILURE:
+               default:
+                       break;
+               }
+
                final File dst = db.fileFor(id);
                throw new ObjectWritingException("Unable to create new object: " + dst);
        }