[JGIT PATCH] Fix: RefUpdate.delete does not prune empty directories

[JGIT PATCH] Fix: RefUpdate.delete does not prune empty directories

[Charles O'Farrell]

When the last loose ref (or reflog) is removed from a directory the
directory itself should also be removed, up to refs/{heads,tags,remotes}.
Otherwise we may fail when doing something like:

  delete refs/heads/foo/bar
  create refs/heads/foo

as refs/heads/foo is still a directory and cannot be a file.

http://code.google.com/p/egit/issues/detail?id=10

Signed-off-by: Charles O'Farrell <charleso@charleso.org>
---
 .../tst/org/spearce/jgit/lib/RefUpdateTest.java    |   34 ++++++++++++++--
 .../src/org/spearce/jgit/lib/RefUpdate.java        |   41 +++++++++++++++++--
 2 files changed, 65 insertions(+), 10 deletions(-)

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
index 1ade2ef..6e2cfa8 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
@@ -76,16 +76,22 @@ public void testDeleteHead() throws IOException {
 	}
 
 	public void testLogDeleted() throws IOException {
-		final File log = new File(db.getDirectory(), Constants.LOGS
-				+ "/refs/heads/a");
-		log.getParentFile().mkdirs();
-		log.createNewFile();
+		String refName = "refs/heads/a";
+		final File log = createLog(refName);
 		assertTrue(log.exists());
-		final RefUpdate ref = updateRef("refs/heads/a");
+		final RefUpdate ref = updateRef(refName);
 		delete(ref, Result.FAST_FORWARD);
 		assertFalse(log.exists());
 	}
 
+	private File createLog(String name) throws IOException {
+		final File log = new File(db.getDirectory(), Constants.LOGS + "/"
+				+ name);
+		log.getParentFile().mkdirs();
+		log.createNewFile();
+		return log;
+	}
+
 	public void testDeleteNotFound() throws IOException {
 		final RefUpdate ref = updateRef("refs/heads/xyz");
 		delete(ref, Result.NEW, false, true);
@@ -103,4 +109,22 @@ public void testDeleteForce() throws IOException {
 		ref.setForceUpdate(true);
 		delete(ref, Result.FORCED);
 	}
+
+	public void testDeleteEmptyDirs() throws IOException {
+		final String top = "refs/heads/a";
+		final String newRef = top + "/b/c";
+		final String newRef2 = top + "/d";
+		updateRef(newRef).update();
+		updateRef(newRef2).update();
+		delete(updateRef(newRef2), Result.NO_CHANGE);
+		assertExists(true, top);
+		createLog(newRef);
+		delete(updateRef(newRef), Result.NO_CHANGE);
+		assertExists(false, top);
+		assertExists(false, Constants.LOGS + "/" + top);
+	}
+
+	private void assertExists(final boolean expected, final String name) {
+		assertEquals(expected, new File(db.getDirectory(), name).exists());
+	}
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
index e9c0e77..86b44c5 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
@@ -459,13 +459,44 @@ Result store(LockFile lock, Result status) throws IOException {
 				return status;
 			if (storage.isPacked())
 				db.removePackedRef(ref.getName());
+
+			final int levels = count(ref.getName(), '/') - 2;
+
+			// Delete logs _before_ unlocking
+			final File gitDir = db.getRepository().getDirectory();
+			final File logDir = new File(gitDir, Constants.LOGS);
+			deleteFileAndEmptyDir(new File(logDir, ref.getName()), levels);
+
+			// We have to unlock before (maybe) deleting the parent directories
+			lock.unlock();
 			if (storage.isLoose())
-				if (!looseFile.delete())
-					throw new IOException("File cannot be deleted: "
-							+ looseFile);
-			new File(db.getRepository().getDirectory(), Constants.LOGS + "/"
-					+ ref.getName()).delete();
+				deleteFileAndEmptyDir(looseFile, levels);
 			return status;
 		}
+
+		private void deleteFileAndEmptyDir(final File file, final int depth)
+				throws IOException {
+			if (file.exists()) {
+				if (!file.delete())
+					throw new IOException("File cannot be deleted: " + file);
+				deleteEmptyDir(file.getParentFile(), depth);
+			}
+		}
+
+		private void deleteEmptyDir(File dir, int depth) {
+			for (; depth > 0 && dir != null; depth--) {
+				if (!dir.delete())
+					break;
+				dir = dir.getParentFile();
+			}
+		}
+	}
+
+	private static int count(final String s, final char c) {
+		int count = 0;
+		for (int p = s.indexOf(c); p >= 0; p = s.indexOf(c, p + 1)) {
+			count++;
+		}
+		return count;
 	}
 }
-- 
1.6.0.1.220.g80d1