Merge branch 'stable-4.5' into stable-4.6

* stable-4.5:
  Fix exception handling for opening bitmap index files

Change-Id: Ifb511238e3e98b1bc9f79a990807b940a17ebaa6
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

2 files changed, 114 insertions, 2 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcConcurrentTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcConcurrentTest.java
index 07a7be7467..776226b4ef 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcConcurrentTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcConcurrentTest.java
@@ -45,8 +45,12 @@ package org.eclipse.jgit.internal.storage.file;
 
 import static java.lang.Integer.valueOf;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNotNull;
 
 import java.io.IOException;
+import java.util.Collection;
+import java.util.Collections;
 import java.util.concurrent.BrokenBarrierException;
 import java.util.concurrent.Callable;
 import java.util.concurrent.CyclicBarrier;
@@ -56,8 +60,14 @@ import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 
 import org.eclipse.jgit.internal.JGitText;
+import org.eclipse.jgit.internal.storage.pack.PackWriter;
+import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.lib.EmptyProgressMonitor;
+import org.eclipse.jgit.lib.NullProgressMonitor;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Sets;
 import org.eclipse.jgit.revwalk.RevBlob;
+import org.eclipse.jgit.revwalk.RevCommit;
 import org.junit.Test;
 
 public class GcConcurrentTest extends GcTestCase {
@@ -116,4 +126,97 @@ public class GcConcurrentTest extends GcTestCase {
 			pool.awaitTermination(Long.MAX_VALUE, TimeUnit.SECONDS);
 		}
 	}
+
+	@Test
+	public void repackAndGetStats() throws Exception {
+		TestRepository<FileRepository>.BranchBuilder test = tr.branch("test");
+		test.commit().add("a", "a").create();
+		GC gc1 = new GC(tr.getRepository());
+		gc1.setPackExpireAgeMillis(0);
+		gc1.gc();
+		test.commit().add("b", "b").create();
+
+		// Create a new Repository instance and trigger a gc
+		// from that instance. Reusing the existing repo instance
+		// tr.getRepository() would not show the problem.
+		FileRepository r2 = new FileRepository(
+				tr.getRepository().getDirectory());
+		GC gc2 = new GC(r2);
+		gc2.setPackExpireAgeMillis(0);
+		gc2.gc();
+
+		new GC(tr.getRepository()).getStatistics();
+	}
+
+	@Test
+	public void repackAndUploadPack() throws Exception {
+		TestRepository<FileRepository>.BranchBuilder test = tr.branch("test");
+		// RevCommit a = test.commit().add("a", "a").create();
+		test.commit().add("a", "a").create();
+
+		GC gc1 = new GC(tr.getRepository());
+		gc1.setPackExpireAgeMillis(0);
+		gc1.gc();
+
+		RevCommit b = test.commit().add("b", "b").create();
+
+		FileRepository r2 = new FileRepository(
+				tr.getRepository().getDirectory());
+		GC gc2 = new GC(r2);
+		gc2.setPackExpireAgeMillis(0);
+		gc2.gc();
+
+		// Simulate parts of an UploadPack. This is the situation on
+		// server side (e.g. gerrit) when when clients are
+		// cloning/fetching while the server side repo's
+		// are gc'ed by an external process (e.g. scheduled
+		// native git gc)
+		try (PackWriter pw = new PackWriter(tr.getRepository())) {
+			pw.setUseBitmaps(true);
+			pw.preparePack(NullProgressMonitor.INSTANCE, Sets.of(b),
+					Collections.<ObjectId> emptySet());
+			new GC(tr.getRepository()).getStatistics();
+		}
+	}
+
+	PackFile getSinglePack(FileRepository r) {
+		Collection<PackFile> packs = r.getObjectDatabase().getPacks();
+		assertEquals(1, packs.size());
+		return packs.iterator().next();
+	}
+
+	@Test
+	public void repackAndCheckBitmapUsage() throws Exception {
+		// create a test repository with one commit and pack all objects. After
+		// packing create loose objects to trigger creation of a new packfile on
+		// the next gc
+		TestRepository<FileRepository>.BranchBuilder test = tr.branch("test");
+		test.commit().add("a", "a").create();
+		FileRepository repository = tr.getRepository();
+		GC gc1 = new GC(repository);
+		gc1.setPackExpireAgeMillis(0);
+		gc1.gc();
+		String oldPackName = getSinglePack(repository).getPackName();
+		RevCommit b = test.commit().add("b", "b").create();
+
+		// start the garbage collection on a new repository instance,
+		FileRepository repository2 = new FileRepository(repository.getDirectory());
+		GC gc2 = new GC(repository2);
+		gc2.setPackExpireAgeMillis(0);
+		gc2.gc();
+		String newPackName = getSinglePack(repository2).getPackName();
+		// make sure gc() has caused creation of a new packfile
+		assertNotEquals(oldPackName, newPackName);
+
+		// Even when asking again for the set of packfiles outdated data
+		// will be returned. As long as the repository can work on cached data
+		// it will do so and not detect that a new packfile exists.
+		assertNotEquals(getSinglePack(repository).getPackName(), newPackName);
+
+		// Only when accessing object content it is required to rescan the pack
+		// directory and the new packfile will be detected.
+		repository.getObjectDatabase().open(b).getSize();
+		assertEquals(getSinglePack(repository).getPackName(), newPackName);
+		assertNotNull(getSinglePack(repository).getBitmapIndex());
+	}
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java
index 038172f92d..b5889f2dd4 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java
@@ -1105,8 +1105,17 @@ public class PackFile implements Iterable<PackIndex.MutableEntry> {
 		if (invalid || invalidBitmap)
 			return null;
 		if (bitmapIdx == null && hasExt(BITMAP_INDEX)) {
-			final PackBitmapIndex idx = PackBitmapIndex.open(
-					extFile(BITMAP_INDEX), idx(), getReverseIdx());
+			final PackBitmapIndex idx;
+			try {
+				idx = PackBitmapIndex.open(extFile(BITMAP_INDEX), idx(),
+						getReverseIdx());
+			} catch (FileNotFoundException e) {
+				// Once upon a time this bitmap file existed. Now it
+				// has been removed. Most likely an external gc  has
+				// removed this packfile and the bitmap
+				 invalidBitmap = true;
+				 return null;
+			}
 
 			// At this point, idx() will have set packChecksum.
 			if (Arrays.equals(packChecksum, idx.packChecksum))