From fa4f00b7ed248d420ba12c576f944e873a3b8095 Mon Sep 17 00:00:00 2001 From: Christian Halstrick Date: Thu, 25 Sep 2014 17:29:35 +0200 Subject: [PATCH] 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 --- .../internal/storage/file/PackWriterTest.java | 29 +++++++++++++++++++ .../internal/storage/pack/PackWriter.java | 7 +++-- .../storage/pack/PackWriterBitmapWalker.java | 14 ++++++--- 3 files changed, 43 insertions(+), 7 deletions(-) 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 aba48bc35f..873b2cda45 100644 --- 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 uninterestings, final boolean thin, final boolean ignoreMissingUninteresting) throws MissingObjectException, IOException { + createVerifyOpenPack(interestings, uninterestings, thin, + ignoreMissingUninteresting, false); + } + + private void createVerifyOpenPack(final Set interestings, + final Set 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 f3791e66fa..76e0b35b35 100644 --- 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 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 2e9ce1ddc1..63a91cd82d 100644 --- 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 start, BitmapBuilder seen) + BitmapBuilder findObjects(Set 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; } } -- 2.39.5