Re: [PATCH 4/4] refs: do not clobber dangling symrefs

From: Jeff King <peff@peff.net>
Date: Sep 23, 2025, 17:33 UTC
Message-ID: <20250923173322.GA1136654@coredump.intra.peff.net>
In-Reply-To: <87cy7hy0gc.fsf@iotcl.com>

On Tue, Sep 23, 2025 at 11:36:51AM +0200, Toon Claes wrote:

Show 11 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > So I dunno. My inclination is to say that the double-null-oid invocation
> > is weird and wrong, and callers should update if they need to. But I
> > could be convinced otherwise.
> 
> Thanks for your feedback, and I have to agree. I'll get in touch with
> the Gitaly team to see if we can rid of this odd invocation. For the
> record, this conversation has been happening here[1].
> 
> [1]: https://gitlab.com/gitlab-org/gitaly/-/merge_requests/8161#note_2767808133

Looking over that conversation, I do think you might consider using symref-delete. As noted there, doing "delete <dangling-symref>" is going to delete unconditionally, whether it's a symref, a real ref, or nothing is there at all.

If you know it's a symref pointing to "refs/heads/foo", then the safest thing is:

  symref-delete FOO_HEAD refs/heads/foo

which guarantees the operation is doing what you expected.

There's an open question there of: how do I know what it's pointing to? But that's kind of the point of the "old-target" (and "old-oid") options. They take information you discovered previously non-atomically and atomically perform the operation while checking (under lock) that things haven't changed unexpectedly.

So from the test perspective, I think you just know what's in the test fixture. From the Gitaly API perspective, the caller should have some idea of what they're deleting (just like they should for a real ref).

-Peff