On Tue, Aug 27, 2019 at 8:46 PM Matheus Tavares Bernardino <matheus.bernardino@xxxxxx> wrote: > > Hi, Duy > > On Tue, Aug 27, 2019 at 6:26 AM Duy Nguyen <pclouds@xxxxxxxxx> wrote: > > > > On Tue, Aug 27, 2019 at 6:57 AM Matheus Tavares > > <matheus.bernardino@xxxxxx> wrote: > > > > > > Currently, config_with_options() relies on the global the_repository > > > when it has to configure from a blob. > > > > Not really reading the patch, but my last experience with moving > > config.c away from the_repo [1] shows that there are more hidden > > dependencies, in git_path() and particularly the git_config_clear() > > call in git_config_set_multivar_... Not really sure if those deps > > really affect your goals or not. Have a look at that branch, filtering > > on config.c for more info (and if you want to pick up some patches > > from that, you have my sign-off). > > Thanks for the advice. Indeed, I see now that do_git_config_sequence() > may call git_pathdup(), which relies on the_repo. For my use in patch > 2/2, repo_config_with_options() won't ever get to call > do_git_config_sequence(), so that's fine. But in other use cases it > may have to, so I'll need to check that. While working on this, I think I may have found a bug: The repo_read_config() function takes a repository R as parameter and calls this chain of functions: repo_read_config(struct repository *R) > config_with_options() > do_git_config_sequence() > git_pathdup("config.worktree") Shouldn't, however, the last call consider R instead of using the_repository? i.e., use repo_git_path(R, "config.worktree"), instead? If so, how could we get R there? I mean, we could pass it through this chain, but the chain already passes a "struct config_options", which carries the "commondir" and "git_dir" fields. So it would probably be confusing to have them and an extra repository parameter (which also has "commondir" and "git_dir"), right? Any ideas on how to better approach this? > > [1] https://gitlab.com/pclouds/git/commits/submodules-in-worktrees > > > > -- > > Duy