Subject: Re: "lock file exists" when fetching in bare clone of repository

From: Junio C Hamano <[email-masked]>
Date: Fri, 22 Aug 2025 10:47:04 -0700

Karthik Nayak <[email-masked]> writes:

> The fix itself isn't too involved:
>
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 088b52c740..5c31b02e6b 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -776,6 +776,8 @@ 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)
> +				ret = REF_TRANSACTION_ERROR_NAME_CONFLICT;
>             goto error_return;
>         }
>     }

We assume that the existing lock is what _we_ created to lock the
ref in the other case, not somebody else locked-and-died some time
ago, or somebody else locked-and-about-to-update-competing-with-us? 

Without this change we'd return REF_TRANSACTION_ERROR_GENERIC; does
the caller treat NAME_CONFLICT any specially?  Or is the "fix" you
talk about just about giving a different message and no other
behaviour changes involved?  I guess a more specific message that is
99% of the time more correct is an improvement over an overly
generic "some error happened" message.  But I thought the original
issue was that the user cannot make any progress when the ref
updates are transactional.  Does returning NAME_CONFLICT from here
tell the machinery that we are allowed to break transactional
semantics somehow?

In any case, I wonder if refs_verify_refnames_available() should
notice that we are using files backend on a case challenged
filesystem, and change the behaviour a bit.  This additional check
needs to be implemented as a backend call via refs->be->something()
to tighten the outcome.  It appears to me that the function in the
current form does not even notice a D/F conflict when there is a
branch 'd' and the transaction requests to create a new branch 'D/f'
on a case challenged system if files backend is in use, since the
function is in the generic layer and behaves case sensitively (which
is the right thing to do---we are talking about detecting backend
specific glitches here).

Thanks.