diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcBasicPackingTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcBasicPackingTest.java index 0742504d2..bbd41237e 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcBasicPackingTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcBasicPackingTest.java @@ -45,13 +45,16 @@ package org.eclipse.jgit.internal.storage.file; import static org.junit.Assert.assertEquals; +import java.io.File; import java.io.IOException; import java.util.Collection; +import java.util.Date; import java.util.Iterator; import org.eclipse.jgit.junit.TestRepository.BranchBuilder; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.storage.pack.PackConfig; +import org.junit.Test; import org.junit.experimental.theories.DataPoints; import org.junit.experimental.theories.Theories; import org.junit.experimental.theories.Theory; @@ -176,6 +179,47 @@ public class GcBasicPackingTest extends GcTestCase { } } + @Test + public void testDonePruneTooYoungPacks() throws Exception { + BranchBuilder bb = tr.branch("refs/heads/master"); + bb.commit().message("M").add("M", "M").create(); + + gc.setExpireAgeMillis(0); + gc.gc(); + stats = gc.getStatistics(); + assertEquals(0, stats.numberOfLooseObjects); + assertEquals(3, stats.numberOfPackedObjects); + assertEquals(1, stats.numberOfPackFiles); + File oldPackfile = tr.getRepository().getObjectDatabase().getPacks() + .iterator().next().getPackFile(); + + fsTick(); + bb.commit().message("B").add("B", "Q").create(); + + // The old packfile is too young to be deleted. We should end up with + // two pack files + gc.setExpire(new Date(oldPackfile.lastModified() - 1)); + gc.gc(); + stats = gc.getStatistics(); + assertEquals(0, stats.numberOfLooseObjects); + // if objects exist in multiple packFiles then they are counted multiple + // times + assertEquals(9, stats.numberOfPackedObjects); + assertEquals(2, stats.numberOfPackFiles); + + // repack again but now without a grace period for packfiles. We should + // end up with one packfile + gc.setExpireAgeMillis(0); + gc.gc(); + stats = gc.getStatistics(); + assertEquals(0, stats.numberOfLooseObjects); + // if objects exist in multiple packFiles then they are counted multiple + // times + assertEquals(6, stats.numberOfPackedObjects); + assertEquals(1, stats.numberOfPackFiles); + + } + private void configureGc(GC myGc, boolean aggressive) { PackConfig pconfig = new PackConfig(repo); if (aggressive) { 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 338106f8e..c5723c059 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 @@ -175,13 +175,17 @@ public class GC { /** * Delete old pack files. What is 'old' is defined by specifying a set of * old pack files and a set of new pack files. Each pack file contained in - * old pack files but not contained in new pack files will be deleted. + * old pack files but not contained in new pack files will be deleted. If an + * expirationDate is set then pack files which are younger than the + * expirationDate will not be deleted. * * @param oldPacks * @param newPacks + * @throws ParseException */ private void deleteOldPacks(Collection oldPacks, - Collection newPacks) { + Collection newPacks) throws ParseException { + long expireDate = getExpireDate(); oldPackLoop: for (PackFile oldPack : oldPacks) { String oldName = oldPack.getPackName(); // check whether an old pack file is also among the list of new @@ -190,7 +194,8 @@ public class GC { if (oldName.equals(newPack.getPackName())) continue oldPackLoop; - if (!oldPack.shouldBeKept()) { + if (!oldPack.shouldBeKept() + && oldPack.getPackFile().lastModified() < expireDate) { oldPack.close(); prunePack(oldName); } @@ -303,22 +308,7 @@ public class GC { */ public void prune(Set objectsToKeep) throws IOException, ParseException { - long expireDate = Long.MAX_VALUE; - - if (expire == null && expireAgeMillis == -1) { - String pruneExpireStr = repo.getConfig().getString( - ConfigConstants.CONFIG_GC_SECTION, null, - ConfigConstants.CONFIG_KEY_PRUNEEXPIRE); - if (pruneExpireStr == null) - pruneExpireStr = PRUNE_EXPIRE_DEFAULT; - expire = GitDateParser.parse(pruneExpireStr, null, SystemReader - .getInstance().getLocale()); - expireAgeMillis = -1; - } - if (expire != null) - expireDate = expire.getTime(); - if (expireAgeMillis != -1) - expireDate = System.currentTimeMillis() - expireAgeMillis; + long expireDate = getExpireDate(); // Collect all loose objects which are old enough, not referenced from // the index and not in objectsToKeep @@ -435,6 +425,26 @@ public class GC { repo.getObjectDatabase().close(); } + private long getExpireDate() throws ParseException { + long expireDate = Long.MAX_VALUE; + + if (expire == null && expireAgeMillis == -1) { + String pruneExpireStr = repo.getConfig().getString( + ConfigConstants.CONFIG_GC_SECTION, null, + ConfigConstants.CONFIG_KEY_PRUNEEXPIRE); + if (pruneExpireStr == null) + pruneExpireStr = PRUNE_EXPIRE_DEFAULT; + expire = GitDateParser.parse(pruneExpireStr, null, SystemReader + .getInstance().getLocale()); + expireAgeMillis = -1; + } + if (expire != null) + expireDate = expire.getTime(); + if (expireAgeMillis != -1) + expireDate = System.currentTimeMillis() - expireAgeMillis; + return expireDate; + } + /** * Remove all entries from a map which key is the id of an object referenced * by the given ObjectWalk @@ -559,7 +569,14 @@ public class GC { if (rest != null) ret.add(rest); } - deleteOldPacks(toBeDeleted, ret); + try { + deleteOldPacks(toBeDeleted, ret); + } catch (ParseException e) { + // TODO: the exception has to be wrapped into an IOException because + // throwing the ParseException directly would break the API, instead + // we should throw a ConfigInvalidException + throw new IOException(e); + } prunePacked(); lastPackedRefs = refsBefore;