Re: [PATCH 0/3] Convert grep to recurse in-process

On Wed, Jul 12, 2017 at 11:09:23AM -0700, Jonathan Nieder wrote:

quoted

I didn't follow the rest of the "struct repository" series closely, but
I don't feel like we ever reached a resolution on how config would be
handled. I notice that the in-process "ls-files" behaves differently
than the old one when config differs between the submodule and the
parent repository. As we convert more commands (that use more config)
this will become more likely to be noticed by somebody.

Do we have a plan for dealing with this? Is our solution just "recursed
operations always respect the parent config, deal with it"?

For settings like branch.<name>.remote, I don't think anyone would
disagree that the right thing to do is to use the per-repository
config of the submodule.  The repository object is already able to
handle per-repository config, so this just involves callers being
careful not to cache values locally in a way that conflates
repositories.  It should be pretty straightforward (for commands like
"git fetch --recurse-submodules", for example).

I agree that's the right approach. What I'm worried about is that I see
in-process work proceeding without the "callers being careful" part
being audited, which can lead to regressions (e.g., ls-files with
core.quotepath is "broken" in next right now). Though at least the
regression would be limited to people using submodules.

For settings like grep.patternType, on the other hand, it would be
very strange for the behavior to change when grep crosses the
submodule boundary.  So I think using the parent project config is the
right thing to do and the old behavior was simply wrong.  In other
words, I don't think this is so much a case of "deal with it" as
"sorry we got the behavior so wrong before --- we've finally fixed it
now".

I think that's not actually about the parent project's config, as much
as it is about the parameters of the current operation. I.e., the
argument is that this particular grep operation is using a particular
pattern type, no matter how we arrived at that decision, and it should
be used for all of the recursive bits. So whether the superproject has
its own grep.patternType set, or whether the user used "-P" on the
command line, the result is the same: we need to tell the submodule to
ignore any config and use the parameters we feed it.

In a multi-process model, that should happen by converting all of the
bits in "struct grep_opt" back into command-line parameters and feeding
them to the recursive processes (which would then give them precedence
over any config). But I'm pretty sure we don't do that.

In the in-process model, that would hopefully be a bit simpler, as we'd
just pass in a pre-made grep_opt.

-Peff