Fix PackWriterBitmapWalker handling non-existing uninteresting objects

When writing new packs it should be allowed to specify objects as "have"
(objects which should not be included in the pack) which do not exist in
the local repository.

This works with the traditional PackWriter, but when PackWriter was
working on a repository with bitmap indexes and used
PackWriterBitmapWalker then this feature was broken. Non-existing "have"
objects lead to MissingObjectExceptions. That broke push and Gerrit
replication. When the replication target had branches unknown to the
replication source then the source repository wanted to build pack files
where "have" included branch-tips which were unknown in the source
repository.

Bug: 427107
Change-Id: I6b6598a1ec49af68aa77ea6f1f06e827982ea4ac
Also-by: Matthias Sohn <matthias.sohn@sap.com>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackWriterTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackWriterTest.java
index aba48bc35ff09dda07b57d2b9d4657b913d65212..873b2cda45980699265848871e53c133f769f3a7 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackWriterTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackWriterTest.java
@@ -54,6 +54,7 @@ import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.text.ParseException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -224,6 +225,25 @@ public class PackWriterTest extends SampleDataRepositoryTestCase {
                // shouldn't throw anything
        }
 
+       /**
+        * Try to pass non-existing object as uninteresting, with ignoring setting.
+        * Use a repo with bitmap indexes because then PackWriter will use
+        * PackWriterBitmapWalker which had problems with this situation.
+        *
+        * @throws IOException
+        * @throws ParseException
+        */
+       @Test
+       public void testIgnoreNonExistingObjectsWithBitmaps() throws IOException,
+                       ParseException {
+               final ObjectId nonExisting = ObjectId
+                               .fromString("0000000000000000000000000000000000000001");
+               new GC(db).gc();
+               createVerifyOpenPack(EMPTY_SET_OBJECT,
+                               Collections.singleton(nonExisting), false, true, true);
+               // shouldn't throw anything
+       }
+
        /**
         * Create pack basing on only interesting objects, then precisely verify
         * content. No delta reuse here.
@@ -604,8 +624,17 @@ public class PackWriterTest extends SampleDataRepositoryTestCase {
                        final Set<ObjectId> uninterestings, final boolean thin,
                        final boolean ignoreMissingUninteresting)
                        throws MissingObjectException, IOException {
+               createVerifyOpenPack(interestings, uninterestings, thin,
+                               ignoreMissingUninteresting, false);
+       }
+
+       private void createVerifyOpenPack(final Set<ObjectId> interestings,
+                       final Set<ObjectId> uninterestings, final boolean thin,
+                       final boolean ignoreMissingUninteresting, boolean useBitmaps)
+                       throws MissingObjectException, IOException {
                NullProgressMonitor m = NullProgressMonitor.INSTANCE;
                writer = new PackWriter(config, db.newObjectReader());
+               writer.setUseBitmaps(useBitmaps);
                writer.setThin(thin);
                writer.setIgnoreMissingUninteresting(ignoreMissingUninteresting);
                writer.preparePack(m, interestings, uninterestings);

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java
index f3791e66faebf822486430c0dc2d5cd39e6028e9..76e0b35b35fd01e72189648614bae983fca6958d 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java
@@ -1846,9 +1846,10 @@ public class PackWriter {
                        Set<? extends ObjectId> have)
                        throws MissingObjectException, IncorrectObjectTypeException,
                        IOException {
-               BitmapBuilder haveBitmap = bitmapWalker.findObjects(have, null);
+               BitmapBuilder haveBitmap = bitmapWalker.findObjects(have, null, true);
                bitmapWalker.reset();
-               BitmapBuilder wantBitmap = bitmapWalker.findObjects(want, haveBitmap);
+               BitmapBuilder wantBitmap = bitmapWalker.findObjects(want, haveBitmap,
+                               false);
                BitmapBuilder needBitmap = wantBitmap.andNot(haveBitmap);
 
                if (useCachedPacks && reuseSupport != null
@@ -2048,7 +2049,7 @@ public class PackWriter {
                                walker = bitmapPreparer.newBitmapWalker();
 
                        BitmapBuilder bitmap = walker.findObjects(
-                                       Collections.singleton(cmit), null);
+                                       Collections.singleton(cmit), null, false);
 
                        if (last != null && cmit.isReuseWalker() && !bitmap.contains(last))
                                throw new IllegalStateException(MessageFormat.format(

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriterBitmapWalker.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriterBitmapWalker.java
index 2e9ce1ddc1e38aa832a8a022af9df0c00f17cf4f..63a91cd82d4449a39cb4838fba695add3866cd93 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriterBitmapWalker.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriterBitmapWalker.java
@@ -78,7 +78,7 @@ final class PackWriterBitmapWalker {
                this.pm = (pm == null) ? NullProgressMonitor.INSTANCE : pm;
        }
 
-       BitmapBuilder findObjects(Set<? extends ObjectId> start, BitmapBuilder seen)
+       BitmapBuilder findObjects(Set<? extends ObjectId> start, BitmapBuilder seen, boolean ignoreMissingStart)
                        throws MissingObjectException, IncorrectObjectTypeException,
                        IOException {
                final BitmapBuilder bitmapResult = bitmapIndex.newBitmapBuilder();
@@ -91,9 +91,15 @@ final class PackWriterBitmapWalker {
 
                boolean marked = false;
                for (ObjectId obj : start) {
-                       if (!bitmapResult.contains(obj)) {
-                               walker.markStart(walker.parseAny(obj));
-                               marked = true;
+                       try {
+                               if (!bitmapResult.contains(obj)) {
+                                       walker.markStart(walker.parseAny(obj));
+                                       marked = true;
+                               }
+                       } catch (MissingObjectException e) {
+                               if (ignoreMissingStart)
+                                       continue;
+                               throw e;
                        }
                }