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. > So I dunno. I had hoped libpcre2 would be a hands-down win on timing, > but it sounds like there's a little back-and-forth depending on the > context. Is there a ticking clock on pcre1 going away? I suspect it will > be around for many years to come, but if they'll continue optimizing > pcre2 but not pcre1, then it may make sense to hitch our wagon to the > one that upstream is actually working on.