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

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

From
Jeff King <peff@peff.net>
Date
Jul 12, 2017, 18:27 UTC
Message-ID
<20170712182751.nzpp4v3vqyrj65sb@sigill.intra.peff.net>
In-Reply-To
<20170712180923.GE93855@aiede.mtv.corp.google.com>
On Wed, Jul 12, 2017 at 11:09:23AM -0700, Jonathan Nieder wrote:
Show 17 quoted lines
> > 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.

Show 7 quoted lines
> 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
Previous: Stefan BellerNext: Brandon Williams
Message 31 of 68 in “Convert grep to recurse in-process”
  1. 0/3 Convert grep to recurse in-processBrandon Williams, Jul 11, 2017
  2. 1/3 repo_read_index: don't discard the indexBrandon Williams, Jul 11, 2017
  3. Jonathan NiederJul 11, 2017
  4. Brandon WilliamsJul 12, 2017
  5. Stefan BellerJul 11, 2017
  6. Brandon WilliamsJul 12, 2017
  7. 2/3 setup: have the_repository use the_indexBrandon Williams, Jul 11, 2017
  8. Jonathan NiederJul 12, 2017
  9. Stefan BellerJul 12, 2017
  10. Brandon WilliamsJul 12, 2017
  11. Junio C HamanoJul 12, 2017
  12. Brandon WilliamsJul 12, 2017
  13. Junio C HamanoJul 12, 2017
  14. Jonathan NiederJul 12, 2017
  15. Junio C HamanoJul 12, 2017
  16. Junio C HamanoJul 18, 2017
  17. 3/3 grep: recurse in-process using 'struct repository'Brandon Williams, Jul 11, 2017
  18. Jacob KellerJul 11, 2017
  19. Brandon WilliamsJul 12, 2017
  20. Stefan BellerJul 12, 2017
  21. Brandon WilliamsJul 12, 2017
  22. Jonathan NiederJul 12, 2017
  23. Brandon WilliamsJul 12, 2017
  24. Jeff KingJul 12, 2017
  25. Brandon WilliamsJul 12, 2017
  26. Jeff KingJul 12, 2017
  27. Jonathan NiederJul 12, 2017
  28. Jeff KingJul 12, 2017
  29. Jonathan NiederJul 12, 2017
  30. Stefan BellerJul 12, 2017
  31. Jeff KingJul 12, 2017
  32. 0/3 Convert grep to recurse in-processBrandon Williams, Jul 14, 2017
  33. 1/3 repo_read_index: don't discard the indexBrandon Williams, Jul 14, 2017
  34. 2/3 repository: have the_repository use the_indexBrandon Williams, Jul 14, 2017
  35. 3/3 grep: recurse in-process using 'struct repository'Brandon Williams, Jul 14, 2017
  36. 00/10 Convert grep to recurse in-processBrandon Williams, Jul 18, 2017
  37. 01/10 repo_read_index: don't discard the indexBrandon Williams, Jul 18, 2017
  38. 02/10 repository: have the_repository use the_indexBrandon Williams, Jul 18, 2017
  39. 04/10 config: add config_from_gitmodulesBrandon Williams, Jul 18, 2017
  40. 03/10 cache.h: add GITMODULES_FILE macroBrandon Williams, Jul 18, 2017
  41. convert any hard coded .gitmodules file string to the MACROStefan Beller, Jul 31, 2017
  42. Jeff HostetlerAug 1, 2017
  43. Stefan BellerAug 1, 2017
  44. Junio C HamanoAug 1, 2017
  45. Brandon WilliamsAug 2, 2017
  46. Brandon WilliamsAug 2, 2017
  47. 08/10 submodule: check for unmerged .gitmodules outside of config parsingBrandon Williams, Jul 18, 2017
  48. 09/10 submodule: merge repo_read_gitmodules and gitmodules_configBrandon Williams, Jul 18, 2017
  49. 10/10 grep: recurse in-process using 'struct repository'Brandon Williams, Jul 18, 2017
  50. 05/10 submodule: remove submodule.fetchjobs from submodule-config parsingBrandon Williams, Jul 18, 2017
  51. 06/10 submodule: remove fetch.recursesubmodules from submodule-config parsingBrandon Williams, Jul 18, 2017
  52. 07/10 submodule: check for unstaged .gitmodules outside of config parsingBrandon Williams, Jul 18, 2017
  53. Stefan BellerJul 31, 2017
  54. Brandon WilliamsAug 2, 2017
  55. Brandon WilliamsAug 2, 2017
  56. Junio C HamanoJul 18, 2017
  57. Brandon WilliamsJul 18, 2017
  58. 00/10 Convert grep to recurse in-processBrandon Williams, Aug 2, 2017
  59. 01/10 repo_read_index: don't discard the indexBrandon Williams, Aug 2, 2017
  60. 02/10 repository: have the_repository use the_indexBrandon Williams, Aug 2, 2017
  61. 04/10 config: add config_from_gitmodulesBrandon Williams, Aug 2, 2017
  62. 06/10 submodule: remove fetch.recursesubmodules from submodule-config parsingBrandon Williams, Aug 2, 2017
  63. 03/10 cache.h: add GITMODULES_FILE macroBrandon Williams, Aug 2, 2017
  64. 08/10 submodule: check for unmerged .gitmodules outside of config parsingBrandon Williams, Aug 2, 2017
  65. 09/10 submodule: merge repo_read_gitmodules and gitmodules_configBrandon Williams, Aug 2, 2017
  66. 10/10 grep: recurse in-process using 'struct repository'Brandon Williams, Aug 2, 2017
  67. 05/10 submodule: remove submodule.fetchjobs from submodule-config parsingBrandon Williams, Aug 2, 2017
  68. 07/10 submodule: check for unstaged .gitmodules outside of config parsingBrandon Williams, Aug 2, 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.