git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 0/3] BUG() and "config --local" outside of repo

From
Jeff King <peff@peff.net>
Date
May 13, 2017, 03:24 UTC
Message-ID
<20170513032414.mfrwabt4hovujde2@sigill.intra.peff.net>
In-Reply-To
<20170513020346.25ijajgtwffh7g4b@sigill.intra.peff.net>
On Fri, May 12, 2017 at 10:03:46PM -0400, Jeff King wrote:
Show 25 quoted lines
> On Sat, May 13, 2017 at 12:31:31AM +0200, Ævar Arnfjörð Bjarmason wrote:
> 
> > > +       if (use_local_config && nongit)
> > > +               die(_("--local only be used inside a git repository"));
> > > +
> > 
> > It would be better to have a test for edge cases that are currently
> > only being discovered by users in the wild.
> 
> I actually started on one earlier, but what would it check? We already
> die() in this case. Should we be grepping for the message? It seems more
> likely to me that we would change the message and cause a false positive
> than that there would be an actual regression.
> 
> What I think might be more interesting is if die("BUG") could learn to
> exit with some error code that the test suite considered invalid. Like
> calling abort(), which would kill us with SIGABRT and cause
> test_must_fail to complain.
> 
> On many systems that would also generate a coredump. Which is handy
> sometimes, but I wonder if it would be inconvenient for others. I guess
> that is no different than what a raised assert() would do.
> 
> But if we were to do that, then the test could easily demonstrate that
> we expect a clean die().

So here's a series which does that (and fixes the gramm-o that Jonathan pointed out). The SIGABRT/coredump thing may seem like overkill, but it's actually something I've found useful before (while developing this very same setup_git_env assertion, in fact).

  [1/3]: usage.c: add BUG() function
  [2/3]: setup_git_env: convert die("BUG") to BUG()
  [3/3]: config: complain about --local outside of a git repo
 builtin/config.c       |  3 +++
 environment.c          |  2 +-
 git-compat-util.h      |  9 +++++++++
 t/t1300-repo-config.sh |  6 ++++++
 usage.c                | 32 ++++++++++++++++++++++++++++++++
 5 files changed, 51 insertions(+), 1 deletion(-)
Previous: Jeff KingNext: Jeff King
Message 7 of 25 in “[Git 2.13.0] BUG: setup_git_env called without repository”
  1. Josh HaginsMay 12, 2017
  2. Johannes SchindelinMay 12, 2017
  3. Josh HaginsMay 15, 2017
  4. config: complain about --local outside of a git repoJeff King, May 12, 2017
  5. Ævar Arnfjörð BjarmasonMay 12, 2017
  6. Jeff KingMay 13, 2017
  7. 0/3 BUG() and "config --local" outside of repoJeff King, May 13, 2017
  8. 1/3 usage.c: add BUG() functionJeff King, May 13, 2017
  9. Jeff KingMay 13, 2017
  10. Junio C HamanoMay 15, 2017
  11. 2/3 setup_git_env: convert die("BUG") to BUG()Jeff King, May 13, 2017
  12. 3/3 config: complain about --local outside of a git repoJeff King, May 13, 2017
  13. 0/4 Finish the conversion from die("BUG: ...") to BUG()Johannes Schindelin, May 2, 2018
  14. 1/4 test-tool: help verifying BUG() code pathsJohannes Schindelin, May 2, 2018
  15. Duy NguyenMay 2, 2018
  16. Johannes SchindelinMay 5, 2018
  17. 2/4 run-command: use BUG() to report bugs, not die()Johannes Schindelin, May 2, 2018
  18. Jeff KingMay 7, 2018
  19. 3/4 Replace all die("BUG: ...") calls by BUG() onesJohannes Schindelin, May 2, 2018
  20. 4/4 Convert remaining die*(BUG) messagesJohannes Schindelin, May 2, 2018
  21. Jeff KingMay 7, 2018
  22. Jonathan NiederMay 13, 2017
  23. Jeff KingMay 13, 2017
  24. Josh HaginsMay 15, 2017
  25. Jeff KingMay 15, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.