git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jonathan Tan <jonathantanmy@google.com>
To: git@vger.kernel.org
Cc: Jonathan Tan <jonathantanmy@google.com>,
	chooglen@google.com, gitster@pobox.com, avarab@gmail.com
Subject: [PATCH v4 2/2] config: include file if remote URL matches a glob
Date: Mon, 29 Nov 2021 12:23:42 -0800	[thread overview]
Message-ID: <3b3af0da983c65ac8282b14141495f6859cd575a.1638217387.git.jonathantanmy@google.com> (raw)
In-Reply-To: <cover.1638217387.git.jonathantanmy@google.com>

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 - it could, for example,
contain a file to be included conditionally and a post-install script
that adds the include directive to the system-wide config file).

In order to do this, Git reruns the config parsing mechanism upon
noticing the first URL-conditional include in order to find all remote
URLs, and these remote URLs are then used to determine if that first and
all subsequent includes are executed. Remote URLs are not allowed to be
configued in any URL-conditionally-included file.

[1] https://lore.kernel.org/git/cover.1623881977.git.jonathantanmy@google.com/

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
 Documentation/config.txt |  12 ++++
 config.c                 | 121 ++++++++++++++++++++++++++++++++++++---
 config.h                 |   7 +++
 t/t1300-config.sh        | 118 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 250 insertions(+), 8 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0c0e6b859f..bfc9e22d78 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -159,6 +159,18 @@ all branches that begin with `foo/`. This is useful if your branches are
 organized hierarchically and you would like to apply a configuration to
 all the branches in that hierarchy.
 
+`hasremoteurl`::
+	The data that follows the keyword `hasremoteurl:` is taken to
+	be a pattern with standard globbing wildcards and two
+	additional ones, `**/` and `/**`, that can match multiple
+	components. The first time this keyword is seen, the rest of
+	the config files will be scanned for remote URLs (without
+	applying any values). If there exists at least one remote URL
+	that matches this pattern, the include condition is met.
++
+Files included by this option (directly or indirectly) are not allowed
+to contain remote URLs.
+
 A few more notes on matching via `gitdir` and `gitdir/i`:
 
  * Symlinks in `$GIT_DIR` are not resolved before matching.
diff --git a/config.c b/config.c
index 94ad5ce913..4ffc1e87e9 100644
--- a/config.c
+++ b/config.c
@@ -125,6 +125,12 @@ struct config_include_data {
 	config_fn_t fn;
 	void *data;
 	const struct config_options *opts;
+	struct git_config_source *config_source;
+
+	/*
+	 * All remote URLs discovered when reading all config files.
+	 */
+	struct string_list *remote_urls;
 };
 #define CONFIG_INCLUDE_INIT { 0 }
 
@@ -316,12 +322,83 @@ static int include_condition_is_true(const struct config_options *opts,
 	return 0;
 }
 
+static int add_remote_url(const char *var, const char *value, void *data)
+{
+	struct string_list *remote_urls = data;
+	const char *remote_name;
+	size_t remote_name_len;
+	const char *key;
+
+	if (!parse_config_key(var, "remote", &remote_name, &remote_name_len,
+			      &key) &&
+	    remote_name &&
+	    !strcmp(key, "url"))
+		string_list_append(remote_urls, value);
+	return 0;
+}
+
+static void populate_remote_urls(struct config_include_data *inc)
+{
+	struct config_options opts;
+
+	struct config_source *store_cf = cf;
+	struct key_value_info *store_kvi = current_config_kvi;
+	enum config_scope store_scope = current_parsing_scope;
+
+	opts = *inc->opts;
+	opts.unconditional_remote_url = 1;
+
+	cf = NULL;
+	current_config_kvi = NULL;
+	current_parsing_scope = 0;
+
+	inc->remote_urls = xmalloc(sizeof(*inc->remote_urls));
+	string_list_init_dup(inc->remote_urls);
+	config_with_options(add_remote_url, inc->remote_urls, inc->config_source, &opts);
+
+	cf = store_cf;
+	current_config_kvi = store_kvi;
+	current_parsing_scope = store_scope;
+}
+
+static int forbid_remote_url(const char *var, const char *value, void *data)
+{
+	const char *remote_name;
+	size_t remote_name_len;
+	const char *key;
+
+	if (!parse_config_key(var, "remote", &remote_name, &remote_name_len,
+			      &key) &&
+	    remote_name &&
+	    !strcmp(key, "url"))
+		die(_("remote URLs cannot be configured in file directly or indirectly included by includeIf.hasremoteurl"));
+	return 0;
+}
+
+static int at_least_one_url_matches_glob(const char *glob, int glob_len,
+					 struct string_list *remote_urls)
+{
+	struct strbuf pattern = STRBUF_INIT;
+	struct string_list_item *url_item;
+	int found = 0;
+
+	strbuf_add(&pattern, glob, glob_len);
+	for_each_string_list_item(url_item, remote_urls) {
+		if (!wildmatch(pattern.buf, url_item->string, WM_PATHNAME)) {
+			found = 1;
+			break;
+		}
+	}
+	strbuf_release(&pattern);
+	return found;
+}
+
 static int git_config_include(const char *var, const char *value, void *data)
 {
 	struct config_include_data *inc = data;
 	const char *cond, *key;
 	size_t cond_len;
-	int ret;
+	int ret = 0;
 
 	/*
 	 * Pass along all values, including "include" directives; this makes it
@@ -335,9 +412,29 @@ static 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(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)) {
+			if (inc->opts->unconditional_remote_url) {
+				config_fn_t old_fn = inc->fn;
+
+				inc->fn = forbid_remote_url;
+				ret = handle_path_include(value, inc);
+				inc->fn = old_fn;
+			} else {
+				if (!inc->remote_urls)
+					populate_remote_urls(inc);
+				if (at_least_one_url_matches_glob(
+						url, url_len, inc->remote_urls))
+					ret = handle_path_include(value, inc);
+			}
+		} else if (include_condition_is_true(inc->opts, cond, cond_len)) {
+			ret = handle_path_include(value, inc);
+		}
+	}
 
 	return ret;
 }
@@ -1933,11 +2030,13 @@ 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;
 		inc.data = data;
 		inc.opts = opts;
+		inc.config_source = config_source;
 		fn = git_config_include;
 		data = &inc;
 	}
@@ -1950,17 +2049,23 @@ 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.remote_urls) {
+		string_list_clear(inc.remote_urls, 0);
+		FREE_AND_NULL(inc.remote_urls);
+	}
+	return ret;
 }
 
 static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
diff --git a/config.h b/config.h
index 48a5e472ca..c24458b10a 100644
--- a/config.h
+++ b/config.h
@@ -89,6 +89,13 @@ struct config_options {
 	unsigned int ignore_worktree : 1;
 	unsigned int ignore_cmdline : 1;
 	unsigned int system_gently : 1;
+
+	/*
+	 * For internal use. Include all includeif.hasremoteurl paths without
+	 * checking if the repo has that remote URL.
+	 */
+	unsigned int unconditional_remote_url : 1;
+
 	const char *commondir;
 	const char *git_dir;
 	config_parser_event_fn_t event_fn;
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 9ff46f3b04..9bd299d3f8 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -2387,4 +2387,122 @@ 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' '
+	git init hasremoteurlTest &&
+	test_when_finished "rm -rf 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' '
+	git init hasremoteurlTest &&
+	test_when_finished "rm -rf hasremoteurlTest" &&
+
+	cat >"$(pwd)"/include-two-three <<-\EOF &&
+	[user]
+		two = included-config
+		three = included-config
+	EOF
+	cat >>hasremoteurlTest/.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 hasremoteurlTest config --get user.one >actual &&
+	test_cmp expect-main-config actual &&
+
+	git -C hasremoteurlTest config --get user.two >actual &&
+	test_cmp expect-included-config actual &&
+
+	git -C hasremoteurlTest config --get user.three >actual &&
+	test_cmp expect-main-config actual
+'
+
+test_expect_success 'includeIf.hasremoteurl globs' '
+	git init hasremoteurlTest &&
+	test_when_finished "rm -rf hasremoteurlTest" &&
+
+	printf "[user]\ndss = yes\n" >"$(pwd)/double-star-start" &&
+	printf "[user]\ndse = yes\n" >"$(pwd)/double-star-end" &&
+	printf "[user]\ndsm = yes\n" >"$(pwd)/double-star-middle" &&
+	printf "[user]\nssm = yes\n" >"$(pwd)/single-star-middle" &&
+	printf "[user]\nno = no\n" >"$(pwd)/no" &&
+
+	cat >>hasremoteurlTest/.git/config <<-EOF &&
+	[remote "foo"]
+		url = https://foo/bar/baz
+	[includeIf "hasremoteurl:**/baz"]
+		path = "$(pwd)/double-star-start"
+	[includeIf "hasremoteurl:**/nomatch"]
+		path = "$(pwd)/no"
+	[includeIf "hasremoteurl:https:/**"]
+		path = "$(pwd)/double-star-end"
+	[includeIf "hasremoteurl:nomatch:/**"]
+		path = "$(pwd)/no"
+	[includeIf "hasremoteurl:https:/**/baz"]
+		path = "$(pwd)/double-star-middle"
+	[includeIf "hasremoteurl:https:/**/nomatch"]
+		path = "$(pwd)/no"
+	[includeIf "hasremoteurl:https://*/bar/baz"]
+		path = "$(pwd)/single-star-middle"
+	[includeIf "hasremoteurl:https://*/baz"]
+		path = "$(pwd)/no"
+	EOF
+
+	git -C hasremoteurlTest config --get user.dss &&
+	git -C hasremoteurlTest config --get user.dse &&
+	git -C hasremoteurlTest config --get user.dsm &&
+	git -C hasremoteurlTest config --get user.ssm &&
+	test_must_fail git -C hasremoteurlTest config --get user.no
+'
+
+test_expect_success 'includeIf.hasremoteurl forbids remote url in such included files' '
+	git init hasremoteurlTest &&
+	test_when_finished "rm -rf hasremoteurlTest" &&
+
+	cat >"$(pwd)"/include-with-url <<-\EOF &&
+	[remote "bar"]
+		url = bar
+	EOF
+	cat >>hasremoteurlTest/.git/config <<-EOF &&
+	[includeIf "hasremoteurl:foo"]
+		path = "$(pwd)/include-with-url"
+	EOF
+
+	# test with any Git command
+	test_must_fail git -C hasremoteurlTest status 2>err &&
+	grep "fatal: remote URLs cannot be configured in file directly or indirectly included by includeIf.hasremoteurl" err
+'
+
 test_done
-- 
2.34.0.rc2.393.gf8c9666880-goog


  parent reply	other threads:[~2021-11-29 22:53 UTC|newest]

Thread overview: 87+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-12 22:57 [RFC PATCH 0/2] Conditional config includes based on remote URL Jonathan Tan
2021-10-12 22:57 ` [RFC PATCH 1/2] config: make git_config_include() static Jonathan Tan
2021-10-12 23:07   ` Jeff King
2021-10-12 23:26   ` Junio C Hamano
2021-10-13  8:26   ` Ævar Arnfjörð Bjarmason
2021-10-13 17:00     ` Junio C Hamano
2021-10-13 18:13       ` Jonathan Tan
2021-10-12 22:57 ` [RFC PATCH 2/2] config: include file if remote URL matches a glob Jonathan Tan
2021-10-12 23:30   ` Jeff King
2021-10-13 18:33     ` Jonathan Tan
2021-10-27 11:40       ` Jeff King
2021-10-27 17:23         ` Jonathan Tan
2021-10-12 23:48   ` Junio C Hamano
2021-10-13 19:52     ` Jonathan Tan
2021-10-13  0:46 ` [RFC PATCH 0/2] Conditional config includes based on remote URL brian m. carlson
2021-10-13 18:17   ` Jonathan Tan
2021-10-18 20:48 ` Jonathan Tan
2021-10-22  3:12   ` Emily Shaffer
2021-10-27 11:55   ` Jeff King
2021-10-27 17:52     ` Jonathan Tan
2021-10-27 20:32       ` Jeff King
2021-10-25 13:03 ` Ævar Arnfjörð Bjarmason
2021-10-25 18:53   ` Jonathan Tan
2021-10-26 10:12     ` Ævar Arnfjörð Bjarmason
2021-10-29 17:31 ` [WIP v2 " Jonathan Tan
2021-10-29 17:31   ` [WIP v2 1/2] config: make git_config_include() static Jonathan Tan
2021-11-05 19:45     ` Emily Shaffer
2021-10-29 17:31   ` [WIP v2 2/2] config: include file if remote URL matches a glob Jonathan Tan
2021-11-05 20:24     ` Emily Shaffer
2021-11-06  4:41       ` Ævar Arnfjörð Bjarmason
2021-11-09  0:25         ` Jonathan Tan
2021-11-09  0:22       ` Jonathan Tan
2021-11-16  0:00 ` [PATCH v3 0/2] Conditional config includes based on remote URL Jonathan Tan
2021-11-16  0:00   ` [PATCH v3 1/2] config: make git_config_include() static Jonathan Tan
2021-11-16  0:00   ` [PATCH v3 2/2] config: include file if remote URL matches a glob Jonathan Tan
2021-11-22 22:59     ` Glen Choo
2021-11-29 17:53       ` Jonathan Tan
2021-11-23  1:22     ` Junio C Hamano
2021-11-29 18:18       ` Jonathan Tan
2021-12-01 18:51         ` Junio C Hamano
2021-12-02 23:14           ` Jonathan Tan
2021-11-23  1:27     ` Ævar Arnfjörð Bjarmason
2021-11-29 18:33       ` Jonathan Tan
2021-11-29 20:50         ` Ævar Arnfjörð Bjarmason
2021-11-29 20:23 ` [PATCH v4 0/2] Conditional config includes based on remote URL Jonathan Tan
2021-11-29 20:23   ` [PATCH v4 1/2] config: make git_config_include() static Jonathan Tan
2021-11-29 20:23   ` Jonathan Tan [this message]
2021-12-02  6:57     ` [PATCH v4 2/2] config: include file if remote URL matches a glob Junio C Hamano
2021-12-02 17:41       ` Jonathan Tan
2021-11-29 20:48   ` [PATCH v4 0/2] Conditional config includes based on remote URL Ævar Arnfjörð Bjarmason
2021-11-30  7:51     ` Junio C Hamano
2021-12-02 23:31 ` [PATCH v5 " Jonathan Tan
2021-12-02 23:31   ` [PATCH v5 1/2] config: make git_config_include() static Jonathan Tan
2021-12-02 23:31   ` [PATCH v5 2/2] config: include file if remote URL matches a glob Jonathan Tan
2021-12-06 22:32     ` Glen Choo
2021-12-07 17:53       ` Jonathan Tan
2021-12-06 18:57   ` [PATCH v5 0/2] Conditional config includes based on remote URL Ævar Arnfjörð Bjarmason
2021-12-07 17:46     ` Jonathan Tan
2021-12-07 17:56       ` Ævar Arnfjörð Bjarmason
2021-12-07 18:52         ` Jonathan Tan
2021-12-07 23:23 ` [PATCH v6 " Jonathan Tan
2021-12-07 23:23   ` [PATCH v6 1/2] config: make git_config_include() static Jonathan Tan
2021-12-07 23:23   ` [PATCH v6 2/2] config: include file if remote URL matches a glob Jonathan Tan
2021-12-08 19:19     ` Glen Choo
2021-12-09 22:16       ` Jonathan Tan
2021-12-08 19:55     ` Glen Choo
2021-12-09 22:39       ` Jonathan Tan
2021-12-09 23:33         ` Glen Choo
2021-12-13 23:35           ` Jonathan Tan
2021-12-10 21:45         ` Junio C Hamano
2021-12-13 23:37           ` Jonathan Tan
2021-12-14 21:31 ` [PATCH v7 0/2] Conditional config includes based on remote URL Jonathan Tan
2021-12-14 21:31   ` [PATCH v7 1/2] config: make git_config_include() static Jonathan Tan
2021-12-14 21:31   ` [PATCH v7 2/2] config: include file if remote URL matches a glob Jonathan Tan
2021-12-16 21:54     ` Glen Choo
2021-12-28  0:55     ` Elijah Newren
2022-01-10 18:58       ` Jonathan Tan
2021-12-16 21:57   ` [PATCH v7 0/2] Conditional config includes based on remote URL Glen Choo
2021-12-28  1:13   ` Elijah Newren
2021-12-28 23:13     ` Glen Choo
2022-01-10 19:22     ` Jonathan Tan
2022-01-10 20:17       ` Elijah Newren
2022-01-25 13:26         ` Scalar vs JGit, was " Johannes Schindelin
2022-01-18 17:47 ` [PATCH v8 " Jonathan Tan
2022-01-18 17:47   ` [PATCH v8 1/2] config: make git_config_include() static Jonathan Tan
2022-01-18 17:47   ` [PATCH v8 2/2] config: include file if remote URL matches a glob Jonathan Tan
2022-01-18 20:54   ` [PATCH v8 0/2] Conditional config includes based on remote URL Elijah Newren

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=3b3af0da983c65ac8282b14141495f6859cd575a.1638217387.git.jonathantanmy@google.com \
    --to=jonathantanmy@google.com \
    --cc=avarab@gmail.com \
    --cc=chooglen@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /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).