Re: [RFC PATCH v2 2/4] config: load the correct config.worktree file
- From
Matheus Tavares Bernardino <matheus.bernardino@usp.br>
- Date
- May 12, 2020, 22:55 UTC
- Message-ID
- <CAHd-oW4J4LVo3HPj-8WnRAOXOhxaLS8v3_SW2kOTO+SFuPas=A@mail.gmail.com>
- In-Reply-To
- <xmqqlflyqocl.fsf@gitster.c.googlers.com>
On Mon, May 11, 2020 at 4:10 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 14 quoted lines
> > Matheus Tavares <matheus.bernardino@usp.br> writes: > > > One of the steps in do_git_config_sequence() is to load the > > worktree-specific config file. Although the function receives a git_dir > > string, it relies on git_pathdup(), which uses the_repository->git_dir, > > to make the path to the file. Thus, when a submodule has a worktree > > setting, a command executed in the superproject that recurses into the > > submodule won't find the said setting. > > This has far wider ramifications than just "git grep" and it may be > an important fix. Anything that wants to read from a per-worktree > configuration is not working as expected when run from a secondary > worktree, right?
Hmm, I think the code should be able to retrieve the per-worktree configuration, in this case, as the_repository->gitdir will be pointing to the secondary worktree's gitdir. But when we want to read a per-worktree configuration from a repo other than the_repository, then the code doesn't find the setting (even if it is in the main worktree of the subrepo).
> Can we add a test or two to protect this fix from future breakages?
Sure! There are already a couple tests, in the following patch, that check this behavior *indirectly*. As we recurse into submodules, in grep, we try to retrieve the core.sparseCheckout setting for each submodule (which is stored in the subrepo's config.worktree file). The said tests make sure we can get this setting, and they indeed fail without this patch. But would it be better to also add a more direct test, in this patch? I think we could do so by adding a new test helper that prints submodules' configs, from the superproject, and then testing the presence of per-worktree configs in the output.