Re: [PATCH 2/4] lock_ref_sha1_basic: simplify errno handling

[Jonathan Nieder <jrnieder@gmail.com>]

Jeff King wrote:

> Now that error() does not clobber errno, we do not have to
> take pains to save it ourselves.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  refs.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 5ff457e..169a46d 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2232,7 +2232,6 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,

All the caller to lock_ref_sha1_basic cares about is whether errno ==
ENOTDIR, since in that case we can print a message suggesting running
"git remote prune".

Longer term, I suspect the caller (transaction_commit) should call
is_refname_available to check for conflicting refs and return early to
give "git fetch" a chance to print its advice.

If a conflicting ref appears after that point, then just printing a
reasonable error message is enough --- it is not so useful to give the
'remote prune' advice when people are doing funny things like running
fetch in one terminal and a ref update creating a D/F conflict against
that fetch in another terminal.[*]

By the way, Stefan was mentioning the other day that it might make
sense for transaction_commit to prevent a conflicting ref from
appearing mid-transaction by locking 'refs/heads' in addition to
'refs/heads/master'.

Anyway, in the meantime this is a nice cleanup.

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

[*] If we want to handle that, the 'struct strbuf *err' could be
replaced with a larger struct with room for structured data, like
Junio was hinting at in another thread.