Subject: [PATCH 1/2] remote: defer repacking packed-refs when deleting refs

From: Jens Lindström <[email-masked]>
Date: Tue, 20 May 2014 12:39:35 +0200

When 'git remote rm' or 'git remote prune' were used in a repository
with many refs, and needed to delete many refs, a lot of time was spent
deleting those refs since for each deleted ref, repack_without_refs()
was called to rewrite packed-refs without just that deleted ref.

To avoid this, defer the repacking until after all refs have been
deleted (by delete_ref()), and then call repack_without_refs() once to
repack without all the deleted refs.

Signed-off-by: Jens Lindström <[email-masked]>
---
This patch changes behavior when the operation is aborted in the
middle, so that loose refs and ref logs might have been deleted, but
not the corresponding entries in packed-refs, since packed-refs is now
only updated at the end.  This is a bit unfortunate, and may
"resurrect" an obsolete packed-refs entry by deleting the loose ref
that had overridden it.

Mitigating factors would be that this only affects remote tracking
branches that we were about to delete anyway, and that in the case of
'git remote prune' were apparently not actually matching a ref in the
remote.

Also, it is a lot harder to interrupt the operation in the middle when
it takes a few seconds compared to when it takes many minutes to
complete.  :-)

 builtin/remote.c | 19 ++++++++++++++++---
 refs.c           | 15 +++++++++------
 refs.h           |  3 +++
 3 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index b3ab4cf..ce60a30 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -749,15 +749,18 @@ static int mv(int argc, const char **argv)
 
 static int remove_branches(struct string_list *branches)
 {
+	const char **branch_names = xmalloc(branches->nr * sizeof(*branch_names));
 	int i, result = 0;
 	for (i = 0; i < branches->nr; i++) {
 		struct string_list_item *item = branches->items + i;
-		const char *refname = item->string;
+		const char *refname = branch_names[i] = item->string;
 		unsigned char *sha1 = item->util;
 
-		if (delete_ref(refname, sha1, 0))
+		if (delete_ref(refname, sha1, REF_DEFERREPACK))
 			result |= error(_("Could not remove branch %s"), refname);
 	}
+	result |= repack_without_refs(branch_names, branches->nr);
+	free(branch_names);
 	return result;
 }
 
@@ -1303,6 +1306,7 @@ static int prune_remote(const char *remote, int dry_run)
 {
 	int result = 0, i;
 	struct ref_states states;
+	const char **delete_refs;
 	const char *dangling_msg = dry_run
 		? _(" %s will become dangling!")
 		: _(" %s has become dangling!");
@@ -1316,13 +1320,16 @@ static int prune_remote(const char *remote, int dry_run)
 		       states.remote->url_nr
 		       ? states.remote->url[0]
 		       : _("(no URL)"));
+		delete_refs = xmalloc(states.stale.nr * sizeof(*delete_refs));
 	}
 
 	for (i = 0; i < states.stale.nr; i++) {
 		const char *refname = states.stale.items[i].util;
 
+		delete_refs[i] = refname;
+
 		if (!dry_run)
-			result |= delete_ref(refname, NULL, 0);
+			result |= delete_ref(refname, NULL, REF_DEFERREPACK);
 
 		if (dry_run)
 			printf_ln(_(" * [would prune] %s"),
@@ -1333,6 +1340,12 @@ static int prune_remote(const char *remote, int dry_run)
 		warn_dangling_symref(stdout, dangling_msg, refname);
 	}
 
+	if (states.stale.nr) {
+		if (!dry_run)
+			result |= repack_without_refs(delete_refs, states.stale.nr);
+		free(delete_refs);
+	}
+
 	free_remote_ref_states(&states);
 	return result;
 }
diff --git a/refs.c b/refs.c
index 28d5eca..3b62aca 100644
--- a/refs.c
+++ b/refs.c
@@ -2431,7 +2431,7 @@ static int curate_packed_ref_fn(struct ref_entry *entry, void *cb_data)
 	return 0;
 }
 
-static int repack_without_refs(const char **refnames, int n)
+int repack_without_refs(const char **refnames, int n)
 {
 	struct ref_dir *packed;
 	struct string_list refs_to_delete = STRING_LIST_INIT_DUP;
@@ -2507,11 +2507,14 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
 		return 1;
 	ret |= delete_ref_loose(lock, flag);
 
-	/* removing the loose one could have resurrected an earlier
-	 * packed one.  Also, if it was not loose we need to repack
-	 * without it.
-	 */
-	ret |= repack_without_ref(lock->ref_name);
+	if (!(delopt & REF_DEFERREPACK))
+	{
+		/* removing the loose one could have resurrected an earlier
+		 * packed one.  Also, if it was not loose we need to repack
+		 * without it.
+		 */
+		ret |= repack_without_ref(lock->ref_name);
+	}
 
 	unlink_or_warn(git_path("logs/%s", lock->ref_name));
 	clear_loose_ref_cache(&ref_cache);
diff --git a/refs.h b/refs.h
index 87a1a79..0db5584 100644
--- a/refs.h
+++ b/refs.h
@@ -132,6 +132,8 @@ extern void rollback_packed_refs(void);
  */
 int pack_refs(unsigned int flags);
 
+extern int repack_without_refs(const char **refnames, int n);
+
 extern int ref_exists(const char *);
 
 /*
@@ -149,6 +151,7 @@ extern struct ref_lock *lock_ref_sha1(const char *refname, const unsigned char *
 
 /** Locks any ref (for 'HEAD' type refs). */
 #define REF_NODEREF	0x01
+#define REF_DEFERREPACK	0x02
 extern struct ref_lock *lock_any_ref_for_update(const char *refname,
 						const unsigned char *old_sha1,
 						int flags, int *type_p);
--
1.9.1