Re: [PATCHv10 02/10] receive-pack.c: die instead of error in assure_connectivity_checked

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

 



Stefan Beller wrote:

> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -1055,15 +1055,15 @@ static void assure_connectivity_checked(struct command *commands,
>  
>  	for (cmd = commands; cmd; cmd = cmd->next) {
>  		if (should_process_cmd(cmd) && si->shallow_ref[cmd->index]) {
> -			error("BUG: connectivity check has not been run on ref %s",
> -			      cmd->ref_name);
> +			die("BUG: connectivity check has not been run on ref %s",
> +			    cmd->ref_name);

This could stay as error() so that the caller gets to see the list of
refs that didn't experience a connectivity check.  Or if that list
isn't important, this error/die call could be dropped and the
'checked_connectivity = 0' setting would be enough.

>  			checked_connectivity = 0;
>  		}
>  	}
>  	if (!checked_connectivity)
> -		error("BUG: run 'git fsck' for safety.\n"
> -		      "If there are errors, try to remove "
> -		      "the reported refs above");
> +		die("BUG: run 'git fsck' for safety.\n"
> +		    "If there are errors, try to remove "
> +		    "the reported refs above");

I find this error message misleading and confusing.  It makes it seem
like this is an expected error that we are trying to help the user
recover from.  Instead, something impossible has happened.  "Try to
remove the reported refs" is actively harmful advice --- that would be
a way for the user to work around a serious bug instead of figuring
out what went wrong and getting git fixed.

Jonathan
--
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]