On Fri, Jun 03, 2022 at 02:05:49PM -0700, Junio C Hamano wrote: > > 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@xxxxxxxxxxxxxxxxxxxxx/ :) 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
