Re: [PATCH v3 3/3] fetch, remote: retain old OIDs when pruning refs
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 22, 2026, 19:16 UTC
- Message-ID
- <xmqqeceldrlw.fsf@gitster.g>
- In-Reply-To
- <3f3062252ac1aa057b9ee9a2dd9892e629ba7a82.1790079917.git.maciej.ciemborowicz@gmail.com>
Maciej Ciemborowicz <maciej.ciemborowicz@gmail.com> writes:
Show 24 quoted lines
> 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, 0);
> }
> + if (result)
> + goto cleanup;
> }Hmph, I may not be reading the code correctly, but the last "goto cleanup" in the above block can happen when refs_delete_refs() call that internally uses the best effort transaction sees an error. If we were about to prune 30 refs but failed to prune one of them, and if we are running with non-negative verbosity, don't we still want to make the "[deleted]" report for the 29 of them and possibly report "[failed to delete]" for the one that failed?
Show 10 quoted lines
>
> if (verbosity >= 0) {
> int summary_width = transport_summary_width(stale_refs);
>
> + if (!refnames.nr)
> + for (ref = stale_refs; ref; ref = ref->next)
> + string_list_append(&refnames, ref->name);
> for (ref = stale_refs; ref; ref = ref->next) {
> display_ref_update(display_state, '-', _("[deleted]"), NULL,
> _("(none)"), ref->name,Show 23 quoted lines
> @@ -1639,17 +1650,24 @@ 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, 0);
> + if (result)
> + goto cleanup;
> + }Ditto. Beyond the post context of this hunk ...
Show 7 quoted lines
> 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 (dry_run)
> printf_ln(_(" * [would prune] %s"),... around here is a code that reports "* [pruned]" for the ones that we successfully removed, which is now ignored when even one of the bulk removal fails.