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

Stefan Beller wrote:

Maybe we should do both?

    die ("BUG: Some refs have not been checked for connectivity."
          "Please contact the git developers (git@vger.kernel.org) 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 git@vger.kernel.org.

It could be that this is a recent, terrible regression and
git@vger.kernel.org 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