On Sat, Jul 06, 2024 at 02:18:50AM -0400, Jeff King wrote: > On Wed, Jul 03, 2024 at 11:44:33PM +0200, Rubén Justo wrote: > > > > Explicitly indicating that the error is being forced due to > > > "GIT_TEST_SANITIZE_LEAK_LOG=true", for a test that doesn't fail when run > > > normally or even when run with just > > > "GIT_TEST_PASSING_SANITIZE_LEAK=yes", could save us some confusion. > > > > > > So, I dunno. > > > > > > Anyway, I agree that this can be addressed later. > > > > > > Thanks. > > > > Maybe what we should do is integrate "GIT_TEST_SANITIZE_LEAK_LOG" into > > "GIT_TEST_PASSING_SANITIZE_LEAK" because I'm not sure what value we get > > by keeping them separate (test performance?). But that's another topic, > > even further out of scope of this patch :-) > > I don't think we want to integrate them, but I'd suggest that > SANITIZE_LEAK_LOG should be the default/only option. > > Without it, you are potentially missing leaks in programs whose failing > exit codes do not trigger a test failure. So there is no point in > running PASSING_SANITIZE_LEAK=check without also checking the logs. But > it is still useful to set SANITIZE_LEAK_LOG just for normal runs to look > for leaks. > > I don't know of any reason we couldn't always check the logs (for a > leak-checking build), and I didn't see anything in the history. I think > it was written that way only because there is otherwise no affirmative > action by the user to say "and btw, look for leaks" (and if we are not > looking for leaks, there might not be any logs!). > > But really, if you have done a leak-checking build, then every run of > the tests is looking for leaks, whether you check the logs or not. So we > should able to just check that $SANITIZE_LEAK is set. > And then there would be one less thing for people checking for leaks > to remember to set. I completely agree. Let's wait for the dust to settle after the fix in this series, and then I'll address the change as you described. Thanks.
