On Sat, Apr 15, 2017 at 10:11 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Jeff King <peff@xxxxxxxx> 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@xxxxxxxxxxxxxx>. 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.