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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxx>
> ---
>  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@xxxxxxxxx>

[*] 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]