This is a feature that supports config file inclusion conditional on whether the repo has a remote with a URL that matches a glob. Similar to my previous work on remote-suggested hooks [1], the main motivation is to allow remote repo administrators to provide recommended configs in a way that can be consumed more easily (e.g. through a package installable by a package manager). NEEDSWORK: The way this works is that if we see such an include, we shunt all subsequent configs into a stash (while looking for URLs), then process the stash. In particular, this means that more memory is needed, and the nature of error reporting changes (currently, if a callback returns nonzero for a variable, processing halts immediately, but with this patch, all the config might be read from disk before the callback even sees the variable). I'll need to expand on this and write a documentation section. One alternative is to rerun the config parsing mechanism upon noticing the first URL-conditional include in order to find all URLs. This would require the config files to be read from disk twice, though. [1] https://lore.kernel.org/git/cover.1623881977.git.jonathantanmy@xxxxxxxxxx/ Signed-off-by: Jonathan Tan <jonathantanmy@xxxxxxxxxx> --- config.c | 132 +++++++++++++++++++++++++++++++++++++++++----- t/t1300-config.sh | 60 +++++++++++++++++++++ 2 files changed, 180 insertions(+), 12 deletions(-) diff --git a/config.c b/config.c index 94ad5ce913..63a37e0a5d 100644 --- a/config.c +++ b/config.c @@ -120,13 +120,30 @@ static long config_buf_ftell(struct config_source *conf) return conf->u.buf.pos; } +struct stashed_var { + char *var; + char *value; + int depth; + + char *url; +}; + struct config_include_data { int depth; config_fn_t fn; void *data; const struct config_options *opts; + + /* + * All remote URLs discovered when reading all config files. + */ + struct string_list remote_urls; + + struct stashed_var *stashed; + size_t stashed_nr, stashed_alloc; + int current_stash_depth; }; -#define CONFIG_INCLUDE_INIT { 0 } +#define CONFIG_INCLUDE_INIT { .remote_urls = STRING_LIST_INIT_DUP } static int git_config_include(const char *var, const char *value, void *data); @@ -316,28 +333,110 @@ static int include_condition_is_true(const struct config_options *opts, return 0; } +static int execute_stashed(struct config_include_data *inc) +{ + size_t i = 0; + while (i < inc->stashed_nr) { + int ret = inc->fn(inc->stashed[i].var, inc->stashed[i].value, + inc->data); + if (ret) + return ret; + + /* + * If it is an include, skip to next entry of the same depth if + * the URL doesn't match + */ + if (inc->stashed[i].url) { + struct strbuf pattern = STRBUF_INIT; + struct string_list_item *url_item; + int found = 0; + + strbuf_addstr(&pattern, inc->stashed[i].url); + add_trailing_starstar_for_dir(&pattern); + for_each_string_list_item(url_item, &inc->remote_urls) { + if (!wildmatch(pattern.buf, url_item->string, + WM_PATHNAME)) { + found = 1; + break; + } + } + strbuf_release(&pattern); + if (found) { + i++; + } else { + int depth = inc->stashed[i].depth; + + i++; + while (i < inc->stashed_nr && + inc->stashed[i].depth != depth) + i++; + } + } else { + i++; + } + } + return 0; +} + static int git_config_include(const char *var, const char *value, void *data) { struct config_include_data *inc = data; + const char *remote_name; + size_t remote_name_len; const char *cond, *key; size_t cond_len; - int ret; + int ret = 0; + + if (!parse_config_key(var, "remote", &remote_name, &remote_name_len, + &key) && + remote_name && + !strcmp(key, "url")) + string_list_append(&inc->remote_urls, value); /* * Pass along all values, including "include" directives; this makes it * possible to query information on the includes themselves. */ - ret = inc->fn(var, value, inc->data); - if (ret < 0) - return ret; + if (inc->stashed_nr || starts_with(var, "includeif.hasremoteurl:")) { + struct stashed_var *last; + + /* + * Start or continue using the stash. (A false positive on + * "includeif.hasremoteurl:?.path" is fine here - this just + * means that some config variables unnecessarily go through + * the stash before being passed to the callback.) + */ + ALLOC_GROW_BY(inc->stashed, inc->stashed_nr, 1, + inc->stashed_alloc); + last = &inc->stashed[inc->stashed_nr - 1]; + last->var = xstrdup(var); + last->value = xstrdup(value); + last->depth = inc->current_stash_depth; + } else { + ret = inc->fn(var, value, inc->data); + if (ret < 0) + return ret; + } if (!strcmp(var, "include.path")) ret = handle_path_include(value, inc); if (!parse_config_key(var, "includeif", &cond, &cond_len, &key) && - (cond && include_condition_is_true(inc->opts, cond, cond_len)) && - !strcmp(key, "path")) - ret = handle_path_include(value, inc); + cond && !strcmp(key, "path")) { + const char *url; + size_t url_len; + + if (skip_prefix_mem(cond, cond_len, "hasremoteurl:", &url, + &url_len)) { + inc->stashed[inc->stashed_nr - 1].url = + xmemdupz(url, url_len); + inc->current_stash_depth++; + ret = handle_path_include(value, inc); + inc->current_stash_depth--; + } else if (include_condition_is_true(inc->opts, cond, cond_len)) { + ret = handle_path_include(value, inc); + } + } return ret; } @@ -1933,6 +2032,7 @@ int config_with_options(config_fn_t fn, void *data, const struct config_options *opts) { struct config_include_data inc = CONFIG_INCLUDE_INIT; + int ret; if (opts->respect_includes) { inc.fn = fn; @@ -1950,17 +2050,25 @@ int config_with_options(config_fn_t fn, void *data, * regular lookup sequence. */ if (config_source && config_source->use_stdin) { - return git_config_from_stdin(fn, data); + ret = git_config_from_stdin(fn, data); } else if (config_source && config_source->file) { - return git_config_from_file(fn, config_source->file, data); + ret = git_config_from_file(fn, config_source->file, data); } else if (config_source && config_source->blob) { struct repository *repo = config_source->repo ? config_source->repo : the_repository; - return git_config_from_blob_ref(fn, repo, config_source->blob, + ret = git_config_from_blob_ref(fn, repo, config_source->blob, data); + } else { + ret = do_git_config_sequence(opts, fn, data); } - return do_git_config_sequence(opts, fn, data); + if (inc.stashed_nr) { + execute_stashed(&inc); + inc.stashed_nr = 0; + } + + string_list_clear(&inc.remote_urls, 0); + return ret; } static void configset_iter(struct config_set *cs, config_fn_t fn, void *data) diff --git a/t/t1300-config.sh b/t/t1300-config.sh index 9ff46f3b04..ea15f7fd46 100755 --- a/t/t1300-config.sh +++ b/t/t1300-config.sh @@ -2387,4 +2387,64 @@ test_expect_success '--get and --get-all with --fixed-value' ' test_must_fail git config --file=config --get-regexp --fixed-value fixed+ non-existent ' +test_expect_success 'includeIf.hasremoteurl' ' + test_create_repo hasremoteurlTest && + + cat >"$(pwd)"/include-this <<-\EOF && + [user] + this = this-is-included + EOF + cat >"$(pwd)"/dont-include-that <<-\EOF && + [user] + that = that-is-not-included + EOF + cat >>hasremoteurlTest/.git/config <<-EOF && + [includeIf "hasremoteurl:foo"] + path = "$(pwd)/include-this" + [includeIf "hasremoteurl:bar"] + path = "$(pwd)/dont-include-that" + [remote "foo"] + url = foo + EOF + + echo this-is-included >expect-this && + git -C hasremoteurlTest config --get user.this >actual-this && + test_cmp expect-this actual-this && + + test_must_fail git -C hasremoteurlTest config --get user.that +' + +test_expect_success 'includeIf.hasremoteurl respects last-config-wins' ' + test_create_repo hasremoteurlTestOverride && + + cat >"$(pwd)"/include-two-three <<-\EOF && + [user] + two = included-config + three = included-config + EOF + cat >>hasremoteurlTestOverride/.git/config <<-EOF && + [remote "foo"] + url = foo + [user] + one = main-config + two = main-config + [includeIf "hasremoteurl:foo"] + path = "$(pwd)/include-two-three" + [user] + three = main-config + EOF + + echo main-config >expect-main-config && + echo included-config >expect-included-config && + + git -C hasremoteurlTestOverride config --get user.one >actual && + test_cmp expect-main-config actual && + + git -C hasremoteurlTestOverride config --get user.two >actual && + test_cmp expect-included-config actual && + + git -C hasremoteurlTestOverride config --get user.three >actual && + test_cmp expect-main-config actual +' + test_done -- 2.33.1.1089.g2158813163f-goog
