Merge branch 'master' into stable-6.9

* master:
  Update SECURITY.md
  DfsObjDatabase: Let object database instantiate DfsPackFiles
  DfsPackFile: Abstract the bitmap loading to support other backends
  Remove unused API problem filters
  Support public key in IdentityFile
  Revert "StartGenerator: Fix parent rewrite with non-default RevFilter"
  DfsReader#getObjectSize: use size index if possible

Change-Id: I60244aec1e44fe5b757a9b7ccb320697e0dc9b29

8 files changed, 187 insertions, 102 deletions

diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters
index e15ed48e28..dda62f22a3 100644
--- a/org.eclipse.jgit/.settings/.api_filters
+++ b/org.eclipse.jgit/.settings/.api_filters
@@ -1,35 +1,5 @@
 <?xml version="1.0" encoding="UTF-8" standalone="no"?>
 <component id="org.eclipse.jgit" version="2">
-    <resource path="src/org/eclipse/jgit/api/GarbageCollectCommand.java" type="org.eclipse.jgit.api.GarbageCollectCommand">
-        <filter id="1142947843">
-            <message_arguments>
-                <message_argument value="5.13.3"/>
-                <message_argument value="setPackKeptObjects(boolean)"/>
-            </message_arguments>
-        </filter>
-    </resource>
-    <resource path="src/org/eclipse/jgit/lib/BitmapIndex.java" type="org.eclipse.jgit.lib.BitmapIndex">
-        <filter id="404000815">
-            <message_arguments>
-                <message_argument value="org.eclipse.jgit.lib.BitmapIndex"/>
-                <message_argument value="addBitmapLookupListener(BitmapIndex.BitmapLookupListener)"/>
-            </message_arguments>
-        </filter>
-    </resource>
-    <resource path="src/org/eclipse/jgit/lib/ConfigConstants.java" type="org.eclipse.jgit.lib.ConfigConstants">
-        <filter id="1142947843">
-            <message_arguments>
-                <message_argument value="5.13.3"/>
-                <message_argument value="CONFIG_KEY_PACK_KEPT_OBJECTS"/>
-            </message_arguments>
-        </filter>
-        <filter id="1142947843">
-            <message_arguments>
-                <message_argument value="5.13.3"/>
-                <message_argument value="CONFIG_REPACK_SECTION"/>
-            </message_arguments>
-        </filter>
-    </resource>
     <resource path="src/org/eclipse/jgit/lib/GpgSignatureVerifier.java" type="org.eclipse.jgit.lib.GpgSignatureVerifier">
         <filter id="404000815">
             <message_arguments>
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsInserter.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsInserter.java
index dfab7bab6f..a07d8416c4 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsInserter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsInserter.java
@@ -217,7 +217,7 @@ public class DfsInserter extends ObjectInserter {
 		db.commitPack(Collections.singletonList(packDsc), null);
 		rollback = false;
 
-		DfsPackFile p = new DfsPackFile(cache, packDsc);
+		DfsPackFile p = db.createDfsPackFile(cache, packDsc);
 		if (index != null)
 			p.setPackIndex(index);
 		db.addPack(p);
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsObjDatabase.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsObjDatabase.java
index 2375698303..9f6eb10256 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsObjDatabase.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsObjDatabase.java
@@ -592,7 +592,7 @@ public abstract class DfsObjDatabase extends ObjectDatabase {
 			if (oldPack != null) {
 				newPacks.add(oldPack);
 			} else if (dsc.hasFileExt(PackExt.PACK)) {
-				newPacks.add(new DfsPackFile(cache, dsc));
+				newPacks.add(createDfsPackFile(cache, dsc));
 				foundNew = true;
 			}
 
@@ -617,6 +617,23 @@ public abstract class DfsObjDatabase extends ObjectDatabase {
 				newReftables.toArray(new DfsReftable[0]));
 	}
 
+	/**
+	 * Create instances of DfsPackFile
+	 *
+	 * Implementors can decide to construct or wrap DfsPackFile in different
+	 * ways.
+	 *
+	 * @param cache
+	 *            block cache
+	 * @param dsc
+	 *            pack description
+	 * @return the dfs packfile
+	 */
+	protected DfsPackFile createDfsPackFile(DfsBlockCache cache,
+			DfsPackDescription dsc) {
+		return new DfsPackFile(cache, dsc);
+	}
+
 	private static Map<DfsPackDescription, DfsPackFile> packMap(PackList old) {
 		Map<DfsPackDescription, DfsPackFile> forReuse = new HashMap<>();
 		for (DfsPackFile p : old.packs) {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackFile.java
index e0f0e13553..42b1d235bf 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackFile.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackFile.java
@@ -65,8 +65,11 @@ import org.eclipse.jgit.util.LongList;
  */
 public final class DfsPackFile extends BlockBasedFile {
 	private static final int REC_SIZE = Constants.OBJECT_ID_LENGTH + 8;
+
 	private static final long REF_POSITION = 0;
 
+	private static final PackBitmapIndexLoader DEFAULT_BITMAP_LOADER = new StreamPackBitmapIndexLoader();
+
 	/** Index mapping {@link ObjectId} to position within the pack stream. */
 	private volatile PackIndex index;
 
@@ -79,6 +82,8 @@ public final class DfsPackFile extends BlockBasedFile {
 	/** Index of compressed commit graph mapping entire object graph. */
 	private volatile CommitGraph commitGraph;
 
+	private final PackBitmapIndexLoader bitmapLoader;
+
 	/** Index by size */
 	private boolean objectSizeIndexLoadAttempted;
 
@@ -105,6 +110,10 @@ public final class DfsPackFile extends BlockBasedFile {
 	 *            description of the pack within the DFS.
 	 */
 	DfsPackFile(DfsBlockCache cache, DfsPackDescription desc) {
+		this(cache, desc, DEFAULT_BITMAP_LOADER);
+	}
+
+	DfsPackFile(DfsBlockCache cache, DfsPackDescription desc, PackBitmapIndexLoader bitmapLoader) {
 		super(cache, desc, PACK);
 
 		int bs = desc.getBlockSize(PACK);
@@ -114,6 +123,8 @@ public final class DfsPackFile extends BlockBasedFile {
 
 		long sz = desc.getFileSize(PACK);
 		length = sz > 0 ? sz : -1;
+
+		this.bitmapLoader = bitmapLoader;
 	}
 
 	/**
@@ -206,7 +217,7 @@ public final class DfsPackFile extends BlockBasedFile {
 	 *             the bitmap index is not available, or is corrupt.
 	 */
 	public PackBitmapIndex getBitmapIndex(DfsReader ctx) throws IOException {
-		if (invalid || isGarbage() || !desc.hasFileExt(BITMAP_INDEX)) {
+		if (invalid || isGarbage() || !bitmapLoader.hasBitmaps(desc)) {
 			return null;
 		}
 
@@ -214,6 +225,15 @@ public final class DfsPackFile extends BlockBasedFile {
 			return bitmapIndex;
 		}
 
+		if (!bitmapLoader.keepInDfs(desc)) {
+			PackBitmapIndexLoader.LoadResult result = bitmapLoader
+					.loadPackBitmapIndex(ctx, this);
+			if (bitmapIndex == null && result.bitmapIndex != null) {
+				bitmapIndex = result.bitmapIndex;
+			}
+			return bitmapIndex;
+		}
+
 		DfsStreamKey bitmapKey = desc.getStreamKey(BITMAP_INDEX);
 		AtomicBoolean cacheHit = new AtomicBoolean(true);
 		DfsBlockCache.Ref<PackBitmapIndex> idxref = cache
@@ -1252,31 +1272,11 @@ public final class DfsPackFile extends BlockBasedFile {
 	private DfsBlockCache.Ref<PackBitmapIndex> loadBitmapIndex(DfsReader ctx,
 			DfsStreamKey bitmapKey) throws IOException {
 		ctx.stats.readBitmap++;
-		long start = System.nanoTime();
-		try (ReadableChannel rc = ctx.db.openFile(desc, BITMAP_INDEX)) {
-			long size;
-			PackBitmapIndex bmidx;
-			try {
-				bmidx = PackBitmapIndex.read(alignTo8kBlocks(rc),
-						() -> idx(ctx), () -> getReverseIdx(ctx),
-						ctx.getOptions().shouldLoadRevIndexInParallel());
-			} finally {
-				size = rc.position();
-				ctx.stats.readBitmapIdxBytes += size;
-				ctx.stats.readBitmapIdxMicros += elapsedMicros(start);
-			}
-			bitmapIndex = bmidx;
-			return new DfsBlockCache.Ref<>(
-					bitmapKey, REF_POSITION, size, bmidx);
-		} catch (EOFException e) {
-			throw new IOException(MessageFormat.format(
-					DfsText.get().shortReadOfIndex,
-					desc.getFileName(BITMAP_INDEX)), e);
-		} catch (IOException e) {
-			throw new IOException(MessageFormat.format(
-					DfsText.get().cannotReadIndex,
-					desc.getFileName(BITMAP_INDEX)), e);
-		}
+		PackBitmapIndexLoader.LoadResult result = bitmapLoader
+				.loadPackBitmapIndex(ctx, this);
+		bitmapIndex = result.bitmapIndex;
+		return new DfsBlockCache.Ref<>(bitmapKey, REF_POSITION,
+				result.bytesRead, result.bitmapIndex);
 	}
 
 	private DfsBlockCache.Ref<CommitGraph> loadCommitGraph(DfsReader ctx,
@@ -1317,4 +1317,105 @@ public final class DfsPackFile extends BlockBasedFile {
 		}
 		return new BufferedInputStream(in, bs);
 	}
+
+	/**
+	 * Loads the PackBitmapIndex associated with this packfile
+	 */
+	public interface PackBitmapIndexLoader {
+		/**
+		 * Does this pack has bitmaps associated?
+		 *
+		 * @param desc
+		 *            the pack
+		 * @return true if the pack has bitmaps
+		 */
+		boolean hasBitmaps(DfsPackDescription desc);
+
+		/**
+		 * If the returned instance must be kept in DFS cache
+		 *
+		 * It should be true when the instance is expensive to load and can be
+		 * reused.
+		 *
+		 * @param desc
+		 *            the pack
+		 * @return true if the returned bitmap index should be kept in DFS
+		 */
+		boolean keepInDfs(DfsPackDescription desc);
+
+		/**
+		 * Returns a PackBitmapIndex for this pack, if the pack has bitmaps
+		 * associated.
+		 *
+		 * @param ctx
+		 *            the reader
+		 * @param pack
+		 *            the pack
+		 * @return the pack bitmap index and bytes size (when applicable)
+		 * @throws IOException
+		 *             error accessing storage
+		 */
+		LoadResult loadPackBitmapIndex(DfsReader ctx, DfsPackFile pack)
+				throws IOException;
+
+		/**
+		 * The instance of the pack bitmap index and the amount of bytes loaded.
+		 *
+		 * The bytes can be 0, if the implementation doesn't do any initial
+		 * loading.
+		 */
+		class LoadResult {
+			final PackBitmapIndex bitmapIndex;
+
+			final long bytesRead;
+
+			LoadResult(PackBitmapIndex packBitmapIndex, long bytesRead) {
+				this.bitmapIndex = packBitmapIndex;
+				this.bytesRead = bytesRead;
+			}
+		}
+	}
+
+	/**
+	 * Load the packbitmapindex from the BITMAP_INDEX pack extension
+	 */
+	private static final class StreamPackBitmapIndexLoader implements PackBitmapIndexLoader {
+		@Override
+		public boolean hasBitmaps(DfsPackDescription desc) {
+			return desc.hasFileExt(BITMAP_INDEX);
+		}
+
+		@Override
+		public boolean keepInDfs(DfsPackDescription desc) {
+			return true;
+		}
+
+		@Override
+		public LoadResult loadPackBitmapIndex(DfsReader ctx, DfsPackFile pack)
+				throws IOException {
+			DfsPackDescription desc = pack.getPackDescription();
+			try (ReadableChannel rc = ctx.db.openFile(desc, BITMAP_INDEX)) {
+				long size;
+				PackBitmapIndex bmidx;
+				try {
+					bmidx = PackBitmapIndex.read(alignTo8kBlocks(rc),
+							() -> pack.idx(ctx), () -> pack.getReverseIdx(ctx),
+							ctx.getOptions().shouldLoadRevIndexInParallel());
+				} finally {
+					size = rc.position();
+				}
+				return new LoadResult(bmidx, size);
+			} catch (EOFException e) {
+				throw new IOException(
+						MessageFormat.format(DfsText.get().shortReadOfIndex,
+								desc.getFileName(BITMAP_INDEX)),
+						e);
+			} catch (IOException e) {
+				throw new IOException(
+						MessageFormat.format(DfsText.get().cannotReadIndex,
+								desc.getFileName(BITMAP_INDEX)),
+						e);
+			}
+		}
+	}
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackParser.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackParser.java
index a38ce91ccd..b19f65c69b 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackParser.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackParser.java
@@ -126,7 +126,7 @@ public class DfsPackParser extends PackParser {
 			objdb.commitPack(Collections.singletonList(packDsc), null);
 			rollback = false;
 
-			DfsPackFile p = new DfsPackFile(blockCache, packDsc);
+			DfsPackFile p = objdb.createDfsPackFile(blockCache, packDsc);
 			p.setBlockSize(blockSize);
 			if (packIndex != null)
 				p.setPackIndex(packIndex);
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsReader.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsReader.java
index c722c06e9c..a342796cbe 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsReader.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsReader.java
@@ -503,30 +503,28 @@ public class DfsReader extends ObjectReader implements ObjectReuseAsIs {
 	public long getObjectSize(AnyObjectId objectId, int typeHint)
 			throws MissingObjectException, IncorrectObjectTypeException,
 			IOException {
-		if (last != null && !skipGarbagePack(last)) {
-			long sz = last.getObjectSize(this, objectId);
-			if (0 <= sz) {
-				return sz;
+		DfsPackFile pack = findPackWithObject(objectId);
+		if (pack == null) {
+			if (typeHint == OBJ_ANY) {
+				throw new MissingObjectException(objectId.copy(),
+						JGitText.get().unknownObjectType2);
 			}
+			throw new MissingObjectException(objectId.copy(), typeHint);
 		}
 
-		PackList packList = db.getPackList();
-		long sz = getObjectSizeImpl(packList, objectId);
-		if (0 <= sz) {
-			return sz;
-		}
-		if (packList.dirty()) {
-			sz = getObjectSizeImpl(packList, objectId);
-			if (0 <= sz) {
-				return sz;
-			}
+		if (typeHint != Constants.OBJ_BLOB || !pack.hasObjectSizeIndex(this)) {
+			return pack.getObjectSize(this, objectId);
 		}
 
-		if (typeHint == OBJ_ANY) {
-			throw new MissingObjectException(objectId.copy(),
-					JGitText.get().unknownObjectType2);
+		long sz = pack.getIndexedObjectSize(this, objectId);
+		if (sz >= 0) {
+			stats.objectSizeIndexHit += 1;
+			return sz;
 		}
-		throw new MissingObjectException(objectId.copy(), typeHint);
+
+		// Object wasn't in the index
+		stats.objectSizeIndexMiss += 1;
+		return pack.getObjectSize(this, objectId);
 	}
 
 
@@ -582,21 +580,6 @@ public class DfsReader extends ObjectReader implements ObjectReuseAsIs {
 		return null;
 	}
 
-	private long getObjectSizeImpl(PackList packList, AnyObjectId objectId)
-			throws IOException {
-		for (DfsPackFile pack : packList.packs) {
-			if (pack == last || skipGarbagePack(pack)) {
-				continue;
-			}
-			long sz = pack.getObjectSize(this, objectId);
-			if (0 <= sz) {
-				last = pack;
-				return sz;
-			}
-		}
-		return -1;
-	}
-
 	@Override
 	public DfsObjectToPack newObjectToPack(AnyObjectId objectId, int type) {
 		return new DfsObjectToPack(objectId, type);
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/StartGenerator.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/StartGenerator.java
index 61a91e70dc..6854b6083d 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/StartGenerator.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/StartGenerator.java
@@ -98,14 +98,14 @@ class StartGenerator extends Generator {
 		} else {
 			pending = RevWalk.newDateRevQueue(q);
 		}
-		if (rf != RevFilter.ALL && w.getRewriteParents()) {
-			pendingOutputType |= HAS_REWRITE | NEEDS_REWRITE;
-		}
 		if (tf != TreeFilter.ALL) {
+			int rewriteFlag;
 			if (w.getRewriteParents()) {
 				pendingOutputType |= HAS_REWRITE | NEEDS_REWRITE;
-			}
-			rf = AndRevFilter.create(new TreeRevFilter(w, tf), rf);
+				rewriteFlag = RevWalk.REWRITE;
+			} else
+				rewriteFlag = 0;
+			rf = AndRevFilter.create(new TreeRevFilter(w, tf, rewriteFlag), rf);
 		}
 
 		walker.queue = q;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/TreeRevFilter.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/TreeRevFilter.java
index 4085954638..43571a6868 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/TreeRevFilter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/TreeRevFilter.java
@@ -60,8 +60,21 @@ public class TreeRevFilter extends RevFilter {
 	 * Create a {@link org.eclipse.jgit.revwalk.filter.RevFilter} from a
 	 * {@link org.eclipse.jgit.treewalk.filter.TreeFilter}.
 	 *
-	 * When revWalk's rewrite parent flag is set, it creates a filter for the
-	 * first phase of a parent-rewriting limited revision walk.
+	 * @param walker
+	 *            walker used for reading trees.
+	 * @param t
+	 *            filter to compare against any changed paths in each commit. If
+	 *            a {@link org.eclipse.jgit.revwalk.FollowFilter}, will be
+	 *            replaced with a new filter following new paths after a rename.
+	 * @since 3.5
+	 */
+	public TreeRevFilter(RevWalk walker, TreeFilter t) {
+		this(walker, t, 0);
+	}
+
+	/**
+	 * Create a filter for the first phase of a parent-rewriting limited
+	 * revision walk.
 	 * <p>
 	 * This filter is ANDed to evaluate before all other filters and ties the
 	 * configured {@link TreeFilter} into the revision walking process.
@@ -78,13 +91,14 @@ public class TreeRevFilter extends RevFilter {
 	 *            filter to compare against any changed paths in each commit. If
 	 *            a {@link FollowFilter}, will be replaced with a new filter
 	 *            following new paths after a rename.
-	 * @since 3.5
+	 * @param rewriteFlag
+	 *            flag to color commits to be removed from the simplified DAT.
 	 */
-	public TreeRevFilter(RevWalk walker, TreeFilter t) {
+	TreeRevFilter(RevWalk walker, TreeFilter t, int rewriteFlag) {
 		pathFilter = new TreeWalk(walker.reader);
 		pathFilter.setFilter(t);
 		pathFilter.setRecursive(t.shouldBeRecursive());
-		this.rewriteFlag = walker.getRewriteParents() ? RevWalk.REWRITE : 0;
+		this.rewriteFlag = rewriteFlag;
 	}
 
 	@Override