Delete all loose refs empty directories

Remove completely the empty directories under refs/<namespace>
including the first level partition of the changes, when they are
completely empty.

Bug: 536777
Change-Id: I88304d34cc42435919c2d1480258684d993dfdca
Signed-off-by: Luca Milanesio <luca.milanesio@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcDeleteEmptyRefsFoldersTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcDeleteEmptyRefsFoldersTest.java
index 76e1534c5940b9766d720af85839f7b0309f5b47..b37cd7ae05aee05a3fc63b0149d36f6a85e72496 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcDeleteEmptyRefsFoldersTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcDeleteEmptyRefsFoldersTest.java
@@ -46,6 +46,7 @@ package org.eclipse.jgit.internal.storage.file;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
+import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -56,8 +57,8 @@ import org.junit.Before;
 import org.junit.Test;
 
 public class GcDeleteEmptyRefsFoldersTest extends GcTestCase {
-       private static final String REF_FOLDER_01 = "01";
-       private static final String REF_FOLDER_02 = "02";
+       private static final String REF_FOLDER_01 = "A/B/01";
+       private static final String REF_FOLDER_02 = "C/D/02";
 
        private Path refsDir;
        private Path heads;
@@ -74,23 +75,36 @@ public class GcDeleteEmptyRefsFoldersTest extends GcTestCase {
        @Test
        public void emptyRefFoldersAreDeleted() throws Exception {
                FileTime fileTime = FileTime.from(Instant.now().minusSeconds(31));
-               Path refDir01 = Files.createDirectory(heads.resolve(REF_FOLDER_01));
-               Path refDir02 = Files.createDirectory(heads.resolve(REF_FOLDER_02));
-               Files.setLastModifiedTime(refDir01, fileTime);
-               Files.setLastModifiedTime(refDir02, fileTime);
+               Path refDir01 = Files.createDirectories(heads.resolve(REF_FOLDER_01));
+               Path refDir02 = Files.createDirectories(heads.resolve(REF_FOLDER_02));
+               setLastModifiedTime(fileTime, heads, REF_FOLDER_01);
+               setLastModifiedTime(fileTime, heads, REF_FOLDER_02);
                assertTrue(refDir01.toFile().exists());
                assertTrue(refDir02.toFile().exists());
                gc.gc();
 
                assertFalse(refDir01.toFile().exists());
+               assertFalse(refDir01.getParent().toFile().exists());
+               assertFalse(refDir01.getParent().getParent().toFile().exists());
                assertFalse(refDir02.toFile().exists());
+               assertFalse(refDir02.getParent().toFile().exists());
+               assertFalse(refDir02.getParent().getParent().toFile().exists());
+       }
+
+       private void setLastModifiedTime(FileTime fileTime, Path path, String folder) throws IOException {
+               long numParents = folder.chars().filter(c -> c == '/').count();
+               Path folderPath = path.resolve(folder);
+               for(int folderLevel = 0; folderLevel <= numParents; folderLevel ++ ) {
+                       Files.setLastModifiedTime(folderPath, fileTime);
+                       folderPath = folderPath.getParent();
+               }
        }
 
        @Test
        public void emptyRefFoldersAreKeptIfTheyAreTooRecent()
                        throws Exception {
-               Path refDir01 = Files.createDirectory(heads.resolve(REF_FOLDER_01));
-               Path refDir02 = Files.createDirectory(heads.resolve(REF_FOLDER_02));
+               Path refDir01 = Files.createDirectories(heads.resolve(REF_FOLDER_01));
+               Path refDir02 = Files.createDirectories(heads.resolve(REF_FOLDER_02));
                assertTrue(refDir01.toFile().exists());
                assertTrue(refDir02.toFile().exists());
                gc.gc();
@@ -101,8 +115,8 @@ public class GcDeleteEmptyRefsFoldersTest extends GcTestCase {
 
        @Test
        public void nonEmptyRefsFoldersAreKept() throws Exception {
-               Path refDir01 = Files.createDirectory(heads.resolve(REF_FOLDER_01));
-               Path refDir02 = Files.createDirectory(heads.resolve(REF_FOLDER_02));
+               Path refDir01 = Files.createDirectories(heads.resolve(REF_FOLDER_01));
+               Path refDir02 = Files.createDirectories(heads.resolve(REF_FOLDER_02));
                Path ref01 = Files.createFile(refDir01.resolve("ref01"));
                Path ref02 = Files.createFile(refDir01.resolve("ref02"));
                assertTrue(refDir01.toFile().exists());

diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
index 68bb68e4261e9ee8d6013d365f3194391fd00c94..16f184babc6479cbef8a7c765a08984240fcc3e8 100644 (file)
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -45,6 +45,7 @@ branchNameInvalid=Branch name {0} is not allowed
 buildingBitmaps=Building bitmaps
 cachedPacksPreventsIndexCreation=Using cached packs prevents index creation
 cachedPacksPreventsListingObjects=Using cached packs prevents listing objects
+cannotAccessLastModifiedForSafeDeletion=Unable to access lastModifiedTime of file {0}, skip deletion since we cannot safely avoid race condition
 cannotBeCombined=Cannot be combined.
 cannotBeRecursiveWhenTreesAreIncluded=TreeWalk shouldn't be recursive when tree objects are included.
 cannotChangeActionOnComment=Cannot change action on comment line in git-rebase-todo file, old action: {0}, new action: {1}.

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
index 56a24482bec19f4ca67381a66f29cd009fc3571f..4b7459c15b45abfed080175f99c5a90728170eb3 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
@@ -104,6 +104,7 @@ public class JGitText extends TranslationBundle {
        /***/ public String buildingBitmaps;
        /***/ public String cachedPacksPreventsIndexCreation;
        /***/ public String cachedPacksPreventsListingObjects;
+       /***/ public String cannotAccessLastModifiedForSafeDeletion;
        /***/ public String cannotBeCombined;
        /***/ public String cannotBeRecursiveWhenTreesAreIncluded;
        /***/ public String cannotChangeActionOnComment;

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 eb5c1db50ee252a41fd0d57acc7e1b5458390e93..d99a48524950052704bd6d5dd49ec18a960f92e2 100644 (file)
--- 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
@@ -885,16 +885,31 @@ public class GC {
 
        private void deleteEmptyRefsFolders() throws IOException {
                Path refs = repo.getDirectory().toPath().resolve(Constants.R_REFS);
+               // Avoid deleting a folder that was created after the threshold so that concurrent
+               // operations trying to create a reference are not impacted
+               Instant threshold = Instant.now().minus(30, ChronoUnit.SECONDS);
                try (Stream<Path> entries = Files.list(refs)) {
                        Iterator<Path> iterator = entries.iterator();
                        while (iterator.hasNext()) {
                                try (Stream<Path> s = Files.list(iterator.next())) {
-                                       s.forEach(this::deleteDir);
+                                       s.filter(path -> canBeSafelyDeleted(path, threshold)).forEach(this::deleteDir);
                                }
                        }
                }
        }
 
+       private boolean canBeSafelyDeleted(Path path, Instant threshold) {
+               try {
+                       return Files.getLastModifiedTime(path).toInstant().isBefore(threshold);
+               }
+               catch (IOException e) {
+                       LOG.warn(MessageFormat.format(
+                                       JGitText.get().cannotAccessLastModifiedForSafeDeletion,
+                                       path), e);
+                       return false;
+               }
+       }
+
        private void deleteDir(Path dir) {
                try (Stream<Path> dirs = Files.walk(dir)) {
                        dirs.filter(this::isDirectory).sorted(Comparator.reverseOrder())
@@ -910,16 +925,7 @@ public class GC {
 
        private void delete(Path d) {
                try {
-                       // Avoid deleting a folder that was just created so that concurrent
-                       // operations trying to create a reference are not impacted
-                       Instant threshold = Instant.now().minus(30, ChronoUnit.SECONDS);
-                       Instant lastModified = Files.getLastModifiedTime(d).toInstant();
-                       if (lastModified.isBefore(threshold)) {
-                               // If the folder is not empty, the delete operation will fail
-                               // silently. This is a cheaper alternative to filtering the
-                               // stream in the calling method.
-                               Files.delete(d);
-                       }
+                       Files.delete(d);
                } catch (IOException e) {
                        LOG.error(MessageFormat.format(JGitText.get().cannotDeleteFile, d),
                                        e);