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

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

[-- Attachment #1: Type: text/plain, Size: 2763 bytes --]

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).

> 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