All of lore.kernel.org
 help / color / mirror / Atom feed
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 2/2] config: handle conditional include when $GIT_DIR is not set up
Date: Sun, 16 Apr 2017 17:41:25 +0700	[thread overview]
Message-ID: <20170416104125.15300-2-pclouds@gmail.com> (raw)
In-Reply-To: <20170416104125.15300-1-pclouds@gmail.com>

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. 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 the
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 <bert.wesarg@googlemail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 cache.h                   |  2 ++
 config.c                  | 35 ++++++++++++++++++++++++++---------
 t/t1305-config-include.sh | 11 +++++++++++
 3 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/cache.h b/cache.h
index e29a093839..27b7286f99 100644
--- a/cache.h
+++ b/cache.h
@@ -1884,6 +1884,8 @@ enum config_origin_type {
 
 struct config_options {
 	unsigned int respect_includes : 1;
+	unsigned int early_config : 1;
+	const char *git_dir; /* only valid when early_config is true */
 };
 
 typedef int (*config_fn_t)(const char *, const char *, void *);
diff --git a/config.c b/config.c
index 042321a3a0..f323b96283 100644
--- a/config.c
+++ b/config.c
@@ -207,13 +207,20 @@ 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;
 
-	strbuf_add_absolute_path(&text, get_git_dir());
+	if (!opts->early_config)
+		strbuf_add_absolute_path(&text, get_git_dir());
+	else if (opts->git_dir)
+		strbuf_add_absolute_path(&text, opts->git_dir);
+	else
+		goto done;
+
 	strbuf_add(&pattern, cond, cond_len);
 	prefix = prepare_include_condition_pattern(&pattern);
 
@@ -242,13 +249,14 @@ static int include_by_gitdir(const char *cond, size_t cond_len, int icase)
 	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 +281,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 +1611,13 @@ 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);
+	opts.early_config = 1;
 
+	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,15 +1626,21 @@ 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 = strbuf_detach(&buf, NULL);
+
+	git_config_with_options(cb, data, NULL, &opts);
+
+	if (opts.git_dir) {
 		struct git_config_source repo_config;
 
 		memset(&repo_config, 0, sizeof(repo_config));
-		strbuf_addstr(&buf, "/config");
+		strbuf_addf(&buf, "%s/config", opts.git_dir);
 		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/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
-- 
2.11.0.157.gd943d85


  reply	other threads:[~2017-04-16 10:41 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         ` Nguyễn Thái Ngọc Duy [this message]
2017-04-16 15:51           ` [PATCH 2/2] config: handle conditional include when $GIT_DIR is not set up 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                 ` [PATCH v2 3/3] config: correct file reading order in read_early_config() Nguyễn Thái Ngọc Duy
2017-04-18  3:53                   ` 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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170416104125.15300-2-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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.