Re: [PATCH 00/12] PCREv2 & more

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Sat, Apr 15, 2017 at 10:11 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
>
>> On Sat, Apr 08, 2017 at 01:24:54PM +0000, Ævar Arnfjörð Bjarmason wrote:
>>
>>> This adds PCRE v2 support, but as I was adding that I kept noticing
>>> other related problems to fix. It's all bundled up into the same
>>> series because much of it conflicts because it modifies the same test
>>> or other code. Notes on each patch below.
>>
>> Overall, the series looks OK to me.
>>
>> I'm not sure if it is worth all the complexity to carry pcre1/pcre2 as
>> run-time options. That does make it easier to do back-to-back
>> comparisons, but it makes the code a lot more complicated. In particular
>> I'm worried about subtle cases where we pcre1 turns into pcre2 (or vice
>> versa) by use of the aliases. That shouldn't matter to a user for
>> correctness, but it would throw off the benchmarking.
>>
>> If we literally just added USE_LIBPCRE2 and built against one or the
>> other, then all the complexity would be limited to a few #ifdefs. The
>> big drawback AFAICT is that anybody doing timing tests would have to
>> recompile in between.
>
> Yeah, having to dl two libs at runtime, even when you would ever use
> just one in a single run, is less than ideal.  A small downside
> inflicted on everybody will add up to million times more than a
> larger downside only suffered by developers, so I tend to agree with
> you that we probably should simplify to choose just one (or zero) at
> compile time.

I'll document & clarify this in v2, but I don't expect / want anyone
who's distributing git to link to both v1 & v2, more details in my
<CACBZZX6HLDmWSGiQ+cJ-p0Ak6SQHcmECaGqsfVz-Js4q7aSEwg@mail.gmail.com>.

It's just something we already have 95% of the code to support anyway,
and doing the remaining 5% makes it easier to test & benchmark it for
us devs without incurring any real maintenance or tech burden.

But as noted elsewhere in that message I'll include a patch to only
add the ability to use one PCRE version. So we can just review &
discuss the tradeoffs of doing that then.