On Sun Oct 13, 2024 at 16:03, Phillip Wood [off-list ref] wrote:

Hi Bence

On 13/10/2024 00:03, Bence Ferdinandy wrote:

quoted

Currently there is only one special error for transaction, for when
there is a naming conflict, all other errors are dumped under a generic
error. Add a new special error case for when the caller requests the
reference to be updated only when it does not yet exist and the
reference actually does exist.

This looks like useful improvement. Are the changes to 
reftable-backend.c correct - it looks like where it previously returned 
TRANSACTION_GENERIC_ERR it now returns TRANSACTION_NAME_CONFLICT which I 
think is used to indicate a file/directory conflict (e.g. trying to 
create refs/heads/topic/one when refs/heads/topic exists)

This passes:
GIT_TEST_DEFAULT_REF_FORMAT=reftable prove --timer --jobs 8 ./t[0-9]*.sh
so I'm hoping it's correct :)

[snip]

I guess you are referring to this part below:

quoted

diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 3c96fbf66f..ebf8e57fbc 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c

@@ -1206,10 +1206,13 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,
  				goto done;
  			}
  		} else if ((u->flags & REF_HAVE_OLD) && !oideq(&current_oid, &u->old_oid)) {
-			if (is_null_oid(&u->old_oid))
+			ret = TRANSACTION_NAME_CONFLICT;
+			if (is_null_oid(&u->old_oid)) {
  				strbuf_addf(err, _("cannot lock ref '%s': "
  						   "reference already exists"),
  					    ref_update_original_update_refname(u));
+				ret = TRANSACTION_CREATE_EXISTS;
+			}
  			else if (is_null_oid(&current_oid))
  				strbuf_addf(err, _("cannot lock ref '%s': "
  						   "reference is missing but expected %s"),

@@ -1221,7 +1224,6 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,
  					    ref_update_original_update_refname(u),
  					    oid_to_hex(&current_oid),
  					    oid_to_hex(&u->old_oid));
-			ret = -1;
  			goto done;
  		}
  

This originally returned -1, and it still returns that if it doesn't return -2,
I just used the named variable instead of the integer itself. It might still be
that this should be -3 GENERIC_ERROR, but if that is the case I think fixing
that should be a different patch? I didn't check if changing that -1 to
something else breaks anything or not.

Best,
Bence

-- 
bence.ferdinandy.com