git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Charles O'Farrell <charleso@charleso.org>
To: git@vger.kernel.org
Subject: [JGIT PATCH] Fix: RefUpdate.delete does not prune empty directories
Date: Sat,  6 Sep 2008 08:56:47 +1000	[thread overview]
Message-ID: <1220655407-5817-1-git-send-email-charleso@charleso.org> (raw)

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

                 reply	other threads:[~2008-09-05 22:58 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1220655407-5817-1-git-send-email-charleso@charleso.org \
    --to=charleso@charleso.org \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).