Re: [PATCH] refs: cleanup directories when deleting packed ref

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/05/21 12.00, Will Chandler wrote:
Subject: [PATCH v2] refs: cleanup directories when deleting packed ref

When deleting a packed ref via 'update-ref -d', a lockfile is made in
the directory that would contain the loose copy of that ref, creating
any directories in the ref's path that do not exist. When the
transaction completes, the lockfile is deleted, but any empty parent
directories made when creating the lockfile are left in place.  These
empty directories are not removed by 'pack-refs' or other housekeeping
tasks and will accumulate over time.

When deleting a loose ref, we remove all empty parent directories at the
end of the transaction.

This commit applies the parent directory cleanup logic used when
deleting loose refs to packed refs as well.

Signed-off-by: Will Chandler <wfc@xxxxxxxxxxxxxx>
---
  refs/files-backend.c  | 12 ++++++------
  t/t1400-update-ref.sh |  9 +++++++++
  2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 119972ee16..49e6ee069a 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -45,10 +45,10 @@
  #define REF_UPDATE_VIA_HEAD (1 << 8)
/*
- * Used as a flag in ref_update::flags when the loose reference has
- * been deleted.
+ * Used as a flag in ref_update::flags when a reference has been
+ * deleted and the ref's parent directories may need cleanup.
   */
-#define REF_DELETED_LOOSE (1 << 9)
+#define REF_DELETED_RMDIR (1 << 9)
struct ref_lock {
  	char *ref_name;
@@ -2852,6 +2852,7 @@ static int files_transaction_finish(struct ref_store *ref_store,
if (update->flags & REF_DELETING &&
  		    !(update->flags & REF_LOG_ONLY)) {
+			update->flags |= REF_DELETED_RMDIR;
  			if (!(update->type & REF_ISPACKED) ||
  			    update->type & REF_ISSYMREF) {
  				/* It is a loose reference. */
@@ -2861,7 +2862,6 @@ static int files_transaction_finish(struct ref_store *ref_store,
  					ret = TRANSACTION_GENERIC_ERROR;
  					goto cleanup;
  				}
-				update->flags |= REF_DELETED_LOOSE;
  			}
  		}
  	}
@@ -2874,9 +2874,9 @@ static int files_transaction_finish(struct ref_store *ref_store,
  	for (i = 0; i < transaction->nr; i++) {
  		struct ref_update *update = transaction->updates[i];
- if (update->flags & REF_DELETED_LOOSE) {
+		if (update->flags & REF_DELETED_RMDIR) {
  			/*
-			 * The loose reference was deleted. Delete any
+			 * The reference was deleted. Delete any
  			 * empty parent directories. (Note that this
  			 * can only work because we have already
  			 * removed the lockfile.)
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index e31f65f381..4506cd435b 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -1598,4 +1598,13 @@ test_expect_success 'transaction cannot restart ongoing transaction' '
  	test_must_fail git show-ref --verify refs/heads/restart
  '
+test_expect_success 'directory not created deleting packed ref' '
+	git branch d1/d2/r1 HEAD &&
+	git pack-refs --all &&
+	test_path_is_missing .git/refs/heads/d1/d2 &&
+	git update-ref -d refs/heads/d1/d2/r1 &&
+	test_path_is_missing .git/refs/heads/d1/d2 &&
+	test_path_is_missing .git/refs/heads/d1
+'
+
  test_done


I ask to you: why did you send v2 patch as reply to v1?

Supposed that I interested to apply only this v2, instead of v1.
With this situation, I downloaded mbox for v1, which contains v2
patch as reply to v1. And git-am would instead apply v1 instead.

So why not send this v2 as separate message-id?

--
An old man doll... just what I always wanted! - Clara



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux