From: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>,
bert.wesarg@googlemail.com,
"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: [PATCH v2 3/3] config: correct file reading order in read_early_config()
Date: Mon, 17 Apr 2017 17:10:02 +0700 [thread overview]
Message-ID: <20170417101002.18272-3-pclouds@gmail.com> (raw)
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
next prev parent reply other threads:[~2017-04-17 10:10 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-14 17:04 includeIf breaks calling dashed externals Bert Wesarg
2017-04-14 17:43 ` Jeff King
2017-04-15 11:49 ` Duy Nguyen
2017-04-16 4:50 ` Jeff King
2017-04-16 10:41 ` [PATCH 1/2] config: prepare to pass more info in git_config_with_options() Nguyễn Thái Ngọc Duy
2017-04-16 10:41 ` [PATCH 2/2] config: handle conditional include when $GIT_DIR is not set up Nguyễn Thái Ngọc Duy
2017-04-16 15:51 ` Jeff King
2017-04-17 2:13 ` Duy Nguyen
2017-04-18 3:56 ` Jeff King
2017-04-17 10:07 ` Duy Nguyen
2017-04-17 10:10 ` [PATCH v2 1/3] config: prepare to pass more info in git_config_with_options() Nguyễn Thái Ngọc Duy
2017-04-17 10:10 ` [PATCH v2 2/3] config: handle conditional include when $GIT_DIR is not set up Nguyễn Thái Ngọc Duy
2017-04-18 2:49 ` Junio C Hamano
2017-04-18 2:56 ` Junio C Hamano
2017-04-18 3:46 ` Jeff King
2017-04-17 10:10 ` Nguyễn Thái Ngọc Duy [this message]
2017-04-18 3:53 ` [PATCH v2 3/3] config: correct file reading order in read_early_config() Jeff King
2017-04-18 2:27 ` [PATCH v2 1/3] config: prepare to pass more info in git_config_with_options() Junio C Hamano
2017-04-18 3:55 ` Jeff King
2017-04-18 4:51 ` Junio C Hamano
2017-04-18 3:17 ` [PATCH 2/2] config: handle conditional include when $GIT_DIR is not set up Junio C Hamano
2017-04-18 4:03 ` Jeff King
2017-04-16 15:31 ` [PATCH 1/2] config: prepare to pass more info in git_config_with_options() Jeff King
2017-04-17 1:42 ` Duy Nguyen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170417101002.18272-3-pclouds@gmail.com \
--to=pclouds@gmail.com \
--cc=bert.wesarg@googlemail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).