git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 11/12] grep: change the internal PCRE code & header names to be PCRE1

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Apr 11, 2017, 18:31 UTC
Message-ID
<CACBZZX6HLDmWSGiQ+cJ-p0Ak6SQHcmECaGqsfVz-Js4q7aSEwg@mail.gmail.com>
In-Reply-To
<20170411165127.GC36152@google.com>
On Tue, Apr 11, 2017 at 6:51 PM, Brandon Williams <bmwill@google.com> wrote:
Show 55 quoted lines
> On 04/11, Jeff King wrote:
>> On Tue, Apr 11, 2017 at 01:02:56PM +0200, Ęvar Arnfjörš Bjarmason wrote:
>>
>> > >> Yes, this is a bug. I'll need to add a git_options along with
>> > >> submodule_options and pass -c grep.patternType=....
>> > >
>> > > Maybe that's an indication we should have --pcre1-regexp and
>> > > --pcre2-regexp, so we don't have to resort to config tweaking.
>> >
>> > I'd rather not. To reply to both your
>> > <20170411103018.dkq5gangx3vcxhp4@sigill.intra.peff.net> & this, one
>> > thing I was trying to do in this series (and I don't think I went far
>> > enough in "grep & rev-list doc: stop promising libpcre for
>> > --perl-regexp") was to stop promising some specific version of PCRE.
>>
>> We don't necessarily have to document them. This is just in the general
>> rule of "if there's config, there should be command-line to override
>> it". Because without that, you get this exact situation where you have
>> to bolt on "-c" options to another part of the command line, which gets
>> awkward.
>>
>> I'm also not sure it would be strictly correct, if the sub-program runs
>> other sub-programs. Providing "-c" affects all child processes, whereas
>> command-line options are propagated manually. So imagine you have a
>> process tree like:
>>
>>   grep
>>    \-grep
>>       \-textconv
>>
>> I.e., grep recurses to a submodule which then has to kick off a textconv
>> filter for one of the files. If you use "-c" to pass options to the
>> second grep, then those options will continue to have an effect inside
>> the textconv filter. Which _probably_ doesn't run git commands that
>> would care, but technically it could do anything.
>>
>> > I.e. as far as the user is concerned they just want perl-y regexes,
>> > but they most likely don't care about the 1% featureset of those
>> > regexes where the various implementations of "perl-y regex" actually
>> > differ, because those cases tend to be really obscure syntax.
>>
>> Yeah, that's what led me to the "why are we even worrying about run-time
>> switching" direction. I'd think a build-time switch would be enough for
>> people to test, and it makes all of this type of complexity go away.
>
> Yeah I agree with Jeff that we should probably avoid needing to pass a
> config option down in addition to a command line switch to do perl
> regex's.  I didn't take too hard of a look at how that would be done in
> the grep code, but it might be slightly more involved than just changing
> the enum name.
>
> From [12/12] it looks like the main purpose of this series is to use a
> more preferment version of PCRE, if all else is equal it doesn't really
> make much sense to have both versions to be select-able at runtime.  Is
> there any benefit of being able to do that, that I'm missing?

Not really no. I don't think any git user is ever going to be using both pcre1 & pcre2 at runtime.

This grew more organically out of how I started to hack the code. Due to how different the two APIs are it's much less messier to have a new set of wrapper functions than to ifdef around v1 & v2. Once I had all the code & config flags it was easy to shimmy it up so I could switch between the two, and it was handy for performance testing.

But the guy developing it is hardly the main target audience for a feature like this, but on the other hand it's easy to support...

I'm hacking up a v2 of this series. It includes some extra goodies like bugfixes, v1 JIT support, and I'll try to tack a patch at the end that removes this facility to switch between the two at runtime, and we can see if that looks better than not having it.

Previous: Brandon WilliamsNext: Jeff King
Message 40 of 43 in “PCREv2 & more”
  1. 00/12 PCREv2 & moreÆvar Arnfjörð Bjarmason, Apr 8, 2017
  2. 01/12 grep: add ability to disable threading with --threads=0 or grep.threads=0Ævar Arnfjörð Bjarmason, Apr 8, 2017
  3. Jeff KingApr 11, 2017
  4. Ævar Arnfjörð BjarmasonApr 11, 2017
  5. Jeff KingApr 11, 2017
  6. Ævar Arnfjörð BjarmasonApr 11, 2017
  7. Jeff KingApr 14, 2017
  8. Ævar Arnfjörð BjarmasonApr 16, 2017
  9. 02/12 grep: remove redundant regflags assignment under PCREÆvar Arnfjörð Bjarmason, Apr 8, 2017
  10. Jeff KingApr 11, 2017
  11. 03/12 Makefile & configure: reword outdated comment about PCREÆvar Arnfjörð Bjarmason, Apr 8, 2017
  12. Jeff KingApr 11, 2017
  13. Ævar Arnfjörð BjarmasonApr 15, 2017
  14. 04/12 grep: add a test for backreferences in PCRE patternsÆvar Arnfjörð Bjarmason, Apr 8, 2017
  15. 05/12 log: add exhaustive tests for pattern style options & configÆvar Arnfjörð Bjarmason, Apr 8, 2017
  16. Jeff KingApr 11, 2017
  17. Ævar Arnfjörð BjarmasonApr 11, 2017
  18. Jeff KingApr 11, 2017
  19. 06/12 log: add -P as a synonym for --perl-regexpÆvar Arnfjörð Bjarmason, Apr 8, 2017
  20. Junio C HamanoApr 10, 2017
  21. Jeff KingApr 11, 2017
  22. 08/12 grep: make grep.patternType=[pcre|pcre1] a synonym for "perl"Ævar Arnfjörð Bjarmason, Apr 8, 2017
  23. Jeff KingApr 11, 2017
  24. 07/12 grep & rev-list doc: stop promising libpcre for --perl-regexpÆvar Arnfjörð Bjarmason, Apr 8, 2017
  25. 09/12 test-lib: rename the LIBPCRE prerequisite to PCREÆvar Arnfjörð Bjarmason, Apr 8, 2017
  26. Jeff KingApr 11, 2017
  27. 10/12 grep: change the internal PCRE macro names to be PCRE1Ævar Arnfjörð Bjarmason, Apr 8, 2017
  28. Jeff KingApr 11, 2017
  29. Ævar Arnfjörð BjarmasonApr 11, 2017
  30. Jeff KingApr 11, 2017
  31. 12/12 grep: add support for PCRE v2Ævar Arnfjörð Bjarmason, Apr 8, 2017
  32. Jeff KingApr 11, 2017
  33. 11/12 grep: change the internal PCRE code & header names to be PCRE1Ævar Arnfjörð Bjarmason, Apr 8, 2017
  34. Jeff KingApr 11, 2017
  35. Ævar Arnfjörð BjarmasonApr 11, 2017
  36. Jeff KingApr 11, 2017
  37. Ævar Arnfjörð BjarmasonApr 11, 2017
  38. Jeff KingApr 11, 2017
  39. Brandon WilliamsApr 11, 2017
  40. Ævar Arnfjörð BjarmasonApr 11, 2017
  41. Jeff KingApr 11, 2017
  42. Junio C HamanoApr 15, 2017
  43. Ævar Arnfjörð BjarmasonApr 15, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.