From: Jonathan Nieder <jrnieder@gmail.com>
To: Stefan Beller <sbeller@google.com>
Cc: Duy Nguyen <pclouds@gmail.com>,
Junio C Hamano <gitster@pobox.com>,
"git@vger.kernel.org" <git@vger.kernel.org>,
Eric Sunshine <sunshine@sunshineco.com>,
Michael Haggerty <mhagger@alum.mit.edu>,
ronnie sahlberg <ronniesahlberg@gmail.com>
Subject: Re: [PATCHv10 02/10] receive-pack.c: die instead of error in assure_connectivity_checked
Date: Mon, 5 Jan 2015 13:25:23 -0800 [thread overview]
Message-ID: <20150105212523.GN29365@google.com> (raw)
In-Reply-To: <CAGZ79kZUiwEbcSPk3Td60umixvh_Q7jXTGoLemKLYsvX1ty39w@mail.gmail.com>
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
next prev parent reply other threads:[~2015-01-05 21:25 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-12-30 23:41 [PATCH 0/9] atomic pushes Stefan Beller
2014-12-30 23:41 ` [PATCHv9 1/9] receive-pack.c: shorten the execute_commands loop over all commands Stefan Beller
2015-01-03 2:20 ` Jonathan Nieder
2015-01-03 9:53 ` Duy Nguyen
2015-01-05 18:02 ` Stefan Beller
2015-01-05 18:25 ` [PATCHv10 01/10] " Stefan Beller
2015-01-05 18:25 ` [PATCHv10 02/10] receive-pack.c: die instead of error in assure_connectivity_checked Stefan Beller
2015-01-05 20:17 ` Jonathan Nieder
2015-01-05 21:15 ` Stefan Beller
2015-01-05 21:25 ` Jonathan Nieder [this message]
2015-01-06 19:40 ` [PATCHv11 02/11] receive-pack.c: die instead of error in case of possible future bug Stefan Beller
2015-01-06 19:46 ` Jonathan Nieder
2015-01-05 20:22 ` [PATCHv10 01/10] receive-pack.c: shorten the execute_commands loop over all commands Jonathan Nieder
2015-01-05 21:07 ` Stefan Beller
2015-01-05 21:18 ` Jonathan Nieder
2015-01-06 19:34 ` [PATCHv11 01/11] " Stefan Beller
2014-12-30 23:41 ` [PATCHv9 2/9] receive-pack.c: move iterating over all commands outside execute_commands Stefan Beller
2014-12-30 23:41 ` [PATCHv9 3/9] receive-pack.c: move transaction handling in a central place Stefan Beller
2014-12-30 23:41 ` [PATCHv9 4/9] receive-pack.c: add execute_commands_atomic function Stefan Beller
2014-12-30 23:41 ` [PATCHv9 5/9] receive-pack.c: negotiate atomic push support Stefan Beller
2014-12-30 23:41 ` [PATCHv9 6/9] send-pack: rename ref_update_to_be_sent to check_to_send_update Stefan Beller
2014-12-30 23:41 ` [PATCHv9 7/9] send-pack.c: add --atomic command line argument Stefan Beller
2014-12-30 23:41 ` [PATCHv9 8/9] push.c: add an --atomic argument Stefan Beller
2014-12-30 23:41 ` [PATCHv9 9/9] t5543-atomic-push.sh: add basic tests for atomic pushes Stefan Beller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150105212523.GN29365@google.com \
--to=jrnieder@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=mhagger@alum.mit.edu \
--cc=pclouds@gmail.com \
--cc=ronniesahlberg@gmail.com \
--cc=sbeller@google.com \
--cc=sunshine@sunshineco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.