Re: [PATCH] ci: add address and undefined sanitizer tasks

[Jeff King <peff@peff.net>]

On Mon, Oct 10, 2022 at 04:25:23PM -0700, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Signed-off-by: Junio C Hamano <gitster@pobox.com>
> > ---
> >  * I've been running my local post-integration-pre-pushout tests of
> >    'seen' with these two sanitizer tests, which has saved me from a
> >    few potential embarrassments early.  As it takes a lot extra time
> >    to run these locally, I am aiming to burden contributors who run
> >    their due diligence "before sending the patch" checks using the
> >    GitHub Actions CI ;-).
> >
> >    The way the patch adds jobs to CI just imitates how -leaks one is
> >    defined.
> 
> One downside is that the usual CI cycle for a branch that takes a
> bit shorter than 40 minutes seems to take between 50 to 60 minutes
> (the primary culprit seems to be the address sanitizer).
> 
> Arguably, that 10 or 20 minutes are time saved from human
> developers' time, so it might not be too bad, but I'll keep
> it out of 'next' for now.

You can make it a bit faster by running both at once as
SANITIZE=address,undefined.

Since we expect both to pass cleanly, there's really no other
complication; the user will either see errors (and correct them) or they
won't. The signal of "passed with asan, but not ubsan" (or vice versa)
is not that useful in practice.

In the long run, I hope that "leaks" can run in the same way, but we're
not there yet since there's a lot of selective test-running. In theory
the regular linux test run is not necessary with this job, but I think
the signal for "broke with/without sanitizers" is a little higher (and
anyway, we have to run the regular suite on a zillion other platforms,
too).

-Peff