Pack: Replace extensions bitset with bitmapIdx PackFile

The only extension that was ever consulted from the bitmap was the
bitmap index. We can simplify the Pack code as well as the code of
all the callers if we focus on just that usage.

Change-Id: I799ddfdee93142af67ce5081d14a430d36aa4c15
Signed-off-by: Nasser Grainawi <quic_nasserg@quicinc.com>

4 files changed, 40 insertions, 48 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackTest.java
index 85bd31d711..a3596541fe 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackTest.java
@@ -261,7 +261,7 @@ public class PackTest extends LocalDiskRepositoryTestCase {
 				new PackIndexWriterV1(f).write(list, footer);
 			}
 
-			Pack pack = new Pack(packName, PackExt.INDEX.getBit());
+			Pack pack = new Pack(packName, null);
 			try {
 				pack.get(wc, b);
 				fail("expected LargeObjectException.ExceedsByteArrayLimit");
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java
index e71a960603..170df57808 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java
@@ -11,8 +11,8 @@
 package org.eclipse.jgit.internal.storage.file;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
-import static org.eclipse.jgit.internal.storage.pack.PackExt.INDEX;
 import static org.eclipse.jgit.internal.storage.pack.PackExt.PACK;
+import static org.eclipse.jgit.internal.storage.pack.PackExt.BITMAP_INDEX;
 
 import java.io.BufferedReader;
 import java.io.File;
@@ -31,7 +31,6 @@ import java.util.concurrent.atomic.AtomicReference;
 
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.internal.storage.pack.ObjectToPack;
-import org.eclipse.jgit.internal.storage.pack.PackExt;
 import org.eclipse.jgit.internal.storage.pack.PackWriter;
 import org.eclipse.jgit.lib.AbbreviatedObjectId;
 import org.eclipse.jgit.lib.AnyObjectId;
@@ -216,26 +215,26 @@ public class ObjectDirectory extends FileObjectDatabase {
 	 * Add a single existing pack to the list of available pack files.
 	 */
 	@Override
-	public Pack openPack(File pack)
-			throws IOException {
-		final String p = pack.getName();
-		if (p.length() != 50 || !p.startsWith("pack-") || !p.endsWith(".pack")) //$NON-NLS-1$ //$NON-NLS-2$
-			throw new IOException(MessageFormat.format(JGitText.get().notAValidPack, pack));
-
-		// The pack and index are assumed to exist. The existence of other
-		// extensions needs to be explicitly checked.
-		//
-		int extensions = PACK.getBit() | INDEX.getBit();
-		final String base = p.substring(0, p.length() - 4);
-		for (PackExt ext : PackExt.values()) {
-			if ((extensions & ext.getBit()) == 0) {
-				final String name = base + ext.getExtension();
-				if (new File(pack.getParentFile(), name).exists())
-					extensions |= ext.getBit();
-			}
-		}
-
-		Pack res = new Pack(pack, extensions);
+	public Pack openPack(File pack) throws IOException {
+		PackFile pf;
+		try {
+			pf = new PackFile(pack);
+		} catch (IllegalArgumentException e) {
+			throw new IOException(
+					MessageFormat.format(JGitText.get().notAValidPack, pack),
+					e);
+		}
+
+		String p = pf.getName();
+		// TODO(nasserg): See if PackFile can do these checks instead
+		if (p.length() != 50 || !p.startsWith("pack-") //$NON-NLS-1$
+				|| !pf.getPackExt().equals(PACK)) {
+			throw new IOException(
+					MessageFormat.format(JGitText.get().notAValidPack, pack));
+		}
+
+		PackFile bitmapIdx = pf.create(BITMAP_INDEX);
+		Pack res = new Pack(pack, bitmapIdx.exists() ? bitmapIdx : null);
 		packed.insert(res);
 		return res;
 	}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java
index fa938b3111..5efd4c5bfc 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java
@@ -12,7 +12,6 @@
 
 package org.eclipse.jgit.internal.storage.file;
 
-import static org.eclipse.jgit.internal.storage.pack.PackExt.BITMAP_INDEX;
 import static org.eclipse.jgit.internal.storage.pack.PackExt.INDEX;
 import static org.eclipse.jgit.internal.storage.pack.PackExt.KEEP;
 
@@ -38,6 +37,7 @@ import java.util.zip.CRC32;
 import java.util.zip.DataFormatException;
 import java.util.zip.Inflater;
 
+import org.eclipse.jgit.annotations.Nullable;
 import org.eclipse.jgit.errors.CorruptObjectException;
 import org.eclipse.jgit.errors.LargeObjectException;
 import org.eclipse.jgit.errors.MissingObjectException;
@@ -51,7 +51,6 @@ import org.eclipse.jgit.errors.UnsupportedPackVersionException;
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.internal.storage.pack.BinaryDelta;
 import org.eclipse.jgit.internal.storage.pack.ObjectToPack;
-import org.eclipse.jgit.internal.storage.pack.PackExt;
 import org.eclipse.jgit.internal.storage.pack.PackOutputStream;
 import org.eclipse.jgit.lib.AbbreviatedObjectId;
 import org.eclipse.jgit.lib.AnyObjectId;
@@ -80,8 +79,6 @@ public class Pack implements Iterable<PackIndex.MutableEntry> {
 
 	private final PackFile packFile;
 
-	private final int extensions;
-
 	private PackFile keepFile;
 
 	final int hash;
@@ -105,7 +102,8 @@ public class Pack implements Iterable<PackIndex.MutableEntry> {
 
 	private volatile Exception invalidatingCause;
 
-	private boolean invalidBitmap;
+	@Nullable
+	private PackFile bitmapIdxFile;
 
 	private AtomicInteger transientErrorCount = new AtomicInteger();
 
@@ -131,14 +129,14 @@ public class Pack implements Iterable<PackIndex.MutableEntry> {
 	 *
 	 * @param packFile
 	 *            path of the <code>.pack</code> file holding the data.
-	 * @param extensions
-	 *            additional pack file extensions with the same base as the pack
+	 * @param bitmapIdxFile
+	 *            existing bitmap index file with the same base as the pack
 	 */
-	public Pack(File packFile, int extensions) {
+	public Pack(File packFile, @Nullable PackFile bitmapIdxFile) {
 		this.packFile = new PackFile(packFile);
 		this.fileSnapshot = PackFileSnapshot.save(packFile);
 		this.packLastModified = fileSnapshot.lastModifiedInstant();
-		this.extensions = extensions;
+		this.bitmapIdxFile = bitmapIdxFile;
 
 		// Multiply by 31 here so we can more directly combine with another
 		// value in WindowCache.hash(), without doing the multiply there.
@@ -1124,26 +1122,28 @@ public class Pack implements Iterable<PackIndex.MutableEntry> {
 	}
 
 	synchronized PackBitmapIndex getBitmapIndex() throws IOException {
-		if (invalid || invalidBitmap)
+		if (invalid || bitmapIdxFile == null) {
 			return null;
-		if (bitmapIdx == null && hasExt(BITMAP_INDEX)) {
+		}
+		if (bitmapIdx == null) {
 			final PackBitmapIndex idx;
 			try {
-				idx = PackBitmapIndex.open(packFile.create(BITMAP_INDEX), idx(),
+				idx = PackBitmapIndex.open(bitmapIdxFile, 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;
+				bitmapIdxFile = null;
+				return null;
 			}
 
 			// At this point, idx() will have set packChecksum.
-			if (Arrays.equals(packChecksum, idx.packChecksum))
+			if (Arrays.equals(packChecksum, idx.packChecksum)) {
 				bitmapIdx = idx;
-			else
-				invalidBitmap = true;
+			} else {
+				bitmapIdxFile = null;
+			}
 		}
 		return bitmapIdx;
 	}
@@ -1179,10 +1179,6 @@ public class Pack implements Iterable<PackIndex.MutableEntry> {
 		}
 	}
 
-	private boolean hasExt(PackExt ext) {
-		return (extensions & ext.getBit()) != 0;
-	}
-
 	@SuppressWarnings("nls")
 	@Override
 	public String toString() {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackDirectory.java
index 2e68d46248..007205e55e 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackDirectory.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackDirectory.java
@@ -421,10 +421,7 @@ class PackDirectory {
 				continue;
 			}
 
-			list.add(new Pack(packFile,
-					packFilesByExt.containsKey(BITMAP_INDEX)
-							? BITMAP_INDEX.getBit()
-							: 0));
+			list.add(new Pack(packFile, packFilesByExt.get(BITMAP_INDEX)));
 			foundNew = true;
 		}