From: Jeff King <peff@peff.net>
Date: Tue, 23 Sep 2025 17:33:22 GMT
Subject: Re: [PATCH 4/4] refs: do not clobber dangling symrefs
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:

> 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