[PATCH v2 3/3] config: correct file reading order in read_early_config()
- From
Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
- Date
- Apr 17, 2017, 10:10 UTC
- Message-ID
- <20170417101002.18272-3-pclouds@gmail.com>
- In-Reply-To
- <20170417101002.18272-1-pclouds@gmail.com>
Config file reading order is important because each file can override values in the previous files and this is expected behavior. Normally we read in this order, all in do_git_config_sequence():
1. $HOME/.gitconfig 2. $GIT_DIR/config 3. config from command line
However in read_early_config() the order may be swapped a bit if setup_git_directory() has not been called:
1. $HOME/.gitconfig 2. $GIT_DIR/config is NOT read because .git dir is not found _yet_ 3. config from command line 4. $GIT_DIR/config is now READ (after discover_git_directory() call)
The reading at step 4 could override config at step 3, which is not the expectation.
Now that we could pass the .git dir around, we could feed discover_git_directory() back to step 2, so that it works again, and remove step 4.
Noticed-by: Jeff King <peff@peff.net> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- config.c | 26 ++++++++++++-------------- t/helper/test-config.c | 4 ++++ t/t1309-early-config.sh | 17 +++++++++++++++++ 3 files changed, 33 insertions(+), 14 deletions(-)
diff --git a/config.c b/config.c index 14f0417460..fffda653e3 100644 --- a/config.c +++ b/config.c @@ -1504,12 +1504,20 @@ int git_config_system(void) return !git_env_bool("GIT_CONFIG_NOSYSTEM", 0); } -static int do_git_config_sequence(config_fn_t fn, void *data) +static int do_git_config_sequence(const struct config_options *opts, + config_fn_t fn, void *data) { int ret = 0; char *xdg_config = xdg_config_home("config"); char *user_config = expand_user_path("~/.gitconfig"); - char *repo_config = have_git_dir() ? git_pathdup("config") : NULL; + char *repo_config; + + if (opts->git_dir) + repo_config = mkpathdup("%s/config", opts->git_dir); + else if (have_git_dir()) + repo_config = git_pathdup("config"); + else + repo_config = NULL; current_parsing_scope = CONFIG_SCOPE_SYSTEM; if (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK, 0)) @@ -1563,7 +1571,7 @@ int git_config_with_options(config_fn_t fn, void *data, else if (config_source && config_source->blob) return git_config_from_blob_ref(fn, config_source->blob, data); - return do_git_config_sequence(fn, data); + return do_git_config_sequence(opts, fn, data); } static void git_config_raw(config_fn_t fn, void *data) @@ -1613,7 +1621,6 @@ void read_early_config(config_fn_t cb, void *data) { struct config_options opts = {0}; struct strbuf buf = STRBUF_INIT; - char *to_free = NULL; opts.respect_includes = 1; @@ -1628,20 +1635,11 @@ void read_early_config(config_fn_t cb, void *data) * call). */ else if (discover_git_directory(&buf)) - opts.git_dir = to_free = xstrdup(buf.buf); + opts.git_dir = buf.buf; git_config_with_options(cb, data, NULL, &opts); - if (!have_git_dir() && opts.git_dir) { - struct git_config_source repo_config; - - memset(&repo_config, 0, sizeof(repo_config)); - strbuf_addstr(&buf, "/config"); - repo_config.file = buf.buf; - git_config_with_options(cb, data, &repo_config, &opts); - } strbuf_release(&buf); - free(to_free); } static void git_config_check_init(void); diff --git a/t/helper/test-config.c b/t/helper/test-config.c index 8e3ed6a76c..696d0a52fd 100644 --- a/t/helper/test-config.c +++ b/t/helper/test-config.c @@ -84,6 +84,10 @@ int cmd_main(int argc, const char **argv) struct config_set cs; if (argc == 3 && !strcmp(argv[1], "read_early_config")) { + const char *cmdline_config = getenv("CMDL_CFG"); + + if (cmdline_config) + git_config_push_parameter(cmdline_config); read_early_config(early_config_cb, (void *)argv[2]); return 0; } diff --git a/t/t1309-early-config.sh b/t/t1309-early-config.sh index b97357b8ab..c15f18dd96 100755 --- a/t/t1309-early-config.sh +++ b/t/t1309-early-config.sh @@ -47,6 +47,23 @@ test_expect_success 'ceiling #2' ' test xdg = "$(cat output)" ' +test_expect_success 'read config file in right order' ' + echo "[test]source = home" >>.gitconfig && + git init foo && + ( + cd foo && + echo "[test]source = repo" >>.git/config && + CMDL_CFG=test.source=cmdline test-config \ + read_early_config test.source >actual && + cat >expected <<-\EOF && + home + repo + cmdline + EOF + test_cmp expected actual + ) +' + test_with_config () { rm -rf throwaway && git init throwaway &&
-- 2.11.0.157.gd943d85