From d79b73c2a9829c56898072fa93c6cc3aabc8ed1e Mon Sep 17 00:00:00 2001 From: Maciej Ciemborowicz Date: Tue, 22 Sep 2026 14:26:07 +0200 Subject: [PATCH] refs: allow callers to supply old OIDs for batch deletion 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. When the array is provided, delete the ref at position N only if it still points at the OID at position N. A null OID requests an unconditional deletion for refs whose old value cannot be resolved, such as broken refs. Use REF_TRANSACTION_ALLOW_FAILURE when old OIDs are supplied. This retains the helper's best-effort behavior: an old-OID mismatch rejects that deletion while independent deletions in the batch can still proceed. Signed-off-by: Maciej Ciemborowicz Signed-off-by: Junio C Hamano --- bisect.c | 2 +- builtin/branch.c | 3 ++- builtin/fetch.c | 2 +- builtin/remote.c | 5 ++-- builtin/tag.c | 3 ++- refs.c | 56 ++++++++++++++++++++++++++++++--------- refs.h | 13 +++++++-- t/helper/test-ref-store.c | 2 +- 8 files changed, 65 insertions(+), 21 deletions(-) diff --git a/bisect.c b/bisect.c index 9cbb3dc677..931a80098f 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, 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 a613148fc7..c9f259d046 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 533fdfe7d8..b662216bf7 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 de989ea3ba..13d3cc52dd 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, 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 06c125b53c..40874a2923 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 92d5df5b71..1e0f432ed8 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" @@ -3069,34 +3070,60 @@ void ref_transaction_for_each_rejected_update(struct ref_transaction *transactio } } -int refs_delete_refs(struct ref_store *refs, const char *logmsg, - struct string_list *refnames, unsigned int flags) +struct delete_refs_rejection_data { + int failures; +}; + +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 = 1; +} + +int refs_delete_refs(struct ref_store *refs, const char *logmsg, + struct string_list *refnames, + const struct oid_array *old_oids, + unsigned int flags) +{ + struct delete_refs_rejection_data rejection_data = { 0 }; struct ref_transaction *transaction; struct strbuf err = STRBUF_INIT; - struct string_list_item *item; + size_t i; int ret = 0, failures = 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"); 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, + old_oids ? REF_TRANSACTION_ALLOW_FAILURE : 0, &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; + + if (old_oid && is_null_oid(old_oid)) + old_oid = 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); @@ -3112,9 +3139,14 @@ int refs_delete_refs(struct ref_store *refs, const char *logmsg, refnames->items[0].string, err.buf); else error(_("could not delete references: %s"), err.buf); - } + } else if (old_oids) + ref_transaction_for_each_rejected_update(transaction, + delete_refs_rejection_handler, + &rejection_data); out: + if (rejection_data.failures) + failures = 1; if (!ret && failures) ret = -1; ref_transaction_free(transaction); diff --git a/refs.h b/refs.h index 9979446d15..3a7aacefec 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,21 @@ int refs_delete_ref(struct ref_store *refs, const char *msg, unsigned int flags); /* - * Delete the specified references. If there are any problems, emit + * 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 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, + unsigned int flags); /** Delete a reflog */ int refs_delete_reflog(struct ref_store *refs, const char *refname); diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c index db58f00589..6d6857cbd3 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, flags); string_list_clear(&refnames, 0); return result; }