Delete expired garbage even when there is no GC pack present.

Delete the condition to check whether the garbage pack creation time
is older than the last GC operation, because it's not possible to
find the last GC operation time when there is no GC pack.

Add additional tests to make sure the contents of the expired garbage
packs are considered during the GC operation and any actively
referenced objects from the garbage packs are copied successfully
into the GC pack before deleting the garbage pack.

Change-Id: I09e8b2656de8ba7f9b996724ad1961d908e937b6
Signed-off-by: Thirumala Reddy Mutchukota <thirumala@google.com>

1 files changed, 109 insertions, 26 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollectorTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollectorTest.java
index c70b6f2990..32002fd01d 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollectorTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollectorTest.java
@@ -7,7 +7,6 @@ import static org.eclipse.jgit.internal.storage.dfs.DfsObjDatabase.PackSource.UN
 import static org.eclipse.jgit.internal.storage.pack.PackExt.PACK;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
@@ -180,41 +179,133 @@ public class DfsGarbageCollectorTest {
 		RevCommit commit1 = commit().message("1").parent(commit0).create();
 		git.update("master", commit0);
 
-		gcNoTtl();
 		gcWithTtl();
-
-		// The repository has an UNREACHABLE_GARBAGE pack that could have
-		// expired, but since we never purge the most recent UNREACHABLE_GARBAGE
-		// pack, it must have survived the GC.
-		boolean commit1Found = false;
+		// The repository should have a GC pack with commit0 and an
+		// UNREACHABLE_GARBAGE pack with commit1.
+		assertEquals(2, odb.getPacks().length);
+		boolean gcPackFound = false;
+		boolean garbagePackFound = false;
 		for (DfsPackFile pack : odb.getPacks()) {
 			DfsPackDescription d = pack.getPackDescription();
 			if (d.getPackSource() == GC) {
+				gcPackFound = true;
 				assertTrue("has commit0", isObjectInPack(commit0, pack));
 				assertFalse("no commit1", isObjectInPack(commit1, pack));
 			} else if (d.getPackSource() == UNREACHABLE_GARBAGE) {
-				commit1Found |= isObjectInPack(commit1, pack);
+				garbagePackFound = true;
+				assertFalse("no commit0", isObjectInPack(commit0, pack));
+				assertTrue("has commit1", isObjectInPack(commit1, pack));
 			} else {
 				fail("unexpected " + d.getPackSource());
 			}
 		}
-		assertTrue("garbage commit1 still readable", commit1Found);
+		assertTrue("gc pack found", gcPackFound);
+		assertTrue("garbage pack found", garbagePackFound);
+
+		gcWithTtl();
+		// The gc operation should have removed UNREACHABLE_GARBAGE pack along with commit1.
+		DfsPackFile[] packs = odb.getPacks();
+		assertEquals(1, packs.length);
+
+		assertEquals(GC, packs[0].getPackDescription().getPackSource());
+		assertTrue("has commit0", isObjectInPack(commit0, packs[0]));
+		assertFalse("no commit1", isObjectInPack(commit1, packs[0]));
+	}
+
+	@Test
+	public void testCollectionWithGarbageAndRereferencingGarbage()
+			throws Exception {
+		RevCommit commit0 = commit().message("0").create();
+		RevCommit commit1 = commit().message("1").parent(commit0).create();
+		git.update("master", commit0);
 
-		// Find oldest UNREACHABLE_GARBAGE; it will be pruned by next GC.
-		DfsPackDescription oldestGarbagePack = null;
+		gcWithTtl();
+		// The repository should have a GC pack with commit0 and an
+		// UNREACHABLE_GARBAGE pack with commit1.
+		assertEquals(2, odb.getPacks().length);
+		boolean gcPackFound = false;
+		boolean garbagePackFound = false;
 		for (DfsPackFile pack : odb.getPacks()) {
 			DfsPackDescription d = pack.getPackDescription();
-			if (d.getPackSource() == UNREACHABLE_GARBAGE) {
-				oldestGarbagePack = oldestPack(oldestGarbagePack, d);
+			if (d.getPackSource() == GC) {
+				gcPackFound = true;
+				assertTrue("has commit0", isObjectInPack(commit0, pack));
+				assertFalse("no commit1", isObjectInPack(commit1, pack));
+			} else if (d.getPackSource() == UNREACHABLE_GARBAGE) {
+				garbagePackFound = true;
+				assertFalse("no commit0", isObjectInPack(commit0, pack));
+				assertTrue("has commit1", isObjectInPack(commit1, pack));
+			} else {
+				fail("unexpected " + d.getPackSource());
 			}
 		}
-		assertNotNull("has UNREACHABLE_GARBAGE", oldestGarbagePack);
+		assertTrue("gc pack found", gcPackFound);
+		assertTrue("garbage pack found", garbagePackFound);
+
+		git.update("master", commit1);
 
 		gcWithTtl();
-		assertTrue("has packs", odb.getPacks().length > 0);
-		for (DfsPackFile pack : odb.getPacks()) {
-			assertNotEquals(oldestGarbagePack, pack.getPackDescription());
-		}
+		// The gc operation should have removed the UNREACHABLE_GARBAGE pack and
+		// moved commit1 into GC pack.
+		DfsPackFile[] packs = odb.getPacks();
+		assertEquals(1, packs.length);
+
+		assertEquals(GC, packs[0].getPackDescription().getPackSource());
+		assertTrue("has commit0", isObjectInPack(commit0, packs[0]));
+		assertTrue("has commit1", isObjectInPack(commit1, packs[0]));
+	}
+
+	@Test
+	public void testCollectionWithPureGarbageAndGarbagePacksPurged()
+			throws Exception {
+		RevCommit commit0 = commit().message("0").create();
+		RevCommit commit1 = commit().message("1").parent(commit0).create();
+
+		gcWithTtl();
+		// The repository should have a single UNREACHABLE_GARBAGE pack with commit0
+		// and commit1.
+		DfsPackFile[] packs = odb.getPacks();
+		assertEquals(1, packs.length);
+
+		assertEquals(UNREACHABLE_GARBAGE, packs[0].getPackDescription().getPackSource());
+		assertTrue("has commit0", isObjectInPack(commit0, packs[0]));
+		assertTrue("has commit1", isObjectInPack(commit1, packs[0]));
+
+		gcWithTtl();
+		// The gc operation should have removed UNREACHABLE_GARBAGE pack along
+		// with commit0 and commit1.
+		assertEquals(0, odb.getPacks().length);
+	}
+
+	@Test
+	public void testCollectionWithPureGarbageAndRereferencingGarbage()
+			throws Exception {
+		RevCommit commit0 = commit().message("0").create();
+		RevCommit commit1 = commit().message("1").parent(commit0).create();
+
+		gcWithTtl();
+		// The repository should have a single UNREACHABLE_GARBAGE pack with commit0
+		// and commit1.
+		DfsPackFile[] packs = odb.getPacks();
+		assertEquals(1, packs.length);
+
+		DfsPackDescription pack = packs[0].getPackDescription();
+		assertEquals(UNREACHABLE_GARBAGE, pack.getPackSource());
+		assertTrue("has commit0", isObjectInPack(commit0, packs[0]));
+		assertTrue("has commit1", isObjectInPack(commit1, packs[0]));
+
+		git.update("master", commit0);
+
+		gcWithTtl();
+		// The gc operation should have moved commit0 into the GC pack and
+		// removed UNREACHABLE_GARBAGE along with commit1.
+		packs = odb.getPacks();
+		assertEquals(1, packs.length);
+
+		pack = packs[0].getPackDescription();
+		assertEquals(GC, pack.getPackSource());
+		assertTrue("has commit0", isObjectInPack(commit0, packs[0]));
+		assertFalse("no commit1", isObjectInPack(commit1, packs[0]));
 	}
 
 	@Test
@@ -588,14 +679,6 @@ public class DfsGarbageCollectorTest {
 		}
 	}
 
-	private static DfsPackDescription oldestPack(DfsPackDescription a,
-			DfsPackDescription b) {
-		if (a != null && a.getLastModified() < b.getLastModified()) {
-			return a;
-		}
-		return b;
-	}
-
 	private int countPacks(PackSource source) throws IOException {
 		int cnt = 0;
 		for (DfsPackFile pack : odb.getPacks()) {