@matheusbsbs / matheus-bagliano / commits / e145a0bc9b

config: correct file reading order in read_early_config()

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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Nguyễn Thái Ngọc Duy committed Apr 17, 2017 at 17:10 UTC e145a0bc9b8711fe1c6cfad29af52ef06ce4c1ec
2 files changed +30 -14
config.c
+12 -14
@@ -1504,12 +1504,20 @@ int git_config_system(void)
1504 return !git_env_bool("GIT_CONFIG_NOSYSTEM", 0);
1505 }
1506
1507 -static int do_git_config_sequence(config_fn_t fn, void *data)
1507 +static int do_git_config_sequence(const struct config_options *opts,
1508 + config_fn_t fn, void *data)
1509 {
1510 int ret = 0;
1511 char *xdg_config = xdg_config_home("config");
1512 char *user_config = expand_user_path("~/.gitconfig");
1512 - char *repo_config = have_git_dir() ? git_pathdup("config") : NULL;
1513 + char *repo_config;
1514 +
1515 + if (opts->git_dir)
1516 + repo_config = mkpathdup("%s/config", opts->git_dir);
1517 + else if (have_git_dir())
1518 + repo_config = git_pathdup("config");
1519 + else
1520 + repo_config = NULL;
1521
1522 current_parsing_scope = CONFIG_SCOPE_SYSTEM;
1523 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,
1571 else if (config_source && config_source->blob)
1572 return git_config_from_blob_ref(fn, config_source->blob, data);
1573
1566 - return do_git_config_sequence(fn, data);
1574 + return do_git_config_sequence(opts, fn, data);
1575 }
1576
1577 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)
1621 {
1622 struct config_options opts = {0};
1623 struct strbuf buf = STRBUF_INIT;
1616 - char *to_free = NULL;
1624
1625 opts.respect_includes = 1;
1626
@@ -1628,20 +1635,11 @@ void read_early_config(config_fn_t cb, void *data)
1635 * call).
1636 */
1637 else if (discover_git_directory(&buf))
1631 - opts.git_dir = to_free = xstrdup(buf.buf);
1638 + opts.git_dir = buf.buf;
1639
1640 git_config_with_options(cb, data, NULL, &opts);
1641
1635 - if (!have_git_dir() && opts.git_dir) {
1636 - struct git_config_source repo_config;
1637 -
1638 - memset(&repo_config, 0, sizeof(repo_config));
1639 - strbuf_addstr(&buf, "/config");
1640 - repo_config.file = buf.buf;
1641 - git_config_with_options(cb, data, &repo_config, &opts);
1642 - }
1642 strbuf_release(&buf);
1644 - free(to_free);
1643 }
1644
1645 static void git_config_check_init(void);
t/t1309-early-config.sh
+18
@@ -47,6 +47,24 @@ test_expect_success 'ceiling #2' '
47 test xdg = "$(cat output)"
48 '
49
50 +cmdline_config="'test.source=cmdline'"
51 +test_expect_success 'read config file in right order' '
52 + echo "[test]source = home" >>.gitconfig &&
53 + git init foo &&
54 + (
55 + cd foo &&
56 + echo "[test]source = repo" >>.git/config &&
57 + GIT_CONFIG_PARAMETERS=$cmdline_config test-config \
58 + read_early_config test.source >actual &&
59 + cat >expected <<-\EOF &&
60 + home
61 + repo
62 + cmdline
63 + EOF
64 + test_cmp expected actual
65 + )
66 +'
67 +
68 test_with_config () {
69 rm -rf throwaway &&
70 git init throwaway &&