From f5f4bf0ad97f67ff56db18033c0a0795b722e96e Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Fri, 11 Aug 2023 20:15:17 +0100 Subject: [PATCH 1/4] Do not exclude objects in locked packs from bitmap processing Packfiles having an equivalent .keep file are associated with in-flight pushes that haven't been completed, with potentially a set of git objects not yet referenced by a ref. If the Git client is not up-to-date, it may result in pushing a packfile, generating a .keep on the server, which may also contain existing commits due to the lack of Git protocol negotiation in the git-receive-pack. The Git protocol negotiation is the phase where the client and the server exchange the list of refs they have for trying to find a common base and minimise the amount of objects to be transferred. The repack phase in GC was previously skipping all objects that were contained in all packfiles having a .keep file associated (aka "locked packfiles"), which did not take into consideration the fact that excluding the existing commits would have resulted in the generation of an invalid bitmap file. The code for excluding the objects in the locked packfiles was written well before the bitmap was introduced, hence could not consider a use case that did not exist at that time. However, when the bitmap was introduced, the exclusion of locked packfiles was not changed, hence creating a potential problem. The issue went unnoticed for many years because the bitmap generation was disabled when JGit noticed any locked packfiles; however, the bitmaps are enabled again since Id722e68d9f , and the the issue is now visible and is impacting the GC repack phase. Introduce the '--pack-kept-objects' option in GC for including the objects contained in the locked packfiles during the repack phase, which is not an issue because of the following: - If there are any existing commits duplicated in the packfiles they will be just considered once anyway because the repack doesn't generate duplicates in the output packfile. - If there are any new commits that do not have any ref pointing to them, they will be automatically excluded from the output repacked packfile. The same identical solution is adopted in the C implementation of git in repack.c. Because the locked packfile is not pruned, any new commits not pointed by any refs will remain in the repository and there will not be any accidental pruning or object loss as it is today before this change. As a side-effect of this change, it is now potentially possible to still have duplicate BLOBs after GC when the keep packfile contained existing objects. However, it is way better to keep the duplication until the next GC phase rather than omitting existing objects from repacking and, therefore generating an invalid bitmap and incorrect packfile. Bug: 582292 Bug: 582455 Change-Id: Ide3445e652fcf256a7912f881cb898897c99b8f8 --- .../jgit/pgm/internal/CLIText.properties | 1 + .../src/org/eclipse/jgit/pgm/Gc.java | 6 ++ .../bin/.project | 28 +++++++ .../storage/file/GcKeepFilesTest.java | 76 +++++++++++++++++++ org.eclipse.jgit/.settings/.api_filters | 8 ++ .../jgit/api/GarbageCollectCommand.java | 16 ++++ .../jgit/internal/storage/file/GC.java | 14 +++- 7 files changed, 148 insertions(+), 1 deletion(-) create mode 100644 org.eclipse.jgit.ssh.apache.agent/bin/.project diff --git a/org.eclipse.jgit.pgm/resources/org/eclipse/jgit/pgm/internal/CLIText.properties b/org.eclipse.jgit.pgm/resources/org/eclipse/jgit/pgm/internal/CLIText.properties index 97450033e..ae10af2f6 100644 --- a/org.eclipse.jgit.pgm/resources/org/eclipse/jgit/pgm/internal/CLIText.properties +++ b/org.eclipse.jgit.pgm/resources/org/eclipse/jgit/pgm/internal/CLIText.properties @@ -265,6 +265,7 @@ usage_MakeCacheTree=Show the current cache tree structure usage_Match=Only consider tags matching the given glob(7) pattern or patterns, excluding the "refs/tags/" prefix. usage_MergeBase=Find as good common ancestors as possible for a merge usage_MergesTwoDevelopmentHistories=Merges two development histories +usage_PackKeptObjects=Include objects in packs locked by a ".keep" file when repacking usage_PreserveOldPacks=Preserve old pack files by moving them into the preserved subdirectory instead of deleting them after repacking usage_PrunePreserved=Remove the preserved subdirectory containing previously preserved old pack files before repacking, and before preserving more old pack files usage_ReadDirCache= Read the DirCache 100 times diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Gc.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Gc.java index c87f0b6dc..35ac7a162 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Gc.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Gc.java @@ -27,6 +27,9 @@ class Gc extends TextBuiltin { @Option(name = "--prune-preserved", usage = "usage_PrunePreserved") private Boolean prunePreserved; + @Option(name = "--pack-kept-objects", usage = "usage_PackKeptObjects") + private Boolean packKeptObjects; + /** {@inheritDoc} */ @Override protected void run() { @@ -40,6 +43,9 @@ protected void run() { if (prunePreserved != null) { command.setPrunePreserved(prunePreserved.booleanValue()); } + if (packKeptObjects != null) { + command.setPackKeptObjects(packKeptObjects.booleanValue()); + } command.call(); } catch (GitAPIException e) { throw die(e.getMessage(), e); diff --git a/org.eclipse.jgit.ssh.apache.agent/bin/.project b/org.eclipse.jgit.ssh.apache.agent/bin/.project new file mode 100644 index 000000000..73358f4a6 --- /dev/null +++ b/org.eclipse.jgit.ssh.apache.agent/bin/.project @@ -0,0 +1,28 @@ + + + org.eclipse.jgit.ssh.apache.agent + + + + + + org.eclipse.jdt.core.javabuilder + + + + + org.eclipse.pde.ManifestBuilder + + + + + org.eclipse.pde.SchemaBuilder + + + + + + org.eclipse.pde.PluginNature + org.eclipse.jdt.core.javanature + + diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcKeepFilesTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcKeepFilesTest.java index 67848ae7e..5919192e5 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcKeepFilesTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcKeepFilesTest.java @@ -12,6 +12,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import java.util.Iterator; @@ -19,9 +20,12 @@ import org.eclipse.jgit.internal.storage.file.PackIndex.MutableEntry; import org.eclipse.jgit.internal.storage.pack.PackExt; import org.eclipse.jgit.junit.TestRepository.BranchBuilder; +import org.eclipse.jgit.lib.ObjectId; import org.junit.Test; public class GcKeepFilesTest extends GcTestCase { + private static final int COMMIT_AND_TREE_OBJECTS = 2; + @Test public void testKeepFiles() throws Exception { BranchBuilder bb = tr.branch("refs/heads/master"); @@ -72,4 +76,76 @@ public void testKeepFiles() throws Exception { + e.toObjectId(), ind2.hasObject(e.toObjectId())); } + + @Test + public void testKeptObjectsAreIncluded() throws Exception { + BranchBuilder bb = tr.branch("refs/heads/master"); + ObjectId commitObjectInLockedPack = bb.commit().create().toObjectId(); + gc.gc(); + stats = gc.getStatistics(); + assertEquals(COMMIT_AND_TREE_OBJECTS, stats.numberOfPackedObjects); + assertEquals(1, stats.numberOfPackFiles); + assertTrue(getSinglePack().getPackFile().create(PackExt.KEEP).createNewFile()); + + bb.commit().create(); + gc.setPackKeptObjects(true); + gc.gc(); + stats = gc.getStatistics(); + assertEquals(2 * COMMIT_AND_TREE_OBJECTS + 1, + stats.numberOfPackedObjects); + assertEquals(2, stats.numberOfPackFiles); + + PackIndex lockedPackIdx = null; + PackIndex newPackIdx = null; + for (Pack pack : repo.getObjectDatabase().getPacks()) { + if (pack.getObjectCount() == COMMIT_AND_TREE_OBJECTS) { + lockedPackIdx = pack.getIndex(); + } else { + newPackIdx = pack.getIndex(); + } + } + assertNotNull(lockedPackIdx); + assertTrue(lockedPackIdx.hasObject(commitObjectInLockedPack)); + assertNotNull(newPackIdx); + assertTrue(newPackIdx.hasObject(commitObjectInLockedPack)); + } + + @Test + public void testKeptObjectsAreNotIncludedByDefault() throws Exception { + BranchBuilder bb = tr.branch("refs/heads/master"); + ObjectId commitObjectInLockedPack = bb.commit().create().toObjectId(); + gc.gc(); + stats = gc.getStatistics(); + assertEquals(COMMIT_AND_TREE_OBJECTS, stats.numberOfPackedObjects); + assertEquals(1, stats.numberOfPackFiles); + assertTrue(getSinglePack().getPackFile().create(PackExt.KEEP).createNewFile()); + + bb.commit().create(); + gc.gc(); + stats = gc.getStatistics(); + assertEquals(COMMIT_AND_TREE_OBJECTS + 1, stats.numberOfPackedObjects); + assertEquals(2, stats.numberOfPackFiles); + + PackIndex lockedPackIdx = null; + PackIndex newPackIdx = null; + for (Pack pack : repo.getObjectDatabase().getPacks()) { + if (pack.getObjectCount() == COMMIT_AND_TREE_OBJECTS) { + lockedPackIdx = pack.getIndex(); + } else { + newPackIdx = pack.getIndex(); + } + } + assertNotNull(lockedPackIdx); + assertTrue(lockedPackIdx.hasObject(commitObjectInLockedPack)); + assertNotNull(newPackIdx); + assertFalse(newPackIdx.hasObject(commitObjectInLockedPack)); + } + + private Pack getSinglePack() { + Iterator packIt = repo.getObjectDatabase().getPacks() + .iterator(); + Pack singlePack = packIt.next(); + assertFalse(packIt.hasNext()); + return singlePack; + } } diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters index 1d3917173..24c6a3150 100644 --- a/org.eclipse.jgit/.settings/.api_filters +++ b/org.eclipse.jgit/.settings/.api_filters @@ -1,5 +1,13 @@ + + + + + + + + diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/GarbageCollectCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/GarbageCollectCommand.java index a2fbd411f..2e09d4a8e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/GarbageCollectCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/GarbageCollectCommand.java @@ -62,6 +62,8 @@ public class GarbageCollectCommand extends GitCommand { private PackConfig pconfig; + private boolean packKeptObjects; + /** * Constructor for GarbageCollectCommand. * @@ -130,6 +132,19 @@ public GarbageCollectCommand setAggressive(boolean aggressive) { return this; } + /** + * Whether to include objects in `.keep` packs when repacking. + * + * @param packKeptObjects + * whether to include objects in `.keep` files when repacking. + * @return this instance + * @since 5.13.3 + */ + public GarbageCollectCommand setPackKeptObjects(boolean packKeptObjects) { + this.packKeptObjects = packKeptObjects; + return this; + } + /** * Whether to preserve old pack files instead of deleting them. * @@ -174,6 +189,7 @@ public Properties call() throws GitAPIException { gc.setProgressMonitor(monitor); if (this.expire != null) gc.setExpire(expire); + gc.setPackKeptObjects(packKeptObjects); try { gc.gc(); 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 4db922b2c..63edfa64b 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 @@ -158,6 +158,8 @@ public static void setExecutor(ExecutorService e) { private Date packExpire; + private boolean packKeptObjects; + private PackConfig pconfig; /** @@ -839,8 +841,9 @@ public Collection repack() throws IOException { List excluded = new LinkedList<>(); for (Pack p : repo.getObjectDatabase().getPacks()) { checkCancelled(); - if (p.shouldBeKept()) + if (!packKeptObjects && p.shouldBeKept()) { excluded.add(p.getIndex()); + } } // Don't exclude tags that are also branch tips @@ -1307,6 +1310,15 @@ private void checkCancelled() throws CancelledException { } } + /** + * Define whether to include objects in `.keep` files when repacking. + * + * @param packKeptObjects Whether to include objects in `.keep` files when repacking. + */ + public void setPackKeptObjects(boolean packKeptObjects) { + this.packKeptObjects = packKeptObjects; + } + /** * A class holding statistical data for a FileRepository regarding how many * objects are stored as loose or packed objects From f103a1d5c605f5f4545050c1176a9a202151f942 Mon Sep 17 00:00:00 2001 From: Antonio Barone Date: Thu, 5 Oct 2023 21:58:13 +0200 Subject: [PATCH 2/4] Add support for git config repack.packKeptObjects Change Ide3445e652 introduced the `--pack-kept-objects` option to GC for including the objects contained in the locked packfiles during the repack phase. Whilst this allowed to explicitly pass a command line argument to the jgit gc program, it did not allow the option to be read from configuration. Allow the pack kept objects option to be configured exactly as C-Git documents [1], by introducing a new `repack.packKeptObjects` configuration. `repack.packKeptObjects` defaults to `true`, when the `pack.buildBitmaps` is `true` (which is the default case), `false` otherwise. [1] https://git-scm.com/docs/git-config#Documentation/git-config.txt-repackpackKeptObjects Bug: 582292 Change-Id: Ia931667277410d71bc079d27c097a57094299840 --- Documentation/config-options.md | 6 ++ .../storage/file/GcKeepFilesTest.java | 96 +++++++++++++++++-- org.eclipse.jgit/.settings/.api_filters | 24 +++++ .../jgit/api/GarbageCollectCommand.java | 9 +- .../jgit/internal/storage/file/GC.java | 12 ++- .../org/eclipse/jgit/lib/ConfigConstants.java | 13 +++ .../eclipse/jgit/storage/pack/PackConfig.java | 48 +++++++++- 7 files changed, 191 insertions(+), 17 deletions(-) diff --git a/Documentation/config-options.md b/Documentation/config-options.md index 5d76483ac..7221cceb3 100644 --- a/Documentation/config-options.md +++ b/Documentation/config-options.md @@ -110,3 +110,9 @@ Proxy configuration uses the standard Java mechanisms via class `java.net.ProxyS | `pack.waitPreventRacyPack` | `false` | ⃞ | Whether we wait before opening a newly written pack to prevent its lastModified timestamp could be racy. | | `pack.window` | `10` | ✅ | Number of objects to try when looking for a delta base per thread searching for deltas. | | `pack.windowMemory` | `0` (unlimited) | ✅ | Maximum number of bytes to put into the delta search window. | + +## __repack__ options + +| option | default | git option | description | +|---------|---------|------------|-------------| +| `repack.packKeptObjects` | `true` when `pack.buildBitmaps` is set, `false` otherwise | ✅ | Include objects in packs locked by a `.keep` file when repacking. | \ No newline at end of file diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcKeepFilesTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcKeepFilesTest.java index 5919192e5..074728b68 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcKeepFilesTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcKeepFilesTest.java @@ -10,6 +10,10 @@ package org.eclipse.jgit.internal.storage.file; +import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_BUILD_BITMAPS; +import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_PACK_KEPT_OBJECTS; +import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_PACK_SECTION; +import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_REPACK_SECTION; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -20,7 +24,9 @@ import org.eclipse.jgit.internal.storage.file.PackIndex.MutableEntry; import org.eclipse.jgit.internal.storage.pack.PackExt; import org.eclipse.jgit.junit.TestRepository.BranchBuilder; +import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.storage.pack.PackConfig; import org.junit.Test; public class GcKeepFilesTest extends GcTestCase { @@ -55,6 +61,7 @@ public void testKeepFiles() throws Exception { PackFile bitmapFile = singlePack.getPackFile().create(PackExt.BITMAP_INDEX); assertTrue(keepFile.exists()); assertTrue(bitmapFile.delete()); + gc.setPackKeptObjects(false); gc.gc(); stats = gc.getStatistics(); assertEquals(0, stats.numberOfLooseObjects); @@ -78,17 +85,91 @@ public void testKeepFiles() throws Exception { } @Test - public void testKeptObjectsAreIncluded() throws Exception { + public void testKeptObjectsAreIncludedByDefault() throws Exception { + testKeptObjectsAreIncluded(); + } + + @Test + public void testKeptObjectsAreIncludedByDefaultWhenBuildBitmapsIsTrue() + throws Exception { + PackConfig packConfig = new PackConfig(); + Config repoConfig = repo.getObjectDatabase().getConfig(); + repoConfig.setBoolean(CONFIG_PACK_SECTION, null, + CONFIG_KEY_BUILD_BITMAPS, true); + packConfig.fromConfig(repoConfig); + gc.setPackConfig(packConfig); + + testKeptObjectsAreIncluded(); + } + + @Test + public void testKeptObjectsAreIncludedWhenPackKeptObjectsIsFalseButOverriddenViaCommandLine() + throws Exception { + PackConfig packConfig = new PackConfig(); + packConfig.setPackKeptObjects(false); + gc.setPackConfig(packConfig); + gc.setPackKeptObjects(true); + + testKeptObjectsAreIncluded(); + } + + @Test + public void testKeptObjectsAreNotIncludedByDefaultWhenBuildBitmapsIsFalse() + throws Exception { + PackConfig packConfig = new PackConfig(); + packConfig.setBuildBitmaps(false); + gc.setPackConfig(packConfig); + + testKeptObjectsAreNotIncluded(); + } + + @Test + public void testKeptObjectsAreIncludedWhenBuildBitmapsIsFalseButPackKeptObjectsIsTrue() + throws Exception { + PackConfig packConfig = new PackConfig(); + Config repoConfig = repo.getObjectDatabase().getConfig(); + repoConfig.setBoolean(CONFIG_PACK_SECTION, null, + CONFIG_KEY_BUILD_BITMAPS, false); + repoConfig.setBoolean(CONFIG_REPACK_SECTION, null, + CONFIG_KEY_PACK_KEPT_OBJECTS, true); + packConfig.fromConfig(repoConfig); + gc.setPackConfig(packConfig); + + testKeptObjectsAreIncluded(); + } + + @Test + public void testKeptObjectsAreNotIncludedWhenPackKeptObjectsIsTrueButOverriddenViaCommandLine() + throws Exception { + PackConfig packConfig = new PackConfig(); + packConfig.setPackKeptObjects(true); + gc.setPackConfig(packConfig); + gc.setPackKeptObjects(false); + + testKeptObjectsAreNotIncluded(); + } + + @Test + public void testKeptObjectsAreNotIncludedWhenPackKeptObjectsConfigIsFalse() + throws Exception { + PackConfig packConfig = new PackConfig(); + packConfig.setPackKeptObjects(false); + gc.setPackConfig(packConfig); + + testKeptObjectsAreNotIncluded(); + } + + private void testKeptObjectsAreIncluded() throws Exception { BranchBuilder bb = tr.branch("refs/heads/master"); ObjectId commitObjectInLockedPack = bb.commit().create().toObjectId(); gc.gc(); stats = gc.getStatistics(); assertEquals(COMMIT_AND_TREE_OBJECTS, stats.numberOfPackedObjects); assertEquals(1, stats.numberOfPackFiles); - assertTrue(getSinglePack().getPackFile().create(PackExt.KEEP).createNewFile()); + assertTrue(getSinglePack().getPackFile().create(PackExt.KEEP) + .createNewFile()); bb.commit().create(); - gc.setPackKeptObjects(true); gc.gc(); stats = gc.getStatistics(); assertEquals(2 * COMMIT_AND_TREE_OBJECTS + 1, @@ -110,15 +191,15 @@ public void testKeptObjectsAreIncluded() throws Exception { assertTrue(newPackIdx.hasObject(commitObjectInLockedPack)); } - @Test - public void testKeptObjectsAreNotIncludedByDefault() throws Exception { + private void testKeptObjectsAreNotIncluded() throws Exception { BranchBuilder bb = tr.branch("refs/heads/master"); ObjectId commitObjectInLockedPack = bb.commit().create().toObjectId(); gc.gc(); stats = gc.getStatistics(); assertEquals(COMMIT_AND_TREE_OBJECTS, stats.numberOfPackedObjects); assertEquals(1, stats.numberOfPackFiles); - assertTrue(getSinglePack().getPackFile().create(PackExt.KEEP).createNewFile()); + assertTrue(getSinglePack().getPackFile().create(PackExt.KEEP) + .createNewFile()); bb.commit().create(); gc.gc(); @@ -142,8 +223,7 @@ public void testKeptObjectsAreNotIncludedByDefault() throws Exception { } private Pack getSinglePack() { - Iterator packIt = repo.getObjectDatabase().getPacks() - .iterator(); + Iterator packIt = repo.getObjectDatabase().getPacks().iterator(); Pack singlePack = packIt.next(); assertFalse(packIt.hasNext()); return singlePack; diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters index 24c6a3150..caf331143 100644 --- a/org.eclipse.jgit/.settings/.api_filters +++ b/org.eclipse.jgit/.settings/.api_filters @@ -53,6 +53,18 @@ + + + + + + + + + + + + @@ -69,6 +81,12 @@ + + + + + + @@ -93,6 +111,12 @@ + + + + + + diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/GarbageCollectCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/GarbageCollectCommand.java index 2e09d4a8e..8b7e3e161 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/GarbageCollectCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/GarbageCollectCommand.java @@ -62,7 +62,7 @@ public class GarbageCollectCommand extends GitCommand { private PackConfig pconfig; - private boolean packKeptObjects; + private Boolean packKeptObjects; /** * Constructor for GarbageCollectCommand. @@ -141,7 +141,7 @@ public GarbageCollectCommand setAggressive(boolean aggressive) { * @since 5.13.3 */ public GarbageCollectCommand setPackKeptObjects(boolean packKeptObjects) { - this.packKeptObjects = packKeptObjects; + this.packKeptObjects = Boolean.valueOf(packKeptObjects); return this; } @@ -189,8 +189,9 @@ public Properties call() throws GitAPIException { gc.setProgressMonitor(monitor); if (this.expire != null) gc.setExpire(expire); - gc.setPackKeptObjects(packKeptObjects); - + if (this.packKeptObjects != null) { + gc.setPackKeptObjects(packKeptObjects.booleanValue()); + } try { gc.gc(); return toProperties(gc.getStatistics()); 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 63edfa64b..c8dd71ca3 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 @@ -158,7 +158,7 @@ public static void setExecutor(ExecutorService e) { private Date packExpire; - private boolean packKeptObjects; + private Boolean packKeptObjects; private PackConfig pconfig; @@ -841,7 +841,7 @@ public Collection repack() throws IOException { List excluded = new LinkedList<>(); for (Pack p : repo.getObjectDatabase().getPacks()) { checkCancelled(); - if (!packKeptObjects && p.shouldBeKept()) { + if (!shouldPackKeptObjects() && p.shouldBeKept()) { excluded.add(p.getIndex()); } } @@ -1316,7 +1316,13 @@ private void checkCancelled() throws CancelledException { * @param packKeptObjects Whether to include objects in `.keep` files when repacking. */ public void setPackKeptObjects(boolean packKeptObjects) { - this.packKeptObjects = packKeptObjects; + this.packKeptObjects = Boolean.valueOf(packKeptObjects); + } + + @SuppressWarnings("boxing") + private boolean shouldPackKeptObjects() { + return Optional.ofNullable(packKeptObjects) + .orElse(pconfig.isPackKeptObjects()); } /** diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java index 634e3f7f8..605b44bee 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java @@ -59,6 +59,12 @@ public final class ConfigConstants { /** The "gc" section */ public static final String CONFIG_GC_SECTION = "gc"; + /** + * The "repack" section + * @since 5.13.3 + */ + public static final String CONFIG_REPACK_SECTION = "repack"; + /** The "pack" section */ public static final String CONFIG_PACK_SECTION = "pack"; @@ -728,6 +734,13 @@ public final class ConfigConstants { */ public static final String CONFIG_KEY_WINDOW_MEMORY = "windowmemory"; + /** + * The "repack.packKeptObjects" key + * + * @since 5.13.3 + */ + public static final String CONFIG_KEY_PACK_KEPT_OBJECTS = "packkeptobjects"; + /** * The "feature" section * diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackConfig.java index 163e47588..47ea733fa 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackConfig.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackConfig.java @@ -37,8 +37,10 @@ import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_WINDOW; import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_WINDOW_MEMORY; import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_PACK_SECTION; +import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_PACK_KEPT_OBJECTS; import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_PRESERVE_OLD_PACKS; import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_PRUNE_PRESERVED; +import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_REPACK_SECTION; import java.time.Duration; import java.util.concurrent.Executor; @@ -168,6 +170,16 @@ public class PackConfig { */ public static final boolean DEFAULT_BUILD_BITMAPS = true; + + /** + * Default value for including objects in packs locked by .keep file when + * repacking: {@value} + * + * @see #setPackKeptObjects(boolean) + * @since 5.13.3 + */ + public static final boolean DEFAULT_PACK_KEPT_OBJECTS = false; + /** * Default count of most recent commits to select for bitmaps. Only applies * when bitmaps are enabled: {@value} @@ -284,6 +296,8 @@ public class PackConfig { private boolean buildBitmaps = DEFAULT_BUILD_BITMAPS; + private boolean packKeptObjects = DEFAULT_PACK_KEPT_OBJECTS; + private int bitmapContiguousCommitCount = DEFAULT_BITMAP_CONTIGUOUS_COMMIT_COUNT; private int bitmapRecentCommitCount = DEFAULT_BITMAP_RECENT_COMMIT_COUNT; @@ -362,6 +376,7 @@ public PackConfig(PackConfig cfg) { this.executor = cfg.executor; this.indexVersion = cfg.indexVersion; this.buildBitmaps = cfg.buildBitmaps; + this.packKeptObjects = cfg.packKeptObjects; this.bitmapContiguousCommitCount = cfg.bitmapContiguousCommitCount; this.bitmapRecentCommitCount = cfg.bitmapRecentCommitCount; this.bitmapRecentCommitSpan = cfg.bitmapRecentCommitSpan; @@ -989,6 +1004,31 @@ public void setBuildBitmaps(boolean buildBitmaps) { this.buildBitmaps = buildBitmaps; } + /** + * Set whether to include objects in `.keep` files when repacking. + * + *

Default setting: {@value #DEFAULT_PACK_KEPT_OBJECTS} + * + * @param packKeptObjects boolean indicating whether to include objects in + * `.keep` files when repacking. + * @since 5.13 + */ + public void setPackKeptObjects(boolean packKeptObjects) { + this.packKeptObjects = packKeptObjects; + } + + /** + * True if objects in `.keep` files should be included when repacking. + * + * Default setting: {@value #DEFAULT_PACK_KEPT_OBJECTS} + * + * @return True if objects in `.keep` files should be included when repacking. + * @since 5.13 + */ + public boolean isPackKeptObjects() { + return packKeptObjects; + } + /** * Get the count of most recent commits for which to build bitmaps. * @@ -1234,8 +1274,12 @@ public void fromConfig(Config rc) { setSinglePack(rc.getBoolean(CONFIG_PACK_SECTION, CONFIG_KEY_SINGLE_PACK, getSinglePack())); - setBuildBitmaps(rc.getBoolean(CONFIG_PACK_SECTION, - CONFIG_KEY_BUILD_BITMAPS, isBuildBitmaps())); + boolean buildBitmapsFromConfig = rc.getBoolean(CONFIG_PACK_SECTION, + CONFIG_KEY_BUILD_BITMAPS, isBuildBitmaps()); + setBuildBitmaps(buildBitmapsFromConfig); + setPackKeptObjects(rc.getBoolean(CONFIG_REPACK_SECTION, + CONFIG_KEY_PACK_KEPT_OBJECTS, + buildBitmapsFromConfig || isPackKeptObjects())); setBitmapContiguousCommitCount(rc.getInt(CONFIG_PACK_SECTION, CONFIG_KEY_BITMAP_CONTIGUOUS_COMMIT_COUNT, getBitmapContiguousCommitCount())); From 244165fc56bd7b15d01f37496a1a5039b4ead300 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Fri, 13 Oct 2023 00:18:59 +0200 Subject: [PATCH 3/4] Remove unused API problem filters Change-Id: I9d5b96cf841478af8613667ef8574423630f8028 --- .../.settings/.api_filters | 17 -- org.eclipse.jgit/.settings/.api_filters | 206 ------------------ 2 files changed, 223 deletions(-) delete mode 100644 org.eclipse.jgit.http.server/.settings/.api_filters delete mode 100644 org.eclipse.jgit/.settings/.api_filters diff --git a/org.eclipse.jgit.http.server/.settings/.api_filters b/org.eclipse.jgit.http.server/.settings/.api_filters deleted file mode 100644 index 951a53bf3..000000000 --- a/org.eclipse.jgit.http.server/.settings/.api_filters +++ /dev/null @@ -1,17 +0,0 @@ - - - - - - - - - - - - - - - - - diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters deleted file mode 100644 index caf331143..000000000 --- a/org.eclipse.jgit/.settings/.api_filters +++ /dev/null @@ -1,206 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - From 4d6671b4cec3b3822c75089f3a8aa16c39d49307 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Fri, 13 Oct 2023 00:19:12 +0200 Subject: [PATCH 4/4] PackConfig: fix @since tags Change-Id: Ia513f7cdbf3c197e8661720fc804984ff165fc5c --- .../org/eclipse/jgit/storage/pack/PackConfig.java | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackConfig.java index 47ea733fa..ff925dbe8 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackConfig.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackConfig.java @@ -1007,11 +1007,13 @@ public void setBuildBitmaps(boolean buildBitmaps) { /** * Set whether to include objects in `.keep` files when repacking. * - *

Default setting: {@value #DEFAULT_PACK_KEPT_OBJECTS} + *

+ * Default setting: {@value #DEFAULT_PACK_KEPT_OBJECTS} * - * @param packKeptObjects boolean indicating whether to include objects in - * `.keep` files when repacking. - * @since 5.13 + * @param packKeptObjects + * boolean indicating whether to include objects in `.keep` files + * when repacking. + * @since 5.13.3 */ public void setPackKeptObjects(boolean packKeptObjects) { this.packKeptObjects = packKeptObjects; @@ -1022,8 +1024,9 @@ public void setPackKeptObjects(boolean packKeptObjects) { * * Default setting: {@value #DEFAULT_PACK_KEPT_OBJECTS} * - * @return True if objects in `.keep` files should be included when repacking. - * @since 5.13 + * @return True if objects in `.keep` files should be included when + * repacking. + * @since 5.13.3 */ public boolean isPackKeptObjects() { return packKeptObjects;