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

[PATCH v4 1/3] refs: allow callers to supply old OIDs for batch deletion

From
Maciej Ciemborowicz <maciej.ciemborowicz@gmail.com>
Date
Sep 22, 2026, 22:31 UTC
Message-ID
<f4a9d065c34451a5f6ade6a6c365baa18adeb780.1790113781.git.maciej.ciemborowicz@gmail.com>
In-Reply-To
<cover.1790113781.git.maciej.ciemborowicz@gmail.com>

refs_delete_refs() performs unconditional deletions, so callers cannot preserve old values that they have already resolved. Consequently, reference-transaction hooks see a null old OID.

Let callers provide an optional array of expected old OIDs in parallel with the refname list. Delete the ref at position N only if it still points at the OID at position N. Treat a null OID as an unconditional deletion in ref_transaction_delete(), allowing callers to include broken refs whose old value cannot be resolved.

refs_delete_refs() has always promised best-effort deletion. Always use REF_TRANSACTION_ALLOW_FAILURE and report rejected updates so one failure does not prevent independent refs in the batch from being deleted. Let callers request the exact set of failed refs when they need to report partial results. This also completes the conversion that was missed when batched transaction failure support was introduced.

Signed-off-by: Maciej Ciemborowicz <maciej.ciemborowicz@gmail.com>
---
 bisect.c                  |  2 +-
 builtin/branch.c          |  3 +-
 builtin/fetch.c           |  2 +-
 builtin/remote.c          |  5 +--
 builtin/tag.c             |  3 +-
 refs.c                    | 67 +++++++++++++++++++++++++++++++--------
 refs.h                    | 31 +++++++++++++-----
 t/helper/test-ref-store.c |  2 +-
 8 files changed, 86 insertions(+), 29 deletions(-)
diff --git a/bisect.c b/bisect.c
index 9cbb3dc67..c8ab16d1e 100644
--- a/bisect.c
+++ b/bisect.c
@@ -1206,7 +1206,7 @@ int bisect_clean_state(void)
 	string_list_append(&refs_for_removal, "BISECT_EXPECTED_REV");
 	result = refs_delete_refs(get_main_ref_store(the_repository),
 				  "bisect: remove", &refs_for_removal,
-				  REF_NO_DEREF);
+				  NULL, NULL, REF_NO_DEREF);
 	string_list_clear(&refs_for_removal, 0);
 	unlink_or_warn(git_path_bisect_ancestors_ok());
 	unlink_or_warn(git_path_bisect_log());
diff --git a/builtin/branch.c b/builtin/branch.c
index a613148fc..c9f259d04 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -351,7 +351,8 @@ static int delete_branches(int argc, const char **argv, int kinds,
 	}
 
 	if (!(flags & DELETE_BRANCH_DRY_RUN) &&
-	    refs_delete_refs(get_main_ref_store(the_repository), NULL, &refs_to_delete, REF_NO_DEREF))
+	    refs_delete_refs(get_main_ref_store(the_repository), NULL,
+			     &refs_to_delete, NULL, REF_NO_DEREF))
 		ret = 1;
 
 	for_each_string_list_item(item, &refs_to_delete) {
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 533fdfe7d..b662216bf 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1486,7 +1486,7 @@ static int prune_refs(struct display_state *display_state,
 		} else {
 			result = refs_delete_refs(get_main_ref_store(the_repository),
 						  "fetch: prune", &refnames,
-						  0);
+						  NULL, 0);
 		}
 	}
 
diff --git a/builtin/remote.c b/builtin/remote.c
index de989ea3b..56b06845b 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -1073,7 +1073,7 @@ static int rm(int argc, const char **argv, const char *prefix,
 	if (!result)
 		result = refs_delete_refs(get_main_ref_store(the_repository),
 					  "remote: remove", &branches,
-					  REF_NO_DEREF);
+					  NULL, NULL, REF_NO_DEREF);
 	string_list_clear(&branches, 0);
 
 	if (skipped.nr) {
@@ -1645,7 +1645,8 @@ static int prune_remote(const char *remote, int dry_run)
 
 	if (!dry_run)
 		result |= refs_delete_refs(get_main_ref_store(the_repository),
-					   "remote: prune", &refs_to_prune, 0);
+					   "remote: prune", &refs_to_prune,
+					   NULL, 0);
 
 	for_each_string_list_item(item, &states.stale) {
 		const char *refname = item->util;
diff --git a/builtin/tag.c b/builtin/tag.c
index 06c125b53..40874a292 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -122,7 +122,8 @@ static int delete_tags(const char **argv)
 	struct string_list_item *item;
 
 	result = for_each_tag_name(argv, collect_tags, (void *)&refs_to_delete);
-	if (refs_delete_refs(get_main_ref_store(the_repository), NULL, &refs_to_delete, REF_NO_DEREF))
+	if (refs_delete_refs(get_main_ref_store(the_repository), NULL,
+			     &refs_to_delete, NULL, REF_NO_DEREF))
 		result = 1;
 
 	for_each_string_list_item(item, &refs_to_delete) {
diff --git a/refs.c b/refs.c
index 92d5df5b7..13ee2d459 100644
--- a/refs.c
+++ b/refs.c
@@ -16,6 +16,7 @@
 #include "refs/refs-internal.h"
 #include "hook.h"
 #include "object-name.h"
+#include "oid-array.h"
 #include "odb.h"
 #include "object.h"
 #include "path.h"
@@ -1523,7 +1524,7 @@ int ref_transaction_delete(struct ref_transaction *transaction,
 			   struct strbuf *err)
 {
 	if (old_oid && is_null_oid(old_oid))
-		BUG("delete called with old_oid set to zeros");
+		old_oid = NULL;
 	if (old_oid && old_target)
 		BUG("delete called with both old_oid and old_target set");
 	if (old_target && !(flags & REF_NO_DEREF))
@@ -3069,39 +3070,73 @@ void ref_transaction_for_each_rejected_update(struct ref_transaction *transactio
 	}
 }
 
+struct delete_refs_rejection_data {
+	int failures;
+	struct string_list *failed_refs;
+};
+
+static void delete_refs_rejection_handler(const char *refname,
+					  const struct object_id *old_oid UNUSED,
+					  const struct object_id *new_oid UNUSED,
+					  const char *old_target UNUSED,
+					  const char *new_target UNUSED,
+					  enum ref_transaction_error err,
+					  const char *details,
+					  void *cb_data)
+{
+	struct delete_refs_rejection_data *data = cb_data;
+
+	warning(_("could not delete reference %s: %s"), refname,
+		details ? details : ref_transaction_error_msg(err));
+	data->failures++;
+	if (data->failed_refs)
+		string_list_insert(data->failed_refs, refname);
+}
+
 int refs_delete_refs(struct ref_store *refs, const char *logmsg,
-		     struct string_list *refnames, unsigned int flags)
+		     struct string_list *refnames,
+		     const struct oid_array *old_oids,
+		     struct string_list *failed_refs,
+		     unsigned int flags)
 {
+	struct delete_refs_rejection_data rejection_data = {
+		.failed_refs = failed_refs,
+	};
 	struct ref_transaction *transaction;
 	struct strbuf err = STRBUF_INIT;
-	struct string_list_item *item;
-	int ret = 0, failures = 0;
+	size_t i;
+	int ret = 0;
 	char *msg;
 
 	if (!refnames->nr)
 		return 0;
+	if (old_oids && old_oids->nr != refnames->nr)
+		BUG("refname and old OID counts do not match");
+	if (failed_refs && !failed_refs->strdup_strings)
+		BUG("failed ref list does not duplicate strings");
 
 	msg = normalize_reflog_message(logmsg);
 
-	/*
-	 * Since we don't check the references' old_oids, the
-	 * individual updates can't fail, so we can pack all of the
-	 * updates into a single transaction.
-	 */
-	transaction = ref_store_transaction_begin(refs, 0, &err);
+	transaction = ref_store_transaction_begin(refs,
+						  REF_TRANSACTION_ALLOW_FAILURE, &err);
 	if (!transaction) {
 		ret = error("%s", err.buf);
 		goto out;
 	}
 
-	for_each_string_list_item(item, refnames) {
+	for (i = 0; i < refnames->nr; i++) {
+		struct string_list_item *item = &refnames->items[i];
+		const struct object_id *old_oid = old_oids ? &old_oids->oid[i] : NULL;
+
 		ret = ref_transaction_delete(transaction, item->string,
-					     NULL, NULL, flags, msg, &err);
+					     old_oid, NULL, flags, msg, &err);
 		if (ret) {
 			warning(_("could not delete reference %s: %s"),
 				item->string, err.buf);
 			strbuf_reset(&err);
-			failures = 1;
+			rejection_data.failures++;
+			if (failed_refs)
+				string_list_insert(failed_refs, item->string);
 		}
 	}
 
@@ -3113,9 +3148,13 @@ int refs_delete_refs(struct ref_store *refs, const char *logmsg,
 		else
 			error(_("could not delete references: %s"), err.buf);
 	}
+	if (!ret)
+		ref_transaction_for_each_rejected_update(transaction,
+							 delete_refs_rejection_handler,
+							 &rejection_data);
 
 out:
-	if (!ret && failures)
+	if (!ret && rejection_data.failures)
 		ret = -1;
 	ref_transaction_free(transaction);
 	strbuf_release(&err);
diff --git a/refs.h b/refs.h
index 9979446d1..43f7a32f2 100644
--- a/refs.h
+++ b/refs.h
@@ -9,6 +9,7 @@
 struct fsck_options;
 struct object_id;
 struct ref_store;
+struct oid_array;
 struct strbuf;
 struct string_list;
 struct string_list_item;
@@ -623,13 +624,26 @@ int refs_delete_ref(struct ref_store *refs, const char *msg,
 		    unsigned int flags);
 
 /*
- * Delete the specified references. If there are any problems, emit
- * errors but attempt to keep going (i.e., the deletes are not done in
- * an all-or-nothing transaction). msg and flags are passed through to
- * ref_transaction_delete().
+ * Delete the specified references. If old_oids is non-NULL, it must contain
+ * an entry for each refname, in the same order. Each non-null OID is used to
+ * verify the current value of the corresponding reference before deleting
+ * it. A null OID requests an unconditional deletion, which allows callers to
+ * include broken refs whose old value cannot be resolved.
+ *
+ * If failed_refs is non-NULL, it must be initialized with
+ * STRING_LIST_INIT_DUP. The names of individual updates that cannot be queued
+ * or are rejected while processing the best-effort batch are inserted into
+ * it. A transaction-wide failure is returned without populating the list.
+ *
+ * If there are any problems, emit errors but attempt to keep going (i.e.,
+ * the deletes are not done in an all-or-nothing transaction). msg and flags
+ * are passed through to ref_transaction_delete().
  */
 int refs_delete_refs(struct ref_store *refs, const char *msg,
-		     struct string_list *refnames, unsigned int flags);
+		     struct string_list *refnames,
+		     const struct oid_array *old_oids,
+		     struct string_list *failed_refs,
+		     unsigned int flags);
 
 /** Delete a reflog */
 int refs_delete_reflog(struct ref_store *refs, const char *refname);
@@ -956,9 +970,10 @@ int ref_transaction_create(struct ref_transaction *transaction,
 			   struct strbuf *err);
 
 /*
- * Add a reference deletion to transaction. If old_oid is non-NULL,
- * then it holds the value that the reference should have had before
- * the update (which must not be null_oid).
+ * Add a reference deletion to transaction. If old_oid is non-NULL and not
+ * null_oid, then it holds the value that the reference should have had before
+ * the update. Passing null_oid is equivalent to passing NULL and disables the
+ * old value check.
  *
  * See the above comment "Reference transaction updates" for more
  * information.
diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index db58f0058..29945f2b8 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -132,7 +132,7 @@ static int cmd_delete_refs(struct ref_store *refs, const char **argv)
 	while (*argv)
 		string_list_append(&refnames, *argv++);
 
-	result = refs_delete_refs(refs, msg, &refnames, flags);
+	result = refs_delete_refs(refs, msg, &refnames, NULL, NULL, flags);
 	string_list_clear(&refnames, 0);
 	return result;
 }

-- 
2.39.3 (Apple Git-146)
Previous: Maciej CiemborowiczNext: Maciej Ciemborowicz
Message 27 of 52 in “[BUG] reference-transaction reports zero OIDs for branch and tag deletion”
  1. Maciej CiemborowiczSep 19, 2026
  2. D. Ben KnobleSep 19, 2026
  3. Maciej CiemborowiczSep 19, 2026
  4. 0/3 refs: report old OIDs for batched deletionsMaciej Ciemborowicz, Sep 19, 2026
  5. 1/3 refs: allow callers to supply old OIDs for batch deletionMaciej Ciemborowicz, Sep 19, 2026
  6. Karthik NayakSep 19, 2026
  7. Maciej CiemborowiczSep 20, 2026
  8. 0/3 refs: report old OIDs for batched deletionsMaciej Ciemborowicz, Sep 20, 2026
  9. 1/3 refs: allow callers to supply old OIDs for batch deletionMaciej Ciemborowicz, Sep 20, 2026
  10. Karthik NayakSep 21, 2026
  11. Junio C HamanoSep 21, 2026
  12. 2/3 branch, tag: retain old OIDs in batched deletionsMaciej Ciemborowicz, Sep 20, 2026
  13. Karthik NayakSep 21, 2026
  14. 3/3 fetch, remote: retain old OIDs when pruning refsMaciej Ciemborowicz, Sep 20, 2026
  15. Karthik NayakSep 21, 2026
  16. Karthik NayakSep 21, 2026
  17. Maciej CiemborowiczSep 21, 2026
  18. 0/3 refs: report old OIDs for batched deletionsMaciej Ciemborowicz, Sep 22, 2026
  19. 1/3 refs: allow callers to supply old OIDs for batch deletionMaciej Ciemborowicz, Sep 22, 2026
  20. Junio C HamanoSep 22, 2026
  21. Maciej CiemborowiczSep 22, 2026
  22. Junio C HamanoSep 22, 2026
  23. 2/3 branch, tag: retain old OIDs in batched deletionsMaciej Ciemborowicz, Sep 22, 2026
  24. 3/3 fetch, remote: retain old OIDs when pruning refsMaciej Ciemborowicz, Sep 22, 2026
  25. Junio C HamanoSep 22, 2026
  26. 0/3 refs: report old OIDs for batched deletionsMaciej Ciemborowicz, Sep 22, 2026
  27. 1/3 refs: allow callers to supply old OIDs for batch deletionMaciej Ciemborowicz, Sep 22, 2026
  28. 2/3 branch, tag: retain old OIDs in batched deletionsMaciej Ciemborowicz, Sep 22, 2026
  29. 3/3 fetch, remote: retain old OIDs when pruning refsMaciej Ciemborowicz, Sep 22, 2026
  30. Junio C HamanoSep 23, 2026
  31. Maciej CiemborowiczSep 23, 2026
  32. 0/3 refs: report old OIDs for batched deletionsMaciej Ciemborowicz, Sep 23, 2026
  33. 1/3 refs: allow callers to supply old OIDs for batch deletionMaciej Ciemborowicz, Sep 23, 2026
  34. Karthik NayakSep 24, 2026
  35. Junio C HamanoSep 24, 2026
  36. Maciej CiemborowiczSep 24, 2026
  37. Patrick SteinhardtSep 24, 2026
  38. Junio C HamanoSep 24, 2026
  39. Maciej CiemborowiczSep 24, 2026
  40. Patrick SteinhardtSep 28, 2026
  41. Patrick SteinhardtSep 28, 2026
  42. Maciej CiemborowiczSep 24, 2026
  43. 2/3 branch, tag: retain old OIDs in batched deletionsMaciej Ciemborowicz, Sep 23, 2026
  44. Patrick SteinhardtSep 24, 2026
  45. 3/3 fetch, remote: retain old OIDs when pruning refsMaciej Ciemborowicz, Sep 23, 2026
  46. Junio C HamanoSep 23, 2026
  47. 0/1 refs: report old values to transaction hooksMaciej Ciemborowicz, Sep 24, 2026
  48. 1/1 refs: report old values to transaction hooksMaciej Ciemborowicz, Sep 24, 2026
  49. Maciej CiemborowiczSep 30, 2026
  50. Maciej CiemborowiczOct 1, 2026
  51. 2/3 branch, tag: retain old OIDs in batched deletionsMaciej Ciemborowicz, Sep 19, 2026
  52. 3/3 fetch, remote: retain old OIDs when pruning refsMaciej Ciemborowicz, Sep 19, 2026

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.