Re: [PATCH v5 2/3] branch, tag: retain old OIDs in batched deletions

On Wed, Sep 23, 2026 at 11:04:41PM +0200, Maciej Ciemborowicz wrote:

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.

Hm. The motivation makes sense to me, but I have to wonder whether we're
approaching it on the wrong level. With your proposed changes, we're now
not force-deleting the refs anymore, which is a user-visible change in
behaviour.

What you're after though is to always have an old object ID available
when the reference-transaction hook kicks in. But if that's the goal,
shouldn't we consider whether we can instead resolve the old value
during the transaction and queue that for the reftx hook, regardless of
whether or not the user has asked for an old object ID? That would now
cover _all_ users that modify refs without us having to update every
single callsite.

Sure, strictly speaking it's a backwards-incompatible change. But we've
always considered the reftx hook to be exposing internals, so we aren't
all that strict about retaining its behaviour and have allowed changes
in behaviour in the past. So I wouldn't mind if we adapted the hook to
always yield the old object ID.

Patrick