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

From: Jonathan Nieder <[email-masked]>
Date: Mon, 5 Jan 2015 13:25:23 -0800

Stefan Beller wrote:

> Maybe we should do both?
>
>     die ("BUG: Some refs have not been checked for connectivity."
>           "Please contact the git developers ([email-masked]) and"
>           "report the problem. As a workaround run 'git fsck'. If there"
>           "are errors, try to remove the reported refs above. (This "
>           "may lead to data loss, backup first.)"

I personally find this kind of message grating when I run into it.
The message is trying to tell me what to do, but it is not in a
position to know what the best thing to do is.

It could be that I am using an ancient version of git with known bugs.
In that case I should upgrade.

It could be that I am using faulty hardware that flips random bits and
confuses software.

It could be that I have a patched version of git, in which case I should
contact the author of my patch instead of [email-masked].

It could be that this is a recent, terrible regression and
[email-masked] is already bombarded with reports about it.

If the message says

	fatal: BUG: connectivity check skipped???

then it has exactly the right amount of information to tell me what to
do.  Now I have

 - a short string to grep for in the source code (or on the web) to
   find out what happened

 - a clear indication that This Can't Happen, so I know to try to
   reproduce it and contact the author of my patched git or upstream
   or whoever, depending on the context

 - no irrelevant guesses to confuse me

The workaround of running 'git fsck' could be actively harmful,
depending on what the bug is.  All that we know is that a bug has
occured and we shouldn't proceed further.

> Just thinking out loud:
[...]
>                  Would it make sense to have an extra die_bug function,

Yes, I think something like the kernel's BUG_ON and WARN_ON would be
very nice (though orthogonal to this change).

Thanks,
Jonathan