{"thread":{"id":"29448","subject":"[RFC/PATCH 0/4] config include directives","startedAt":"2012-01-26T07:35:47Z","lastAt":"2012-01-27T23:10:30Z","messageCount":36,"participants":["Jeff King","Johannes Sixt","Junio C Hamano","Ævar Arnfjörð Bjarmason","Nguyen Thai Ngoc Duy","Michael Haggerty","Jens Lehmann"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"183109","messageId":"20120126073547.GA28689@sigill.intra.peff.net","threadId":"29448","inReplyTo":null,"subject":"[RFC/PATCH 0/4] config include directives","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-01-26T07:35:47Z","receivedAt":"2012-01-26T07:35:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This series provides a way for config files to include other config\nfiles in two ways:\n\n 1. From other files in the filesystem. This is implemented by patch 1\n below, and is hopefully straightforward and uncontroversial. See\n that patch for more rationale.\n\n 2. From blobs in the repo. This is implemented by patch 4, with\n patches 2 and 3 providing the necessary refactoring. This\n is one way of implementing the often asked-for \"respect shared\n config inside the repo\" feature, but attempts to mitigate some of\n the security concerns. The interface for using it safely is a bit\n raw, but I think it's a sane building block, and somebody could\n write a fancier shared-config updater on top of it if they wanted\n to.\n\n [1/4]: config: add include directive\n [2/4]: config: factor out config file stack management\n [3/4]: config: support parsing config data from buffers\n [4/4]: config: allow including config from repository blobs\n\n-Peff\n"},{"id":"183110","messageId":"20120126073752.GA30474@sigill.intra.peff.net","threadId":"29448","inReplyTo":"20120126073547.GA28689@sigill.intra.peff.net","subject":"[PATCH 1/4] config: add include directive","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-01-26T07:37:52Z","receivedAt":"2012-01-26T07:37:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"It can be useful to split your ~/.gitconfig across multiple\nfiles. For example, you might have a \"main\" file which is\nused on many machines, but a small set of per-machine\ntweaks. Or you may want to make some of your config public\n(e.g., clever aliases) while keeping other data back (e.g.,\nyour name or other identifying information). Or you may want\nto include a number of config options in some subset of your\nrepos without copying and pasting (e.g., you want to\nreference them from the .git/config of participating repos).\n\nThis patch introduces an include directive for config files.\nIt looks like:\n\n [include]\n path = /path/to/file\n\nThis is syntactically backwards-compatible with existing git\nconfig parsers (i.e., they will see it as another config\nentry and ignore it unless you are looking up include.path).\n\nThe implementation provides a \"git_config_include\" callback\nwhich wraps regular config callbacks. Callers can pass it\nto git_config_from_file, and it will transparently follow\nany include directives, passing all of the discovered\noptions to the real callback.\n\nInclude directives are turned on for regular git config\nparsing (i.e., when you call git_config()), as well as for\nlookups via the \"git config\" program. They are not turned on\nin other cases, including:\n\n 1. Parsing of other config-like files, like .gitmodules.\n There isn't a real need, and I'd rather be conservative\n and avoid unnecessary incompatibility or confusion.\n\n 2. Writing files via \"git config\"; we want to treat\n include.* variables as literal items to be copied (or\n modified), and not expand them. So \"git config\n --unset-all foo.bar\" would operate _only_ on\n .git/config, not any of its included files (just as it\n also does not operate on ~/.gitconfig).\n\nSigned-off-by: Jeff King \n---\n Documentation/config.txt | 15 ++++++\n Documentation/git-config.txt | 5 ++\n builtin/config.c | 29 +++++++++---\n cache.h | 6 +++\n config.c | 58 +++++++++++++++++++++++++\n t/t1305-config-include.sh | 98 ++++++++++++++++++++++++++++++++++++++++++\n 6 files changed, 204 insertions(+), 7 deletions(-)\n create mode 100755 t/t1305-config-include.sh\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex abeb82b..e55dae1 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -84,6 +84,17 @@ customary UNIX fashion.\n \n Some variables may require a special value format.\n \n+Includes\n+~~~~~~~~\n+\n+You can include one config file from another by setting the special\n+`include.path` variable to the name of the file to be included. The\n+included file is expanded immediately, as if its contents had been\n+found at the location of the include directive. If the value of the\n+`include.path` variable is a relative path, the path is considered to be\n+relative to the configuration file in which the include directive was\n+found. See below for examples.\n+\n Example\n ~~~~~~~\n \n@@ -106,6 +117,10 @@ Example\n \t\tgitProxy=\"ssh\" for \"kernel.org\"\n \t\tgitProxy=default-proxy ; for the rest\n \n+\t[include]\n+\t\tpath = /path/to/foo.inc ; include by absolute path\n+\t\tpath = foo ; expand \"foo\" relative to the current file\n+\n Variables\n ~~~~~~~~~\n \ndiff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\nindex e7ecf5d..aa8303b 100644\n--- a/Documentation/git-config.txt\n+++ b/Documentation/git-config.txt\n@@ -178,6 +178,11 @@ See also <>.\n \tOpens an editor to modify the specified config file; either\n \t'--system', '--global', or repository (default).\n \n+--includes::\n+--no-includes::\n+\tRespect `include.*` directives in config files when looking up\n+\tvalues. Defaults to on.\n+\n [[FILES]]\n FILES\n -----\ndiff --git a/builtin/config.c b/builtin/config.c\nindex d35c06a..9105f87 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -25,6 +25,7 @@ static const char *given_config_file;\n static int actions, types;\n static const char *get_color_slot, *get_colorbool_slot;\n static int end_null;\n+static int respect_includes = 1;\n \n #define ACTION_GET (1<<0)\n #define ACTION_GET_ALL (1<<1)\n@@ -74,6 +75,8 @@ static struct option builtin_config_options[] = {\n \tOPT_BIT(0, \"path\", &types, \"value is a path (file or directory name)\", TYPE_PATH),\n \tOPT_GROUP(\"Other\"),\n \tOPT_BOOLEAN('z', \"null\", &end_null, \"terminate values with NUL byte\"),\n+\tOPT_BOOLEAN(0, \"includes\", &respect_includes,\n+\t\t \"respect include directives on lookup\"),\n \tOPT_END(),\n };\n \n@@ -161,6 +164,9 @@ static int get_value(const char *key_, const char *regex_)\n \tint ret = -1;\n \tchar *global = NULL, *repo_config = NULL;\n \tconst char *system_wide = NULL, *local;\n+\tstruct git_config_include_data inc;\n+\tconfig_fn_t fn;\n+\tvoid *data;\n \n \tlocal = config_exclusive_filename;\n \tif (!local) {\n@@ -213,19 +219,28 @@ static int get_value(const char *key_, const char *regex_)\n \t\t}\n \t}\n \n+\tfn = show_config;\n+\tdata = NULL;\n+\tif (respect_includes) {\n+\t\tinc.fn = fn;\n+\t\tinc.data = data;\n+\t\tfn = git_config_include;\n+\t\tdata = &inc;\n+\t}\n+\n \tif (do_all && system_wide)\n-\t\tgit_config_from_file(show_config, system_wide, NULL);\n+\t\tgit_config_from_file(fn, system_wide, data);\n \tif (do_all && global)\n-\t\tgit_config_from_file(show_config, global, NULL);\n+\t\tgit_config_from_file(fn, global, data);\n \tif (do_all)\n-\t\tgit_config_from_file(show_config, local, NULL);\n-\tgit_config_from_parameters(show_config, NULL);\n+\t\tgit_config_from_file(fn, local, data);\n+\tgit_config_from_parameters(fn, data);\n \tif (!do_all && !seen)\n-\t\tgit_config_from_file(show_config, local, NULL);\n+\t\tgit_config_from_file(fn, local, data);\n \tif (!do_all && !seen && global)\n-\t\tgit_config_from_file(show_config, global, NULL);\n+\t\tgit_config_from_file(fn, global, data);\n \tif (!do_all && !seen && system_wide)\n-\t\tgit_config_from_file(show_config, system_wide, NULL);\n+\t\tgit_config_from_file(fn, system_wide, data);\n \n \tfree(key);\n \tif (regexp) {\ndiff --git a/cache.h b/cache.h\nindex 10afd71..21bbb0a 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1138,6 +1138,12 @@ extern const char *get_commit_output_encoding(void);\n \n extern int git_config_parse_parameter(const char *, config_fn_t fn, void *data);\n \n+struct git_config_include_data {\n+\tconfig_fn_t fn;\n+\tvoid *data;\n+};\n+int git_config_include(const char *name, const char *value, void *vdata);\n+\n extern const char *config_exclusive_filename;\n \n #define MAX_GITNAME (1000)\ndiff --git a/config.c b/config.c\nindex 40f9c6d..a6966c1 100644\n--- a/config.c\n+++ b/config.c\n@@ -874,10 +874,68 @@ int git_config_system(void)\n \treturn !git_env_bool(\"GIT_CONFIG_NOSYSTEM\", 0);\n }\n \n+static int handle_path_include(const char *path, void *data)\n+{\n+\tint ret = 0;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\n+\t/*\n+\t * Use an absolute value as-is, but interpret relative paths\n+\t * based on the including config file.\n+\t */\n+\tif (!is_absolute_path(path)) {\n+\t\tchar *slash;\n+\t\tif (!cf)\n+\t\t\treturn error(\"relative config includes must come from files\");\n+\t\tstrbuf_addstr(&buf, absolute_path(cf->name));\n+\t\tslash = find_last_dir_sep(buf.buf);\n+\t\tif (!slash)\n+\t\t\tdie(\"BUG: no directory separator in an absolute path?\");\n+\t\tstrbuf_setlen(&buf, slash - buf.buf + 1);\n+\t\tstrbuf_addf(&buf, \"%s\", path);\n+\t\tpath = buf.buf;\n+\t}\n+\n+\tif (!access(path, R_OK))\n+\t\tret = git_config_from_file(git_config_include, path, data);\n+\tstrbuf_release(&buf);\n+\treturn ret;\n+}\n+\n+int git_config_include(const char *name, const char *value, void *vdata)\n+{\n+\tconst struct git_config_include_data *data = vdata;\n+\tconst char *type;\n+\tint ret;\n+\n+\t/*\n+\t * Pass along all values, including \"include\" directives; this makes it\n+\t * possible to query information on the includes themselves.\n+\t */\n+\tret = data->fn(name, value, data->data);\n+\tif (ret < 0)\n+\t\treturn ret;\n+\n+\tif (prefixcmp(name, \"include.\"))\n+\t\treturn ret;\n+\ttype = strrchr(name, '.') + 1;\n+\n+\tif (!strcmp(type, \"path\"))\n+\t\tret = handle_path_include(value, vdata);\n+\n+\treturn ret;\n+}\n+\n int git_config_early(config_fn_t fn, void *data, const char *repo_config)\n {\n \tint ret = 0, found = 0;\n \tconst char *home = NULL;\n+\tstruct git_config_include_data inc;\n+\n+\tinc.fn = fn;\n+\tinc.data = data;\n+\tfn = git_config_include;\n+\tdata = &inc;\n \n \t/* Setting $GIT_CONFIG makes git read _only_ the given config file. */\n \tif (config_exclusive_filename)\ndiff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh\nnew file mode 100755\nindex 0000000..4db3091\n--- /dev/null\n+++ b/t/t1305-config-include.sh\n@@ -0,0 +1,98 @@\n+#!/bin/sh\n+\n+test_description='test config file include directives'\n+. ./test-lib.sh\n+\n+test_expect_success 'include file by absolute path' '\n+\techo \"[test]one = 1\" >one &&\n+\techo \"[include]path = \\\"$PWD/one\\\"\" >base &&\n+\techo 1 >expect &&\n+\tgit config -f base test.one >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'include file by relative path' '\n+\techo \"[test]one = 1\" >one &&\n+\techo \"[include]path = one\" >base &&\n+\techo 1 >expect &&\n+\tgit config -f base test.one >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'recursive relative paths' '\n+\tmkdir subdir &&\n+\techo \"[test]three = 3\" >subdir/three &&\n+\techo \"[include]path = three\" >subdir/two &&\n+\techo \"[include]path = subdir/two\" >base &&\n+\techo 3 >expect &&\n+\tgit config -f base test.three >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'include options can still be examined' '\n+\techo \"[test]one = 1\" >one &&\n+\techo \"[include]path = one\" >base &&\n+\techo one >expect &&\n+\tgit config -f base include.path >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'listing includes option and expansion' '\n+\techo \"[test]one = 1\" >one &&\n+\techo \"[include]path = one\" >base &&\n+\tcat >expect <<-\\EOF &&\n+\tinclude.path=one\n+\ttest.one=1\n+\tEOF\n+\tgit config -f base --list >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'writing config file does not expand includes' '\n+\techo \"[test]one = 1\" >one &&\n+\techo \"[include]path = one\" >base &&\n+\tgit config -f base test.two 2 &&\n+\techo 2 >expect &&\n+\tgit config -f base --no-includes test.two >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_must_fail git config -f base --no-includes test.one\n+'\n+\n+test_expect_success 'config modification does not affect includes' '\n+\techo \"[test]one = 1\" >one &&\n+\techo \"[include]path = one\" >base &&\n+\tgit config -f base test.one 2 &&\n+\techo 1 >expect &&\n+\tgit config -f one test.one >actual &&\n+\ttest_cmp expect actual &&\n+\tcat >expect <<-\\EOF &&\n+\t1\n+\t2\n+\tEOF\n+\tgit config -f base --get-all test.one >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'missing include files are ignored' '\n+\tcat >base <<-\\EOF &&\n+\t[include]path = foo\n+\t[test]value = yes\n+\tEOF\n+\techo yes >expect &&\n+\tgit config -f base test.value >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'absolute includes from command line work' '\n+\techo \"[test]one = 1\" >one &&\n+\techo 1 >expect &&\n+\tgit -c include.path=\"$PWD/one\" config test.one >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'relative includes from command line fail' '\n+\techo \"[test]one = 1\" >one &&\n+\ttest_must_fail git -c include.path=one config test.one\n+'\n+\n+test_done\n-- \n1.7.9.rc2.293.gaae2\n"},{"id":"183111","messageId":"20120126073821.GB30474@sigill.intra.peff.net","threadId":"29448","inReplyTo":"20120126073547.GA28689@sigill.intra.peff.net","subject":"[PATCH 2/4] config: factor out config file stack management","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-01-26T07:38:22Z","receivedAt":"2012-01-26T07:38:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Because a config callback may start parsing a new file, the\nglobal context regarding the current config file is stored\nas a stack. Currently we only need to manage that stack from\ngit_config_from_file. Let's factor it out to allow new\nsources of config data.\n\nSigned-off-by: Jeff King \n---\n config.c | 30 +++++++++++++++++++-----------\n 1 files changed, 19 insertions(+), 11 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex a6966c1..b82f749 100644\n--- a/config.c\n+++ b/config.c\n@@ -826,6 +826,23 @@ int git_default_config(const char *var, const char *value, void *dummy)\n \treturn 0;\n }\n \n+static void config_file_push(config_file *top, const char *name)\n+{\n+\ttop->prev = cf;\n+\ttop->f = NULL;\n+\ttop->name = name;\n+\ttop->linenr = 1;\n+\ttop->eof = 0;\n+\tstrbuf_init(&top->value, 1024);\n+\tcf = top;\n+}\n+\n+static void config_file_pop(config_file *top)\n+{\n+\tstrbuf_release(&top->value);\n+\tcf = top->prev;\n+}\n+\n int git_config_from_file(config_fn_t fn, const char *filename, void *data)\n {\n \tint ret;\n@@ -835,21 +852,12 @@ int git_config_from_file(config_fn_t fn, const char *filename, void *data)\n \tif (f) {\n \t\tconfig_file top;\n \n-\t\t/* push config-file parsing state stack */\n-\t\ttop.prev = cf;\n+\t\tconfig_file_push(&top, filename);\n \t\ttop.f = f;\n-\t\ttop.name = filename;\n-\t\ttop.linenr = 1;\n-\t\ttop.eof = 0;\n-\t\tstrbuf_init(&top.value, 1024);\n-\t\tcf = ⊤\n \n \t\tret = git_parse_file(fn, data);\n \n-\t\t/* pop config-file parsing state stack */\n-\t\tstrbuf_release(&top.value);\n-\t\tcf = top.prev;\n-\n+\t\tconfig_file_pop(&top);\n \t\tfclose(f);\n \t}\n \treturn ret;\n-- \n1.7.9.rc2.293.gaae2\n"},{"id":"183112","messageId":"20120126074031.GC30474@sigill.intra.peff.net","threadId":"29448","inReplyTo":"20120126073547.GA28689@sigill.intra.peff.net","subject":"[PATCH 3/4] config: support parsing config data from buffers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-01-26T07:40:31Z","receivedAt":"2012-01-26T07:40:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The only two ways to parse config data are from a file or\nfrom the command-line. Because the command-line format is\ntotally different from the file format, they don't share any\ncode. Therefore, to add new sources of file-like config data,\nwe have to refactor git_parse_file to handle reading from\nsomething besides stdio.\n\nTo fix this, our config_file structure now holds either a\n\"FILE *\" pointer or a memory buffer. We intercept calls to\nfgetc and ungetc and either pass them along to stdio, or\nfake them with our buffer. This leaves the main parsing code\nintact and easy to read.\n\nSigned-off-by: Jeff King \n---\n cache.h | 1 +\n config.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----\n 2 files changed, 54 insertions(+), 5 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 21bbb0a..a298897 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1110,6 +1110,7 @@ extern int update_server_info(int);\n typedef int (*config_fn_t)(const char *, const char *, void *);\n extern int git_default_config(const char *, const char *, void *);\n extern int git_config_from_file(config_fn_t fn, const char *, void *);\n+extern int git_config_from_buffer(config_fn_t fn, void *, const char *, char *, unsigned long );\n extern void git_config_push_parameter(const char *text);\n extern int git_config_from_parameters(config_fn_t fn, void *data);\n extern int git_config(config_fn_t fn, void *);\ndiff --git a/config.c b/config.c\nindex b82f749..49a3d1a 100644\n--- a/config.c\n+++ b/config.c\n@@ -18,6 +18,9 @@ typedef struct config_file {\n \tconst char *name;\n \tint linenr;\n \tint eof;\n+\tchar *buf;\n+\tunsigned long size;\n+\tunsigned long cur;\n \tstruct strbuf value;\n \tchar var[MAXNAME];\n } config_file;\n@@ -101,19 +104,45 @@ int git_config_from_parameters(config_fn_t fn, void *data)\n \treturn nr > 0;\n }\n \n+static int get_one_char(void)\n+{\n+\tif (cf->f)\n+\t\treturn fgetc(cf->f);\n+\telse if (cf->buf) {\n+\t\tif (cf->cur < cf->size)\n+\t\t\treturn cf->buf[cf->cur++];\n+\t\treturn EOF;\n+\t}\n+\n+\tdie(\"BUG: attempt to read from NULL config_file\");\n+}\n+\n+static int unget_one_char(int c)\n+{\n+\tif (cf->f)\n+\t\tungetc(c, cf->f);\n+\telse if (cf->buf) {\n+\t\tif (cf->cur == 0)\n+\t\t\treturn EOF;\n+\t\tcf->buf[--cf->cur] = c;\n+\t\treturn c;\n+\t}\n+\n+\tdie(\"BUG: attempt to ungetc NULL config_file\");\n+}\n+\n static int get_next_char(void)\n {\n \tint c;\n-\tFILE *f;\n \n \tc = '\\n';\n-\tif (cf && ((f = cf->f) != NULL)) {\n-\t\tc = fgetc(f);\n+\tif (cf && (cf->f || cf->buf)) {\n+\t\tc = get_one_char();\n \t\tif (c == '\\r') {\n \t\t\t/* DOS like systems */\n-\t\t\tc = fgetc(f);\n+\t\t\tc = get_one_char();\n \t\t\tif (c != '\\n') {\n-\t\t\t\tungetc(c, f);\n+\t\t\t\tunget_one_char(c);\n \t\t\t\tc = '\\r';\n \t\t\t}\n \t\t}\n@@ -833,6 +862,9 @@ static void config_file_push(config_file *top, const char *name)\n \ttop->name = name;\n \ttop->linenr = 1;\n \ttop->eof = 0;\n+\ttop->buf = NULL;\n+\ttop->size = 0;\n+\ttop->cur = 0;\n \tstrbuf_init(&top->value, 1024);\n \tcf = top;\n }\n@@ -863,6 +895,22 @@ int git_config_from_file(config_fn_t fn, const char *filename, void *data)\n \treturn ret;\n }\n \n+int git_config_from_buffer(config_fn_t fn, void *data, const char *name,\n+\t\t\t char *buf, unsigned long size)\n+{\n+\tint ret;\n+\tconfig_file top;\n+\n+\tconfig_file_push(&top, name);\n+\ttop.buf = buf;\n+\ttop.size = size;\n+\n+\tret = git_parse_file(fn, data);\n+\n+\tconfig_file_pop(&top);\n+\treturn ret;\n+}\n+\n const char *git_etc_gitconfig(void)\n {\n \tstatic const char *system_wide;\n-- \n1.7.9.rc2.293.gaae2\n"},{"id":"183113","messageId":"20120126074208.GD30474@sigill.intra.peff.net","threadId":"29448","inReplyTo":"20120126073547.GA28689@sigill.intra.peff.net","subject":"[PATCH 4/4] config: allow including config from repository blobs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-01-26T07:42:08Z","receivedAt":"2012-01-26T07:42:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"One often-requested feature is to allow projects to ship\nsuggested config to people who clone. The most obvious way\nof implementing this would be to respect .gitconfig files\nwithin the working tree. However, this has two problems:\n\n 1. Because git configuration can cause the execution of\n arbitrary code, that creates a potential security problem.\n While you may be comfortable running \"make\" on a newly\n cloned project, you at least have the opportunity to\n inspect the downloaded contents. But by automatically\n respecting downloaded git configuration, you cannot\n even safely use git to inspect those contents!\n\n 2. Configuration options tend not to be tied to a specific\n version of the project. So if you are using \"git\n checkout\" to sight-see to an older revision, you\n probably still want to be using the most recent version\n of the suggested config.\n\nInstead, this patch lets you include configuration directly\nfrom a blob in the repository (using the usual object name\nlookup rules). This avoids (2) by pointing directly to a tag\nor branch tip. It is still possible to be dangerous as in\n(1) above, but the danger can be avoided by not pointing\ndirectly into remote blobs (and the documentation warns of\nthis and gives a safe example).\n\nSigned-off-by: Jeff King \n---\n Documentation/config.txt | 41 ++++++++++++++++++++++++++++++++++++++++-\n config.c | 25 ++++++++++++++++++++++++-\n t/t1305-config-include.sh | 38 ++++++++++++++++++++++++++++++++++++++\n 3 files changed, 102 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex e55dae1..38e83df 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -93,7 +93,14 @@ included file is expanded immediately, as if its contents had been\n found at the location of the include directive. If the value of the\n `include.path` variable is a relative path, the path is considered to be\n relative to the configuration file in which the include directive was\n-found. See below for examples.\n+found.\n+\n+You can also include configuration from a blob stored in your repository\n+by setting the special `include.ref` variable to the name of an object\n+containing your configuration data (in the same format as a regular\n+config file).\n+\n+See below for examples.\n \n Example\n ~~~~~~~\n@@ -120,6 +127,38 @@ Example\n \t[include]\n \t\tpath = /path/to/foo.inc ; include by absolute path\n \t\tpath = foo ; expand \"foo\" relative to the current file\n+\t\tref = config:.gitconfig ; look on \"config\" branch\n+\t\tref = origin/master:.gitconfig ; this is unsafe! see below\n+\n+\n+Security Considerations\n+~~~~~~~~~~~~~~~~~~~~~~~\n+\n+Because git configuration may cause git to execute arbitrary shell\n+commands, it is important to verify any configuration you receive over\n+the network. In particular, it is not a good idea to point `include.ref`\n+directly at a remote tracking branch like `origin/master:shared-config`.\n+After a fetch, you have no way of inspecting the shared-config you have\n+just received without running git (and thus respecting the downloaded\n+config). Instead, you can create a local tag representing the last\n+verified version of the config, and only update the tag after inspecting\n+any new content.\n+\n+For example:\n+\n+\t# initially, look at their suggested config\n+\tgit show origin/master:shared-config\n+\n+\t# if it looks good to you, point a local ref at it\n+\tgit tag config origin/master\n+\tgit config include.ref config:shared-config\n+\n+\t# much later, fetch any changes and examine them\n+\tgit fetch origin\n+\tgit diff config origin/master -- shared-config\n+\n+\t# If the changes look OK, update your local version\n+\tgit tag -f config origin/master\n \n Variables\n ~~~~~~~~~\ndiff --git a/config.c b/config.c\nindex 49a3d1a..c41fb3b 100644\n--- a/config.c\n+++ b/config.c\n@@ -941,7 +941,7 @@ static int handle_path_include(const char *path, void *data)\n \t */\n \tif (!is_absolute_path(path)) {\n \t\tchar *slash;\n-\t\tif (!cf)\n+\t\tif (!cf || !cf->f)\n \t\t\treturn error(\"relative config includes must come from files\");\n \t\tstrbuf_addstr(&buf, absolute_path(cf->name));\n \t\tslash = find_last_dir_sep(buf.buf);\n@@ -958,6 +958,27 @@ static int handle_path_include(const char *path, void *data)\n \treturn ret;\n }\n \n+static int handle_ref_include(const char *ref, void *data)\n+{\n+\tunsigned char sha1[20];\n+\tchar *buf;\n+\tunsigned long size;\n+\tenum object_type type;\n+\tint ret;\n+\n+\tif (get_sha1(ref, sha1))\n+\t\treturn 0;\n+\tbuf = read_sha1_file(sha1, &type, &size);\n+\tif (!buf)\n+\t\treturn error(\"unable to read include ref '%s'\", ref);\n+\tif (type != OBJ_BLOB)\n+\t\treturn error(\"include ref '%s' is not a blob\", ref);\n+\n+\tret = git_config_from_buffer(git_config_include, data, ref, buf, size);\n+\tfree(buf);\n+\treturn ret;\n+}\n+\n int git_config_include(const char *name, const char *value, void *vdata)\n {\n \tconst struct git_config_include_data *data = vdata;\n@@ -978,6 +999,8 @@ int git_config_include(const char *name, const char *value, void *vdata)\n \n \tif (!strcmp(type, \"path\"))\n \t\tret = handle_path_include(value, vdata);\n+\telse if (!strcmp(type, \"ref\"))\n+\t\tret = handle_ref_include(value, vdata);\n \n \treturn ret;\n }\ndiff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh\nindex 4db3091..31d3b9b 100755\n--- a/t/t1305-config-include.sh\n+++ b/t/t1305-config-include.sh\n@@ -95,4 +95,42 @@ test_expect_success 'relative includes from command line fail' '\n \ttest_must_fail git -c include.path=one config test.one\n '\n \n+test_expect_success 'include from ref' '\n+\techo \"[test]one = 1\" >one &&\n+\tgit add one &&\n+\tgit commit -m one &&\n+\trm one &&\n+\techo \"[include]ref = HEAD:one\" >base &&\n+\techo 1 >expect &&\n+\tgit config -f base test.one >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'relative file include from ref fails' '\n+\techo \"[test]two = 2\" >two &&\n+\techo \"[include]path = two\" >one &&\n+\tgit add one &&\n+\tgit commit -m one &&\n+\techo \"[include]ref = HEAD:one\" >base &&\n+\ttest_must_fail git config -f base test.two\n+'\n+\n+test_expect_success 'non-existent include refs are ignored' '\n+\tcat >base <<-\\EOF &&\n+\t[include]ref = my-missing-config-branch:foo.cfg\n+\t[test]value = yes\n+\tEOF\n+\techo yes >expect &&\n+\tgit config -f base test.value >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'non-blob include refs fail' '\n+\tcat >base <<-\\EOF &&\n+\t[include]ref = HEAD\n+\t[test]value = yes\n+\tEOF\n+\ttest_must_fail git config -f base test.value\n+'\n+\n test_done\n-- \n1.7.9.rc2.293.gaae2\n"},{"id":"183115","messageId":"4F2119E6.8010109@viscovery.net","threadId":"29448","inReplyTo":"20120126073752.GA30474@sigill.intra.peff.net","subject":"Re: [PATCH 1/4] config: add include directive","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2012-01-26T09:16:22Z","receivedAt":"2012-01-26T09:16:22Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 1/26/2012 8:37, schrieb Jeff King:\n> This patch introduces an include directive for config files.\n\nNice. I haven't had a need for it, yet, but the concept looks good.\n\n> +test_expect_success 'recursive relative paths' '\n> +\tmkdir subdir &&\n> +\techo \"[test]three = 3\" >subdir/three &&\n> +\techo \"[include]path = three\" >subdir/two &&\n> +\techo \"[include]path = subdir/two\" >base &&\n> +\techo 3 >expect &&\n> +\tgit config -f base test.three >actual &&\n> +\ttest_cmp expect actual\n> +'\n\nIsn't it rather \"chained relative paths\"? Recursive would be if I write\n\n [include]path = .gitconfig\n\nin my ~/.gitconfig. What happens in this case?\n\n-- Hannes\n"},{"id":"183116","messageId":"4F211C0C.7060400@viscovery.net","threadId":"29448","inReplyTo":"20120126074208.GD30474@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] config: allow including config from repository blobs","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2012-01-26T09:25:32Z","receivedAt":"2012-01-26T09:25:32Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 1/26/2012 8:42, schrieb Jeff King:\n> +static int handle_ref_include(const char *ref, void *data)\n> +{\n> +\tunsigned char sha1[20];\n> +\tchar *buf;\n> +\tunsigned long size;\n> +\tenum object_type type;\n> +\tint ret;\n> +\n> +\tif (get_sha1(ref, sha1))\n> +\t\treturn 0;\n> +\tbuf = read_sha1_file(sha1, &type, &size);\n> +\tif (!buf)\n> +\t\treturn error(\"unable to read include ref '%s'\", ref);\n> +\tif (type != OBJ_BLOB)\n> +\t\treturn error(\"include ref '%s' is not a blob\", ref);\n> +\n> +\tret = git_config_from_buffer(git_config_include, data, ref, buf, size);\n> +\tfree(buf);\n> +\treturn ret;\n> +}\n\nWhat happens if a ref cannot be resolved, for example due to repository\ncorruption? Does git just emit an error and then carries on, or does it\nalways die? Can I run at least git-fsck in such a case?\n\n-- Hannes\n"},{"id":"183123","messageId":"20120126165456.GA5278@sigill.intra.peff.net","threadId":"29448","inReplyTo":"4F2119E6.8010109@viscovery.net","subject":"Re: [PATCH 1/4] config: add include directive","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-01-26T16:54:56Z","receivedAt":"2012-01-26T16:54:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 26, 2012 at 10:16:22AM +0100, Johannes Sixt wrote:\n\n> > +test_expect_success 'recursive relative paths' '\n> > +\tmkdir subdir &&\n> > +\techo \"[test]three = 3\" >subdir/three &&\n> > +\techo \"[include]path = three\" >subdir/two &&\n> > +\techo \"[include]path = subdir/two\" >base &&\n> > +\techo 3 >expect &&\n> > +\tgit config -f base test.three >actual &&\n> > +\ttest_cmp expect actual\n> > +'\n> \n> Isn't it rather \"chained relative paths\"? Recursive would be if I write\n> \n> [include]path = .gitconfig\n> \n> in my ~/.gitconfig. What happens in this case?\n\nGood point. I used \"recursive\" because it is recursing in the include\nfunction within git, but obviously from the user's perspective, it is\nnot a recursion.\n\nAnd no, I didn't do any cycle detection. We could either do:\n\n 1. Record some canonical name for each source we look at (probably\n realpath() for files, and the sha1 for refs), and don't descend\n into already-seen sources.\n\n 2. Simply provide a maximum depth, and don't include beyond it.\n\nThe latter is much simpler to implement, but I think the former is a\nlittle nicer for the user.\n\n-Peff\n"},{"id":"183124","messageId":"20120126172259.GB5278@sigill.intra.peff.net","threadId":"29448","inReplyTo":"4F211C0C.7060400@viscovery.net","subject":"Re: [PATCH 4/4] config: allow including config from repository blobs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-01-26T17:22:59Z","receivedAt":"2012-01-26T17:22:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 26, 2012 at 10:25:32AM +0100, Johannes Sixt wrote:\n\n> Am 1/26/2012 8:42, schrieb Jeff King:\n> > +static int handle_ref_include(const char *ref, void *data)\n> > +{\n> > +\tunsigned char sha1[20];\n> > +\tchar *buf;\n> > +\tunsigned long size;\n> > +\tenum object_type type;\n> > +\tint ret;\n> > +\n> > +\tif (get_sha1(ref, sha1))\n> > +\t\treturn 0;\n> > +\tbuf = read_sha1_file(sha1, &type, &size);\n> > +\tif (!buf)\n> > +\t\treturn error(\"unable to read include ref '%s'\", ref);\n> > +\tif (type != OBJ_BLOB)\n> > +\t\treturn error(\"include ref '%s' is not a blob\", ref);\n> > +\n> > +\tret = git_config_from_buffer(git_config_include, data, ref, buf, size);\n> > +\tfree(buf);\n> > +\treturn ret;\n> > +}\n> \n> What happens if a ref cannot be resolved, for example due to repository\n> corruption? Does git just emit an error and then carries on, or does it\n> always die? Can I run at least git-fsck in such a case?\n\nNames which do not resolve are explicitly ignored, because I wanted to\nflexibility in specifying the includes. E.g., you might say put:\n\n [include]\n ref = refs/config\n\nin your ~/.gitconfig, and then only use the feature in some of your\nrepositories (I'm not sure if that is a good idea yet in practice or\nnot. But as I said before, I think of this is a building block, and I'd\nlike people to experiment and see if it fills their needs).\n\nObviously the trade-off is that we would silently ignore a typo in the\nobject name.\n\nHowever, I did explicitly return an error for a failure to find a sha1,\nor a non-blob sha1, as those are more severe configuration errors (where\nthe former is basically repository corruption). We only return an error\nhere, but git_config will eventually die() because of it, noting the\nfile and line number where the include happened[1].\n\nIt's then up to you to fix the config file before you can continue using\ngit. You can do so by hand, but I think using \"git config\" to do so will\nnot work; even though it correctly does not expand includes while\nwriting, the git wrapper incidentally reads the config before actually\nrunning the config command.\n\nWe already have similar problems where setting a bool option to a\nnon-bool value will cause many git commands git to die (e.g., try\nsetting \"color.ui\" to \"foo\"). But it's less often an issue, because\nunless the config option you have messed up is very basic (like\ncore.bare), you can still run \"git config\".\n\nSo it's certainly recoverable if you are comfortable editing the config\nfile. But we could also make it a little friendlier by turning those\nerrors into warnings, at the minor cost of making errors less\nnoticeable.\n\n-Peff\n\n[1] The error reporting just shows the source of the deepest file,\n because git_parse_file will actually call die(). So if I have a\n .git/config that includes a ref that includes another ref that has\n an error, I see only:\n\n $ git config foo.value\n error: include ref 'HEAD:subdir' is not a blob\n fatal: bad config file line 5 in HEAD:config\n\n But solving the problem without git is hard. I know the problem is\n in the ref, but I can't edit the ref. The source of the include\n chain has to come from a file I can edit outside of git (since we\n always start from the files), but I'm not told which file included\n it.\n\n So it would be a little nicer to say something like:\n\n error: include ref 'HEAD:subdir' is not a blob (at HEAD:config, line 5)\n error: included file 'HEAD:config' had errors at .git/config, line 9\n fatal: unable to parse configuration\n\n which shows the complete trail, and you know to edit .git/config. In\n practice, I don't know if it is much of an issue. There are only 3\n places that git actually reads config from, and it is likely that\n you just edited one.\n"},{"id":"183132","messageId":"7vmx9a2o23.fsf@alter.siamese.dyndns.org","threadId":"29448","inReplyTo":"20120126165456.GA5278@sigill.intra.peff.net","subject":"Re: [PATCH 1/4] config: add include directive","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-26T20:42:28Z","receivedAt":"2012-01-26T20:42:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> And no, I didn't do any cycle detection. We could either do:\n>\n> 1. Record some canonical name for each source we look at (probably\n> realpath() for files, and the sha1 for refs), and don't descend\n> into already-seen sources.\n>\n> 2. Simply provide a maximum depth, and don't include beyond it.\n>\n> The latter is much simpler to implement, but I think the former is a\n> little nicer for the user.\n\nAnother thing I wondered after reading this patch was that it will be a\nrather common \"mistake\" to include the same file twice, one in ~/.gitconfig\nand then another in project specific .git/config, or more likely, people\nstart using useful ones in ~their/.gitconfig, and then the site administrator\nby popular demand adding the same include in /etc/gitconfig to retroactively\ncause the same file included twice for them.\n\nYour first alternative solution should solve this case nicely as well, I\nwould think.\n"},{"id":"183133","messageId":"7vipjy2nbi.fsf@alter.siamese.dyndns.org","threadId":"29448","inReplyTo":"20120126073752.GA30474@sigill.intra.peff.net","subject":"Re: [PATCH 1/4] config: add include directive","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-26T20:58:25Z","receivedAt":"2012-01-26T20:58:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Include directives are turned on for regular git config\n> parsing (i.e., when you call git_config()), as well as for\n> lookups via the \"git config\" program. They are not turned on\n> in other cases, including:\n>\n> 1. Parsing of other config-like files, like .gitmodules.\n> There isn't a real need, and I'd rather be conservative\n> and avoid unnecessary incompatibility or confusion.\n\nThis is a good design decision, I think, but I am not sure where this \"we\ndo not let gitmodules to honor inclusion\" is implemented in this patch. I\nwould have expected a patch to \"git-submodule.sh\" that adds --no-includes\nwhere it invokes \"git config\"?\n\n> +Includes\n> +~~~~~~~~\n> +\n> +You can include one config file from another by setting the special\n> +`include.path` variable to the name of the file to be included. The\n> +included file is expanded immediately, as if its contents had been\n> +found at the location of the include directive.\n\nI cannot offer better wording, but the first reaction I had to this was\n\"Eh, if I include a file A that includes another file B, to which config\nfile the '[include] path = B' directive found in file A relative to?\"\n\nLogically it should be relative to A, and I think your implementation\nmakes it so, but the above \"as if its contents had been found at...\"\nsounds as if it should be the same as writing '[include] path = B' in the\noriginal config file that included A.\n\nBy the way, I was a bit surprised that you did not have to add the source\nstacking to git_config_from_file().\n\nIt was added in 924aaf3 (config.c: Make git_config() work correctly when\ncalled recursively, 2011-06-16) to address the situation where\ngit_config() ends up calling git_config() recursively. My gut feeling is\nthat logically that issue shouldn't be limited to configuration that is\ncoming from file, which in turn makes me suspect that the source stacking\nmay be better in one level higher than git_config_from_file() so that the\nones coming from parameters could also be treated as a different kind of\nsource to read configuration from.\n\nBut in practice, including from the command line (i.e. -c include.path=...)\nis somewhat crazy and we probably would not want to support it, so...\n"},{"id":"183134","messageId":"7vbopq2mk9.fsf@alter.siamese.dyndns.org","threadId":"29448","inReplyTo":"20120126074208.GD30474@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] config: allow including config from repository blobs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-26T21:14:46Z","receivedAt":"2012-01-26T21:14:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> +You can also include configuration from a blob stored in your repository\n> +by setting the special `include.ref` variable to the name of an object\n> +containing your configuration data (in the same format as a regular\n> +config file).\n\nHmm, the concept is surely *fun*, but is this really worth it?\n\nWith this, projects could include README that says something like this:\n\n\tWhen working with our project, we would suggest using some tweaks\n\tto the configuration file. For your convenience, a copy of it\n\talready exists in the clone of your repository, and all you have\n\tto do is to run this in your repository:\n\n\t$ git config include.ref 4774acaf6657efed\n\n\tWe may update the set of recommended tweaks from time to time, so\n watch this README file for such updates, and re-run the above command\n\twith the updated blob object name as needed.\n\n\tNote: if you are paranoid and suspect that the project might\n\tgive you trojan horse, you could inspect the recommended\n\ttweaks first before including them, like this:\n\n\t$ git cat-file -p 4774acaf6657efed\n\nThe blob will be included in the repository and the most natural way for\nsuch a project to arrange things is to keep it together with the source\ntree, perhaps in a separate hierarchy, say \"dev_tools/std_gitconfig\" or\nsomething. Without the update in patches 3 and 4, the project should be\nable to update the above for its participants with minimum fuss, e.g.\n\ndiff --git a/README b/README\nindex af31555..203d255 100644\n--- a/README\n+++ b/README\n@@ -3,14 +3,15 @@\n \talready exists in the clone of your repository, and all you have\n \tto do is to run this in your repository:\n \n-\t$ git config include.ref 4774acaf6657efed\n+\t$ cp dev_tools/std_gitconfig .git/std_gitconfig\n+\t$ git config include.path std_gitconfig\n \n \tWe may update the set of recommended tweaks from time to time, so\n- watch this README file for such updates, and re-run the above command\n-\twith the updated blob object name as needed.\n+ watch \"git log -p dev_tools/std_gitconfig\" for such updates and\n+\tupdate your .git/std_gitconfig as needed.\n \n \tNote: if you are paranoid and suspect that the project might\n \tgive you trojan horse, you could inspect the recommended\n \ttweaks first before including them, like this:\n \n-\t$ git cat-file -p 4774acaf6657efed\n+\t$ less dev_tools/std_gitconfig\n"},{"id":"183136","messageId":"20120126222532.GA12855@sigill.intra.peff.net","threadId":"29448","inReplyTo":"7vmx9a2o23.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/4] config: add include directive","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-01-26T22:25:32Z","receivedAt":"2012-01-26T22:25:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 26, 2012 at 12:42:28PM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > And no, I didn't do any cycle detection. We could either do:\n> >\n> > 1. Record some canonical name for each source we look at (probably\n> > realpath() for files, and the sha1 for refs), and don't descend\n> > into already-seen sources.\n> >\n> > 2. Simply provide a maximum depth, and don't include beyond it.\n> >\n> > The latter is much simpler to implement, but I think the former is a\n> > little nicer for the user.\n> \n> Another thing I wondered after reading this patch was that it will be a\n> rather common \"mistake\" to include the same file twice, one in ~/.gitconfig\n> and then another in project specific .git/config, or more likely, people\n> start using useful ones in ~their/.gitconfig, and then the site administrator\n> by popular demand adding the same include in /etc/gitconfig to retroactively\n> cause the same file included twice for them.\n> \n> Your first alternative solution should solve this case nicely as well, I\n> would think.\n\nAgreed. I'll implement that, then.\n\nThere's a slight complication with when to clear the \"I have seen this\"\nmark for each source. If you are interested only in breaking cycles,\nthen obviously you can just clear the marks as you pop the stack. But if\nyou want to stop repeated inclusion down different branches of the\ninclude tree, you need to keep the marks until you're done (e.g., the\nsame file referenced by .git/config and ~/.gitconfig). But you do still\nneed to clear them, because we repeatedly call git_config with different\ncallback functions, and we need to re-parse each time.\n\nI think it should be sufficient to make the marks live through\ngit_config(), and get cleared after that (so all of .git/config,\n~/.gitconfig, and /etc/gitconfig will only load a given include once,\nbut another call to git_config will load it again).\n\n-Peff\n"},{"id":"183138","messageId":"20120126224350.GA13241@sigill.intra.peff.net","threadId":"29448","inReplyTo":"20120126222532.GA12855@sigill.intra.peff.net","subject":"Re: [PATCH 1/4] config: add include directive","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-01-26T22:43:50Z","receivedAt":"2012-01-26T22:43:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 26, 2012 at 05:25:32PM -0500, Jeff King wrote:\n\n> There's a slight complication with when to clear the \"I have seen this\"\n> mark for each source. If you are interested only in breaking cycles,\n> then obviously you can just clear the marks as you pop the stack. But if\n> you want to stop repeated inclusion down different branches of the\n> include tree, you need to keep the marks until you're done (e.g., the\n> same file referenced by .git/config and ~/.gitconfig). But you do still\n> need to clear them, because we repeatedly call git_config with different\n> callback functions, and we need to re-parse each time.\n> \n> I think it should be sufficient to make the marks live through\n> git_config(), and get cleared after that (so all of .git/config,\n> ~/.gitconfig, and /etc/gitconfig will only load a given include once,\n> but another call to git_config will load it again).\n\nUgh, actually it's a little trickier. Because a git_config callback can\ncall git_config again, we want one set of marks per git_config\ninvocation, but we need to handle two simultaneous invocations. But I\nthink it's OK. We can stuff the data into the \"git_config_include_data\",\nwhich has the proper scope.\n\n-Peff\n"},{"id":"183139","messageId":"20120126225140.GB12855@sigill.intra.peff.net","threadId":"29448","inReplyTo":"7vipjy2nbi.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/4] config: add include directive","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-01-26T22:51:40Z","receivedAt":"2012-01-26T22:51:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 26, 2012 at 12:58:25PM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > Include directives are turned on for regular git config\n> > parsing (i.e., when you call git_config()), as well as for\n> > lookups via the \"git config\" program. They are not turned on\n> > in other cases, including:\n> >\n> > 1. Parsing of other config-like files, like .gitmodules.\n> > There isn't a real need, and I'd rather be conservative\n> > and avoid unnecessary incompatibility or confusion.\n> \n> This is a good design decision, I think, but I am not sure where this \"we\n> do not let gitmodules to honor inclusion\" is implemented in this patch. I\n> would have expected a patch to \"git-submodule.sh\" that adds --no-includes\n> where it invokes \"git config\"?\n\nMy goal was to leave git_config_from_file untouched, so that the code in\nsubmodule.c:gitmodules_config would work without modification. And that\nworks, obviously.\n\nHowever, I didn't think about the fact that git-submodule.sh would be\ncalling git-config separately, and that is accidentally changed by my\npatch. Even if we changed git-submodule to use \"git config\n--no-includes\" that would break any third-party scripts that use \"git\nconfig\" to read git-like files.\n\nBut it would be nice for callers doing \"git config foo.bar\" to get the\nincludes by default. So maybe the right rule is:\n\n 1. In C:\n a. git_config() respects includes automatically.\n b. other callers do not do so automatically (e.g., gitmodules via\n submodule.c).\n\n (i.e., what is implemented by this patch)\n\n 2. Callers of git-config:\n a. respect includes for lookup that checks all of the \"normal\"\n config spots in sequence: .git/config, ~/.gitconfig, and\n /etc/gitconfig. These are the shell equivalent of calling\n git_config().\n\n b. when we are looking in a specific file (via GIT_CONFIG or \"git\n config -f\"), do not respect includes (but allow --includes if\n the caller chooses). This specific file may be something like\n .gitmodules. Or perhaps somebody is saying \"no, I really just\n want to know what is in _this_ file, not what the config\n ecosystem tells me in general\".\n\nAnd then because of 1a and 2a, most programs should Just Work without\nany changes, but because of 1b and 2b, any special uses will have to\ndecide manually whether they would want to allow includes.\n\nDoes that make sense?\n\n> > +Includes\n> > +~~~~~~~~\n> > +\n> > +You can include one config file from another by setting the special\n> > +`include.path` variable to the name of the file to be included. The\n> > +included file is expanded immediately, as if its contents had been\n> > +found at the location of the include directive.\n> \n> I cannot offer better wording, but the first reaction I had to this was\n> \"Eh, if I include a file A that includes another file B, to which config\n> file the '[include] path = B' directive found in file A relative to?\"\n> \n> Logically it should be relative to A, and I think your implementation\n> makes it so, but the above \"as if its contents had been found at...\"\n> sounds as if it should be the same as writing '[include] path = B' in the\n> original config file that included A.\n\nYes, it is relative to A. Anything else would be insane (because it\nwould mean \"A\" has to care about who is including it).\n\nI had a similar thought while writing it, but hoped the sentence after\n(that you snipped) would make it clear. But I see the \"as if...\" can be\nambiguous (because it's only mostly \"as if\"). I started trying to write\nsomething like \"with the exception of further includes, which...\" but I\nthink that is just getting more confusing.\n\nHow about:\n\n The included file is processed immediately, before any other\n directives from the surrounding file.\n\nWhat I wanted to make clear there is the ordering, which sometimes\nmatters.\n\n> By the way, I was a bit surprised that you did not have to add the source\n> stacking to git_config_from_file().\n> \n> It was added in 924aaf3 (config.c: Make git_config() work correctly when\n> called recursively, 2011-06-16) to address the situation where\n> git_config() ends up calling git_config() recursively.\n\nYeah. I remembered discussing that patch with Ramsay a few months ago,\nand I was pleased that it just worked in this case.\n\n> My gut feeling is that logically that issue shouldn't be limited to\n> configuration that is coming from file, which in turn makes me suspect\n> that the source stacking may be better in one level higher than\n> git_config_from_file() so that the ones coming from parameters could\n> also be treated as a different kind of source to read configuration\n> from.\n\nI do factor it out in the second patch. I also considered how\ngit_config_from_parameters doesn't share the same source marking. But it\nfollows a completely different code-path, as its syntax is totally\ndifferent. So there's really not much point in adding to the config_file\nstack (if you had another source that could be included from a file, you\nwould want to mention it in the stack. But the command-line parameters\nmust always be the \"root\" of the stack, so it's sufficient to simply\nhave a NULL stack to mark that).\n\n> But in practice, including from the command line (i.e. -c include.path=...)\n> is somewhat crazy and we probably would not want to support it, so...\n\nActually, I made sure it works, and it's in the tests. It even complains\nif you use a relative path (I thought about using $PWD, but that is too\ninsane, as git changes the working directory behind the scenes).\n\nThe one use I think is to bundle a bunch of related config options, and\nthen turn them on selectively. So you _could_ do:\n\n cat >>~/.bashrc <<-\\EOF\n FOO_OPTIONS=\"-c foo.one=1 -c foo.two=2\"\n EOF\n # without FOO\n git blah\n # with FOO\n git $FOO_OPTIONS blah\n\nbut with this patch, you can do:\n\n cat >~/.gitconfig.foo <<-\\EOF\n [foo]\n one = 1\n two = 2\n EOF\n git -c include.path=$HOME/.gitconfig.foo blah\n\nwhich to my mind is a little more natural. I don't personally plan on\nusing it, but it was easy to implement (in fact, I'd have to write to\ndisallow it, since the include magic is in the callback handler).\n\n-Peff\n"},{"id":"183140","messageId":"20120126230054.GC12855@sigill.intra.peff.net","threadId":"29448","inReplyTo":"7vbopq2mk9.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/4] config: allow including config from repository blobs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-01-26T23:00:54Z","receivedAt":"2012-01-26T23:00:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 26, 2012 at 01:14:46PM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > +You can also include configuration from a blob stored in your repository\n> > +by setting the special `include.ref` variable to the name of an object\n> > +containing your configuration data (in the same format as a regular\n> > +config file).\n> \n> Hmm, the concept is surely *fun*, but is this really worth it?\n> [you could just tell people to copy the file into .git]\n\nYes, that does work. I liked the idea of putting it in the repo, though,\nbecause it means you can actually _track_ the contents, including any\nlocal changes you make.\n\nSo yeah, if you are just going to copy it once, or even periodically, it\nis not that big an advantage. And the example I gave using \"git tag\" did\njust that. But I also wanted to allow more complex things, like this:\n\n # clone and inspect\n git clone git://example.com/project.git\n cd project\n git show origin:devtools/std_gitconfig\n\n # well, that looks pretty good. But I'd like to tweak something.\n git checkout -b config origin\n $EDITOR devtools/std_gitconfig\n git commit -a -m \"drop the foo option, which I hate\"\n\n # OK, let's use it now.\n git config include.ref config:devtools/std_gitconfig\n\n # Weeks pass. Somebody else updates the std_gitconfig.\n git fetch\n # let's inspect the changes\n git checkout config\n git diff @{u} -- devtools/std_gitconfig\n # looks good, let's merge (not copy!) them in\n git merge @{u}\n\nThis is obviously an advanced thing to be doing. But it's cool to me,\nbecause even if you aren't working on a shared project, it is a means of\nversioning your config.\n\n-Peff\n"},{"id":"183144","messageId":"CACBZZX5_qjC6WZsZ9hKvSR5vQJPs=jgWn-R4EnWZGVq+RvjRyg@mail.gmail.com","threadId":"29448","inReplyTo":"20120126073752.GA30474@sigill.intra.peff.net","subject":"Re: [PATCH 1/4] config: add include directive","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2012-01-27T00:02:52Z","receivedAt":"2012-01-27T00:02:52Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Thu, Jan 26, 2012 at 08:37, Jeff King wrote:\n> This patch introduces an include directive for config files.\n> It looks like:\n>\n>  [include]\n>    path = /path/to/file\n\nVery nice, I'd been meaning to resurrect my gitconfig.d series, and\nthis series implements a lot of the structural changes needed for that\nsort of thing.\n\nWhat do you think of an option (e.g. include.gitconfig_d = true) that\nwould cause git to look in:\n\n /etc/gitconfig.d/*\n ~/.gitconfig.d/*\n .git/config.d/*\n\nAs well as the usual:\n\n /etc/gitconfig\n ~/.gitconfig\n .git/config\n\nIt would make including third-party config easy since you could just\nsymlink it in, and it would follow the convention of a lot of other\nprograms that have a foo and a foo.d directory.\n"},{"id":"183146","messageId":"20120127003241.GA15165@sigill.intra.peff.net","threadId":"29448","inReplyTo":"CACBZZX5_qjC6WZsZ9hKvSR5vQJPs=jgWn-R4EnWZGVq+RvjRyg@mail.gmail.com","subject":"Re: [PATCH 1/4] config: add include directive","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-01-27T00:32:41Z","receivedAt":"2012-01-27T00:32:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 27, 2012 at 01:02:52AM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> On Thu, Jan 26, 2012 at 08:37, Jeff King wrote:\n> > This patch introduces an include directive for config files.\n> > It looks like:\n> >\n> >  [include]\n> >    path = /path/to/file\n> \n> Very nice, I'd been meaning to resurrect my gitconfig.d series, and\n> this series implements a lot of the structural changes needed for that\n> sort of thing.\n\nYeah, that seems like a reasonable thing to do. It could make life\neasier for package managers (I think the only reason it has not come up\nmuch is that there simply isn't a lot of third-party git config).\n\n> What do you think of an option (e.g. include.gitconfig_d = true) that\n> would cause git to look in:\n> \n> /etc/gitconfig.d/*\n> ~/.gitconfig.d/*\n> .git/config.d/*\n\nHmm. Is that really worth having an option? I.e., why not just always\ncheck those directories?\n\nI could see having\n\n [include]\n dir = /path/to/gitconfig.d\n\nfor non-standard directories, though (or perhaps even simpler, the\n\"path\" directive should auto-detect a file versus a directory. Similarly\nthe \"ref\" form could detect and expand a tree).\n\n-Peff\n"},{"id":"183148","messageId":"7vsjj20yog.fsf@alter.siamese.dyndns.org","threadId":"29448","inReplyTo":"20120126230054.GC12855@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] config: allow including config from repository blobs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-27T00:35:59Z","receivedAt":"2012-01-27T00:35:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> So yeah, if you are just going to copy it once, or even periodically, it\n> is not that big an advantage. And the example I gave using \"git tag\" did\n> just that. But I also wanted to allow more complex things, like this:\n>\n> # clone and inspect\n> git clone git://example.com/project.git\n> cd project\n> git show origin:devtools/std_gitconfig\n>\n> # well, that looks pretty good. But I'd like to tweak something.\n> git checkout -b config origin\n> $EDITOR devtools/std_gitconfig\n> git commit -a -m \"drop the foo option, which I hate\"\n>\n> # OK, let's use it now.\n> git config include.ref config:devtools/std_gitconfig\n>\n> # Weeks pass. Somebody else updates the std_gitconfig.\n> git fetch\n> # let's inspect the changes\n> git checkout config\n> git diff @{u} -- devtools/std_gitconfig\n> # looks good, let's merge (not copy!) them in\n> git merge @{u}\n>\n> This is obviously an advanced thing to be doing.\n\nThe \"which *I* hate\" in the log message makes it sound as if it is a\npersonal preference, but in fact this is more about maintaining the\nrecommended configuration among participants, no? And if you have the\nsource of the configuration on a branch so that people can work on it\namong themselves, then \"config.path = ../devtools/std_gitconfig\" should be\nsufficient, no?\n\nThe pros-and-cons between the volume of the change to read include from\nblobs and the benefit illustrated in the use case did not look too good to\nme, at least from the messages in this thread so far.\n"},{"id":"183150","messageId":"20120127004902.GA15257@sigill.intra.peff.net","threadId":"29448","inReplyTo":"7vsjj20yog.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/4] config: allow including config from repository blobs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-01-27T00:49:02Z","receivedAt":"2012-01-27T00:49:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 26, 2012 at 04:35:59PM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > So yeah, if you are just going to copy it once, or even periodically, it\n> > is not that big an advantage. And the example I gave using \"git tag\" did\n> > just that. But I also wanted to allow more complex things, like this:\n> >\n> > # clone and inspect\n> > git clone git://example.com/project.git\n> > cd project\n> > git show origin:devtools/std_gitconfig\n> >\n> > # well, that looks pretty good. But I'd like to tweak something.\n> > git checkout -b config origin\n> > $EDITOR devtools/std_gitconfig\n> > git commit -a -m \"drop the foo option, which I hate\"\n> >\n> > # OK, let's use it now.\n> > git config include.ref config:devtools/std_gitconfig\n> >\n> > # Weeks pass. Somebody else updates the std_gitconfig.\n> > git fetch\n> > # let's inspect the changes\n> > git checkout config\n> > git diff @{u} -- devtools/std_gitconfig\n> > # looks good, let's merge (not copy!) them in\n> > git merge @{u}\n> >\n> > This is obviously an advanced thing to be doing.\n> \n> The \"which *I* hate\" in the log message makes it sound as if it is a\n> personal preference, but in fact this is more about maintaining the\n> recommended configuration among participants, no?\n\nNo, I meant it explicitly to be about this single user hating it. Note\nhow the resulting commits are never pushed. It is purely a local\noverride, but with the added bonus that history is tracked so you can\nmerge in further changes from upstream.\n\nOf course, you could also share it with others, or do whatever. Once\nit's tracked by git, you can be as flexible as you like.\n\n> And if you have the source of the configuration on a branch so that\n> people can work on it among themselves, then \"config.path =\n> ../devtools/std_gitconfig\" should be sufficient, no?\n\nYes, you _could_ just keep it in a branch, merge upstream's changes into\nthe branch, and then periodically copy it out to your .git directory.\nBut this removes that final step.\n\nIt also does allow \"[include]ref = origin/meta:gitconfig\" if you want to\nlive dangerously. I consider that a feature, because it lets the user\nmake the security tradeoff they deem appropriate. Yes, I want to have\ngit be secure by default, and yes I want to encourage awareness of the\nissues in the documentation for the feature. But I suspect in practice\nthat many people fetch changes and run \"make\" without looking at them,\nwhich is basically the exact same hole. If a user has already accepted\nthat risk, why deny them the convenience of accepting it somewhere else?\n\n> The pros-and-cons between the volume of the change to read include from\n> blobs and the benefit illustrated in the use case did not look too good to\n> me, at least from the messages in this thread so far.\n\nI didn't think the read-from-blob code was very big or complex (most of\nthe refactoring was to support parsing an arbitrary buffer, but I think\nthat may be a good thing to have in the long run, anyway).\n\nBut I guess that is all a matter of opinion.\n\n-Peff\n"},{"id":"183154","messageId":"CACsJy8ASXrmBvxj4DxjGiqYhPkr1Yp02CAyMqKrfyfrzaAw-2g@mail.gmail.com","threadId":"29448","inReplyTo":"4F211C0C.7060400@viscovery.net","subject":"Re: [PATCH 4/4] config: allow including config from repository blobs","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-01-27T03:47:29Z","receivedAt":"2012-01-27T03:47:29Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Jan 26, 2012 at 4:25 PM, Johannes Sixt wrote:\n> Am 1/26/2012 8:42, schrieb Jeff King:\n>> +static int handle_ref_include(const char *ref, void *data)\n>> +{\n>> +     unsigned char sha1[20];\n>> +     char *buf;\n>> +     unsigned long size;\n>> +     enum object_type type;\n>> +     int ret;\n>> +\n>> +     if (get_sha1(ref, sha1))\n>> +             return 0;\n>> +     buf = read_sha1_file(sha1, &type, &size);\n>> +     if (!buf)\n>> +             return error(\"unable to read include ref '%s'\", ref);\n>> +     if (type != OBJ_BLOB)\n>> +             return error(\"include ref '%s' is not a blob\", ref);\n>> +\n>> +     ret = git_config_from_buffer(git_config_include, data, ref, buf, size);\n>> +     free(buf);\n>> +     return ret;\n>> +}\n>\n> What happens if a ref cannot be resolved, for example due to repository\n> corruption? Does git just emit an error and then carries on, or does it\n> always die? Can I run at least git-fsck in such a case?\n\nMoreover, if I specify sha-1 in the config (it's discouraged but not\nforbidden from the code), can git-prune remove the blob?\n-- \nDuy\n"},{"id":"183155","messageId":"CACsJy8DPcgJtw_xxqZ2pOtpV-w=PAVQ7uHs+CJ+ynECYdL50og@mail.gmail.com","threadId":"29448","inReplyTo":"20120126074208.GD30474@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] config: allow including config from repository blobs","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-01-27T04:01:00Z","receivedAt":"2012-01-27T04:01:00Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Jan 26, 2012 at 2:42 PM, Jeff King wrote:\n> +Security Considerations\n> +~~~~~~~~~~~~~~~~~~~~~~~\n> +\n> +Because git configuration may cause git to execute arbitrary shell\n> +commands, it is important to verify any configuration you receive over\n> +the network. In particular, it is not a good idea to point `include.ref`\n> +directly at a remote tracking branch like `origin/master:shared-config`.\n> +After a fetch, you have no way of inspecting the shared-config you have\n> +just received without running git (and thus respecting the downloaded\n> +config). Instead, you can create a local tag representing the last\n> +verified version of the config, and only update the tag after inspecting\n> +any new content.\n\nIt may be a good idea to tell users the ref include.ref points to has\nbeen updated at the end of git-fetch. Showing a diff is even better.\n-- \nDuy\n"},{"id":"183158","messageId":"4F223115.3050004@alum.mit.edu","threadId":"29448","inReplyTo":"20120126073752.GA30474@sigill.intra.peff.net","subject":"Re: [PATCH 1/4] config: add include directive","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-01-27T05:07:33Z","receivedAt":"2012-01-27T05:07:33Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 01/26/2012 08:37 AM, Jeff King wrote:\n> [...]\n> This patch introduces an include directive for config files.\n> It looks like:\n> \n> [include]\n> path = /path/to/file\n\nI like it.\n\n> diff --git a/cache.h b/cache.h\n> index 10afd71..21bbb0a 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -1138,6 +1138,12 @@ extern const char *get_commit_output_encoding(void);\n> \n> extern int git_config_parse_parameter(const char *, config_fn_t fn, void *data);\n> \n> +struct git_config_include_data {\n> +\tconfig_fn_t fn;\n> +\tvoid *data;\n> +};\n> +int git_config_include(const char *name, const char *value, void *vdata);\n> +\n> extern const char *config_exclusive_filename;\n> \n> #define MAX_GITNAME (1000)\n\nHow about a short comment or two?\n\n> diff --git a/config.c b/config.c\n> index 40f9c6d..a6966c1 100644\n> --- a/config.c\n> +++ b/config.c\n> [...]\n> +int git_config_include(const char *name, const char *value, void *vdata)\n> +{\n> +\tconst struct git_config_include_data *data = vdata;\n> +\tconst char *type;\n> +\tint ret;\n> +\n> +\t/*\n> +\t * Pass along all values, including \"include\" directives; this makes it\n> +\t * possible to query information on the includes themselves.\n> +\t */\n> +\tret = data->fn(name, value, data->data);\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\n> +\tif (prefixcmp(name, \"include.\"))\n> +\t\treturn ret;\n> +\ttype = strrchr(name, '.') + 1;\n> +\n> +\tif (!strcmp(type, \"path\"))\n> +\t\tret = handle_path_include(value, vdata);\n> +\n> +\treturn ret;\n> +}\n> +\n\nDoesn't this code accept all keys of the form \"include\\.(.*\\.)?path\"\n(e.g., \"include.foo.path\")? If that is your intention, then the\ndocumentation should be fixed. If not, then a single strcmp(name,\n\"include.path\") would seem sufficient.\n\n> int git_config_early(config_fn_t fn, void *data, const char *repo_config)\n> {\n> \tint ret = 0, found = 0;\n> \tconst char *home = NULL;\n> +\tstruct git_config_include_data inc;\n> +\n> +\tinc.fn = fn;\n> +\tinc.data = data;\n> +\tfn = git_config_include;\n> +\tdata = &inc;\n> \n> \t/* Setting $GIT_CONFIG makes git read _only_ the given config file. */\n> \tif (config_exclusive_filename)\n\nThe comment just after your addition should be adjusted, since now \"the\ngiven config file and any files that it includes\" are read.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"183159","messageId":"7vk44d1zww.fsf@alter.siamese.dyndns.org","threadId":"29448","inReplyTo":"20120126225140.GB12855@sigill.intra.peff.net","subject":"Re: [PATCH 1/4] config: add include directive","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-27T05:23:59Z","receivedAt":"2012-01-27T05:23:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> And then because of 1a and 2a, most programs should Just Work without\n> any changes, but because of 1b and 2b, any special uses will have to\n> decide manually whether they would want to allow includes.\n>\n> Does that make sense?\n\nIn short, the \"git config\" interface defaults to \"--no-includes\" when\nreading from an explicit file with \"-f\" and \"--includes\" otherwise, which\nsounds like a 100% sensible default to me.\n\n> I had a similar thought while writing it, but hoped the sentence after\n> (that you snipped) would make it clear.\n\nI think the whole paragraph makes it reasonably clear; I was merely being\npedantic to see if you or others can come up with a clear and simple way\nto rephrase it that can also satisfy such pedantry.\n\n> How about:\n>\n> The included file is processed immediately, before any other\n> directives from the surrounding file.\n>\n> What I wanted to make clear there is the ordering, which sometimes\n> matters.\n\nHmm, I think the original is probably easier to read.\n\n> The one use I think is to bundle a bunch of related config options, and\n> then turn them on selectively.\n> ...\n> but with this patch, you can do:\n>\n> cat >~/.gitconfig.foo <<-\\EOF\n> [foo]\n> one = 1\n> two = 2\n> EOF\n> git -c include.path=$HOME/.gitconfig.foo blah\n\nThat is quite a sensible use case actually.\n\nThanks.\n"},{"id":"183160","messageId":"7vd3a51zlb.fsf@alter.siamese.dyndns.org","threadId":"29448","inReplyTo":"20120127004902.GA15257@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] config: allow including config from repository blobs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-27T05:30:56Z","receivedAt":"2012-01-27T05:30:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> No, I meant it explicitly to be about this single user hating it. Note\n> how the resulting commits are never pushed. It is purely a local\n> override, but with the added bonus that history is tracked so you can\n> merge in further changes from upstream.\n> ...\n> It also does allow \"[include]ref = origin/meta:gitconfig\" if you want to\n> live dangerously. I consider that a feature, because it lets the user\n> make the security tradeoff they deem appropriate.\n\nWhile I do not think origin/meta:config is a sensible default, I actually\ndo think that:\n\n\t[include]\n \tref = meta:gitconfig\n [branch \"meta\"]\n\t\tremote = origin\n \tmerge = refs/heads/meta\n\nmakes some sense. The earlier example with the in-tree dev_tools/config in\nthe same line of history as the usual source material to keep track of\nprivate changes (\"this single user hating it\") was not realistic as it\nforbids the user from sharing the rest of the source once she decides to\nfork the config preference.\n"},{"id":"183162","messageId":"20120127054216.GA23633@sigill.intra.peff.net","threadId":"29448","inReplyTo":"7vd3a51zlb.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/4] config: allow including config from repository blobs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-01-27T05:42:16Z","receivedAt":"2012-01-27T05:42:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 26, 2012 at 09:30:56PM -0800, Junio C Hamano wrote:\n\n> While I do not think origin/meta:config is a sensible default, I actually\n> do think that:\n> \n> \t[include]\n> \tref = meta:gitconfig\n> [branch \"meta\"]\n> \t\tremote = origin\n> \tmerge = refs/heads/meta\n> \n> makes some sense. The earlier example with the in-tree dev_tools/config in\n> the same line of history as the usual source material to keep track of\n> private changes (\"this single user hating it\") was not realistic as it\n> forbids the user from sharing the rest of the source once she decides to\n> fork the config preference.\n\nI don't think having it in-tree makes a difference. I can fork the\nregular tree into my config branch, and it contains only my config\nchanges. If I want to share config changes with people, then I do so by\nsharing that branch. But it need not have any impact on the \"real\"\nbranch I create from the regular tree. The fact that the rest of the\nsource files are in the config branch are irrelevant.\n\nThat being said, I think it would be nicer for projects to carry meta\ninformation like this out-of-tree in a special ref. It's just simpler to\nwork with, and it means the project's source isn't polluted with extra\njunk.\n\n-Peff\n"},{"id":"183164","messageId":"20120127055405.GB23633@sigill.intra.peff.net","threadId":"29448","inReplyTo":"4F223115.3050004@alum.mit.edu","subject":"Re: [PATCH 1/4] config: add include directive","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-01-27T05:54:05Z","receivedAt":"2012-01-27T05:54:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 27, 2012 at 06:07:33AM +0100, Michael Haggerty wrote:\n\n> > +struct git_config_include_data {\n> > +\tconfig_fn_t fn;\n> > +\tvoid *data;\n> > +};\n> > +int git_config_include(const char *name, const char *value, void *vdata);\n> > +\n> > extern const char *config_exclusive_filename;\n> > \n> > #define MAX_GITNAME (1000)\n> \n> How about a short comment or two?\n\nI had originally planned to document this somewhat non-intuitive\ninterface in the config API documentation. But then I noticed we didn't\nhave such a document, and promptly forgot about documenting.\n\nI'd rather have an API document, but I admit that the thought of\ndescribing the config interface frightens me. It has some nasty corners.\nBut maybe starting one with the non-scary bits would be better, and then\nI could add this to it.\n\n> > +int git_config_include(const char *name, const char *value, void *vdata)\n> > +{\n> > +\tconst struct git_config_include_data *data = vdata;\n> > +\tconst char *type;\n> > +\tint ret;\n> > +\n> > +\t/*\n> > +\t * Pass along all values, including \"include\" directives; this makes it\n> > +\t * possible to query information on the includes themselves.\n> > +\t */\n> > +\tret = data->fn(name, value, data->data);\n> > +\tif (ret < 0)\n> > +\t\treturn ret;\n> > +\n> > +\tif (prefixcmp(name, \"include.\"))\n> > +\t\treturn ret;\n> > +\ttype = strrchr(name, '.') + 1;\n> > +\n> > +\tif (!strcmp(type, \"path\"))\n> > +\t\tret = handle_path_include(value, vdata);\n> > +\n> > +\treturn ret;\n> > +}\n> > +\n> \n> Doesn't this code accept all keys of the form \"include\\.(.*\\.)?path\"\n> (e.g., \"include.foo.path\")? If that is your intention, then the\n> documentation should be fixed. If not, then a single strcmp(name,\n> \"include.path\") would seem sufficient.\n\nIt does. I was considering (but haven't yet written) a patch that would\nallow for conditional inclusion, like:\n\n [include \"foo\"]\n path = /some/file\n\nwhere \"foo\" would be the condition. Specifically, I wanted to enable\nincludes when certain features were available in the parsing version of\ngit. For example, the pager.* variables were originally bools, but later\nlearned to take arbitrary strings. So my config with arbitrary strings\nworks on modern git, but causes earlier versions of git to barf. I'd\nlike to be able to do something like:\n\n [include \"per-command-pager-strings\"]\n path = /path/to/my/pager.config\n\nwhere \"per-command-pager-strings\" would be a flag known internally to\ngit versions that support that feature.\n\nI didn't end up implementing it right away, because of course those same\nearly versions of git also don't know about \"include\" at all. So using\nany include effectively works as a conditional for that particular\nfeature. But as new incompatible config semantics are added\npost-include, they could take advantage of a similar scheme.\n\nSo I wanted to leave the code open to adding such a patch later, if and\nwhen it becomes useful. That being said, the code above is wrong.\nFor my scheme to work, versions of git that handle includes but don't\nhave the conditional-include patch (if it ever comes) would want to\nexplicitly disallow includes with subsections.\n\nI'll fix it in the re-roll.\n\n> > +\tstruct git_config_include_data inc;\n> > +\n> > +\tinc.fn = fn;\n> > +\tinc.data = data;\n> > +\tfn = git_config_include;\n> > +\tdata = &inc;\n> > \n> > \t/* Setting $GIT_CONFIG makes git read _only_ the given config file. */\n> > \tif (config_exclusive_filename)\n> \n> The comment just after your addition should be adjusted, since now \"the\n> given config file and any files that it includes\" are read.\n\nWill do.\n\n-Peff\n"},{"id":"183165","messageId":"20120127055515.GC23633@sigill.intra.peff.net","threadId":"29448","inReplyTo":"7vk44d1zww.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/4] config: add include directive","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-01-27T05:55:15Z","receivedAt":"2012-01-27T05:55:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 26, 2012 at 09:23:59PM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > And then because of 1a and 2a, most programs should Just Work without\n> > any changes, but because of 1b and 2b, any special uses will have to\n> > decide manually whether they would want to allow includes.\n> >\n> > Does that make sense?\n> \n> In short, the \"git config\" interface defaults to \"--no-includes\" when\n> reading from an explicit file with \"-f\" and \"--includes\" otherwise, which\n> sounds like a 100% sensible default to me.\n\nYes, exactly. Thank you for explaining it in about 1/10 the words I\nneeded to use. :)\n\nI'll put that behavior in the re-roll.\n\n> > How about:\n> >\n> > The included file is processed immediately, before any other\n> > directives from the surrounding file.\n> >\n> > What I wanted to make clear there is the ordering, which sometimes\n> > matters.\n> \n> Hmm, I think the original is probably easier to read.\n\nOK, then I'll leave it unless somebody else wants to come up with\nsomething brilliant.\n\n-Peff\n"},{"id":"183166","messageId":"20120127055722.GD23633@sigill.intra.peff.net","threadId":"29448","inReplyTo":"CACsJy8ASXrmBvxj4DxjGiqYhPkr1Yp02CAyMqKrfyfrzaAw-2g@mail.gmail.com","subject":"Re: [PATCH 4/4] config: allow including config from repository blobs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-01-27T05:57:22Z","receivedAt":"2012-01-27T05:57:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 27, 2012 at 10:47:29AM +0700, Nguyen Thai Ngoc Duy wrote:\n\n> > What happens if a ref cannot be resolved, for example due to repository\n> > corruption? Does git just emit an error and then carries on, or does it\n> > always die? Can I run at least git-fsck in such a case?\n> \n> Moreover, if I specify sha-1 in the config (it's discouraged but not\n> forbidden from the code), can git-prune remove the blob?\n\nYes. I don't think we want to get into connectivity guarantees for\nconfig (because they can be quite complex, and involve files totally\noutside the repo). I think it's OK for the user to be responsible for\neither using a ref, or making sure that a bare sha1 they point to is\nreachable from a ref.\n\n-Peff\n"},{"id":"183167","messageId":"20120127055930.GE23633@sigill.intra.peff.net","threadId":"29448","inReplyTo":"CACsJy8DPcgJtw_xxqZ2pOtpV-w=PAVQ7uHs+CJ+ynECYdL50og@mail.gmail.com","subject":"Re: [PATCH 4/4] config: allow including config from repository blobs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-01-27T05:59:30Z","receivedAt":"2012-01-27T05:59:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 27, 2012 at 11:01:00AM +0700, Nguyen Thai Ngoc Duy wrote:\n\n> On Thu, Jan 26, 2012 at 2:42 PM, Jeff King wrote:\n> > +Security Considerations\n> > +~~~~~~~~~~~~~~~~~~~~~~~\n> > +\n> > +Because git configuration may cause git to execute arbitrary shell\n> > +commands, it is important to verify any configuration you receive over\n> > +the network. In particular, it is not a good idea to point `include.ref`\n> > +directly at a remote tracking branch like `origin/master:shared-config`.\n> > +After a fetch, you have no way of inspecting the shared-config you have\n> > +just received without running git (and thus respecting the downloaded\n> > +config). Instead, you can create a local tag representing the last\n> > +verified version of the config, and only update the tag after inspecting\n> > +any new content.\n> \n> It may be a good idea to tell users the ref include.ref points to has\n> been updated at the end of git-fetch. Showing a diff is even better.\n\nI really didn't want to have to let other parts of git know or care\nabout this mechanism. At least not for now. In the long run, I have no\nproblem with some porcelain growing up around the feature to make it\nsimpler to use. But I'd really rather focus on the bare-bones\nfunctionality for now, see how people use it, and then find ways to\naddress deficiencies in their workflows once we have data.\n\n-Peff\n"},{"id":"183171","messageId":"4F2251D4.9060005@viscovery.net","threadId":"29448","inReplyTo":"20120127054216.GA23633@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] config: allow including config from repository blobs","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2012-01-27T07:27:16Z","receivedAt":"2012-01-27T07:27:16Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 1/27/2012 6:42, schrieb Jeff King:\n> That being said, I think it would be nicer for projects to carry meta\n> information like this out-of-tree in a special ref. It's just simpler to\n> work with, and it means the project's source isn't polluted with extra\n> junk.\n\nReally? I doubt that carrying configuration in a special ref outside the\nnormal contents will have any practical relevance:\n\nTo manage such a config file would mean to switch to a branch with\nentirely different contents. But before you can test the new configuration\nin action, you have to commit, switch branches, which exchanges the\nworktree completely; and if the config change didn't work out, repeat the\nprocess (and if we are talking about source code repository, this usally\nincludes a complete rebuild). Sure, you could keep the config branch in a\nseparate repository, but, again, how do you test an updated configuration?\nIt is not funny, and nobody will go this route.\n\nWhich raises doubts about the usefulness of the include.ref feature.\n\n-- Hannes\n"},{"id":"183178","messageId":"CACBZZX5MH7bEY_0081zTQbofQ5FCHGR7AQBpxXXd2c9oFUo=+A@mail.gmail.com","threadId":"29448","inReplyTo":"20120127003241.GA15165@sigill.intra.peff.net","subject":"Re: [PATCH 1/4] config: add include directive","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2012-01-27T09:33:32Z","receivedAt":"2012-01-27T09:33:32Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Fri, Jan 27, 2012 at 01:32, Jeff King wrote:\n> On Fri, Jan 27, 2012 at 01:02:52AM +0100, Ævar Arnfjörð Bjarmason wrote:\n>\n>> On Thu, Jan 26, 2012 at 08:37, Jeff King wrote:\n>> > This patch introduces an include directive for config files.\n>> > It looks like:\n>> >\n>> >  [include]\n>> >    path = /path/to/file\n>>\n>> Very nice, I'd been meaning to resurrect my gitconfig.d series, and\n>> this series implements a lot of the structural changes needed for that\n>> sort of thing.\n>\n> Yeah, that seems like a reasonable thing to do. It could make life\n> easier for package managers (I think the only reason it has not come up\n> much is that there simply isn't a lot of third-party git config).\n>\n>> What do you think of an option (e.g. include.gitconfig_d = true) that\n>> would cause git to look in:\n>>\n>>     /etc/gitconfig.d/*\n>>     ~/.gitconfig.d/*\n>>     .git/config.d/*\n>\n> Hmm. Is that really worth having an option? I.e., why not just always\n> check those directories?\n\nYou're right, always just including those directories is a much better\noption, an extra stat() doesn't cost us much.\n\nThanks again for working on this.\n"},{"id":"183180","messageId":"CACBZZX59sur4_61LkN_sMOvXQ4Jdnt1P8O-UOgm0SooBQpjFdQ@mail.gmail.com","threadId":"29448","inReplyTo":"20120126073547.GA28689@sigill.intra.peff.net","subject":"Re: [RFC/PATCH 0/4] config include directives","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2012-01-27T09:51:34Z","receivedAt":"2012-01-27T09:51:34Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Thu, Jan 26, 2012 at 08:35, Jeff King wrote:\n> This series provides a way for config files to include other config\n> files in two ways:\n>\n>  1. From other files in the filesystem. This is implemented by patch 1\n>     below, and is hopefully straightforward and uncontroversial.  See\n>     that patch for more rationale.\n>\n>  2. From blobs in the repo. This is implemented by patch 4, with\n>     patches 2 and 3 providing the necessary refactoring. This\n>     is one way of implementing the often asked-for \"respect shared\n>     config inside the repo\" feature, but attempts to mitigate some of\n>     the security concerns. The interface for using it safely is a bit\n>     raw, but I think it's a sane building block, and somebody could\n>     write a fancier shared-config updater on top of it if they wanted\n>     to.\n>\n>  [1/4]: config: add include directive\n>  [2/4]: config: factor out config file stack management\n>  [3/4]: config: support parsing config data from buffers\n>  [4/4]: config: allow including config from repository blobs\n\nI expect you've thought about this, but our current API is (from\nadd.c):\n\n\tgit_config(add_config, NULL);\n\nFollowed by:\n\n static int add_config(const char *var, const char *value, void *cb)\n {\n \tif (!strcmp(var, \"add.ignoreerrors\") ||\n \t !strcmp(var, \"add.ignore-errors\")) {\n \t\tignore_add_errors = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n \treturn git_default_config(var, value, cb);\n }\n\nI.e. that function gets called with one key at a time, and stashes it\nto a local value.\n\nIf you write the function like that it means your patch series just\nworks since values encountered later will override earlier ones, but\nhave you checked git's code to make sure we don't have anything like:\n\n static int ignore_add_errors_is_set = 0;\n static int add_config(const char *var, const char *value, void *cb)\n {\n \tif (!ignore_add_errors_is_set &&\n (!strcmp(var, \"add.ignoreerrors\") ||\n \t !strcmp(var, \"add.ignore-errors\"))) {\n \t\tignore_add_errors = git_config_bool(var, value);\n ignore_add_errors_is_set = 1;\n \t\treturn 0;\n \t}\n \treturn git_default_config(var, value, cb);\n }\n\nWhich would mean that the include config support would be silently\nignored.\n"},{"id":"183192","messageId":"4F22D8EA.6080003@web.de","threadId":"29448","inReplyTo":"20120126225140.GB12855@sigill.intra.peff.net","subject":"Re: [PATCH 1/4] config: add include directive","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-01-27T17:03:38Z","receivedAt":"2012-01-27T17:03:38Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 26.01.2012 23:51, schrieb Jeff King:\n> However, I didn't think about the fact that git-submodule.sh would be\n> calling git-config separately, and that is accidentally changed by my\n> patch. Even if we changed git-submodule to use \"git config\n> --no-includes\" that would break any third-party scripts that use \"git\n> config\" to read git-like files.\n> \n> But it would be nice for callers doing \"git config foo.bar\" to get the\n> includes by default. So maybe the right rule is:\n> \n> 1. In C:\n> a. git_config() respects includes automatically.\n> b. other callers do not do so automatically (e.g., gitmodules via\n> submodule.c).\n> \n> (i.e., what is implemented by this patch)\n> \n> 2. Callers of git-config:\n> a. respect includes for lookup that checks all of the \"normal\"\n> config spots in sequence: .git/config, ~/.gitconfig, and\n> /etc/gitconfig. These are the shell equivalent of calling\n> git_config().\n> \n> b. when we are looking in a specific file (via GIT_CONFIG or \"git\n> config -f\"), do not respect includes (but allow --includes if\n> the caller chooses). This specific file may be something like\n> .gitmodules. Or perhaps somebody is saying \"no, I really just\n> want to know what is in _this_ file, not what the config\n> ecosystem tells me in general\".\n> \n> And then because of 1a and 2a, most programs should Just Work without\n> any changes, but because of 1b and 2b, any special uses will have to\n> decide manually whether they would want to allow includes.\n> \n> Does that make sense?\n\nTo me it really does. It lets submodule.c:gitmodules_config and\n\"git config -f .gitmodules\" behave in the same way, which is very\nimportant for consistent behavior between the submodule script and\nthe submodule functionality that is already handled in c. And I don't\nknow of a use case for includes in .gitmodules (the main reason for\nadding includes seems to be to enable users to have configuration\nstored in the repo, which the .gitmodules file already is. And if it\nis about having out of repo configuration blended in, .gitmodules\nsettings are always overridden by those in .git/config, and you can\nuse includes there).\n\nThe only thing I'm not so sure about is the GIT_CONFIG case. I don't\nknow if using this is rather a \"I moved my config there, but please\nrespect includes there too\" or a \"I want git config to look at a\ncompletely different file\" use case. Probably both. But also the\nquestion of where to look for relative paths seems not so easy to\nanswer for the GIT_CONFIG case, so it might be best to just disable\nincludes there too.\n"},{"id":"183193","messageId":"20120127173408.GA3053@sigill.intra.peff.net","threadId":"29448","inReplyTo":"CACBZZX59sur4_61LkN_sMOvXQ4Jdnt1P8O-UOgm0SooBQpjFdQ@mail.gmail.com","subject":"Re: [RFC/PATCH 0/4] config include directives","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-01-27T17:34:09Z","receivedAt":"2012-01-27T17:34:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 27, 2012 at 10:51:34AM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> If you write the function like that it means your patch series just\n> works since values encountered later will override earlier ones, but\n> have you checked git's code to make sure we don't have anything like:\n> \n> static int ignore_add_errors_is_set = 0;\n> static int add_config(const char *var, const char *value, void *cb)\n> {\n> \tif (!ignore_add_errors_is_set &&\n> (!strcmp(var, \"add.ignoreerrors\") ||\n> \t !strcmp(var, \"add.ignore-errors\"))) {\n> \t\tignore_add_errors = git_config_bool(var, value);\n> ignore_add_errors_is_set = 1;\n> \t\treturn 0;\n> \t}\n> \treturn git_default_config(var, value, cb);\n> }\n> \n> Which would mean that the include config support would be silently\n> ignored.\n\nI'm not sure what the issue is. If you write code like this, it will\nalready ignore the second invocation when it is found later in the same\nfile, or when it is found in a later file (i.e., in both .git/config and\n.gitconfig). So I don't think includes introduce a new problem with\nrespect to code like this (and no, I didn't check exhaustively, but I\ndon't recall seeing code like this in git).\n\nA bigger potential problem is multi-key values that form lists. For\nexample, I cannot use a later \"remote.foo.url\" line to override an\nearlier one; instead, it gets appended to the list of URLs for \"foo\".\nIn practice, it's not a problem because the list-like options don't tend\nto be found in multiple places. And again, this is not a new problem of\nincludes, since we already handle multiple files.\n\nAccidentally including the same file twice would cause duplicates for\nmulti-key values. But I'm going to take Junio's suggestion to avoid\nincluding the same file twice (which also prevents infinite loops due to\ncycles).\n\n-Peff\n"},{"id":"183208","messageId":"7v39b0zqqh.fsf@alter.siamese.dyndns.org","threadId":"29448","inReplyTo":"4F2251D4.9060005@viscovery.net","subject":"Re: [PATCH 4/4] config: allow including config from repository blobs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-27T23:10:30Z","receivedAt":"2012-01-27T23:10:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Am 1/27/2012 6:42, schrieb Jeff King:\n>> That being said, I think it would be nicer for projects to carry meta\n>> information like this out-of-tree in a special ref. It's just simpler to\n>> work with, and it means the project's source isn't polluted with extra\n>> junk.\n>\n> Really? I doubt that carrying configuration in a special ref outside the\n> normal contents will have any practical relevance:\n>\n> To manage such a config file would mean to switch to a branch with\n> entirely different contents. But before you can test the new configuration\n> in action, you have to commit, switch branches, which exchanges the\n> worktree completely; and if the config change didn't work out, repeat the\n> process (and if we are talking about source code repository, this usally\n> includes a complete rebuild). Sure, you could keep the config branch in a\n> separate repository, but, again, how do you test an updated configuration?\n> It is not funny, and nobody will go this route.\n>\n> Which raises doubts about the usefulness of the include.ref feature.\n\nHmm, good point.\n\nWhat I envisioned, when I said \"meta:gitconfig\" might make sense, was to\ndo something like:\n\n * have a separate worktree via git-new-worktree in meta/, so that you\n do not have to switch away from the \"source\" branch and trash the\n working tree for it; and\n\n * update meta/gitconfig, perhaps make commit there, and possibly push\n it back for public consuption.\n\nIn other words, I think \"you *could* keep the config branch in a separate\nrepository\" is more like \"you would most likely want to have a separate\ncheckout of the config branch for this 'meta' branch to be useful\".\n\nAnd at that point, as you said, setting include.path = meta/gitconfig\n(with possibly adding meta/ in .git/info/exclude) would be far more\npleasant, because you would have a chance to experiment your changes to\nthe file before committing it.\n\nSo having include.ref, while it is a fun thought experiment, would not\nhelp very much in the real life.\n"}]}