On Thu, Aug 29, 2019 at 11:44 PM Matheus Tavares Bernardino <matheus.bernardino@xxxxxx> wrote: > > I'm sure there are other gotchas in the config code, though, related to > > things for which we _do_ need a repository. E.g., include_by_branch() > > looks at the_repository, and should use a repository struct matching the > > git_dir we're looking at (though it may be acceptable to bail during > > early pre-repo-initialization config and just disallow branch includes, > > which is what happens now). > > I think config_with_options() is another example of a place where we > should have a reference to a repo (but we currently don't). When > configuring from a given blob, it will call > git_config_from_blob_ref(), which calls get_oid() and > read_object_file(). Both of these functions will use the_repo by > default. But the git_dir and commondir fields passed to > config_with_options() through 'struct config_options' may not refer to > the_repo, right? > > I'm not sure what is the best solution to this, though. I mean, we > could add a 'struct repository' in 'struct config_options', but as you > already pointed out, some callers might not have a repository struct > yet... Early setup code has always been special (there's a lot of stuff you don't have access too). Ideally we could have a lower level API that takes git_dir and git_commondir only, no access to 'struct repository'. This is used for early access. And we have a higher level API that only takes struct repo and pass repo->gitdir down to the that lowlevel one. But I guess that's not the reality we're in. Since early setup code is special, perhaps you could make 'struct config_options' take both git_dir and struct repo, but not both at the same time. Early setup code sets repo to NULL and git_dir something else. Other code always leave git_dir and git_common_dir to NULL, documented to say those are for early setup only. PS. Again still not looking at the code so I may just be talking rubbish here. -- Duy
