[PATCH v3 2/3] branch, tag: retain old OIDs in batched deletions
- From
Maciej Ciemborowicz <maciej.ciemborowicz@gmail.com>
- Date
- Sep 22, 2026, 12:26 UTC
- Message-ID
- <2065188aabecd857456b3f2d791edf9f7c8c6c6a.1790079917.git.maciej.ciemborowicz@gmail.com>
- In-Reply-To
- <cover.1790079917.git.maciej.ciemborowicz@gmail.com>
Before 8198907795 (use delete_refs when deleting tags or branches, 2021-01-21), branch and tag deletion passed each resolved old OID to delete_ref(). This prevented the command from deleting a ref that another process had changed after it was inspected.
The conversion to batched deletion dropped those old OIDs. Besides making the deletions unconditional, this causes reference-transaction hooks to report zero as both the old and new OID.
Both commands still resolve the old OIDs before starting the deletion. Pass those values to refs_delete_refs(). This restores the old race protection and lets hooks receive useful old values without adding ref reads. If a ref changes concurrently, reject its deletion and preserve the new value.
Signed-off-by: Maciej Ciemborowicz <maciej.ciemborowicz@gmail.com> --- builtin/branch.c | 6 ++++- builtin/tag.c | 27 ++++++++++++++------ t/t1416-ref-transaction-hooks.sh | 44 ++++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 9 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c index c9f259d04..f222a2644 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -16,6 +16,7 @@ #include "commit.h" #include "gettext.h" #include "object-name.h" +#include "oid-array.h" #include "remote.h" #include "parse-options.h" #include "branch.h" @@ -248,6 +249,7 @@ static int delete_branches(int argc, const char **argv, int kinds, struct strbuf bname = STRBUF_INIT; enum interpret_branch_kind allowed_interpret; struct string_list refs_to_delete = STRING_LIST_INIT_DUP; + struct oid_array old_oids = OID_ARRAY_INIT; struct string_list_item *item; int branch_name_pos; const char *fmt_remotes = "refs/remotes/%s"; @@ -342,6 +344,7 @@ static int delete_branches(int argc, const char **argv, int kinds, } item = string_list_append(&refs_to_delete, name); + oid_array_append(&old_oids, &oid); item->util = xstrdup((ref_flags & REF_ISBROKEN) ? "broken" : (ref_flags & REF_ISSYMREF) ? target : repo_find_unique_abbrev(the_repository, &oid, DEFAULT_ABBREV)); @@ -352,7 +355,7 @@ 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, NULL, REF_NO_DEREF)) + &refs_to_delete, &old_oids, REF_NO_DEREF)) ret = 1; for_each_string_list_item(item, &refs_to_delete) { @@ -377,6 +380,7 @@ static int delete_branches(int argc, const char **argv, int kinds, free(describe_ref); } string_list_clear(&refs_to_delete, 0); + oid_array_clear(&old_oids); free(name); strbuf_release(&bname); diff --git a/builtin/tag.c b/builtin/tag.c index 40874a292..32b70c369 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -105,28 +105,38 @@ static int for_each_tag_name(const char **argv, each_tag_name_fn fn, return had_error; } +struct tags_to_delete { + struct string_list refs; + struct oid_array old_oids; +}; + static int collect_tags(const char *name UNUSED, const char *ref, const struct object_id *oid, void *cb_data) { - struct string_list *ref_list = cb_data; + struct tags_to_delete *data = cb_data; + struct string_list_item *item; - string_list_append(ref_list, ref); - ref_list->items[ref_list->nr - 1].util = oiddup(oid); + item = string_list_append(&data->refs, ref); + item->util = oiddup(oid); + oid_array_append(&data->old_oids, oid); return 0; } static int delete_tags(const char **argv) { int result; - struct string_list refs_to_delete = STRING_LIST_INIT_DUP; + struct tags_to_delete data = { + .refs = STRING_LIST_INIT_DUP, + .old_oids = OID_ARRAY_INIT, + }; struct string_list_item *item; - result = for_each_tag_name(argv, collect_tags, (void *)&refs_to_delete); + result = for_each_tag_name(argv, collect_tags, &data); if (refs_delete_refs(get_main_ref_store(the_repository), NULL, - &refs_to_delete, NULL, REF_NO_DEREF)) + &data.refs, &data.old_oids, REF_NO_DEREF)) result = 1; - for_each_string_list_item(item, &refs_to_delete) { + for_each_string_list_item(item, &data.refs) { const char *name = item->string; struct object_id *oid = item->util; if (!refs_ref_exists(get_main_ref_store(the_repository), name)) @@ -136,7 +146,8 @@ static int delete_tags(const char **argv) free(oid); } - string_list_clear(&refs_to_delete, 0); + string_list_clear(&data.refs, 0); + oid_array_clear(&data.old_oids); return result; } diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh index 4fe9d9b23..01b5ba8c4 100755 --- a/t/t1416-ref-transaction-hooks.sh +++ b/t/t1416-ref-transaction-hooks.sh @@ -14,6 +14,50 @@ test_expect_success setup ' POST_OID=$(git rev-parse POST) ' +test_expect_success 'hook gets old values for batched branch/tag deletion' ' + test_when_finished "rm -f actual" && + git branch to-delete PRE && + git tag delete-tag POST && + git pack-refs --all && + test_hook reference-transaction <<-\EOF && + if test "$1" = committed + then + # Ignore backend-internal zero-to-zero records. + while read -r old new ref + do + case "$old" in + *[!0]*) + echo "$old $new $ref" + ;; + esac + done >>actual + fi + EOF + cat >expect <<-EOF && + $PRE_OID $ZERO_OID refs/heads/to-delete + $POST_OID $ZERO_OID refs/tags/delete-tag + EOF + git branch -D to-delete && + git tag -d delete-tag && + test_cmp expect actual +' + +test_expect_success 'branch deletion rejects a concurrent update' ' + git branch delete-race PRE && + test_hook reference-transaction <<-\EOF && + marker=$(git rev-parse --git-path delete-race-once) + if test "$1" = preparing && test ! -e "$marker" + then + >"$marker" + git update-ref refs/heads/delete-race POST + fi + exit 0 + EOF + test_must_fail git branch -D delete-race 2>err && + test_grep "is at $POST_OID but expected $PRE_OID" err && + test_cmp_rev POST refs/heads/delete-race +' + test_expect_success 'hook allows updating ref if successful' ' git reset --hard PRE && test_hook reference-transaction <<-\EOF &&
-- 2.39.3 (Apple Git-146)