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