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

From: Jeff King <peff@peff.net>
Date: Apr 11, 2017, 12:57 UTC
Message-ID: <20170411125741.7b7hlmuucid37b3r@sigill.intra.peff.net>
In-Reply-To: <CACBZZX6A8tcXy-hCcQqqDsGNDtXWmZMdTY=RxeAOn8QFYDypyg@mail.gmail.com>

On Tue, Apr 11, 2017 at 01:02:56PM +0200, Ævar Arnfjörð Bjarmason wrote:

Show 11 quoted lines

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

-Peff