Re: [PATCH v3 2/4] refs/files: use correct error type when lock exists
- From
Karthik Nayak <karthik.188@gmail.com>
- Date
- Sep 17, 2025, 07:37 UTC
- Message-ID
- <CAOLa=ZTeduuWYmkbOknxGhAQ6QOazPSFUU=AbeBg7Bs_fru2BA@mail.gmail.com>
- In-Reply-To
- <aMkW-HOAsU8YZ8Lp@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
Show 32 quoted lines
> On Sat, Sep 13, 2025 at 10:54:30PM +0200, Karthik Nayak wrote:
>> 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.
>I did contemplate about this in my mind, thanks for raising this. I will change it locally for now and push a new version if needed.
> Other than that my feedback got addressed, so this looks good to me. > Thanks! > > Patrick
Thanks for your reviews!