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>

org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackTest.java

@@ -261,7 +261,7 @@ public void testDelta_FailsOver2GiB() throws Exception {
 				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");

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 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 Collection<Pack> getPacks() {
 	 * Add a single existing pack to the list of available pack files.
 	 */
 	@Override
-	public Pack openPack(File pack)
+	public Pack openPack(File pack) throws IOException {
-			throws IOException {
+		PackFile pf;
-		final String p = pack.getName();
+		try {
-		if (p.length() != 50 || !p.startsWith("pack-") || !p.endsWith(".pack")) //$NON-NLS-1$ //$NON-NLS-2$
+			pf = new PackFile(pack);
-			throw new IOException(MessageFormat.format(JGitText.get().notAValidPack, pack));
+		} catch (IllegalArgumentException e) {
-
+			throw new IOException(
-		// The pack and index are assumed to exist. The existence of other
+					MessageFormat.format(JGitText.get().notAValidPack, pack),
-		// extensions needs to be explicitly checked.
+					e);
-		//
-		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);
+		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;
 	}

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.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.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
+	 * @param bitmapIdxFile
-	 *            additional pack file extensions with the same base as the pack
+	 *            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 @@ private long findEndOffset(long startOffset)
 	}
 
 	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;
+				bitmapIdxFile = null;
-				 return 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
+			} else {
-				invalidBitmap = true;
+				bitmapIdxFile = null;
+			}
 		}
 		return bitmapIdx;
 	}
@@ -1179,10 +1179,6 @@ private void setCorrupt(long offset) {
 		}
 	}
 
-	private boolean hasExt(PackExt ext) {
-		return (extensions & ext.getBit()) != 0;
-	}
-
 	@SuppressWarnings("nls")
 	@Override
 	public String toString() {

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackDirectory.java

@@ -421,10 +421,7 @@ private PackList scanPacksImpl(PackList old) {
 				continue;
 			}
 
-			list.add(new Pack(packFile,
+			list.add(new Pack(packFile, packFilesByExt.get(BITMAP_INDEX)));
-					packFilesByExt.containsKey(BITMAP_INDEX)
-							? BITMAP_INDEX.getBit()
-							: 0));
 			foundNew = true;
 		}