Re: [musl] Re: Regression: git no longer works with musl libc's regex impl

From: Rich Felker <dalias@libc.org>
Date: Oct 4, 2016, 17:39 UTC
Message-ID: <20161004173926.GA19318@brightrain.aerifal.cx>
In-Reply-To: <alpine.DEB.2.20.1610041802310.35196@virtualbox>

On Tue, Oct 04, 2016 at 06:08:33PM +0200, Johannes Schindelin wrote:

Show 28 quoted lines

> Hi Rich,
> 
> On Tue, 4 Oct 2016, Rich Felker wrote:
> 
> > On Tue, Oct 04, 2016 at 11:27:22AM -0400, Jeff King wrote:
> > > On Tue, Oct 04, 2016 at 11:08:48AM -0400, Rich Felker wrote:
> > > 
> > > > 1. is nonzero mod page size, it just works; the remainder of the last
> > > >    page reads as zero bytes when mmapped.
> > > 
> > > Is that a portable assumption?
> > 
> > Yes.
> 
> No, it is not. You quote POSIX, but the matter of the fact is that we use
> a subset of POSIX in order to be able to keep things running on Windows.
> 
> And quite honestly, there are lots of reasons to keep things running on
> Windows, and even to favor Windows support over musl support. Over four
> million reasons: the Git for Windows users.
> 
> So rather than getting into an ideological discussion about "broken"
> systems, it would be good to keep things practical, realizing that those
> users make up a very real chunk of all of Git's users.
> 
> As to making NO_REGEX conditional on REG_STARTEND: you are talking about
> apples and oranges here. NO_REGEX is a Makefile flag, while REG_STARTEND
> is a C preprocessor macro.

It seems like you could just always compile the source file, and just have it all inside #if defined(NO_REGEX) || !defined(REG_STARTEND) or similar.

> And lastly, the best alternative would be to teach musl about
> REG_STARTEND, as it is rather useful a feature.

Maybe, but it seems fundamentally costly to support -- it's extra state in the inner loops that imposes costly spill/reload on archs with too few registers (x86). I'll look at doing this when we overhaul/replace the regex implementation, and I'm happy to do some performance-regression tests for adding it now if someone has a simple patch (as was mentioned on the musl list).

Rich