Phillip Wood <phillip.wood123@xxxxxxxxx> writes: > Thanks for working on this, I like the idea but looking at > read_very_early_config() it sets "opts.ignore_cmdline = 1" which means > that this will ignore any config options passed with "git -c > key=value". I think it would be better to call config_with_options() > with the appropriate options directly. hmph. > For this to work all the commands that run outside a repository would > have to read the config via repo_config(), and take care not to call > any of the repo_config_get_*() functions. They mostly seem to do that > but "git for-each-repo" calls repo_config_get_string_multi() - it > should be easy enough to convert that to a callback when that command > is updated to stop using "the_repository" Well the only reason why we want to allow NULL in repo_config() calls is to accomodate this pattern, if I am reading the discussion correctly. * A command that wants to run in a repository (i.e. marked with RUN_SETUP, not RUN_SETUP_GENTLY) is invoked with "-h" and outside the repository. * cmd_foo() is called with "-h" option in argv[]; repo is set to NULL. * The typical start-up sequence for any builtin cmd_foo() is to read the configuration and then call parseopt, and the latter is how "git foo -h" is handled. * Historically, the "call the configuration" was done by making a call to git_config(), which used the_repository [*]. But recent trend is to use the repo passed down to cmd_foo() instead, and replacing git_config() with repo_config() blindly would break unless repo_config() is prepared to take NULL. So, any command that requires to be in a repository would not go beyond its call to parse_options() when called with repo==NULL. Either it would have already died inside getup_git_directory() when "-h" is not given, or parse_options() would have gave the usage string and exited. For commands like "for-each-repo" that itself wants to be able to run outside a repository by marking itself as RUN_SETUP_GENTLY, they do have to update the code after the parse_options() themselves, of course, but I view it as a separate issue from this "we make git_config() as the first thing in everything---we want to replace it with repo_config()" patch. Thanks. [Footnote] * I personally think that it is an unhealthy fundamentalism to try eradicating the use of the_repository even from the top-level calls of cmd_foo() functions, which are like traditional main(). They are never meant to be reused as a subroutine that can work on an arbitrary repository from any arbitrary codepaths. The special repository instance, the_repository, is set up to be suitable to work in the current repository, if exists, and gives a reasonable fallback behaviour when we are not inside a repository, which behaves exactly how we want it to behave there. The more library-ish helper functions (I am talking about distinction between what is in say builtin/diff.c and diff-lib.c here and referring to the latter) are totally different matter, of course. They are meant to be reused and they should be made to work on an arbitrary repository from arbitrary codepaths, hence reducing the reliance on the_repository is a good goal for them to aim at.
