Re: [PATCH v5 1/3] refs: allow callers to supply old OIDs for batch deletion

On Thu, Sep 24, 2026 at 12:05 PM Karthik Nayak [off-list ref] wrote:

This also completes the conversion that was missed when
batched transaction failure support was introduced.

Agreed, that sentence is unclear. I will try to remove the batch deletion
API changes, along with this sentence and the caller-specific approach.

This change is totally different from the rest of the commit, I think it
should be a precursor with adequate explanation regarding why this is
done and why that's okay.

I'm also still of the opinion that this shouldn't be done.

I will try to leave ref_transaction_delete() and its null_oid
BUG() unchanged. Instead, store the value observed for the
hook in separate fields that do not set REF_HAVE_OLD and therefore do not
constrain the transaction.

I'm assuming failures is to count the number of refs which failed,
wouldn't failed_refs->nr give us the same result?

failed_refs is optional, which is why a separate counter is needed. If I
move the solution into the common transaction layer, both the failed_refs
parameter and the counter should no longer be needed.

Nit: we could inline the size_t here.

I will fix this in the next version.

Thanks for the review.
- Maciej Ciemborowicz