Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: May 4, 2017, 09:11 UTC
Message-ID: <alpine.DEB.2.21.1.1705041104070.4905@virtualbox>
In-Reply-To: <CACBZZX6_5krLp93PmsW639-N4f1efUT5rPnN+5im=d9-66=QbQ@mail.gmail.com>

Hi Ævar,

On Wed, 3 May 2017, Ævar Arnfjörð Bjarmason wrote:

> [Just replying to you & Duy in the same mail, easier]

It makes it harder on everybody else, though, as two slightly different discussion points are conflated now. Also, no single online mail archive will be able to render the thread correctly (assuming that you edited in the In-Reply-To header to loop back to Duy's mail).

Show 10 quoted lines

> On Wed, May 3, 2017 at 11:45 AM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> > At this point, I feel that someone should recall into our collective
> > memory what happened when we made a change similar in nature that
> > broke existing build setups: by requiring REG_STARTEND all of a sudden
> > ("you can easily flip the NO_REGEX switch", as if everybody should
> > know about those Makefile flags we have).
> 
> And as a result grep/log -G got faster by default,

Sure. For those developers where the build was not broken.

Software maintenance is always a trade-off, and with software as popular as Git, maintainers bear a special responsibility to *not* break builds easily, as it is more likely than not that anybody who wants to build Git is *unfamiliar* with the specifics.

That is the main reason why we have a configure, even if we try hard to make things work with a straight `make`: people who are happily oblivious of our discussions on this here high-volume mailing list will be able to build Git without even consulting the documentation, just by doing what they would do with any Unix-based software: ./configure && make

> and more importantly since v2.10.1 which includes your 2f8952250a and
> made a REG_STARTEND engine a hard requirement nobody using git is
> mysteriously going to miss grep results because of some stray \0 in the
> string being matched.

That is a misinterpretation of what the REG_STARTEND flag is supposed to do. In *some* implementations, REG_STARTEND allows NULs in the haystack. Some other implementations do not allow that. It is ill-defined.

> I agree that I should drop the patch to make v2 the default from this
> series for now. Clearly it's controversial, and can be considered on
> its own merits once the supporting code is in. I'll do that in the
> next submission, which I'm planning to send after v2.13.0 comes out.

Good. I am really glad that we agree that the move to v2 should be a two-step process, with the uncontroversial "optionally use PCRE v2 for speed and robustness" first.

Once enough users like it (and speaking for myself, once I heard from enough users how good it is so that I can justify to set aside enough time to support PCRE v2 in MSYS2), you will find it much smoother sailing to go to phase 2.

Ciao, Dscho