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

[Maciej Ciemborowicz <maciej.ciemborowicz@gmail.com>]

Thanks. Yes, supplying old_oid changes these deletions from
unconditional to compare-and-delete, so a concurrent ref change can make
the transaction fail. I should have called that out explicitly.

I think that failure is desirable here: otherwise the command can delete
a value that it never examined. For branch and tag deletion this also
restores the behavior from before 8198907795 (use delete_refs when
deleting tags or branches, 2021-01-21), where delete_ref() was passed the
OID that had been resolved by the caller. That commit batched the deletes
through delete_refs(), but the expected OIDs were lost in the conversion.

I reproduced the race with a reference-transaction hook that updates the
branch during the "preparing" phase, after delete_branches() has collected
its OID. Current Git returns success and deletes the concurrently updated
branch. With this series, the outer transaction fails its old-OID check and
leaves the new value intact. The same check prevents pruning based on a
stale scan from deleting a ref that another process updated meanwhile.

I will make this behavior change explicit in the commit messages and add a
regression test for the concurrent update. Your comment also exposed that
remote prune can print "[pruned]" after such a deletion failure; I will fix
that reporting in v2 as well.

And agreed on using a local item variable for the loop; I will include that
in v2.

Thanks,
Maciej