Re: [PATCH v3 2/4] refs/files: use correct error type when lock exists
From: Patrick Steinhardt <hidden>
Date: 2025-09-16 07:51:27
On Sat, Sep 13, 2025 at 10:54:30PM +0200, Karthik Nayak wrote:
quoted hunk ↗ jump to hunk
diff --git a/refs/files-backend.c b/refs/files-backend.c index 01df32904b..69e50a16db 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c@@ -797,9 +797,23 @@ 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 && - transaction_has_case_conflicting_update(transaction, update)) - ret = REF_TRANSACTION_ERROR_CASE_CONFLICT; + if (myerr == EEXIST) { + if (ignore_case && + transaction_has_case_conflicting_update(transaction, update)) + /* + * In case-insensitive filesystems, ensure that conflicts within a + * given transaction are handled. Pre-existing refs on a + * case-insensitive system will be overridden without any issue. + */ + ret = REF_TRANSACTION_ERROR_CASE_CONFLICT; + else + /* + * Pre-existing case-conflicting reference locks should also be + * specially categorized to avoid failing all batched updates. + */ + ret = REF_TRANSACTION_ERROR_CREATE_EXISTS; + }
Tiniest nit: I think it would be preferable to use curly braces for such multi-line comments. This nit isn't worth a reroll though. Other than that my feedback got addressed, so this looks good to me. Thanks! Patrick