From c48f4b379eb3a0d093f5a87cce1d2ed6c175104f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Mon, 17 Apr 2017 17:10:00 +0700 Subject: config: prepare to pass more info in git_config_with_options() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit So far we can only pass one flag, respect_includes, to thie function. We need to pass some more (non-flag even), so let's make it accept a struct instead of an integer. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- builtin/config.c | 21 ++++++++++++--------- cache.h | 7 ++++++- config.c | 16 +++++++++++----- 3 files changed, 29 insertions(+), 15 deletions(-) diff --git a/builtin/config.c b/builtin/config.c index 4f49a0edb9..b937d175a9 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -26,7 +26,8 @@ static int use_global_config, use_system_config, use_local_config; static struct git_config_source given_config_source; static int actions, types; static int end_null; -static int respect_includes = -1; +static int respect_includes_opt = -1; +static struct config_options config_options; static int show_origin; #define ACTION_GET (1<<0) @@ -81,7 +82,7 @@ static struct option builtin_config_options[] = { OPT_GROUP(N_("Other")), OPT_BOOL('z', "null", &end_null, N_("terminate values with NUL byte")), OPT_BOOL(0, "name-only", &omit_values, N_("show variable names only")), - OPT_BOOL(0, "includes", &respect_includes, N_("respect include directives on lookup")), + OPT_BOOL(0, "includes", &respect_includes_opt, N_("respect include directives on lookup")), OPT_BOOL(0, "show-origin", &show_origin, N_("show origin of config (file, standard input, blob, command line)")), OPT_END(), }; @@ -242,7 +243,7 @@ static int get_value(const char *key_, const char *regex_) } git_config_with_options(collect_config, &values, - &given_config_source, respect_includes); + &given_config_source, &config_options); ret = !values.nr; @@ -320,7 +321,7 @@ static void get_color(const char *var, const char *def_color) get_color_found = 0; parsed_color[0] = '\0'; git_config_with_options(git_get_color_config, NULL, - &given_config_source, respect_includes); + &given_config_source, &config_options); if (!get_color_found && def_color) { if (color_parse(def_color, parsed_color) < 0) @@ -352,7 +353,7 @@ static int get_colorbool(const char *var, int print) get_diff_color_found = -1; get_color_ui_found = -1; git_config_with_options(git_get_colorbool_config, NULL, - &given_config_source, respect_includes); + &given_config_source, &config_options); if (get_colorbool_found < 0) { if (!strcmp(get_colorbool_slot, "color.diff")) @@ -441,7 +442,7 @@ static int get_urlmatch(const char *var, const char *url) } git_config_with_options(urlmatch_config_entry, &config, - &given_config_source, respect_includes); + &given_config_source, &config_options); ret = !values.nr; @@ -530,8 +531,10 @@ int cmd_config(int argc, const char **argv, const char *prefix) prefix_filename(prefix, given_config_source.file); } - if (respect_includes == -1) - respect_includes = !given_config_source.file; + if (respect_includes_opt == -1) + config_options.respect_includes = !given_config_source.file; + else + config_options.respect_includes = respect_includes_opt; if (end_null) { term = '\0'; @@ -578,7 +581,7 @@ int cmd_config(int argc, const char **argv, const char *prefix) check_argc(argc, 0, 0); if (git_config_with_options(show_all_config, NULL, &given_config_source, - respect_includes) < 0) { + &config_options) < 0) { if (given_config_source.file) die_errno("unable to read config file '%s'", given_config_source.file); diff --git a/cache.h b/cache.h index 556468c25b..a6294d2573 100644 --- a/cache.h +++ b/cache.h @@ -1885,6 +1885,10 @@ enum config_origin_type { CONFIG_ORIGIN_CMDLINE }; +struct config_options { + unsigned int respect_includes : 1; +}; + typedef int (*config_fn_t)(const char *, const char *, void *); extern int git_default_config(const char *, const char *, void *); extern int git_config_from_file(config_fn_t fn, const char *, void *); @@ -1898,7 +1902,7 @@ extern void read_early_config(config_fn_t cb, void *data); extern void git_config(config_fn_t fn, void *); extern int git_config_with_options(config_fn_t fn, void *, struct git_config_source *config_source, - int respect_includes); + const struct config_options *opts); extern int git_parse_ulong(const char *, unsigned long *); extern int git_parse_maybe_bool(const char *); extern int git_config_int(const char *, const char *); @@ -1950,6 +1954,7 @@ struct config_include_data { int depth; config_fn_t fn; void *data; + const struct config_options *opts; }; #define CONFIG_INCLUDE_INIT { 0 } extern int git_config_include(const char *name, const char *value, void *data); diff --git a/config.c b/config.c index 1a4d85537b..042321a3a0 100644 --- a/config.c +++ b/config.c @@ -1530,13 +1530,14 @@ static int do_git_config_sequence(config_fn_t fn, void *data) int git_config_with_options(config_fn_t fn, void *data, struct git_config_source *config_source, - int respect_includes) + const struct config_options *opts) { struct config_include_data inc = CONFIG_INCLUDE_INIT; - if (respect_includes) { + if (opts->respect_includes) { inc.fn = fn; inc.data = data; + inc.opts = opts; fn = git_config_include; data = &inc; } @@ -1557,7 +1558,10 @@ int git_config_with_options(config_fn_t fn, void *data, static void git_config_raw(config_fn_t fn, void *data) { - if (git_config_with_options(fn, data, NULL, 1) < 0) + struct config_options opts = {0}; + + opts.respect_includes = 1; + if (git_config_with_options(fn, data, NULL, &opts) < 0) /* * git_config_with_options() normally returns only * zero, as most errors are fatal, and @@ -1597,9 +1601,11 @@ static void configset_iter(struct config_set *cs, config_fn_t fn, void *data) void read_early_config(config_fn_t cb, void *data) { + struct config_options opts = {0}; struct strbuf buf = STRBUF_INIT; - git_config_with_options(cb, data, NULL, 1); + opts.respect_includes = 1; + git_config_with_options(cb, data, NULL, &opts); /* * When setup_git_directory() was not yet asked to discover the @@ -1615,7 +1621,7 @@ void read_early_config(config_fn_t cb, void *data) memset(&repo_config, 0, sizeof(repo_config)); strbuf_addstr(&buf, "/config"); repo_config.file = buf.buf; - git_config_with_options(cb, data, &repo_config, 1); + git_config_with_options(cb, data, &repo_config, &opts); } strbuf_release(&buf); } -- cgit v1.2.1 From 2185fde56328942d5be9603cc199ee7c6d004085 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Mon, 17 Apr 2017 17:10:01 +0700 Subject: config: handle conditional include when $GIT_DIR is not set up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If setup_git_directory() and friends have not been called, get_git_dir() (because of includeIf.gitdir:XXX) would lead to die("BUG: setup_git_env called without repository"); There are two cases when a config file could be read before $GIT_DIR is located. The first one is check_repository_format(), where we read just the one file $GIT_DIR/config to check if we could understand this repository. This case should be safe. We do not parse include directives, which can only be triggered from git_config_with_options, but setup code uses a lower-level function. The concerned variables should never be hidden away behind includes anyway. The second one is triggered in check_pager_config() when we're about to run an external git command. We might be able to find $GIT_DIR in this case, which is exactly what read_early_config() does (and also is what check_pager_config() uses). Conditional includes and get_git_dir() could be triggered by the first git_config_with_options() call there, before discover_git_directory() is used as a fallback $GIT_DIR detection. Detect this special "early reading" case, pass down the $GIT_DIR, either from previous setup or detected by discover_git_directory(), and make conditional include use it. Noticed-by: Bert Wesarg Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- cache.h | 1 + config.c | 34 ++++++++++++++++++++++++++-------- t/t1305-config-include.sh | 11 +++++++++++ 3 files changed, 38 insertions(+), 8 deletions(-) diff --git a/cache.h b/cache.h index a6294d2573..878e1d441f 100644 --- a/cache.h +++ b/cache.h @@ -1887,6 +1887,7 @@ enum config_origin_type { struct config_options { unsigned int respect_includes : 1; + const char *git_dir; }; typedef int (*config_fn_t)(const char *, const char *, void *); diff --git a/config.c b/config.c index 042321a3a0..14f0417460 100644 --- a/config.c +++ b/config.c @@ -207,13 +207,22 @@ static int prepare_include_condition_pattern(struct strbuf *pat) return prefix; } -static int include_by_gitdir(const char *cond, size_t cond_len, int icase) +static int include_by_gitdir(const struct config_options *opts, + const char *cond, size_t cond_len, int icase) { struct strbuf text = STRBUF_INIT; struct strbuf pattern = STRBUF_INIT; int ret = 0, prefix; + const char *git_dir; - strbuf_add_absolute_path(&text, get_git_dir()); + if (opts->git_dir) + git_dir = opts->git_dir; + else if (have_git_dir()) + git_dir = get_git_dir(); + else + goto done; + + strbuf_add_absolute_path(&text, git_dir); strbuf_add(&pattern, cond, cond_len); prefix = prepare_include_condition_pattern(&pattern); @@ -242,13 +251,14 @@ done: return ret; } -static int include_condition_is_true(const char *cond, size_t cond_len) +static int include_condition_is_true(const struct config_options *opts, + const char *cond, size_t cond_len) { if (skip_prefix_mem(cond, cond_len, "gitdir:", &cond, &cond_len)) - return include_by_gitdir(cond, cond_len, 0); + return include_by_gitdir(opts, cond, cond_len, 0); else if (skip_prefix_mem(cond, cond_len, "gitdir/i:", &cond, &cond_len)) - return include_by_gitdir(cond, cond_len, 1); + return include_by_gitdir(opts, cond, cond_len, 1); /* unknown conditionals are always false */ return 0; @@ -273,7 +283,7 @@ int git_config_include(const char *var, const char *value, void *data) ret = handle_path_include(value, inc); if (!parse_config_key(var, "includeif", &cond, &cond_len, &key) && - (cond && include_condition_is_true(cond, cond_len)) && + (cond && include_condition_is_true(inc->opts, cond, cond_len)) && !strcmp(key, "path")) ret = handle_path_include(value, inc); @@ -1603,10 +1613,12 @@ 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; - git_config_with_options(cb, data, NULL, &opts); + if (have_git_dir()) + opts.git_dir = get_git_dir(); /* * When setup_git_directory() was not yet asked to discover the * GIT_DIR, we ask discover_git_directory() to figure out whether there @@ -1615,7 +1627,12 @@ void read_early_config(config_fn_t cb, void *data) * notably, the current working directory is still the same after the * call). */ - if (!have_git_dir() && discover_git_directory(&buf)) { + else if (discover_git_directory(&buf)) + opts.git_dir = to_free = xstrdup(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)); @@ -1624,6 +1641,7 @@ void read_early_config(config_fn_t cb, void *data) 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/t1305-config-include.sh b/t/t1305-config-include.sh index e833939320..fddb47bafa 100755 --- a/t/t1305-config-include.sh +++ b/t/t1305-config-include.sh @@ -208,6 +208,17 @@ test_expect_success 'conditional include, both unanchored, icase' ' ) ' +test_expect_success 'conditional include, early config reading' ' + ( + cd foo && + echo "[includeIf \"gitdir:foo/\"]path=bar6" >>.git/config && + echo "[test]six=6" >.git/bar6 && + echo 6 >expect && + test-config read_early_config test.six >actual && + test_cmp expect actual + ) +' + test_expect_success 'include cycles are detected' ' cat >.gitconfig <<-\EOF && [test]value = gitconfig -- cgit v1.2.1 From e145a0bc9b8711fe1c6cfad29af52ef06ce4c1ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Mon, 17 Apr 2017 17:10:02 +0700 Subject: config: correct file reading order in read_early_config() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- config.c | 26 ++++++++++++-------------- t/t1309-early-config.sh | 18 ++++++++++++++++++ 2 files changed, 30 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/t1309-early-config.sh b/t/t1309-early-config.sh index b97357b8ab..1af8c454cf 100755 --- 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)" ' +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 && -- cgit v1.2.1