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

Re: [RFC PATCH 1/3] test-tool: don't fake up BUG() exits as code 99

From
Jeff King <peff@peff.net>
Date
Jun 8, 2022, 19:17 UTC
Message-ID
<YqD100DRVJoZCC+x@coredump.intra.peff.net>
In-Reply-To
<xmqqczfpe9ua.fsf@gitster.g>
On Fri, Jun 03, 2022 at 02:05:49PM -0700, Junio C Hamano wrote:
Show 9 quoted lines
> > Are we sure that the reason no longer applies?  How do we know?  We
> > would want to explain that to future developers in the proposed log
> > message, I would think.
> 
> We can flip it the other way around.  
> 
> I do not think I ever saw anybody asked anybody on this list who got
> a BUG() message to use the coredump to do something useful.  Don't
> modern distros ship with "ulimit -c 0" these days?

Certainly I have found the coredumps useful, though I would expect most Git developers to be able to run under a debugger and stop at BUG() anyway. So we could probably live without that, but...

> It might be possible that a better direction is to introduce
> GIT_ABORT_ON_BUG environment or core.abortOnBUG configuration that
> chooses between abort() and exit(99), or something like that, and
> then we switch to use the latter by default over time?

It really should continue to die with a signal (or an exit code that pretends to be one) to continue triggering even under test_must_fail().

IMHO the whole "let's trigger BUG() intentionally via test-tool" stuff in t1406 is misguided. It's literally introducing broken code that is not run in the normal Git binary in order to make sure that it's broken. If that were the only spot, I'd suggest just getting rid of it entirely. But the code in t0210 that checks the handling of trace2 and BUG() is probably worth keeping.

I do agree that an environment variable would be a better selector than "this code is linked against test-tool". I thought so even back in:

  https://lore.kernel.org/git/20180507090109.GA367@sigill.intra.peff.net/

:) That message also covers the flip-side case discussed earlier in this thread: why calling abort() unconditionally in the test suite can be a pain.

-Peff
Previous: Ævar Arnfjörð BjarmasonNext: Junio C Hamano
Message 7 of 16 in “trace2: log "signal" end events if we invoke BUG()”
  1. 0/3 trace2: log "signal" end events if we invoke BUG()Ævar Arnfjörð Bjarmason, May 26, 2022
  2. 2/3 refs API: rename "abort" callback to avoid macro clashÆvar Arnfjörð Bjarmason, May 26, 2022
  3. 1/3 test-tool: don't fake up BUG() exits as code 99Ævar Arnfjörð Bjarmason, May 26, 2022
  4. Junio C HamanoJun 3, 2022
  5. Junio C HamanoJun 3, 2022
  6. Ævar Arnfjörð BjarmasonJun 3, 2022
  7. Jeff KingJun 8, 2022
  8. Junio C HamanoJun 8, 2022
  9. Ævar Arnfjörð BjarmasonJun 9, 2022
  10. Jeff KingJun 9, 2022
  11. Ævar Arnfjörð BjarmasonJun 3, 2022
  12. 3/3 trace2: emit "signal" events after calling BUG()Ævar Arnfjörð Bjarmason, May 26, 2022
  13. Bagas SanjayaMay 26, 2022
  14. Josh SteadmonMay 31, 2022
  15. Junio C HamanoMay 26, 2022
  16. Josh SteadmonMay 31, 2022

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.