From 3a6eec9bb697a599a32ccb08ee176e6d4982f90f Mon Sep 17 00:00:00 2001
From: Luca Milanesio <luca.milanesio@gmail.com>
Date: Sat, 10 Jun 2023 02:21:07 +0100
Subject: [PATCH] Express the explicit intention of creating bitmaps in GC

Add an explicit flag to PackWriter for allowing the
GC.repack() phase to explicitly generate bitmaps only for the
heads packfile and not for the others.

Previously the bitmap generation was conditioned to the
presence of object ids exclusion from the PackWriter.

The introduction of the bitmap generation in the PackWriter
done in Icdb0cdd66 has accidentally made the .keep files not
completely transparent, because their presence have disabled
the generation of the bitmap index, even if the generation
of bitmaps is enabled.

This bug has been an accidental consequence of the intention
of the bitmap generator to avoid generating bitmaps for the
non-heads packfile, however the implementation done by Colby
decided to use the excludeInPacks variable (see [1]) which
is unfortunately also used for excluding the packfiles having
an associated .keep file (see [2]).

[1] https://git.eclipse.org/r/c/jgit/jgit/+/7940/18/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackWriter.java#1617
[2] https://git.eclipse.org/r/plugins/gitiles/jgit/jgit/+/dafcb8f6db82b899c917832768f1c240d273190c/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/GC.java#506

Bug: 582039
Change-Id: Id722e68d9ff4ac24e73bf765ab11017586b6766e
---
 .../jgit/internal/storage/file/GC.java        |  9 +++----
 .../internal/storage/pack/PackWriter.java     | 24 ++++++++++++++++++-
 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java
index 4bb7c8f3d3..4db922b2c1 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java
@@ -862,7 +862,7 @@ public class GC {
 		Pack heads = null;
 		if (!allHeadsAndTags.isEmpty()) {
 			heads = writePack(allHeadsAndTags, PackWriter.NONE, allTags,
-					refsToExcludeFromBitmap, tagTargets, excluded);
+					refsToExcludeFromBitmap, tagTargets, excluded, true);
 			if (heads != null) {
 				ret.add(heads);
 				excluded.add(0, heads.getIndex());
@@ -870,13 +870,13 @@ public class GC {
 		}
 		if (!nonHeads.isEmpty()) {
 			Pack rest = writePack(nonHeads, allHeadsAndTags, PackWriter.NONE,
-					PackWriter.NONE, tagTargets, excluded);
+					PackWriter.NONE, tagTargets, excluded, false);
 			if (rest != null)
 				ret.add(rest);
 		}
 		if (!txnHeads.isEmpty()) {
 			Pack txn = writePack(txnHeads, PackWriter.NONE, PackWriter.NONE,
-					PackWriter.NONE, null, excluded);
+					PackWriter.NONE, null, excluded, false);
 			if (txn != null)
 				ret.add(txn);
 		}
@@ -1146,7 +1146,7 @@ public class GC {
 	private Pack writePack(@NonNull Set<? extends ObjectId> want,
 			@NonNull Set<? extends ObjectId> have, @NonNull Set<ObjectId> tags,
 			@NonNull Set<ObjectId> excludedRefsTips,
-			Set<ObjectId> tagTargets, List<ObjectIdSet> excludeObjects)
+			Set<ObjectId> tagTargets, List<ObjectIdSet> excludeObjects, boolean createBitmap)
 			throws IOException {
 		checkCancelled();
 		File tmpPack = null;
@@ -1177,6 +1177,7 @@ public class GC {
 			if (excludeObjects != null)
 				for (ObjectIdSet idx : excludeObjects)
 					pw.excludeObjects(idx);
+			pw.setCreateBitmaps(createBitmap);
 			pw.preparePack(pm, want, have, PackWriter.NONE,
 					union(tags, excludedRefsTips));
 			if (pw.getObjectCount() == 0)
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 61f92d2e16..4f28b0d37c 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
@@ -254,6 +254,8 @@ public class PackWriter implements AutoCloseable {
 
 	private boolean useBitmaps;
 
+	private boolean createBitmaps = true;
+
 	private boolean ignoreMissingUninteresting = true;
 
 	private boolean pruneCurrentObjectList;
@@ -574,6 +576,26 @@ public class PackWriter implements AutoCloseable {
 		this.useBitmaps = useBitmaps;
 	}
 
+	/**
+	 * Whether to generate bitmaps.
+	 *
+	 * @param createBitmaps
+	 *            if set to true, bitmaps will be generated when creating a pack.
+	 */
+	public void setCreateBitmaps(boolean createBitmaps) {
+		this.createBitmaps = createBitmaps;
+	}
+
+	/**
+	 * Whether the bitmap file is to be created by this PackWriter.
+	 *
+	 * @return {@code true} if the bitmap file is to be created by this
+	 *         PackWriter.
+	 */
+	public boolean isCreateBitmaps() {
+		return createBitmaps;
+	}
+
 	/**
 	 * Whether the index file cannot be created by this PackWriter.
 	 *
@@ -1910,7 +1932,7 @@ public class PackWriter implements AutoCloseable {
 		canBuildBitmaps = config.isBuildBitmaps()
 				&& !shallowPack
 				&& have.isEmpty()
-				&& (excludeInPacks == null || excludeInPacks.length == 0);
+				&& createBitmaps;
 		if (!shallowPack && useBitmaps) {
 			BitmapIndex bitmapIndex = reader.getBitmapIndex();
 			if (bitmapIndex != null) {
-- 
2.39.5