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

[Brandon Williams <bmwill@google.com>]

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?

-- 
Brandon Williams