Subject: Re: [PATCH 3/9] Makefile: turn on NO_MMAP when building with ASan

From: Patrick Steinhardt <[email-masked]>
Date: Thu, 13 Nov 2025 07:34:17 +0100

On Wed, Nov 12, 2025 at 10:12:02PM -0500, Taylor Blau wrote:
> On Wed, Nov 12, 2025 at 12:26:13PM +0100, Patrick Steinhardt wrote:
> > On Wed, Nov 12, 2025 at 03:02:15AM -0500, Jeff King wrote:
> > > diff --git a/Makefile b/Makefile
> > > index 7e0f77e298..0f44268405 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -1587,6 +1587,7 @@ SANITIZE_LEAK = YesCompiledWithIt
> > >  endif
> > >  ifneq ($(filter address,$(SANITIZERS)),)
> > >  NO_REGEX = NeededForASAN
> > > +NO_MMAP = NeededForASAN
> > >  SANITIZE_ADDRESS = YesCompiledWithIt
> > >  endif
> > >  endif
> >
> > Let's also apply this to Meson. Thanks!
> 
> Not to derail us too far off topic, but... ;-)
> 
> I wonder what (if anything) our policy should be for keeping the
> Makefile and Meson build scripts in sync. On the one hand, I do not want
> the two of them to drift (too far) apart. But on the other, I am not
> sure that everyone who may be touching the Makefile are necessarily
> familiar enough to make the equivalent changes to the Meson build files.

They might not be, true. That's basically the reason why I'm always on
the lookout for such changes and try to help them with diffs that they
can simply apply.

> I genuinely don't have a very strong opinion here or even really a clear
> sense of what the right thing to do is. Just something that crossed my
> mind while reading and figured I'd write down in case others had similar

I don't think there needs to be a policy any stricter than "Meson
pipelines need to stay green". So new code files and tests need to be
added, but that's easy enough to do even for somebody who isn't familiar
with Meson, I think.

Other changes like this one here are not really _that_ important in most
cases. Git compiles just fine without such a change, and any real-world
user is likely to not care. So I think it's fine to handle these on a
best effort basis.

Thanks!

Patrick