Re: [PATCH v3 2/2] lock_ref_sha1_basic: handle REF_NODEREF with invalid refs

Jeff King [off-list ref] writes:

I also notice that if we are deleting, we _do_ set
RESOLVE_REF_NO_RECURSE from the very beginning, which means we would
generally not get a valid lock->old_oid.hash for a symref. But I'm not
sure what it would mean to delete a symref while asking for its current
value (it cannot have one!). So I don't think it is a bug.

I started scratching my head after noticing that the NO_RECURSE bit
set in the DELETING codepath before reading the above, and I am
still doing so.

A transaction that attempts to delete an existing symref presumably
wants to make sure that the "old" value it read hasn't changed, but
ensuring the object name (obtained by reading the ref that is
pointed by the symref by dereferencing) are the same is not the
right way to ensure nobody raced with us in the meantime anyway (we
should rather be making sure that the symref is still pointing at
the same ref), so in that sense, in the context of acquiring the
lock, old oid value is meaningless for symrefs.

This patch is a strict improvement as the behaviour for REF_DELETING
case is unchanged by it (an idempotent resolve-ref-unsafe may be
called one more time in some cases), and other cases are better, I
think.