Git development
 help / color / mirror / Atom feed
From: Maciej Ciemborowicz <maciej.ciemborowicz@gmail.com>
To: git@vger.kernel.org
Cc: "Karthik Nayak" <karthik.188@gmail.com>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Patrick Steinhardt" <ps@pks.im>,
	"Phil Hord" <phil.hord@gmail.com>,
	"Elijah Newren" <newren@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"D . Ben Knoble" <ben.knoble@gmail.com>,
	"Maciej Ciemborowicz" <maciej.ciemborowicz@gmail.com>
Subject: [PATCH v5 0/3] refs: report old OIDs for batched deletions
Date: Wed, 23 Sep 2026 23:04:39 +0200	[thread overview]
Message-ID: <cover.1790196627.git.maciej.ciemborowicz@gmail.com> (raw)
In-Reply-To: <cover.1790113781.git.maciej.ciemborowicz@gmail.com>

Thanks for catching the broken intermediate commits. This version updates
every refs_delete_refs() call site in 1/3, using NULL for the optional data
that is not supplied until the later patches. Each patch now builds on its
predecessor. The final tree is identical to v4.

This follows up on the reference-transaction bug report at [1].

The reference-transaction hook receives zero as both the old and new OID
when branch, tag, fetch, and remote delete refs through refs_delete_refs().
Those callers already know the values that they selected for deletion.

Teach refs_delete_refs() to accept a parallel array of expected old OIDs and
pass them into the transaction. Besides making hook records useful, this
restores conditional deletion for branch and tag and adds it to pruning
without additional ref reads. Non-atomic batches preserve best-effort
behavior, while atomic fetches remain all-or-nothing.

Changes since v4:

 * Update all refs_delete_refs() call sites in 1/3 for the new signature.
 * Verify that 1/3, 1/3--2/3, and the complete series each build with
   DEVELOPER=1.

The focused reference-transaction tests pass with SHA-1 and SHA-256 using
both the files and reftable backends. The full test suite passed on the
identical final tree in v4.

[1] https://lore.kernel.org/git/CACQ=SRGTTdQ+dHXhN6F52dBv5KxZBRfk_Em2fvmEmGJDoB6oTg@mail.gmail.com/

Maciej Ciemborowicz (3):
  refs: allow callers to supply old OIDs for batch deletion
  branch, tag: retain old OIDs in batched deletions
  fetch, remote: retain old OIDs when pruning refs

 bisect.c                         |   2 +-
 builtin/branch.c                 |   7 +-
 builtin/fetch.c                  |  30 ++++--
 builtin/remote.c                 |  47 +++++++--
 builtin/tag.c                    |  28 ++++--
 refs.c                           |  67 ++++++++++---
 refs.h                           |  31 ++++--
 t/helper/test-ref-store.c        |   2 +-
 t/t1416-ref-transaction-hooks.sh | 160 +++++++++++++++++++++++++++++++
 9 files changed, 324 insertions(+), 50 deletions(-)

Range-diff against v4:
1:  f4a9d065c ! 1:  9b76cc2c4 refs: allow callers to supply old OIDs for batch deletion
    @@ builtin/branch.c: static int delete_branches(int argc, const char **argv, int ki
      	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))
    ++			     &refs_to_delete, NULL, NULL, REF_NO_DEREF))
      		ret = 1;
      
      	for_each_string_list_item(item, &refs_to_delete) {
    @@ builtin/fetch.c: static int prune_refs(struct display_state *display_state,
      			result = refs_delete_refs(get_main_ref_store(the_repository),
      						  "fetch: prune", &refnames,
     -						  0);
    -+						  NULL, 0);
    ++						  NULL, NULL, 0);
      		}
      	}
      
    @@ builtin/remote.c: static int prune_remote(const char *remote, int 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);
    ++					   NULL, NULL, 0);
      
      	for_each_string_list_item(item, &states.stale) {
      		const char *refname = item->util;
    @@ builtin/tag.c: static int delete_tags(const char **argv)
      	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))
    ++			     &refs_to_delete, NULL, NULL, REF_NO_DEREF))
      		result = 1;
      
      	for_each_string_list_item(item, &refs_to_delete) {
2:  918c97d2b ! 2:  6a8401c44 branch, tag: retain old OIDs in batched deletions
    @@ builtin/branch.c: static int delete_branches(int argc, const char **argv, int ki
      
      	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, NULL, NULL, REF_NO_DEREF))
     +			     &refs_to_delete, &old_oids, NULL, REF_NO_DEREF))
      		ret = 1;
      
    @@ builtin/tag.c: static int for_each_tag_name(const char **argv, each_tag_name_fn
     -	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))
    +-			     &refs_to_delete, NULL, NULL, REF_NO_DEREF))
     +			     &data.refs, &data.old_oids, NULL, REF_NO_DEREF))
      		result = 1;
      
3:  6f34853c7 ! 3:  541da44c3 fetch, remote: retain old OIDs when pruning refs
    @@ builtin/fetch.c: static int prune_refs(struct display_state *display_state,
     +			}
      			result = refs_delete_refs(get_main_ref_store(the_repository),
      						  "fetch: prune", &refnames,
    --						  NULL, 0);
    +-						  NULL, NULL, 0);
     +						  &old_oids, &failed_refs, 0);
     +			if (result && !failed_refs.nr)
     +				goto cleanup;
    @@ builtin/remote.c: 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,
    --					   NULL, 0);
    +-					   NULL, NULL, 0);
     +					   &old_oids, &failed_refs, 0);
     +		if (result && !failed_refs.nr)
     +			goto cleanup;
-- 
2.39.3 (Apple Git-146)

  parent reply	other threads:[~2026-09-23 21:04 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-19 13:34 [BUG] reference-transaction reports zero OIDs for branch and tag deletion Maciej Ciemborowicz
2026-09-19 14:59 ` D. Ben Knoble
2026-09-19 15:42   ` Maciej Ciemborowicz
2026-09-19 20:11     ` [PATCH 0/3] refs: report old OIDs for batched deletions Maciej Ciemborowicz
2026-09-19 20:11       ` [PATCH 1/3] refs: allow callers to supply old OIDs for batch deletion Maciej Ciemborowicz
2026-09-19 20:41         ` Karthik Nayak
2026-09-20 10:38           ` Maciej Ciemborowicz
2026-09-20 10:54           ` [PATCH v2 0/3] refs: report old OIDs for batched deletions Maciej Ciemborowicz
2026-09-20 10:54             ` [PATCH v2 1/3] refs: allow callers to supply old OIDs for batch deletion Maciej Ciemborowicz
2026-09-21 13:12               ` Karthik Nayak
2026-09-21 23:55               ` Junio C Hamano
2026-09-20 10:54             ` [PATCH v2 2/3] branch, tag: retain old OIDs in batched deletions Maciej Ciemborowicz
2026-09-21 13:19               ` Karthik Nayak
2026-09-20 10:54             ` [PATCH v2 3/3] fetch, remote: retain old OIDs when pruning refs Maciej Ciemborowicz
2026-09-21 13:56               ` Karthik Nayak
2026-09-21 13:57             ` [PATCH v2 0/3] refs: report old OIDs for batched deletions Karthik Nayak
2026-09-21 20:01               ` Maciej Ciemborowicz
2026-09-22 12:26             ` [PATCH v3 " Maciej Ciemborowicz
2026-09-22 12:26               ` [PATCH v3 1/3] refs: allow callers to supply old OIDs for batch deletion Maciej Ciemborowicz
2026-09-22 18:55                 ` Junio C Hamano
2026-09-22 19:21                   ` Maciej Ciemborowicz
2026-09-22 23:31                     ` Junio C Hamano
2026-09-22 12:26               ` [PATCH v3 2/3] branch, tag: retain old OIDs in batched deletions Maciej Ciemborowicz
2026-09-22 12:26               ` [PATCH v3 3/3] fetch, remote: retain old OIDs when pruning refs Maciej Ciemborowicz
2026-09-22 19:16                 ` Junio C Hamano
2026-09-22 22:29               ` [PATCH v4 0/3] refs: report old OIDs for batched deletions Maciej Ciemborowicz
2026-09-22 22:31                 ` [PATCH v4 1/3] refs: allow callers to supply old OIDs for batch deletion Maciej Ciemborowicz
2026-09-22 22:31                 ` [PATCH v4 2/3] branch, tag: retain old OIDs in batched deletions Maciej Ciemborowicz
2026-09-22 22:31                 ` [PATCH v4 3/3] fetch, remote: retain old OIDs when pruning refs Maciej Ciemborowicz
2026-09-23 20:03                   ` Junio C Hamano
2026-09-23 21:02                     ` Maciej Ciemborowicz
2026-09-23 21:04                 ` Maciej Ciemborowicz [this message]
2026-09-23 21:04                   ` [PATCH v5 1/3] refs: allow callers to supply old OIDs for batch deletion Maciej Ciemborowicz
2026-09-24 10:04                     ` Karthik Nayak
2026-09-24 16:34                       ` Junio C Hamano
2026-09-24 19:43                       ` Maciej Ciemborowicz
2026-09-24 11:07                     ` Patrick Steinhardt
2026-09-24 16:45                       ` Junio C Hamano
2026-09-24 20:13                         ` Maciej Ciemborowicz
2026-09-28  6:44                           ` Patrick Steinhardt
2026-09-28  6:43                         ` Patrick Steinhardt
2026-09-24 19:56                       ` Maciej Ciemborowicz
2026-09-23 21:04                   ` [PATCH v5 2/3] branch, tag: retain old OIDs in batched deletions Maciej Ciemborowicz
2026-09-24 11:08                     ` Patrick Steinhardt
2026-09-23 21:04                   ` [PATCH v5 3/3] fetch, remote: retain old OIDs when pruning refs Maciej Ciemborowicz
2026-09-23 21:55                   ` [PATCH v5 0/3] refs: report old OIDs for batched deletions Junio C Hamano
2026-09-24 22:33                   ` [PATCH v6 0/1] refs: report old values to transaction hooks Maciej Ciemborowicz
2026-09-24 22:33                     ` [PATCH v6 1/1] " Maciej Ciemborowicz
2026-09-30  3:11                       ` Maciej Ciemborowicz
2026-10-01 17:37                         ` Maciej Ciemborowicz
2026-09-19 20:11       ` [PATCH 2/3] branch, tag: retain old OIDs in batched deletions Maciej Ciemborowicz
2026-09-19 20:11       ` [PATCH 3/3] fetch, remote: retain old OIDs when pruning refs Maciej Ciemborowicz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1790196627.git.maciej.ciemborowicz@gmail.com \
    --to=maciej.ciemborowicz@gmail.com \
    --cc=avarab@gmail.com \
    --cc=ben.knoble@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=karthik.188@gmail.com \
    --cc=newren@gmail.com \
    --cc=phil.hord@gmail.com \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox