diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GCTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GCTest.java index 9d2a03b09..35455f48a 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GCTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GCTest.java @@ -49,6 +49,7 @@ import static org.junit.Assert.assertTrue; import java.io.File; +import java.io.FileInputStream; import java.io.IOException; import java.util.Collection; import java.util.Collections; @@ -66,6 +67,7 @@ import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.internal.storage.file.GC.RepoStatistics; import org.eclipse.jgit.internal.storage.file.PackIndex.MutableEntry; +import org.eclipse.jgit.internal.storage.pack.PackExt; import org.eclipse.jgit.junit.LocalDiskRepositoryTestCase; import org.eclipse.jgit.junit.RepositoryTestCase; import org.eclipse.jgit.junit.TestRepository; @@ -620,6 +622,40 @@ public void testPackCommitsAndLooseOneWithPruneNow() throws Exception { assertEquals(2, stats.numberOfPackFiles); } + @Test + public void testPruneOldPacksWithOpenHandleOnPack() throws Exception { + gc.setExpireAgeMillis(0); + + BranchBuilder bb = tr.branch("refs/heads/master"); + bb.commit().add("A", "A").add("B", "B").create(); + fsTick(); + gc.gc(); + + Collection packs = repo.getObjectDatabase().getPacks(); + assertEquals(1, packs.size()); + PackFile pack = packs.iterator().next(); + File packFile = pack.getPackFile(); + File indexFile = new File(packFile.getParentFile(), "pack-" + + pack.getPackName() + + "." + + PackExt.INDEX.getExtension()); + FileInputStream fis = new FileInputStream(packFile); + try { + bb.commit().add("A", "A2").add("B", "B2").create(); + fsTick(); + gc.gc(); + if (packFile.exists()) { + assertTrue( + "The pack was present but the index file was missing.", + indexFile.exists()); + } + + } finally { + fis.close(); + } + + } + @Test public void testPackCommitsAndLooseOneWithPruneNowNoReflog() throws Exception { 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 3e26bc3e6..9eaeaa8c1 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,21 +175,9 @@ public Collection gc() throws IOException, ParseException { * * @param oldPacks * @param newPacks - * @param ignoreErrors - * true if we should ignore the fact that a certain - * pack files or index files couldn't be deleted. - * false if an exception should be thrown in such - * cases - * @throws IOException - * if a pack file couldn't be deleted and - * ignoreErrors is set to false */ private void deleteOldPacks(Collection oldPacks, - Collection newPacks, boolean ignoreErrors) - throws IOException { - int deleteOptions = FileUtils.RETRY | FileUtils.SKIP_MISSING; - if (ignoreErrors) - deleteOptions |= FileUtils.IGNORE_ERRORS; + Collection newPacks) { oldPackLoop: for (PackFile oldPack : oldPacks) { String oldName = oldPack.getPackName(); // check whether an old pack file is also among the list of new @@ -200,10 +188,7 @@ private void deleteOldPacks(Collection oldPacks, if (!oldPack.shouldBeKept()) { oldPack.close(); - for (PackExt ext : PackExt.values()) { - File f = nameFor(oldName, "." + ext.getExtension()); //$NON-NLS-1$ - FileUtils.delete(f, deleteOptions); - } + prunePack(oldName); } } // close the complete object database. Thats my only chance to force @@ -211,6 +196,42 @@ private void deleteOldPacks(Collection oldPacks, repo.getObjectDatabase().close(); } + /** + * Delete files associated with a single pack file. First try to delete the + * ".pack" file because on some platforms the ".pack" file may be locked and + * can't be deleted. In such a case it is better to detect this early and + * give up on deleting files for this packfile. Otherwise we may delete the + * ".index" file and when failing to delete the ".pack" file we are left + * with a ".pack" file without a ".index" file. + * + * @param packName + */ + private void prunePack(String packName) { + PackExt[] extensions = PackExt.values(); + try { + // Delete the .pack file first and if this fails give up on deleting + // the other files + int deleteOptions = FileUtils.RETRY | FileUtils.SKIP_MISSING; + for (PackExt ext : extensions) + if (PackExt.PACK.equals(ext)) { + File f = nameFor(packName, "." + ext.getExtension()); //$NON-NLS-1$ + FileUtils.delete(f, deleteOptions); + break; + } + // The .pack file has been deleted. Delete as many as the other + // files as you can. + deleteOptions |= FileUtils.IGNORE_ERRORS; + for (PackExt ext : extensions) { + if (!PackExt.PACK.equals(ext)) { + File f = nameFor(packName, "." + ext.getExtension()); //$NON-NLS-1$ + FileUtils.delete(f, deleteOptions); + } + } + } catch (IOException e) { + // Deletion of the .pack file failed. Silently return. + } + } + /** * Like "git prune-packed" this method tries to prune all loose objects * which can be found in packs. If certain objects can't be pruned (e.g. @@ -533,7 +554,7 @@ public Collection repack() throws IOException { if (rest != null) ret.add(rest); } - deleteOldPacks(toBeDeleted, ret, true); + deleteOldPacks(toBeDeleted, ret); prunePacked(); lastPackedRefs = refsBefore;