[PATCH v4 3/3] fetch, remote: retain old OIDs when pruning refs
From: Maciej Ciemborowicz <hidden>
Date: 2026-09-22 22:31:17
Subsystem:
the rest · Maintainer:
Linus Torvalds
get_stale_heads() records the current value of each stale local ref in its new_oid member. The pruning paths discard that value and request unconditional deletion, so reference-transaction hooks receive a null old OID. Pass the recorded values into the deletion transactions. If a ref changes after the stale scan, reject that deletion and preserve the new value. Non-atomic pruning uses refs_delete_refs(), whose partial-failure mode still deletes unaffected stale refs. An atomic fetch remains all-or-nothing. Continue reporting successful non-atomic deletions when another deletion is rejected, but do not report the rejected ref as deleted or use it when checking for newly dangling symrefs. Use the rejected-ref list returned by refs_delete_refs() so reporting reflects the transaction result without additional ref reads. Signed-off-by: Maciej Ciemborowicz <redacted> --- builtin/fetch.c | 30 +++++--- builtin/remote.c | 44 +++++++++--- t/t1416-ref-transaction-hooks.sh | 116 +++++++++++++++++++++++++++++++ 3 files changed, 174 insertions(+), 16 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index b662216bf..95789edb8 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c@@ -1471,22 +1471,29 @@ static int prune_refs(struct display_state *display_state, struct ref *ref, *stale_refs = get_stale_heads(rs, ref_map); struct strbuf err = STRBUF_INIT; struct string_list refnames = STRING_LIST_INIT_NODUP; - - for (ref = stale_refs; ref; ref = ref->next) - string_list_append(&refnames, ref->name); + struct string_list deleted_refs = STRING_LIST_INIT_NODUP; + struct string_list failed_refs = STRING_LIST_INIT_DUP; + struct oid_array old_oids = OID_ARRAY_INIT; if (!dry_run) { if (transaction) { for (ref = stale_refs; ref; ref = ref->next) { - result = ref_transaction_delete(transaction, ref->name, NULL, - NULL, 0, "fetch: prune", &err); + result = ref_transaction_delete(transaction, ref->name, + &ref->new_oid, NULL, 0, + "fetch: prune", &err); if (result) goto cleanup; } } else { + for (ref = stale_refs; ref; ref = ref->next) { + string_list_append(&refnames, ref->name); + oid_array_append(&old_oids, &ref->new_oid); + } result = refs_delete_refs(get_main_ref_store(the_repository), "fetch: prune", &refnames, - NULL, 0); + &old_oids, &failed_refs, 0); + if (result && !failed_refs.nr) + goto cleanup; } }
@@ -1494,18 +1501,25 @@ static int prune_refs(struct display_state *display_state, int summary_width = transport_summary_width(stale_refs); for (ref = stale_refs; ref; ref = ref->next) { + if (string_list_has_string(&failed_refs, ref->name)) + continue; + display_ref_update(display_state, '-', _("[deleted]"), NULL, _("(none)"), ref->name, &ref->new_oid, &ref->old_oid, summary_width); + string_list_append(&deleted_refs, ref->name); } - string_list_sort(&refnames); + string_list_sort(&deleted_refs); refs_warn_dangling_symrefs(get_main_ref_store(the_repository), - stderr, " ", dry_run, &refnames); + stderr, " ", dry_run, &deleted_refs); } cleanup: string_list_clear(&refnames, 0); + string_list_clear(&deleted_refs, 0); + string_list_clear(&failed_refs, 0); + oid_array_clear(&old_oids); strbuf_release(&err); free_refs(stale_refs); return result;
diff --git a/builtin/remote.c b/builtin/remote.c
index 56b06845b..2d9ee6db1 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c@@ -17,6 +17,7 @@ #include "refs.h" #include "refspec.h" #include "odb.h" +#include "oid-array.h" #include "strvec.h" #include "commit-reach.h" #include "progress.h"
@@ -380,6 +381,11 @@ struct ref_states { int queried; }; +struct stale_ref { + struct object_id oid; + char name[FLEX_ARRAY]; +}; + #define REF_STATES_INIT { \ .new_refs = STRING_LIST_INIT_DUP, \ .skipped = STRING_LIST_INIT_DUP, \
@@ -410,9 +416,13 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat } stale_refs = get_stale_heads(&states->remote->fetch, fetch_map); for (ref = stale_refs; ref; ref = ref->next) { + struct stale_ref *stale_ref; struct string_list_item *item = string_list_append(&states->stale, abbrev_branch(ref->name)); - item->util = xstrdup(ref->name); + + FLEX_ALLOC_STR(stale_ref, name, ref->name); + oidcpy(&stale_ref->oid, &ref->new_oid); + item->util = stale_ref; } free_refs(stale_refs); free_refs(fetch_map);
@@ -1627,6 +1637,9 @@ static int prune_remote(const char *remote, int dry_run) int result = 0; struct ref_states states = REF_STATES_INIT; struct string_list refs_to_prune = STRING_LIST_INIT_NODUP; + struct string_list pruned_refs = STRING_LIST_INIT_NODUP; + struct string_list failed_refs = STRING_LIST_INIT_DUP; + struct oid_array old_oids = OID_ARRAY_INIT; struct string_list_item *item; get_remote_ref_states(remote, &states, GET_REF_STATES);
@@ -1639,17 +1652,27 @@ static int prune_remote(const char *remote, int dry_run) printf_ln(_("Pruning %s"), remote); printf_ln(_("URL: %s"), states.remote->url.v[0]); - for_each_string_list_item(item, &states.stale) - string_list_append(&refs_to_prune, item->util); - string_list_sort(&refs_to_prune); + for_each_string_list_item(item, &states.stale) { + struct stale_ref *stale_ref = item->util; + + string_list_append(&refs_to_prune, stale_ref->name); + oid_array_append(&old_oids, &stale_ref->oid); + } - if (!dry_run) + if (!dry_run) { result |= refs_delete_refs(get_main_ref_store(the_repository), "remote: prune", &refs_to_prune, - NULL, 0); + &old_oids, &failed_refs, 0); + if (result && !failed_refs.nr) + goto cleanup; + } for_each_string_list_item(item, &states.stale) { - const char *refname = item->util; + struct stale_ref *stale_ref = item->util; + const char *refname = stale_ref->name; + + if (string_list_has_string(&failed_refs, refname)) + continue; if (dry_run) printf_ln(_(" * [would prune] %s"),
@@ -1657,12 +1680,17 @@ static int prune_remote(const char *remote, int dry_run) else printf_ln(_(" * [pruned] %s"), abbrev_ref(refname, "refs/remotes/")); + string_list_append(&pruned_refs, refname); } refs_warn_dangling_symrefs(get_main_ref_store(the_repository), - stdout, " ", dry_run, &refs_to_prune); + stdout, " ", dry_run, &pruned_refs); +cleanup: string_list_clear(&refs_to_prune, 0); + string_list_clear(&pruned_refs, 0); + string_list_clear(&failed_refs, 0); + oid_array_clear(&old_oids); free_remote_ref_states(&states); return result; }
diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh
index 01b5ba8c4..e7c16cd87 100755
--- a/t/t1416-ref-transaction-hooks.sh
+++ b/t/t1416-ref-transaction-hooks.sh@@ -58,6 +58,122 @@ test_expect_success 'branch deletion rejects a concurrent update' ' test_cmp_rev POST refs/heads/delete-race ' +test_expect_success 'hook gets old values when pruning remote refs' ' + test_when_finished "rm -rf empty.git prune" && + git init --bare empty.git && + git init prune && + ( + cd prune && + git remote add origin ../empty.git && + git commit --allow-empty -m one && + one=$(git rev-parse HEAD) && + git commit --allow-empty -m two && + two=$(git rev-parse HEAD) && + git update-ref refs/remotes/origin/remote-prune-z "$one" && + git update-ref refs/remotes/origin/remote-prune-a "$two" + ) && + test_hook -C prune 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 + ( + cd prune && + one=$(git rev-parse HEAD^) && + two=$(git rev-parse HEAD) && + git remote prune origin && + git update-ref refs/remotes/origin/fetch-prune "$one" && + git fetch --prune origin && + git update-ref refs/remotes/origin/atomic-prune "$one" && + git fetch --atomic --prune origin && + cat >expect <<-EOF && + $two $ZERO_OID refs/remotes/origin/remote-prune-a + $one $ZERO_OID refs/remotes/origin/remote-prune-z + $one $ZERO_OID refs/remotes/origin/fetch-prune + $one $ZERO_OID refs/remotes/origin/atomic-prune + EOF + test_cmp expect actual + ) +' + +test_expect_success 'remote prune reports deletions around a concurrent update' ' + test_when_finished "rm -rf race-empty.git race-prune" && + git init --bare race-empty.git && + git init race-prune && + ( + cd race-prune && + git commit --allow-empty -m one && + one=$(git rev-parse HEAD) && + git commit --allow-empty -m two && + two=$(git rev-parse HEAD) && + git remote add origin ../race-empty.git && + git update-ref refs/remotes/origin/race "$one" && + git update-ref refs/remotes/origin/other "$one" + ) && + test_hook -C race-prune reference-transaction <<-\EOF && + marker=$(git rev-parse --git-path prune-race-once) + if test "$1" = preparing && test ! -e "$marker" + then + >"$marker" + git update-ref refs/remotes/origin/race HEAD + fi + exit 0 + EOF + ( + cd race-prune && + two=$(git rev-parse HEAD) && + test_must_fail git remote prune origin >out 2>err && + test_cmp_rev "$two" refs/remotes/origin/race && + test_must_fail git rev-parse --verify refs/remotes/origin/other && + test_grep "\[pruned\].*origin/other" out && + test_grep ! "\[pruned\].*origin/race" out && + test_grep "could not delete reference refs/remotes/origin/race" err + ) +' + +test_expect_success 'fetch prune reports deletions around a concurrent update' ' + test_when_finished "rm -rf fetch-empty.git fetch-prune" && + git init --bare fetch-empty.git && + git init fetch-prune && + ( + cd fetch-prune && + git commit --allow-empty -m one && + one=$(git rev-parse HEAD) && + git commit --allow-empty -m two && + git remote add origin ../fetch-empty.git && + git update-ref refs/remotes/origin/race "$one" && + git update-ref refs/remotes/origin/other "$one" + ) && + test_hook -C fetch-prune reference-transaction <<-\EOF && + marker=$(git rev-parse --git-path prune-race-once) + if test "$1" = preparing && test ! -e "$marker" + then + >"$marker" + git update-ref refs/remotes/origin/race HEAD + fi + exit 0 + EOF + ( + cd fetch-prune && + two=$(git rev-parse HEAD) && + test_must_fail git fetch --prune origin >out 2>err && + test_cmp_rev "$two" refs/remotes/origin/race && + test_must_fail git rev-parse --verify refs/remotes/origin/other && + test_grep "\[deleted\].*origin/other" err && + test_grep ! "\[deleted\].*origin/race" err && + test_grep "could not delete reference refs/remotes/origin/race" err + ) +' + test_expect_success 'hook allows updating ref if successful' ' git reset --hard PRE && test_hook reference-transaction <<-\EOF &&
--
2.39.3 (Apple Git-146)