git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 20/20] ref_transaction_commit(): clean up empty directories

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Feb 25, 2016, 13:16 UTC
Message-ID
<1c2176ceec2f28709ca9f16adf291c34abb00cf3.1456405699.git.mhagger@alum.mit.edu>
In-Reply-To
<cover.1456405698.git.mhagger@alum.mit.edu>

When deleting/pruning references, remove any directories that are made empty by the deletion of loose references or of reflogs. Otherwise such empty directories can survive forever and accumulate over time. (Even 'pack-refs', which is smart enough to remove the parent directories of loose references that it prunes, leaves directories that were already empty.)

And now that ref_transaction_commit() takes care of deleting the parent directories of loose references that it prunes, we don't have to do that in prune_ref() anymore.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs/files-backend.c  | 37 +++++++++++++++++++++++++++++++------
 refs/refs-internal.h  |  9 ++++++++-
 t/t1400-update-ref.sh | 27 +++++++++++++++++++++++++++
 3 files changed, 66 insertions(+), 7 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 9ebb188..9973958 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2263,7 +2263,6 @@ static void prune_ref(struct ref_to_prune *r)
 	}
 	ref_transaction_free(transaction);
 	strbuf_release(&err);
-	try_remove_empty_parents(r->name, REMOVE_EMPTY_PARENTS_REF);
 }
 
 static void prune_refs(struct ref_to_prune *r)
@@ -3261,6 +3260,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,
 					ret = TRANSACTION_GENERIC_ERROR;
 					goto cleanup;
 				}
+				update->flags |= REF_DELETED_LOOSE;
 			}
 
 			if (!(update->flags & REF_ISPRUNING))
@@ -3273,16 +3273,41 @@ int ref_transaction_commit(struct ref_transaction *transaction,
 		ret = TRANSACTION_GENERIC_ERROR;
 		goto cleanup;
 	}
-	for_each_string_list_item(ref_to_delete, &refs_to_delete)
-		unlink_or_warn(git_path("logs/%s", ref_to_delete->string));
+
+	/* Delete the reflogs of any references that were deleted: */
+	for_each_string_list_item(ref_to_delete, &refs_to_delete) {
+		if (!unlink_or_warn(git_path("logs/%s", ref_to_delete->string)))
+			try_remove_empty_parents(ref_to_delete->string,
+						 REMOVE_EMPTY_PARENTS_REFLOG);
+	}
+
 	clear_loose_ref_cache(&ref_cache);
 
 cleanup:
 	transaction->state = REF_TRANSACTION_CLOSED;
 
-	for (i = 0; i < n; i++)
-		if (updates[i]->lock)
-			unlock_ref(updates[i]->lock);
+	for (i = 0; i < n; i++) {
+		struct ref_update *update = updates[i];
+
+		if (!update->lock)
+			continue;
+
+		if (update->flags & REF_DELETED_LOOSE) {
+			/*
+			 * The loose reference was deleted. We want to
+			 * delete any empty parent directories, but
+			 * that can only work after we have removed
+			 * the lockfile:
+			 */
+			char *path = xstrdup(update->lock->ref_name);
+			unlock_ref(update->lock);
+			try_remove_empty_parents(path, REMOVE_EMPTY_PARENTS_REF);
+			free(path);
+		} else {
+			unlock_ref(update->lock);
+		}
+	}
+
 	string_list_clear(&refs_to_delete, 0);
 	string_list_clear(&affected_refnames, 0);
 	return ret;
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index c7dded3..dd09be1 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -43,6 +43,12 @@
  */
 
 /*
+ * Used as a flag in ref_update::flags when the loose reference has
+ * been deleted.
+ */
+#define REF_DELETED_LOOSE 0x100
+
+/*
  * Return true iff refname is minimally safe. "Safe" here means that
  * deleting a loose reference by this name will not do any damage, for
  * example by causing a file that is not a reference to be deleted.
@@ -141,7 +147,8 @@ struct ref_update {
 	unsigned char old_sha1[20];
 	/*
 	 * One or more of REF_HAVE_NEW, REF_HAVE_OLD, REF_NODEREF,
-	 * REF_DELETING, and REF_ISPRUNING:
+	 * REF_DELETING, REF_ISPRUNING, REF_NEEDS_COMMIT, and
+	 * REF_DELETED_LOOSE:
 	 */
 	unsigned int flags;
 	struct ref_lock *lock;
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index af1b20d..00284eb 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -191,6 +191,33 @@ test_expect_success \
 	 git update-ref HEAD'" $A &&
 	 test $A"' = $(cat .git/'"$m"')'
 
+test_expect_success "empty directory removal" '
+	git branch d1/d2/r1 HEAD &&
+	git branch d1/r2 HEAD &&
+	test -f .git/refs/heads/d1/d2/r1 &&
+	test -f .git/logs/refs/heads/d1/d2/r1 &&
+	git branch -d d1/d2/r1 &&
+	! test -e .git/refs/heads/d1/d2 &&
+	! test -e .git/logs/refs/heads/d1/d2 &&
+	test -f .git/refs/heads/d1/r2 &&
+	test -f .git/logs/refs/heads/d1/r2
+'
+
+test_expect_success "symref empty directory removal" '
+	git branch e1/e2/r1 HEAD &&
+	git branch e1/r2 HEAD &&
+	git checkout e1/e2/r1 &&
+	test_when_finished "git checkout master" &&
+	test -f .git/refs/heads/e1/e2/r1 &&
+	test -f .git/logs/refs/heads/e1/e2/r1 &&
+	git update-ref -d HEAD &&
+	! test -e .git/refs/heads/e1/e2 &&
+	! test -e .git/logs/refs/heads/e1/e2 &&
+	test -f .git/refs/heads/e1/r2 &&
+	test -f .git/logs/refs/heads/e1/r2 &&
+	test -f .git/logs/HEAD
+'
+
 cat >expect <<EOF
 $Z $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000	Initial Creation
 $A $B $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150260 +0000	Switch

-- 
2.7.0
Previous: Michael Haggerty
Message 21 of 21 in “Delete directories left empty after ref deletion”
  1. 00/20 Delete directories left empty after ref deletionMichael Haggerty, Feb 25, 2016
  2. 01/20 safe_create_leading_directories_const(): preserve errnoMichael Haggerty, Feb 25, 2016
  3. 02/20 safe_create_leading_directories(): set errno on SCLD_EXISTSMichael Haggerty, Feb 25, 2016
  4. 03/20 raceproof_create_file(): new functionMichael Haggerty, Feb 25, 2016
  5. 04/20 lock_ref_sha1_basic(): use raceproof_create_file()Michael Haggerty, Feb 25, 2016
  6. 05/20 rename_tmp_log(): use raceproof_create_file()Michael Haggerty, Feb 25, 2016
  7. 06/20 rename_tmp_log(): improve error reportingMichael Haggerty, Feb 25, 2016
  8. 07/20 log_ref_setup(): separate code for create vs non-createMichael Haggerty, Feb 25, 2016
  9. 08/20 log_ref_setup(): improve robustness against racesMichael Haggerty, Feb 25, 2016
  10. 09/20 log_ref_setup(): pass the open file descriptor back to the callerMichael Haggerty, Feb 25, 2016
  11. 10/20 log_ref_write_1(): don't depend on logfileMichael Haggerty, Feb 25, 2016
  12. 11/20 log_ref_setup(): manage the name of the reflog file internallyMichael Haggerty, Feb 25, 2016
  13. 12/20 log_ref_write_1(): inline functionMichael Haggerty, Feb 25, 2016
  14. 13/20 try_remove_empty_parents(): rename parameter "name" -> "refname"Michael Haggerty, Feb 25, 2016
  15. 14/20 try_remove_empty_parents(): don't trash argument contentsMichael Haggerty, Feb 25, 2016
  16. 15/20 try_remove_empty_parents(): don't accommodate consecutive slashesMichael Haggerty, Feb 25, 2016
  17. 16/20 t5505: use "for-each-ref" to test for the non-existence of referencesMichael Haggerty, Feb 25, 2016
  18. 17/20 delete_ref_loose(): derive loose reference path from lockMichael Haggerty, Feb 25, 2016
  19. 18/20 delete_ref_loose(): inline functionMichael Haggerty, Feb 25, 2016
  20. 19/20 try_remove_empty_parents(): teach to remove parents of reflogs, tooMichael Haggerty, Feb 25, 2016
  21. 20/20 ref_transaction_commit(): clean up empty directoriesMichael Haggerty, Feb 25, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.