On Mon, Jan 29, 2024 at 02:15:15PM -0800, Junio C Hamano wrote:

quoted

Let's mark it as leak-free to make sure it stays that way (and to reduce
noise when looking for other leak-free scripts after we fix some leaks).

For other tests in this series, that rationale is a very sensible
thing, but does it apply to this one?

The point of the t-basic tests is to ensure the lightweight unit
test framework that requires nothing from Git behaves (and keeps
behaving) sensibly.  The point of running t[0-9][0-9][0-9][0-9]
tests under leak sanitizer is to exercise production Git code to
catch leaks in Git code.

So it is not quite clear if we even want to run this t0080 under
leak sanitizer to begin with.  t0080 is a relatively tiny test, but
do we even want to spend leak sanitizer cycles on it?  I dunno.

I think you are right that we do not particularly care about leaks in
the t-basic code. That is also true of other test harness code (other
unit-tests, but also stuff in t/helper). But IMHO it is less work to
just keep that code leak-free than it is to try to distinguish between
production and test code.

Right now, it is not that hard to simply leave the PASSES_SANITIZE_LEAK
flag off of t0080, and then it won't be run in the leak-checking CI job.
But I think the end-game of all of this leak-checking stuff is that
eventually _everything_ will be leak-free, and we will discard the whole
PASSES_SANITIZE_LEAK mechanism entirely. And in that end-game, it is
simpler for everything, including t-basic, to just be leak-free and
checked under the same regime.

Setting the flag now just makes sure we continue correctly on that path,
rather than getting surprised near the end of the road that t-basic has
some dumb leak. Plus it avoids the script popping up as a false positive
when checking for scripts which can be marked.

-Peff