[PATCH v2 1/3] refs: allow callers to supply old OIDs for batch deletion
- From
Maciej Ciemborowicz <maciej.ciemborowicz@gmail.com>
- Date
- Sep 20, 2026, 10:54 UTC
- Message-ID
- <5c96a5a1ebafd49a301c5c1dc77a2e19d6677ab0.1789901584.git.maciej.ciemborowicz@gmail.com>
- In-Reply-To
- <cover.1789901584.git.maciej.ciemborowicz@gmail.com>
refs_delete_refs() currently performs unconditional deletions. Thus callers cannot preserve old values that they have already resolved, and reference-transaction hooks consequently see a null old OID.
Add an optional oid_array whose entries correspond to the refnames. Pass each non-null OID to ref_transaction_delete(). Supplying an OID makes the deletion conditional: if the ref changed after the caller resolved it, the transaction fails instead of deleting the new value. Existing callers that pass NULL retain the unconditional behavior.
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 | 23 ++++++++++++++--------- refs.h | 12 ++++++++++-- t/helper/test-ref-store.c | 2 +- 8 files changed, 34 insertions(+), 18 deletions(-)
diff --git a/bisect.c b/bisect.c index 94c7028d2..9aa3bace9 100644 --- a/bisect.c +++ b/bisect.c @@ -1203,7 +1203,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 1572a4f9e..f1abeb681 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -322,7 +322,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds, free(target); } - 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)) ret = 1; for_each_string_list_item(item, &refs_to_delete) { diff --git a/builtin/fetch.c b/builtin/fetch.c index c1d7c672f..d202147b2 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1467,7 +1467,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..13d3cc52d 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 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 d3caa9a63..9c593baea 100644 --- a/refs.c +++ b/refs.c @@ -18,6 +18,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" @@ -3056,33 +3057,37 @@ 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 string_list *refnames, + const struct oid_array *old_oids, + unsigned int flags) { 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 (old_oids && old_oids->nr != refnames->nr) + BUG("refname and old OID counts do not match"); if (!refnames->nr) return 0; 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); 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); diff --git a/refs.h b/refs.h index 71d5c186d..b76b556cf 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; @@ -613,13 +614,20 @@ 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 entry is used + * to verify the current value of the corresponding reference before deleting + * it. A null entry disables verification for that reference. + * + * 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 3866d0aca..c2c7dfb06 100644 --- a/t/helper/test-ref-store.c +++ b/t/helper/test-ref-store.c @@ -140,7 +140,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; }
-- 2.39.3 (Apple Git-146)