Subject: [PATCH/RFC v2] Hacky version of a glob() driven config include

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Fri, 7 May 2010 20:11:19 +0000

Add support for a special `voodoo.include` config variable. Its
contents will be globbed, and the files it expands to included as
config files.

This enables support for keeping around a ~/.gitconfig.d/* directory,
or for distros to keep a /etc/gitconfig.d/* directory. Some use cases:

  * Keep secret portions of the Git config file separate,
    e.g. github.token. Useful if the ~/.gitconfig itself is kept in
    a public Git repository.

  * Ability to selectively include config libraries. You could include
    e.g. ~/.gitconfig.d/pretty-colors.ini or
    ~/.gitconfig.d/better-submodule-handling.ini from a remote source.

Known bugs:

  * Breaks the model of being able to *set* config values. That
    doesn't work for the included files, i.e. setting a value that
    might be considered derived from a value in an included file won't
    end up in the "right" place. Maybe not a bug.

  * Errors in the git_config_from_file() call in glob_include_config()
    aren't passed upwards.

  * It relies on the GNU GLOB_TILDE extension with no
    alternative. That can be done by calling getenv("HOME") and
    s/~/$home/.

  * The whole bit with saving/restoring global state for config
    inclusion is evil, but then again so is the global state.

  * We don't check for recursion. But Git gives up eventually after
    after spewing a *lot* of duplicate entry errors. Not sure how to
    do this sanely w/symlinks.

Signed-off-by: Ævar Arnfjörð Bjarmason <[email-masked]>
---

On Fri, May 7, 2010 at 19:52, Bert Wesarg <[email-masked]> wrote:

> But where does the Developer's Certificate of Origin talks about
> non-legal aspects of patch submitting? E.g. correctness, quality, ...
>
> I think the part "if you want your work included in git.git" is very
> misleading in this paragraph, and I propose to remove it.

Fair enough. My reading of it was literal, i.e. `if (want_inclusion &&
DCO) { -s }`. Since I didn't want inclusion for the patch I didn't add a SOB.

Here's a version v2 of the patch with a Signed-off-by, and a better
commit message.

 config.c               |   55 +++++++++++++++++++++++++++++++++++++++++++++++-
 t/t1300-repo-config.sh |   43 +++++++++++++++++++++++++++++++++++++
 2 files changed, 97 insertions(+), 1 deletions(-)

diff --git a/config.c b/config.c
index 6963fbe..e7581b4 100644
--- a/config.c
+++ b/config.c
@@ -7,6 +7,7 @@
  */
 #include "cache.h"
 #include "exec_cmd.h"
+#include <glob.h>
 
 #define MAXNAME (256)
 
@@ -111,6 +112,52 @@ static inline int iskeychar(int c)
 	return isalnum(c) || c == '-';
 }
 
+static void glob_include_config(config_fn_t fn, void *data, char *pattern)
+{
+	glob_t globber;
+	int glob_ret;
+	int conf_ret;
+	size_t i = 0;
+	char *cfile = NULL;
+	FILE *saved_config_file;
+	char *saved_config_file_name;
+	int saved_config_linenr;
+	int saved_config_file_eof;
+#ifdef GLOB_TILDE
+	int glob_flags = GLOB_TILDE;
+#else
+	/* XXX: Non-GNU support for ~ expansion */
+	int glob_flags = 0;
+#endif
+
+	glob_ret = glob(pattern, glob_flags, NULL, &globber);
+
+	if (glob_ret == GLOB_NOSPACE || glob_ret == GLOB_ABORTED) {
+		globfree(&globber);
+		die("Unable to include config with pattern %s", pattern);
+	}
+
+	for (i = 0; i < globber.gl_pathc; i++) {
+		cfile = globber.gl_pathv[i];
+
+		/* Save away global state for including another file */
+		saved_config_file = config_file;
+		saved_config_file_name = config_file_name;
+		saved_config_linenr = config_linenr;
+		saved_config_file_eof = config_file_eof;
+
+		conf_ret = git_config_from_file(fn, cfile, data);
+
+		/* Restore it again */
+		config_file = saved_config_file;
+		config_file_name = saved_config_file_name;
+		config_linenr = saved_config_linenr;
+		config_file_eof = saved_config_file_eof;
+	}
+
+	globfree(&globber);
+}
+
 static int get_value(config_fn_t fn, void *data, char *name, unsigned int len)
 {
 	int c;
@@ -139,7 +186,13 @@ static int get_value(config_fn_t fn, void *data, char *name, unsigned int len)
 		if (!value)
 			return -1;
 	}
-	return fn(name, value, data);
+
+	if (!strcmp(name, "voodoo.include")) {
+		glob_include_config(fn, data, value);
+		return 0;
+	} else {
+		return fn(name, value, data);
+	}
 }
 
 static int get_extended_base_var(char *name, int baselen, int c)
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index f11f98c..4df6658 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -824,4 +824,47 @@ test_expect_success 'check split_cmdline return' "
 	test_must_fail git merge master
 	"
 
+cat > .git/config << EOF
+[some]
+	variable = blah
+[voodoo]
+	include = .git/more_config_*
+EOF
+
+cat > .git/more_config_1 << EOF
+[another]
+	variable = blah blah
+EOF
+
+cat > .git/more_config_2 << EOF
+[evenmore]
+	variable = blah bluh 
+EOF
+
+test_expect_success 'The voodoo include variable is hidden from us' \
+    'test_must_fail git config --get voodoo.include'
+test_expect_success 'get some included variable' \
+    'git config --get some.variable'
+test_expect_success 'get another included variable' \
+    'git config --get another.variable'
+test_expect_success 'get evenmore included variable' \
+    'git config --get evenmore.variable'
+
+rm .git/more_config*
+
+cat > .git/config << EOF
+[voodoo]
+	include = .git/more_config_*
+EOF
+
+cat > .git/more_config_1 << EOF
+[foo]
+    bar = zar
+[voodoo]
+	include = .git/more_config_*
+EOF
+
+test_expect_success 'circular config inclusion' \
+    'test_must_fail git config --get foo.bar'
+
 test_done
-- 
1.7.1.dirty