Re: [PATCH v3 2/4] refs/files: use correct error type when lock exists
- From
Justin Tobler <jltobler@gmail.com>
- Date
- Sep 16, 2025, 21:31 UTC
- Message-ID
- <672jdldtxegx4y3gdtcyrnsz5nsy4sh6pk76eftqrukwug3f3c@xwso5f4vukn5>
- In-Reply-To
- <20250913-587-git-fetch-1-fails-fetches-on-case-insensitive-repositories-v3-2-195569740b57@gmail.com>
On 25/09/13 10:54PM, Karthik Nayak wrote:
Show 5 quoted lines
> When fetching references into a repository, if a lock for a particular > reference exists, then `lock_raw_ref()` throws: > > - REF_TRANSACTION_ERROR_CASE_CONFLICT: when there is a conflict > because transaction contains conflicting references while being on a
s/transaction/the transaction/
Show 6 quoted lines
> case-insensitive filesystem. > > - REF_TRANSACTION_ERROR_GENERIC: for all other errors. > > The latter causes the entire set of batched updates to fail, even in > case sensitive filessystems.
Ok so this issue isn't related to case-insensitive filesystems. The issue is that now we use batch updated, a single pre-existing lockfile causes the entire transaction to fail. Prior to batch updates, only the individual update would fail, but wouldn't stop others.
> Instead, return a 'REF_TRANSACTION_ERROR_CREATE_EXISTS' error. This > allows batched updates to reject the individual update which conflicts > with the existing file, while updating the rest of the references.
Make sense.
Show 32 quoted lines
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
> refs/files-backend.c | 20 +++++++++++++++++---
> t/t5510-fetch.sh | 26 ++++++++++++++++++++++++++
> 2 files changed, 43 insertions(+), 3 deletions(-)
>
> 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;IIUC, by returning a non-generic error here the individual reference will be rejected during batch updates instead of aborting the transaction.
-Justin