Karthik Nayak <karthik.188@xxxxxxxxx> 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.
