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>

diff --git a/config.c b/config.c
index 14f041746027ee18ac2959434e26f8501e3582da..fffda653e359379b5df26aaf847b026700a88d82 100644 (file)
--- a/config.c
+++ b/config.c
@@ -1504,12 +1504,20 @@ int git_config_system(void)
        return !git_env_bool("GIT_CONFIG_NOSYSTEM", 0);
 }
 
        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");
 {
        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))
 
        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);
 
        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)
 }
 
 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;
 {
        struct config_options opts = {0};
        struct strbuf buf = STRBUF_INIT;

-       char *to_free = NULL;

 
        opts.respect_includes = 1;
 
 
        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))
         * 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);
 
 
        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);
        strbuf_release(&buf);

-       free(to_free);

 }
 
 static void git_config_check_init(void);
 }
 
 static void git_config_check_init(void);

diff --git a/t/t1309-early-config.sh b/t/t1309-early-config.sh
index b97357b8ab3aa47aff100e4012083891b57d9b46..1af8c454cfa715d5694c16b4848428ae0b75eda6 100755 (executable)
--- a/t/t1309-early-config.sh
+++ b/t/t1309-early-config.sh
@@ -47,6 +47,24 @@ test_expect_success 'ceiling #2' '
        test xdg = "$(cat output)"
 '
 
        test xdg = "$(cat output)"
 '
 

+cmdline_config="'test.source=cmdline'"
+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 &&
+               GIT_CONFIG_PARAMETERS=$cmdline_config 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 &&
 test_with_config () {
        rm -rf throwaway &&
        git init throwaway &&