On Mon Sep 30, 2024 at 08:40, Patrick Steinhardt [off-list ref] wrote:

I don't think that is a good reason to introduce this new flag though.
If we need to have a proper way to identify this specific failure case
we should rather update the already-existing mechanism to give us useful
signals, shouldn't we?

The problem with this flag is that it basically duplicates functionality
that already exists, and it needs to be wired up by every ref backend
that we have and that we're adding in the future. Your patch for example
only implements the functionality for the "files" backend, but it must
also be wired up for the "reftable" backend or otherwise it would be
broken.

Another issue is that it gives you more ways to create nonsensical ref
updates. With it you could for example create requests with a non-zero
old object ID, and if it has `REF_CREATE_ONLY` set it would never be
possible to fulfill the request. There's probably other cases where you
can create nonsensical ref updates already, but we shouldn't add more
ways of doing that.

Mind you, if we go the way I propose and improve the error reporting
we'd also have to adapt both backends to do so. But that would be
plugging a gap for which we have no proper solution right now instead of
circumventing the current design by duplicating the functionality that
we already have in a way that makes us able to handle this.

Patrick

You make a convincing argument :) I'll try to see if I can add another
transaction error code for when you only want to create not overwrite and the
ref already exists and pass it up (for both files and reftables, you also don't
mention it, but I think this would not concern packed after a quick glance at
the code).


Best,
Bence

-- 
bence.ferdinandy.com