Re: [PATCH v2 2/4] refs/files: use correct error type when lock exists
From: Karthik Nayak <hidden>
Date: 2025-09-11 10:14:22
Patrick Steinhardt [off-list ref] writes:
On Mon, Sep 08, 2025 at 02:37:36PM +0200, Karthik Nayak wrote:quoted
When fetching references into a repository, if a lock for a particular reference exists, then `lock_raw_ref()` throws the generic error 'REF_TRANSACTION_ERROR_GENERIC'. This causes the entire set of batched updates to fail.This isn't quite accurate anymore as we may also raise `REF_TRANSACTION_ERROR_CASE_CONFLICT` now.
Good catch, will fixup.
quoted
diff --git a/refs/files-backend.c b/refs/files-backend.c index 58005d2732..2730713d23 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c@@ -790,9 +790,13 @@ static enum ref_transaction_error lock_raw_ref(struct files_ref_store *refs, goto retry; } else { unable_to_lock_message(ref_file.buf, myerr, err); - if (myerr == EEXIST && ignore_case && - duplicate_reference_case_cmp(transaction, update)) - ret = REF_TRANSACTION_ERROR_CASE_CONFLICT; + if (myerr == EEXIST) { + if (ignore_case && duplicate_reference_case_cmp(transaction, update)) + ret = REF_TRANSACTION_ERROR_CASE_CONFLICT; + else + ret = REF_TRANSACTION_ERROR_CREATE_EXISTS; + } + goto error_return; } }Hm. So if I understand correctly, we now return CREATE_EXISTS in case we have a conflict with a preexisting case-conflicting reference, but we return CASE_CONFLICT in case we have a conflict with an update in the same transaction?
It's not a reference, rather the lock that causes the conflict. So a preexisting case-conflicting reference lock would raise the GENERIC error. Which would fail all updates.
It feels awkward, but I guess that's the best thing we can do. We happily overwrite case-conflicting preexisting refs, so we wouldn't even see EEXIST in that case. So the only case where we still see that error is on a D/F conflict, and in that case it makes sense to return CREATE_EXISTS. I feel like this should be added as a comment though, as it gives some important non-obvious context.
Yeah, it would be beneficial, will add some comments here.
Patrick
Attachments
- signature.asc [application/pgp-signature] 690 bytes