Web lists-archives.com

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




On Fri, May 12, 2017 at 10:03:46PM -0400, Jeff King wrote:

> 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(-)