From: Jeff King <peff@peff.net>
To: Taylor Blau <me@ttaylorr.com>
Cc: Garima Singh <garimasigit@gmail.com>,
stolee@gmail.com, git@vger.kernel.org
Subject: Re: [RFC PATCH 0/1] commit-graph.c: handle corrupt commit trees
Date: Thu, 5 Sep 2019 02:08:39 -0400 [thread overview]
Message-ID: <20190905060838.GB21450@sigill.intra.peff.net> (raw)
In-Reply-To: <20190904212121.GB20904@syl.local>
On Wed, Sep 04, 2019 at 05:21:21PM -0400, Taylor Blau wrote:
> > I like the idea of completely bailing if the commit can't be parsed too.
> > Only question: Is there a reason you chose to die() instead of BUG() like
> > the other two places in that function? What is the criteria of choosing one
> > over the other?
>
> I did not call 'BUG' here because 'BUG' is traditionally used to
> indicate an internal bug, e.g., an unexpected state or some such. On the
> other side of that coin, 'BUG' is _not_ used to indicate repository
> corruption, since that is not an issue in the Git codebase, rather in
> the user's repository.
>
> Though, to be honest, I've never seen that rule written out explicitly
> (maybe if it were to be written somewhere, it could be stored in
> Documentation/CodingGuidelines?). I think that this is some good
> #leftoverbits material.
That rule matches my understanding. A BUG() should be about asserting
invariants or catching should-not-happen cases, etc. Any time a BUG()
triggers, that is truly a bug in Git, no matter what input got thrown at
it, what syscalls failed, etc, and is worth fixing (even if the only
sensible thing is to die()).
As a side note, we've generally treated segfaults the same way. It
doesn't matter if the files on disk or the program input is garbage, we
should say so and abort the operation cleanly.
-Peff
next prev parent reply other threads:[~2019-09-05 6:15 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-04 2:22 [RFC PATCH 0/1] commit-graph.c: handle corrupt commit trees Taylor Blau
2019-09-04 2:22 ` [RFC PATCH 1/1] commit-graph.c: die on un-parseable commits Taylor Blau
2019-09-04 3:04 ` Jeff King
2019-09-04 21:18 ` Taylor Blau
2019-09-05 6:47 ` Jeff King
2019-09-06 16:48 ` Derrick Stolee
2019-09-06 17:04 ` Jeff King
2019-09-06 17:19 ` Derrick Stolee
2019-09-06 17:20 ` Derrick Stolee
2019-09-05 22:19 ` Junio C Hamano
2019-09-06 6:35 ` Jeff King
2019-09-06 6:56 ` Jeff King
2019-09-06 16:59 ` Junio C Hamano
2019-09-06 17:04 ` Jeff King
2019-09-09 16:39 ` Junio C Hamano
2019-09-09 16:54 ` Jeff King
2019-09-04 18:25 ` [RFC PATCH 0/1] commit-graph.c: handle corrupt commit trees Garima Singh
2019-09-04 21:21 ` Taylor Blau
2019-09-05 6:08 ` Jeff King [this message]
2019-09-06 16:48 ` Derrick Stolee
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190905060838.GB21450@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=garimasigit@gmail.com \
--cc=git@vger.kernel.org \
--cc=me@ttaylorr.com \
--cc=stolee@gmail.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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).