{"thread":{"id":"35875","subject":"[PATCH 3/3] config: teach \"git config --file -\" to read from the standard input","startedAt":"2014-02-14T23:37:31Z","lastAt":"2014-02-18T23:02:46Z","messageCount":8,"participants":["Kirill A. Shutemov","Eric Sunshine","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"234827","messageId":"1392421053-937-1-git-send-email-kirill@shutemov.name","threadId":"35875","inReplyTo":null,"subject":"[PATCH 1/3] builtin/config.c: rename check_blob_write() -> check_write()","fromName":"Kirill A. Shutemov","fromEmail":"kirill@shutemov.name","sentAt":"2014-02-14T23:37:31Z","receivedAt":"2014-02-14T23:37:31Z","isPatch":true,"sender":{"key":"kirill@shutemov.name","avatar":"https://avatars.githubusercontent.com/u/588661?v=4"},"body":"The function will be reused to check for other conditions which prevent\nwrite.\n\nSigned-off-by: Kirill A. Shutemov <kirill@shutemov.name>\n---\n builtin/config.c | 18 +++++++++---------\n 1 file changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/config.c b/builtin/config.c\nindex 92ebf23f0a9a..a7c55e68883c 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -362,7 +362,7 @@ static int get_colorbool(int print)\n \t\treturn get_colorbool_found ? 0 : 1;\n }\n \n-static void check_blob_write(void)\n+static void check_write(void)\n {\n \tif (given_config_blob)\n \t\tdie(\"writing config blobs is not supported\");\n@@ -572,7 +572,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t}\n \telse if (actions == ACTION_SET) {\n \t\tint ret;\n-\t\tcheck_blob_write();\n+\t\tcheck_write();\n \t\tcheck_argc(argc, 2, 2);\n \t\tvalue = normalize_value(argv[0], argv[1]);\n \t\tret = git_config_set_in_file(given_config_file, argv[0], value);\n@@ -582,21 +582,21 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\treturn ret;\n \t}\n \telse if (actions == ACTION_SET_ALL) {\n-\t\tcheck_blob_write();\n+\t\tcheck_write();\n \t\tcheck_argc(argc, 2, 3);\n \t\tvalue = normalize_value(argv[0], argv[1]);\n \t\treturn git_config_set_multivar_in_file(given_config_file,\n \t\t\t\t\t\t       argv[0], value, argv[2], 0);\n \t}\n \telse if (actions == ACTION_ADD) {\n-\t\tcheck_blob_write();\n+\t\tcheck_write();\n \t\tcheck_argc(argc, 2, 2);\n \t\tvalue = normalize_value(argv[0], argv[1]);\n \t\treturn git_config_set_multivar_in_file(given_config_file,\n \t\t\t\t\t\t       argv[0], value, \"^$\", 0);\n \t}\n \telse if (actions == ACTION_REPLACE_ALL) {\n-\t\tcheck_blob_write();\n+\t\tcheck_write();\n \t\tcheck_argc(argc, 2, 3);\n \t\tvalue = normalize_value(argv[0], argv[1]);\n \t\treturn git_config_set_multivar_in_file(given_config_file,\n@@ -623,7 +623,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\treturn get_urlmatch(argv[0], argv[1]);\n \t}\n \telse if (actions == ACTION_UNSET) {\n-\t\tcheck_blob_write();\n+\t\tcheck_write();\n \t\tcheck_argc(argc, 1, 2);\n \t\tif (argc == 2)\n \t\t\treturn git_config_set_multivar_in_file(given_config_file,\n@@ -633,14 +633,14 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t\t      argv[0], NULL);\n \t}\n \telse if (actions == ACTION_UNSET_ALL) {\n-\t\tcheck_blob_write();\n+\t\tcheck_write();\n \t\tcheck_argc(argc, 1, 2);\n \t\treturn git_config_set_multivar_in_file(given_config_file,\n \t\t\t\t\t\t       argv[0], NULL, argv[1], 1);\n \t}\n \telse if (actions == ACTION_RENAME_SECTION) {\n \t\tint ret;\n-\t\tcheck_blob_write();\n+\t\tcheck_write();\n \t\tcheck_argc(argc, 2, 2);\n \t\tret = git_config_rename_section_in_file(given_config_file,\n \t\t\t\t\t\t\targv[0], argv[1]);\n@@ -651,7 +651,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t}\n \telse if (actions == ACTION_REMOVE_SECTION) {\n \t\tint ret;\n-\t\tcheck_blob_write();\n+\t\tcheck_write();\n \t\tcheck_argc(argc, 1, 1);\n \t\tret = git_config_rename_section_in_file(given_config_file,\n \t\t\t\t\t\t\targv[0], NULL);\n-- \n1.8.5.2\n"},{"id":"234826","messageId":"1392421053-937-2-git-send-email-kirill@shutemov.name","threadId":"35875","inReplyTo":"1392421053-937-1-git-send-email-kirill@shutemov.name","subject":"[PATCH 2/3] config: change git_config_with_options() interface","fromName":"Kirill A. Shutemov","fromEmail":"kirill@shutemov.name","sentAt":"2014-02-14T23:37:32Z","receivedAt":"2014-02-14T23:37:32Z","isPatch":true,"sender":{"key":"kirill@shutemov.name","avatar":"https://avatars.githubusercontent.com/u/588661?v=4"},"body":"We're going to have more options for config source.\n\nLet's alter git_config_with_options() interface to accept struct with\nall source options.\n\nSigned-off-by: Kirill A. Shutemov <kirill@shutemov.name>\n---\n builtin/config.c | 75 ++++++++++++++++++++++++++------------------------------\n cache.h          |  8 ++++--\n config.c         | 13 +++++-----\n 3 files changed, 47 insertions(+), 49 deletions(-)\n\ndiff --git a/builtin/config.c b/builtin/config.c\nindex a7c55e68883c..de41ba50e9ca 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -21,8 +21,7 @@ static char key_delim = ' ';\n static char term = '\\n';\n \n static int use_global_config, use_system_config, use_local_config;\n-static const char *given_config_file;\n-static const char *given_config_blob;\n+static struct git_config_source given_config_source;\n static int actions, types;\n static const char *get_color_slot, *get_colorbool_slot;\n static int end_null;\n@@ -55,8 +54,8 @@ static struct option builtin_config_options[] = {\n \tOPT_BOOL(0, \"global\", &use_global_config, N_(\"use global config file\")),\n \tOPT_BOOL(0, \"system\", &use_system_config, N_(\"use system config file\")),\n \tOPT_BOOL(0, \"local\", &use_local_config, N_(\"use repository config file\")),\n-\tOPT_STRING('f', \"file\", &given_config_file, N_(\"file\"), N_(\"use given config file\")),\n-\tOPT_STRING(0, \"blob\", &given_config_blob, N_(\"blob-id\"), N_(\"read config from given blob object\")),\n+\tOPT_STRING('f', \"file\", &given_config_source.file, N_(\"file\"), N_(\"use given config file\")),\n+\tOPT_STRING(0, \"blob\", &given_config_source.blob, N_(\"blob-id\"), N_(\"read config from given blob object\")),\n \tOPT_GROUP(N_(\"Action\")),\n \tOPT_BIT(0, \"get\", &actions, N_(\"get value: name [value-regex]\"), ACTION_GET),\n \tOPT_BIT(0, \"get-all\", &actions, N_(\"get all values: key [value-regex]\"), ACTION_GET_ALL),\n@@ -224,8 +223,7 @@ static int get_value(const char *key_, const char *regex_)\n \t}\n \n \tgit_config_with_options(collect_config, &values,\n-\t\t\t\tgiven_config_file, given_config_blob,\n-\t\t\t\trespect_includes);\n+\t\t\t\t&given_config_source, respect_includes);\n \n \tret = !values.nr;\n \n@@ -309,8 +307,7 @@ static void get_color(const char *def_color)\n \tget_color_found = 0;\n \tparsed_color[0] = '\\0';\n \tgit_config_with_options(git_get_color_config, NULL,\n-\t\t\t\tgiven_config_file, given_config_blob,\n-\t\t\t\trespect_includes);\n+\t\t\t\t&given_config_source, respect_includes);\n \n \tif (!get_color_found && def_color)\n \t\tcolor_parse(def_color, \"command line\", parsed_color);\n@@ -339,8 +336,7 @@ static int get_colorbool(int print)\n \tget_diff_color_found = -1;\n \tget_color_ui_found = -1;\n \tgit_config_with_options(git_get_colorbool_config, NULL,\n-\t\t\t\tgiven_config_file, given_config_blob,\n-\t\t\t\trespect_includes);\n+\t\t\t\t&given_config_source, respect_includes);\n \n \tif (get_colorbool_found < 0) {\n \t\tif (!strcmp(get_colorbool_slot, \"color.diff\"))\n@@ -364,7 +360,7 @@ static int get_colorbool(int print)\n \n static void check_write(void)\n {\n-\tif (given_config_blob)\n+\tif (given_config_source.blob)\n \t\tdie(\"writing config blobs is not supported\");\n }\n \n@@ -435,7 +431,7 @@ static int get_urlmatch(const char *var, const char *url)\n \t}\n \n \tgit_config_with_options(urlmatch_config_entry, &config,\n-\t\t\t\tgiven_config_file, NULL, respect_includes);\n+\t\t\t\t&given_config_source, respect_includes);\n \n \tfor_each_string_list_item(item, &values) {\n \t\tstruct urlmatch_current_candidate_value *matched = item->util;\n@@ -464,14 +460,14 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \tint nongit = !startup_info->have_repository;\n \tchar *value;\n \n-\tgiven_config_file = getenv(CONFIG_ENVIRONMENT);\n+\tgiven_config_source.file = getenv(CONFIG_ENVIRONMENT);\n \n \targc = parse_options(argc, argv, prefix, builtin_config_options,\n \t\t\t     builtin_config_usage,\n \t\t\t     PARSE_OPT_STOP_AT_NON_OPTION);\n \n \tif (use_global_config + use_system_config + use_local_config +\n-\t    !!given_config_file + !!given_config_blob > 1) {\n+\t    !!given_config_source.file + !!given_config_source.blob > 1) {\n \t\terror(\"only one config file at a time.\");\n \t\tusage_with_options(builtin_config_usage, builtin_config_options);\n \t}\n@@ -493,24 +489,24 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \n \t\tif (access_or_warn(user_config, R_OK, 0) &&\n \t\t    xdg_config && !access_or_warn(xdg_config, R_OK, 0))\n-\t\t\tgiven_config_file = xdg_config;\n+\t\t\tgiven_config_source.file = xdg_config;\n \t\telse\n-\t\t\tgiven_config_file = user_config;\n+\t\t\tgiven_config_source.file = user_config;\n \t}\n \telse if (use_system_config)\n-\t\tgiven_config_file = git_etc_gitconfig();\n+\t\tgiven_config_source.file = git_etc_gitconfig();\n \telse if (use_local_config)\n-\t\tgiven_config_file = git_pathdup(\"config\");\n-\telse if (given_config_file) {\n-\t\tif (!is_absolute_path(given_config_file) && prefix)\n-\t\t\tgiven_config_file =\n+\t\tgiven_config_source.file = git_pathdup(\"config\");\n+\telse if (given_config_source.file) {\n+\t\tif (!is_absolute_path(given_config_source.file) && prefix)\n+\t\t\tgiven_config_source.file =\n \t\t\t\txstrdup(prefix_filename(prefix,\n \t\t\t\t\t\t\tstrlen(prefix),\n-\t\t\t\t\t\t\tgiven_config_file));\n+\t\t\t\t\t\t\tgiven_config_source.file));\n \t}\n \n \tif (respect_includes == -1)\n-\t\trespect_includes = !given_config_file;\n+\t\trespect_includes = !given_config_source.file;\n \n \tif (end_null) {\n \t\tterm = '\\0';\n@@ -549,25 +545,24 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \tif (actions == ACTION_LIST) {\n \t\tcheck_argc(argc, 0, 0);\n \t\tif (git_config_with_options(show_all_config, NULL,\n-\t\t\t\t\t    given_config_file,\n-\t\t\t\t\t    given_config_blob,\n+\t\t\t\t\t    &given_config_source,\n \t\t\t\t\t    respect_includes) < 0) {\n-\t\t\tif (given_config_file)\n+\t\t\tif (given_config_source.file)\n \t\t\t\tdie_errno(\"unable to read config file '%s'\",\n-\t\t\t\t\t  given_config_file);\n+\t\t\t\t\t  given_config_source.file);\n \t\t\telse\n \t\t\t\tdie(\"error processing config file(s)\");\n \t\t}\n \t}\n \telse if (actions == ACTION_EDIT) {\n \t\tcheck_argc(argc, 0, 0);\n-\t\tif (!given_config_file && nongit)\n+\t\tif (!given_config_source.file && nongit)\n \t\t\tdie(\"not in a git directory\");\n-\t\tif (given_config_blob)\n+\t\tif (given_config_source.blob)\n \t\t\tdie(\"editing blobs is not supported\");\n \t\tgit_config(git_default_config, NULL);\n-\t\tlaunch_editor(given_config_file ?\n-\t\t\t      given_config_file : git_path(\"config\"),\n+\t\tlaunch_editor(given_config_source.file ?\n+\t\t\t      given_config_source.file : git_path(\"config\"),\n \t\t\t      NULL, NULL);\n \t}\n \telse if (actions == ACTION_SET) {\n@@ -575,7 +570,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\tcheck_write();\n \t\tcheck_argc(argc, 2, 2);\n \t\tvalue = normalize_value(argv[0], argv[1]);\n-\t\tret = git_config_set_in_file(given_config_file, argv[0], value);\n+\t\tret = git_config_set_in_file(given_config_source.file, argv[0], value);\n \t\tif (ret == CONFIG_NOTHING_SET)\n \t\t\terror(\"cannot overwrite multiple values with a single value\\n\"\n \t\t\t\"       Use a regexp, --add or --replace-all to change %s.\", argv[0]);\n@@ -585,21 +580,21 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\tcheck_write();\n \t\tcheck_argc(argc, 2, 3);\n \t\tvalue = normalize_value(argv[0], argv[1]);\n-\t\treturn git_config_set_multivar_in_file(given_config_file,\n+\t\treturn git_config_set_multivar_in_file(given_config_source.file,\n \t\t\t\t\t\t       argv[0], value, argv[2], 0);\n \t}\n \telse if (actions == ACTION_ADD) {\n \t\tcheck_write();\n \t\tcheck_argc(argc, 2, 2);\n \t\tvalue = normalize_value(argv[0], argv[1]);\n-\t\treturn git_config_set_multivar_in_file(given_config_file,\n+\t\treturn git_config_set_multivar_in_file(given_config_source.file,\n \t\t\t\t\t\t       argv[0], value, \"^$\", 0);\n \t}\n \telse if (actions == ACTION_REPLACE_ALL) {\n \t\tcheck_write();\n \t\tcheck_argc(argc, 2, 3);\n \t\tvalue = normalize_value(argv[0], argv[1]);\n-\t\treturn git_config_set_multivar_in_file(given_config_file,\n+\t\treturn git_config_set_multivar_in_file(given_config_source.file,\n \t\t\t\t\t\t       argv[0], value, argv[2], 1);\n \t}\n \telse if (actions == ACTION_GET) {\n@@ -626,23 +621,23 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\tcheck_write();\n \t\tcheck_argc(argc, 1, 2);\n \t\tif (argc == 2)\n-\t\t\treturn git_config_set_multivar_in_file(given_config_file,\n+\t\t\treturn git_config_set_multivar_in_file(given_config_source.file,\n \t\t\t\t\t\t\t       argv[0], NULL, argv[1], 0);\n \t\telse\n-\t\t\treturn git_config_set_in_file(given_config_file,\n+\t\t\treturn git_config_set_in_file(given_config_source.file,\n \t\t\t\t\t\t      argv[0], NULL);\n \t}\n \telse if (actions == ACTION_UNSET_ALL) {\n \t\tcheck_write();\n \t\tcheck_argc(argc, 1, 2);\n-\t\treturn git_config_set_multivar_in_file(given_config_file,\n+\t\treturn git_config_set_multivar_in_file(given_config_source.file,\n \t\t\t\t\t\t       argv[0], NULL, argv[1], 1);\n \t}\n \telse if (actions == ACTION_RENAME_SECTION) {\n \t\tint ret;\n \t\tcheck_write();\n \t\tcheck_argc(argc, 2, 2);\n-\t\tret = git_config_rename_section_in_file(given_config_file,\n+\t\tret = git_config_rename_section_in_file(given_config_source.file,\n \t\t\t\t\t\t\targv[0], argv[1]);\n \t\tif (ret < 0)\n \t\t\treturn ret;\n@@ -653,7 +648,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\tint ret;\n \t\tcheck_write();\n \t\tcheck_argc(argc, 1, 1);\n-\t\tret = git_config_rename_section_in_file(given_config_file,\n+\t\tret = git_config_rename_section_in_file(given_config_source.file,\n \t\t\t\t\t\t\targv[0], NULL);\n \t\tif (ret < 0)\n \t\t\treturn ret;\ndiff --git a/cache.h b/cache.h\nindex dc040fb1aa99..9d94bd69f7db 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1146,6 +1146,11 @@ extern int update_server_info(int);\n #define CONFIG_INVALID_PATTERN 6\n #define CONFIG_GENERIC_ERROR 7\n \n+struct git_config_source {\n+\tconst char *file;\n+\tconst char *blob;\n+};\n+\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@@ -1155,8 +1160,7 @@ 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 *);\n extern int git_config_with_options(config_fn_t fn, void *,\n-\t\t\t\t   const char *filename,\n-\t\t\t\t   const char *blob_ref,\n+\t\t\t\t   struct git_config_source *config_source,\n \t\t\t\t   int respect_includes);\n extern int git_config_early(config_fn_t fn, void *, const char *repo_config);\n extern int git_parse_ulong(const char *, unsigned long *);\ndiff --git a/config.c b/config.c\nindex d969a5aefc2b..6269da907964 100644\n--- a/config.c\n+++ b/config.c\n@@ -1170,8 +1170,7 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)\n }\n \n int git_config_with_options(config_fn_t fn, void *data,\n-\t\t\t    const char *filename,\n-\t\t\t    const char *blob_ref,\n+\t\t\t    struct git_config_source *config_source,\n \t\t\t    int respect_includes)\n {\n \tchar *repo_config = NULL;\n@@ -1189,10 +1188,10 @@ int git_config_with_options(config_fn_t fn, void *data,\n \t * If we have a specific filename, use it. Otherwise, follow the\n \t * regular lookup sequence.\n \t */\n-\tif (filename)\n-\t\treturn git_config_from_file(fn, filename, data);\n-\telse if (blob_ref)\n-\t\treturn git_config_from_blob_ref(fn, blob_ref, data);\n+\tif (config_source && config_source->file)\n+\t\treturn git_config_from_file(fn, config_source->file, data);\n+\telse if (config_source && config_source->blob)\n+\t\treturn git_config_from_blob_ref(fn, config_source->blob, data);\n \n \trepo_config = git_pathdup(\"config\");\n \tret = git_config_early(fn, data, repo_config);\n@@ -1203,7 +1202,7 @@ int git_config_with_options(config_fn_t fn, void *data,\n \n int git_config(config_fn_t fn, void *data)\n {\n-\treturn git_config_with_options(fn, data, NULL, NULL, 1);\n+\treturn git_config_with_options(fn, data, NULL, 1);\n }\n \n /*\n-- \n1.8.5.2\n"},{"id":"234825","messageId":"1392421053-937-3-git-send-email-kirill@shutemov.name","threadId":"35875","inReplyTo":"1392421053-937-1-git-send-email-kirill@shutemov.name","subject":"[PATCH 3/3] config: teach \"git config --file -\" to read from the standard input","fromName":"Kirill A. Shutemov","fromEmail":"kirill@shutemov.name","sentAt":"2014-02-14T23:37:33Z","receivedAt":"2014-02-14T23:37:33Z","isPatch":true,"sender":{"key":"kirill@shutemov.name","avatar":"https://avatars.githubusercontent.com/u/588661?v=4"},"body":"The patch extends git config --file interface to allow read config from\nstdin.\n\nEditing stdin or setting value in stdin is an error.\n\nInclude by absolute path is allowed in stdin config, but not by relative\npath.\n\nSigned-off-by: Kirill A. Shutemov <kirill@shutemov.name>\n---\n builtin/config.c          | 11 +++++++++\n cache.h                   |  1 +\n config.c                  | 58 ++++++++++++++++++++++++++++++++---------------\n t/t1300-repo-config.sh    | 17 ++++++++++++--\n t/t1305-config-include.sh | 14 ++++++++++++\n 5 files changed, 81 insertions(+), 20 deletions(-)\n\ndiff --git a/builtin/config.c b/builtin/config.c\nindex de41ba50e9ca..5677c942b693 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -360,6 +360,9 @@ static int get_colorbool(int print)\n \n static void check_write(void)\n {\n+\tif (given_config_source.use_stdin)\n+\t\tdie(\"writing to stdin is not supported\");\n+\n \tif (given_config_source.blob)\n \t\tdie(\"writing config blobs is not supported\");\n }\n@@ -472,6 +475,12 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\tusage_with_options(builtin_config_usage, builtin_config_options);\n \t}\n \n+\tif (given_config_source.file &&\n+\t\t\t!strcmp(given_config_source.file, \"-\")) {\n+\t\tgiven_config_source.file = NULL;\n+\t\tgiven_config_source.use_stdin = 1;\n+\t}\n+\n \tif (use_global_config) {\n \t\tchar *user_config = NULL;\n \t\tchar *xdg_config = NULL;\n@@ -558,6 +567,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\tcheck_argc(argc, 0, 0);\n \t\tif (!given_config_source.file && nongit)\n \t\t\tdie(\"not in a git directory\");\n+\t\tif (given_config_source.use_stdin)\n+\t\t\tdie(\"editing stdin is not supported\");\n \t\tif (given_config_source.blob)\n \t\t\tdie(\"editing blobs is not supported\");\n \t\tgit_config(git_default_config, NULL);\ndiff --git a/cache.h b/cache.h\nindex 9d94bd69f7db..4db19b537059 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1147,6 +1147,7 @@ extern int update_server_info(int);\n #define CONFIG_GENERIC_ERROR 7\n \n struct git_config_source {\n+\tunsigned int use_stdin:1;\n \tconst char *file;\n \tconst char *blob;\n };\ndiff --git a/config.c b/config.c\nindex 6269da907964..7b42608f5c89 100644\n--- a/config.c\n+++ b/config.c\n@@ -443,10 +443,20 @@ static int git_parse_source(config_fn_t fn, void *data)\n \t\tif (get_value(fn, data, var) < 0)\n \t\t\tbreak;\n \t}\n-\tif (cf->die_on_error)\n-\t\tdie(\"bad config file line %d in %s\", cf->linenr, cf->name);\n-\telse\n-\t\treturn error(\"bad config file line %d in %s\", cf->linenr, cf->name);\n+\tif (cf->die_on_error) {\n+\t\tif (cf->name)\n+\t\t\tdie(\"bad config file line %d in %s\",\n+\t\t\t\t\tcf->linenr, cf->name);\n+\t\telse\n+\t\t\tdie(\"bad config file line %d\", cf->linenr);\n+\n+\t} else {\n+\t\tif (cf->name)\n+\t\t\treturn error(\"bad config file line %d in %s\",\n+\t\t\t\t\tcf->linenr, cf->name);\n+\t\telse\n+\t\t\treturn error(\"bad config file line %d\", cf->linenr);\n+\t}\n }\n \n static int parse_unit_factor(const char *end, uintmax_t *val)\n@@ -1030,24 +1040,34 @@ static int do_config_from(struct config_source *top, config_fn_t fn, void *data)\n \treturn ret;\n }\n \n-int git_config_from_file(config_fn_t fn, const char *filename, void *data)\n+static int do_config_from_file(config_fn_t fn, const char *filename, FILE *f,\n+\t\t\t       void *data)\n {\n-\tint ret;\n-\tFILE *f = fopen(filename, \"r\");\n+\tstruct config_source top;\n \n-\tret = -1;\n-\tif (f) {\n-\t\tstruct config_source top;\n+\ttop.u.file = f;\n+\ttop.name = filename;\n+\ttop.die_on_error = 1;\n+\ttop.do_fgetc = config_file_fgetc;\n+\ttop.do_ungetc = config_file_ungetc;\n+\ttop.do_ftell = config_file_ftell;\n \n-\t\ttop.u.file = f;\n-\t\ttop.name = filename;\n-\t\ttop.die_on_error = 1;\n-\t\ttop.do_fgetc = config_file_fgetc;\n-\t\ttop.do_ungetc = config_file_ungetc;\n-\t\ttop.do_ftell = config_file_ftell;\n+\treturn do_config_from(&top, fn, data);\n+}\n+\n+static int git_config_from_stdin(config_fn_t fn, void *data)\n+{\n+\treturn do_config_from_file(fn, NULL, stdin, data);\n+}\n \n-\t\tret = do_config_from(&top, fn, data);\n+int git_config_from_file(config_fn_t fn, const char *filename, void *data)\n+{\n+\tint ret = -1;\n+\tFILE *f;\n \n+\tf = fopen(filename, \"r\");\n+\tif (f) {\n+\t\tret = do_config_from_file(fn, filename, f, data);\n \t\tfclose(f);\n \t}\n \treturn ret;\n@@ -1188,7 +1208,9 @@ int git_config_with_options(config_fn_t fn, void *data,\n \t * If we have a specific filename, use it. Otherwise, follow the\n \t * regular lookup sequence.\n \t */\n-\tif (config_source && config_source->file)\n+\tif (config_source && config_source->use_stdin)\n+\t\treturn git_config_from_stdin(fn, data);\n+\telse if (config_source && config_source->file)\n \t\treturn git_config_from_file(fn, config_source->file, data);\n \telse if (config_source && config_source->blob)\n \t\treturn git_config_from_blob_ref(fn, config_source->blob, data);\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex 967359344dab..c9c426c273e5 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -475,15 +475,28 @@ ein.bahn=strasse\n EOF\n \n test_expect_success 'alternative GIT_CONFIG' '\n-\tGIT_CONFIG=other-config git config -l >output &&\n+\tGIT_CONFIG=other-config git config --list >output &&\n \ttest_cmp expect output\n '\n \n test_expect_success 'alternative GIT_CONFIG (--file)' '\n-\tgit config --file other-config -l > output &&\n+\tgit config --file other-config --list >output &&\n \ttest_cmp expect output\n '\n \n+test_expect_success 'alternative GIT_CONFIG (--file=-)' '\n+\tgit config --file - --list <other-config >output &&\n+\ttest_cmp expect output\n+'\n+\n+test_expect_success 'setting a value in stdin is an error' '\n+\ttest_must_fail git config --file - some.value foo\n+'\n+\n+test_expect_success 'editing stdin is an error' '\n+\ttest_must_fail git config --file - --edit\n+'\n+\n test_expect_success 'refer config from subdirectory' '\n \tmkdir x &&\n \t(\ndiff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh\nindex a70707620f14..fda6555024c5 100755\n--- a/t/t1305-config-include.sh\n+++ b/t/t1305-config-include.sh\n@@ -122,6 +122,20 @@ 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 'absolute includes from stdin work' '\n+\techo \"[test]one = 1\" >one &&\n+\techo 1 >expect &&\n+\techo \"[include]path=\\\"$PWD/one\\\"\" |\n+\tgit config --file - test.one >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'relative includes from stdin line fail' '\n+\techo \"[test]one = 1\" >one &&\n+\techo \"[include]path=one\" |\n+\ttest_must_fail git config --file - test.one\n+'\n+\n test_expect_success 'include cycles are detected' '\n \tcat >.gitconfig <<-\\EOF &&\n \t[test]value = gitconfig\n-- \n1.8.5.2\n"},{"id":"234870","messageId":"CAPig+cSRgpABwzC36FoBst52hCOPieMBTvsTx9CPFoHifHG9yg@mail.gmail.com","threadId":"35875","inReplyTo":"1392421053-937-3-git-send-email-kirill@shutemov.name","subject":"Re: [PATCH 3/3] config: teach \"git config --file -\" to read from the standard input","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-02-16T02:12:29Z","receivedAt":"2014-02-16T02:12:29Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Feb 14, 2014 at 6:37 PM, Kirill A. Shutemov\n<kirill@shutemov.name> wrote:\n> The patch extends git config --file interface to allow read config from\n> stdin.\n>\n> Editing stdin or setting value in stdin is an error.\n>\n> Include by absolute path is allowed in stdin config, but not by relative\n> path.\n>\n> Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>\n> ---\n> diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh\n> index a70707620f14..fda6555024c5 100755\n> --- a/t/t1305-config-include.sh\n> +++ b/t/t1305-config-include.sh\n> @@ -122,6 +122,20 @@ test_expect_success 'relative includes from command line fail' '\n>         test_must_fail git -c include.path=one config test.one\n>  '\n>\n> +test_expect_success 'absolute includes from stdin work' '\n> +       echo \"[test]one = 1\" >one &&\n> +       echo 1 >expect &&\n> +       echo \"[include]path=\\\"$PWD/one\\\"\" |\n\nTo be Windows-friendly, you may want to use $(pwd). Quoting from t/README:\n\n   When a test checks for an absolute path that a git command generated,\n   construct the expected value using $(pwd) rather than $PWD,\n   $TEST_DIRECTORY, or $TRASH_DIRECTORY. It makes a difference on\n   Windows, where the shell (MSYS bash) mangles absolute path names.\n   For details, see the commit message of 4114156ae9.\n\n> +       git config --file - test.one >actual &&\n> +       test_cmp expect actual\n> +'\n> +\n> +test_expect_success 'relative includes from stdin line fail' '\n> +       echo \"[test]one = 1\" >one &&\n> +       echo \"[include]path=one\" |\n> +       test_must_fail git config --file - test.one\n> +'\n> +\n>  test_expect_success 'include cycles are detected' '\n>         cat >.gitconfig <<-\\EOF &&\n>         [test]value = gitconfig\n> --\n> 1.8.5.2\n"},{"id":"234885","messageId":"1392552781-23275-1-git-send-email-kirill@shutemov.name","threadId":"35875","inReplyTo":"CAPig+cSRgpABwzC36FoBst52hCOPieMBTvsTx9CPFoHifHG9yg@mail.gmail.com","subject":"[PATCH] config: teach \"git config --file -\" to read from the standard input","fromName":"Kirill A. Shutemov","fromEmail":"kirill@shutemov.name","sentAt":"2014-02-16T12:13:01Z","receivedAt":"2014-02-16T12:13:01Z","isPatch":true,"sender":{"key":"kirill@shutemov.name","avatar":"https://avatars.githubusercontent.com/u/588661?v=4"},"body":"The patch extends git config --file interface to allow read config from\nstdin.\n\nEditing stdin or setting value in stdin is an error.\n\nInclude by absolute path is allowed in stdin config, but not by relative\npath.\n\nSigned-off-by: Kirill A. Shutemov <kirill@shutemov.name>\n---\n builtin/config.c          | 11 +++++++++\n cache.h                   |  1 +\n config.c                  | 58 ++++++++++++++++++++++++++++++++---------------\n t/t1300-repo-config.sh    | 17 ++++++++++++--\n t/t1305-config-include.sh | 16 ++++++++++++-\n 5 files changed, 82 insertions(+), 21 deletions(-)\n\ndiff --git a/builtin/config.c b/builtin/config.c\nindex de41ba50e9ca..5677c942b693 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -360,6 +360,9 @@ static int get_colorbool(int print)\n \n static void check_write(void)\n {\n+\tif (given_config_source.use_stdin)\n+\t\tdie(\"writing to stdin is not supported\");\n+\n \tif (given_config_source.blob)\n \t\tdie(\"writing config blobs is not supported\");\n }\n@@ -472,6 +475,12 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\tusage_with_options(builtin_config_usage, builtin_config_options);\n \t}\n \n+\tif (given_config_source.file &&\n+\t\t\t!strcmp(given_config_source.file, \"-\")) {\n+\t\tgiven_config_source.file = NULL;\n+\t\tgiven_config_source.use_stdin = 1;\n+\t}\n+\n \tif (use_global_config) {\n \t\tchar *user_config = NULL;\n \t\tchar *xdg_config = NULL;\n@@ -558,6 +567,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\tcheck_argc(argc, 0, 0);\n \t\tif (!given_config_source.file && nongit)\n \t\t\tdie(\"not in a git directory\");\n+\t\tif (given_config_source.use_stdin)\n+\t\t\tdie(\"editing stdin is not supported\");\n \t\tif (given_config_source.blob)\n \t\t\tdie(\"editing blobs is not supported\");\n \t\tgit_config(git_default_config, NULL);\ndiff --git a/cache.h b/cache.h\nindex 9d94bd69f7db..4db19b537059 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1147,6 +1147,7 @@ extern int update_server_info(int);\n #define CONFIG_GENERIC_ERROR 7\n \n struct git_config_source {\n+\tunsigned int use_stdin:1;\n \tconst char *file;\n \tconst char *blob;\n };\ndiff --git a/config.c b/config.c\nindex 6269da907964..7b42608f5c89 100644\n--- a/config.c\n+++ b/config.c\n@@ -443,10 +443,20 @@ static int git_parse_source(config_fn_t fn, void *data)\n \t\tif (get_value(fn, data, var) < 0)\n \t\t\tbreak;\n \t}\n-\tif (cf->die_on_error)\n-\t\tdie(\"bad config file line %d in %s\", cf->linenr, cf->name);\n-\telse\n-\t\treturn error(\"bad config file line %d in %s\", cf->linenr, cf->name);\n+\tif (cf->die_on_error) {\n+\t\tif (cf->name)\n+\t\t\tdie(\"bad config file line %d in %s\",\n+\t\t\t\t\tcf->linenr, cf->name);\n+\t\telse\n+\t\t\tdie(\"bad config file line %d\", cf->linenr);\n+\n+\t} else {\n+\t\tif (cf->name)\n+\t\t\treturn error(\"bad config file line %d in %s\",\n+\t\t\t\t\tcf->linenr, cf->name);\n+\t\telse\n+\t\t\treturn error(\"bad config file line %d\", cf->linenr);\n+\t}\n }\n \n static int parse_unit_factor(const char *end, uintmax_t *val)\n@@ -1030,24 +1040,34 @@ static int do_config_from(struct config_source *top, config_fn_t fn, void *data)\n \treturn ret;\n }\n \n-int git_config_from_file(config_fn_t fn, const char *filename, void *data)\n+static int do_config_from_file(config_fn_t fn, const char *filename, FILE *f,\n+\t\t\t       void *data)\n {\n-\tint ret;\n-\tFILE *f = fopen(filename, \"r\");\n+\tstruct config_source top;\n \n-\tret = -1;\n-\tif (f) {\n-\t\tstruct config_source top;\n+\ttop.u.file = f;\n+\ttop.name = filename;\n+\ttop.die_on_error = 1;\n+\ttop.do_fgetc = config_file_fgetc;\n+\ttop.do_ungetc = config_file_ungetc;\n+\ttop.do_ftell = config_file_ftell;\n \n-\t\ttop.u.file = f;\n-\t\ttop.name = filename;\n-\t\ttop.die_on_error = 1;\n-\t\ttop.do_fgetc = config_file_fgetc;\n-\t\ttop.do_ungetc = config_file_ungetc;\n-\t\ttop.do_ftell = config_file_ftell;\n+\treturn do_config_from(&top, fn, data);\n+}\n+\n+static int git_config_from_stdin(config_fn_t fn, void *data)\n+{\n+\treturn do_config_from_file(fn, NULL, stdin, data);\n+}\n \n-\t\tret = do_config_from(&top, fn, data);\n+int git_config_from_file(config_fn_t fn, const char *filename, void *data)\n+{\n+\tint ret = -1;\n+\tFILE *f;\n \n+\tf = fopen(filename, \"r\");\n+\tif (f) {\n+\t\tret = do_config_from_file(fn, filename, f, data);\n \t\tfclose(f);\n \t}\n \treturn ret;\n@@ -1188,7 +1208,9 @@ int git_config_with_options(config_fn_t fn, void *data,\n \t * If we have a specific filename, use it. Otherwise, follow the\n \t * regular lookup sequence.\n \t */\n-\tif (config_source && config_source->file)\n+\tif (config_source && config_source->use_stdin)\n+\t\treturn git_config_from_stdin(fn, data);\n+\telse if (config_source && config_source->file)\n \t\treturn git_config_from_file(fn, config_source->file, data);\n \telse if (config_source && config_source->blob)\n \t\treturn git_config_from_blob_ref(fn, config_source->blob, data);\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex 967359344dab..c9c426c273e5 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -475,15 +475,28 @@ ein.bahn=strasse\n EOF\n \n test_expect_success 'alternative GIT_CONFIG' '\n-\tGIT_CONFIG=other-config git config -l >output &&\n+\tGIT_CONFIG=other-config git config --list >output &&\n \ttest_cmp expect output\n '\n \n test_expect_success 'alternative GIT_CONFIG (--file)' '\n-\tgit config --file other-config -l > output &&\n+\tgit config --file other-config --list >output &&\n \ttest_cmp expect output\n '\n \n+test_expect_success 'alternative GIT_CONFIG (--file=-)' '\n+\tgit config --file - --list <other-config >output &&\n+\ttest_cmp expect output\n+'\n+\n+test_expect_success 'setting a value in stdin is an error' '\n+\ttest_must_fail git config --file - some.value foo\n+'\n+\n+test_expect_success 'editing stdin is an error' '\n+\ttest_must_fail git config --file - --edit\n+'\n+\n test_expect_success 'refer config from subdirectory' '\n \tmkdir x &&\n \t(\ndiff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh\nindex a70707620f14..01e50c5882d0 100755\n--- a/t/t1305-config-include.sh\n+++ b/t/t1305-config-include.sh\n@@ -113,7 +113,7 @@ test_expect_success 'missing include files are ignored' '\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+\tgit -c include.path=\"$(pwd)/one\" config test.one >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -122,6 +122,20 @@ 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 'absolute includes from stdin work' '\n+\techo \"[test]one = 1\" >one &&\n+\techo 1 >expect &&\n+\techo \"[include]path=\\\"$(pwd)/one\\\"\" |\n+\tgit config --file - test.one >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'relative includes from stdin line fail' '\n+\techo \"[test]one = 1\" >one &&\n+\techo \"[include]path=one\" |\n+\ttest_must_fail git config --file - test.one\n+'\n+\n test_expect_success 'include cycles are detected' '\n \tcat >.gitconfig <<-\\EOF &&\n \t[test]value = gitconfig\n-- \n1.8.5.2\n"},{"id":"234946","messageId":"20140218084144.GA2692@sigill.intra.peff.net","threadId":"35875","inReplyTo":"1392552781-23275-1-git-send-email-kirill@shutemov.name","subject":"Re: [PATCH] config: teach \"git config --file -\" to read from the standard input","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-18T08:41:44Z","receivedAt":"2014-02-18T08:41:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 16, 2014 at 02:13:01PM +0200, Kirill A. Shutemov wrote:\n\n> The patch extends git config --file interface to allow read config from\n> stdin.\n> \n> Editing stdin or setting value in stdin is an error.\n> \n> Include by absolute path is allowed in stdin config, but not by relative\n> path.\n\nThanks, this looks very cleanly done.\n\nOne nit:\n\n> diff --git a/config.c b/config.c\n> index 6269da907964..7b42608f5c89 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -443,10 +443,20 @@ static int git_parse_source(config_fn_t fn, void *data)\n>  \t\tif (get_value(fn, data, var) < 0)\n>  \t\t\tbreak;\n>  \t}\n> -\tif (cf->die_on_error)\n> -\t\tdie(\"bad config file line %d in %s\", cf->linenr, cf->name);\n> -\telse\n> -\t\treturn error(\"bad config file line %d in %s\", cf->linenr, cf->name);\n> +\tif (cf->die_on_error) {\n> +\t\tif (cf->name)\n> +\t\t\tdie(\"bad config file line %d in %s\",\n> +\t\t\t\t\tcf->linenr, cf->name);\n> +\t\telse\n> +\t\t\tdie(\"bad config file line %d\", cf->linenr);\n> +\n> +\t} else {\n> +\t\tif (cf->name)\n> +\t\t\treturn error(\"bad config file line %d in %s\",\n> +\t\t\t\t\tcf->linenr, cf->name);\n> +\t\telse\n> +\t\t\treturn error(\"bad config file line %d\", cf->linenr);\n> +\t}\n\nI think I preferred the earlier version where you had \"<stdin>\" in the\nname field, and this hunk could just go away. I know you switched it to\nNULL here to avoid making bogus relative filenames in includes.\n\nBut that would be pretty straightforward to fix by splitting the \"name\"\nfield into an additional \"path\" field. It looks like \"git config --blob\"\nhas the same problem (it will try relative lookups in the filesystem,\neven though that is nonsensical from the blob). So we could fix the\nexisting bug at the same time. :)\n\nPerhaps this could go at the start of your series?\n\n-- >8 --\nSubject: config: disallow relative include paths from blobs\n\nWhen we see a relative config include like:\n\n  [include]\n  path = foo\n\nwe make it relative to the containing directory of the file\nthat contains the snippet. This makes no sense for config\nread from a blob, as it is not on the filesystem.  Something\nlike \"HEAD:some/path\" could have a relative path within the\ntree, but:\n\n  1. It would not be part of include.path, which explicitly\n     refers to the filesystem.\n\n  2. It would need different parsing rules anyway to\n     determine that it is a tree path.\n\nThe current code just uses the \"name\" field, which is wrong.\nLet's split that into \"name\" and \"path\" fields, use the\nlatter for relative includes, and fill in only the former\nfor blobs.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI don't think we considered includes at all when adding --blob. I cannot\nthink of any good reason to have even an absolute filesystem include\nfrom a blob. And I wonder if it is possible to cause security mischief\nby convincing git to look at /etc/passwd or similar (it would not parse,\nof course, but you might be able to glean information from the error\nmessages or something).\n\nIt's probably OK, though, because you would generally not read real\nconfig from an untrusted source (there are many bad things they could\nset), and other code which uses the config reader explicitly does not\nturn on includes.\n\n config.c                  | 10 ++++++----\n t/t1305-config-include.sh | 16 ++++++++++++++++\n 2 files changed, 22 insertions(+), 4 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex d969a5a..b295310 100644\n--- a/config.c\n+++ b/config.c\n@@ -21,6 +21,7 @@ struct config_source {\n \t\t} buf;\n \t} u;\n \tconst char *name;\n+\tconst char *path;\n \tint die_on_error;\n \tint linenr;\n \tint eof;\n@@ -97,12 +98,12 @@ static int handle_path_include(const char *path, struct config_include_data *inc\n \tif (!is_absolute_path(path)) {\n \t\tchar *slash;\n \n-\t\tif (!cf || !cf->name)\n+\t\tif (!cf || !cf->path)\n \t\t\treturn error(\"relative config includes must come from files\");\n \n-\t\tslash = find_last_dir_sep(cf->name);\n+\t\tslash = find_last_dir_sep(cf->path);\n \t\tif (slash)\n-\t\t\tstrbuf_add(&buf, cf->name, slash - cf->name + 1);\n+\t\t\tstrbuf_add(&buf, cf->path, slash - cf->path + 1);\n \t\tstrbuf_addstr(&buf, path);\n \t\tpath = buf.buf;\n \t}\n@@ -1040,7 +1041,7 @@ int git_config_from_file(config_fn_t fn, const char *filename, void *data)\n \t\tstruct config_source top;\n \n \t\ttop.u.file = f;\n-\t\ttop.name = filename;\n+\t\ttop.name = top.path = filename;\n \t\ttop.die_on_error = 1;\n \t\ttop.do_fgetc = config_file_fgetc;\n \t\ttop.do_ungetc = config_file_ungetc;\n@@ -1062,6 +1063,7 @@ int git_config_from_buf(config_fn_t fn, const char *name, const char *buf,\n \ttop.u.buf.len = len;\n \ttop.u.buf.pos = 0;\n \ttop.name = name;\n+\ttop.path = NULL;\n \ttop.die_on_error = 0;\n \ttop.do_fgetc = config_buf_fgetc;\n \ttop.do_ungetc = config_buf_ungetc;\ndiff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh\nindex a707076..6edd38b 100755\n--- a/t/t1305-config-include.sh\n+++ b/t/t1305-config-include.sh\n@@ -122,6 +122,22 @@ 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 'absolute includes from blobs work' '\n+\techo \"[test]one = 1\" >one &&\n+\techo \"[include]path=$(pwd)/one\" >blob &&\n+\tblob=$(git hash-object -w blob) &&\n+\techo 1 >expect &&\n+\tgit config --blob=$blob test.one >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'relative includes from blobs fail' '\n+\techo \"[test]one = 1\" >one &&\n+\techo \"[include]path=one\" >blob &&\n+\tblob=$(git hash-object -w blob) &&\n+\ttest_must_fail git config --blob=$blob test.one\n+'\n+\n test_expect_success 'include cycles are detected' '\n \tcat >.gitconfig <<-\\EOF &&\n \t[test]value = gitconfig\n-- \n1.8.5.2.500.g8060133\n"},{"id":"235003","messageId":"xmqq7g8sfd19.fsf@gitster.dls.corp.google.com","threadId":"35875","inReplyTo":"20140218084144.GA2692@sigill.intra.peff.net","subject":"Re: [PATCH] config: teach \"git config --file -\" to read from the standard input","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-18T19:15:14Z","receivedAt":"2014-02-18T19:15:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n>> +\t} else {\n>> +\t\tif (cf->name)\n>> +\t\t\treturn error(\"bad config file line %d in %s\",\n>> +\t\t\t\t\tcf->linenr, cf->name);\n>> +\t\telse\n>> +\t\t\treturn error(\"bad config file line %d\", cf->linenr);\n>> +\t}\n>\n> I think I preferred the earlier version where you had \"<stdin>\" in the\n> name field, and this hunk could just go away. I know you switched it to\n> NULL here to avoid making bogus relative filenames in includes.\n\nExactly the same comment here.  I really like the way how this\nseries becomes more polished every time we see it.\n\n> But that would be pretty straightforward to fix by splitting the \"name\"\n> field into an additional \"path\" field. It looks like \"git config --blob\"\n> has the same problem (it will try relative lookups in the filesystem,\n> even though that is nonsensical from the blob). So we could fix the\n> existing bug at the same time. :)\n>\n> Perhaps this could go at the start of your series?\n\nSounds like the right thing to do.\n\nThanks.\n\n> -- >8 --\n> Subject: config: disallow relative include paths from blobs\n>\n> When we see a relative config include like:\n>\n>   [include]\n>   path = foo\n>\n> we make it relative to the containing directory of the file\n> that contains the snippet. This makes no sense for config\n> read from a blob, as it is not on the filesystem.  Something\n> like \"HEAD:some/path\" could have a relative path within the\n> tree, but:\n>\n>   1. It would not be part of include.path, which explicitly\n>      refers to the filesystem.\n>\n>   2. It would need different parsing rules anyway to\n>      determine that it is a tree path.\n>\n> The current code just uses the \"name\" field, which is wrong.\n> Let's split that into \"name\" and \"path\" fields, use the\n> latter for relative includes, and fill in only the former\n> for blobs.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> I don't think we considered includes at all when adding --blob. I cannot\n> think of any good reason to have even an absolute filesystem include\n> from a blob. And I wonder if it is possible to cause security mischief\n> by convincing git to look at /etc/passwd or similar (it would not parse,\n> of course, but you might be able to glean information from the error\n> messages or something).\n>\n> It's probably OK, though, because you would generally not read real\n> config from an untrusted source (there are many bad things they could\n> set), and other code which uses the config reader explicitly does not\n> turn on includes.\n>\n>  config.c                  | 10 ++++++----\n>  t/t1305-config-include.sh | 16 ++++++++++++++++\n>  2 files changed, 22 insertions(+), 4 deletions(-)\n>\n> diff --git a/config.c b/config.c\n> index d969a5a..b295310 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -21,6 +21,7 @@ struct config_source {\n>  \t\t} buf;\n>  \t} u;\n>  \tconst char *name;\n> +\tconst char *path;\n>  \tint die_on_error;\n>  \tint linenr;\n>  \tint eof;\n> @@ -97,12 +98,12 @@ static int handle_path_include(const char *path, struct config_include_data *inc\n>  \tif (!is_absolute_path(path)) {\n>  \t\tchar *slash;\n>  \n> -\t\tif (!cf || !cf->name)\n> +\t\tif (!cf || !cf->path)\n>  \t\t\treturn error(\"relative config includes must come from files\");\n>  \n> -\t\tslash = find_last_dir_sep(cf->name);\n> +\t\tslash = find_last_dir_sep(cf->path);\n>  \t\tif (slash)\n> -\t\t\tstrbuf_add(&buf, cf->name, slash - cf->name + 1);\n> +\t\t\tstrbuf_add(&buf, cf->path, slash - cf->path + 1);\n>  \t\tstrbuf_addstr(&buf, path);\n>  \t\tpath = buf.buf;\n>  \t}\n> @@ -1040,7 +1041,7 @@ int git_config_from_file(config_fn_t fn, const char *filename, void *data)\n>  \t\tstruct config_source top;\n>  \n>  \t\ttop.u.file = f;\n> -\t\ttop.name = filename;\n> +\t\ttop.name = top.path = filename;\n>  \t\ttop.die_on_error = 1;\n>  \t\ttop.do_fgetc = config_file_fgetc;\n>  \t\ttop.do_ungetc = config_file_ungetc;\n> @@ -1062,6 +1063,7 @@ int git_config_from_buf(config_fn_t fn, const char *name, const char *buf,\n>  \ttop.u.buf.len = len;\n>  \ttop.u.buf.pos = 0;\n>  \ttop.name = name;\n> +\ttop.path = NULL;\n>  \ttop.die_on_error = 0;\n>  \ttop.do_fgetc = config_buf_fgetc;\n>  \ttop.do_ungetc = config_buf_ungetc;\n> diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh\n> index a707076..6edd38b 100755\n> --- a/t/t1305-config-include.sh\n> +++ b/t/t1305-config-include.sh\n> @@ -122,6 +122,22 @@ 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 'absolute includes from blobs work' '\n> +\techo \"[test]one = 1\" >one &&\n> +\techo \"[include]path=$(pwd)/one\" >blob &&\n> +\tblob=$(git hash-object -w blob) &&\n> +\techo 1 >expect &&\n> +\tgit config --blob=$blob test.one >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'relative includes from blobs fail' '\n> +\techo \"[test]one = 1\" >one &&\n> +\techo \"[include]path=one\" >blob &&\n> +\tblob=$(git hash-object -w blob) &&\n> +\ttest_must_fail git config --blob=$blob test.one\n> +'\n> +\n>  test_expect_success 'include cycles are detected' '\n>  \tcat >.gitconfig <<-\\EOF &&\n>  \t[test]value = gitconfig\n"},{"id":"235035","messageId":"xmqqy518ausp.fsf@gitster.dls.corp.google.com","threadId":"35875","inReplyTo":"xmqq7g8sfd19.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] config: teach \"git config --file -\" to read from the standard input","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-18T23:02:46Z","receivedAt":"2014-02-18T23:02:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>> I think I preferred the earlier version where you had \"<stdin>\" in the\n>> name field, and this hunk could just go away. I know you switched it to\n>> NULL here to avoid making bogus relative filenames in includes.\n>\n> Exactly the same comment here.  I really like the way how this\n> series becomes more polished every time we see it.\n>\n>> But that would be pretty straightforward to fix by splitting the \"name\"\n>> field into an additional \"path\" field. It looks like \"git config --blob\"\n>> has the same problem (it will try relative lookups in the filesystem,\n>> even though that is nonsensical from the blob). So we could fix the\n>> existing bug at the same time. :)\n>>\n>> Perhaps this could go at the start of your series?\n>\n> Sounds like the right thing to do.\n>\n> Thanks.\n\nAnd [PATCH 3/3] rebased on the others may look like this.\n\n builtin/config.c          | 11 +++++++++++\n cache.h                   |  1 +\n config.c                  | 42 ++++++++++++++++++++++++++++--------------\n t/t1300-repo-config.sh    | 17 +++++++++++++++--\n t/t1305-config-include.sh | 16 +++++++++++++++-\n 5 files changed, 70 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/config.c b/builtin/config.c\nindex de41ba5..5677c94 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -360,6 +360,9 @@ static int get_colorbool(int print)\n \n static void check_write(void)\n {\n+\tif (given_config_source.use_stdin)\n+\t\tdie(\"writing to stdin is not supported\");\n+\n \tif (given_config_source.blob)\n \t\tdie(\"writing config blobs is not supported\");\n }\n@@ -472,6 +475,12 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\tusage_with_options(builtin_config_usage, builtin_config_options);\n \t}\n \n+\tif (given_config_source.file &&\n+\t\t\t!strcmp(given_config_source.file, \"-\")) {\n+\t\tgiven_config_source.file = NULL;\n+\t\tgiven_config_source.use_stdin = 1;\n+\t}\n+\n \tif (use_global_config) {\n \t\tchar *user_config = NULL;\n \t\tchar *xdg_config = NULL;\n@@ -558,6 +567,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\tcheck_argc(argc, 0, 0);\n \t\tif (!given_config_source.file && nongit)\n \t\t\tdie(\"not in a git directory\");\n+\t\tif (given_config_source.use_stdin)\n+\t\t\tdie(\"editing stdin is not supported\");\n \t\tif (given_config_source.blob)\n \t\t\tdie(\"editing blobs is not supported\");\n \t\tgit_config(git_default_config, NULL);\ndiff --git a/cache.h b/cache.h\nindex 9d94bd6..4db19b5 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1147,6 +1147,7 @@ extern int update_server_info(int);\n #define CONFIG_GENERIC_ERROR 7\n \n struct git_config_source {\n+\tunsigned int use_stdin:1;\n \tconst char *file;\n \tconst char *blob;\n };\ndiff --git a/config.c b/config.c\nindex a21b0dd..9dd0bdb 100644\n--- a/config.c\n+++ b/config.c\n@@ -1031,24 +1031,36 @@ static int do_config_from(struct config_source *top, config_fn_t fn, void *data)\n \treturn ret;\n }\n \n-int git_config_from_file(config_fn_t fn, const char *filename, void *data)\n+static int do_config_from_file(config_fn_t fn,\n+\t\t\t       const char *filename, FILE *f,\n+\t\t\t       const char *label, void *data)\n {\n-\tint ret;\n-\tFILE *f = fopen(filename, \"r\");\n+\tstruct config_source top;\n \n-\tret = -1;\n-\tif (f) {\n-\t\tstruct config_source top;\n+\ttop.u.file = f;\n+\ttop.name = label;\n+\ttop.path = filename;\n+\ttop.die_on_error = 1;\n+\ttop.do_fgetc = config_file_fgetc;\n+\ttop.do_ungetc = config_file_ungetc;\n+\ttop.do_ftell = config_file_ftell;\n+\n+\treturn do_config_from(&top, fn, data);\n+}\n \n-\t\ttop.u.file = f;\n-\t\ttop.name = top.path = filename;\n-\t\ttop.die_on_error = 1;\n-\t\ttop.do_fgetc = config_file_fgetc;\n-\t\ttop.do_ungetc = config_file_ungetc;\n-\t\ttop.do_ftell = config_file_ftell;\n+static int git_config_from_stdin(config_fn_t fn, void *data)\n+{\n+\treturn do_config_from_file(fn, NULL, stdin, \"<stdin>\", data);\n+}\n \n-\t\tret = do_config_from(&top, fn, data);\n+int git_config_from_file(config_fn_t fn, const char *filename, void *data)\n+{\n+\tint ret = -1;\n+\tFILE *f;\n \n+\tf = fopen(filename, \"r\");\n+\tif (f) {\n+\t\tret = do_config_from_file(fn, filename, f, filename, data);\n \t\tfclose(f);\n \t}\n \treturn ret;\n@@ -1190,7 +1202,9 @@ int git_config_with_options(config_fn_t fn, void *data,\n \t * If we have a specific filename, use it. Otherwise, follow the\n \t * regular lookup sequence.\n \t */\n-\tif (config_source && config_source->file)\n+\tif (config_source && config_source->use_stdin)\n+\t\treturn git_config_from_stdin(fn, data);\n+\telse if (config_source && config_source->file)\n \t\treturn git_config_from_file(fn, config_source->file, data);\n \telse if (config_source && config_source->blob)\n \t\treturn git_config_from_blob_ref(fn, config_source->blob, data);\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex 9673593..c9c426c 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -475,15 +475,28 @@ ein.bahn=strasse\n EOF\n \n test_expect_success 'alternative GIT_CONFIG' '\n-\tGIT_CONFIG=other-config git config -l >output &&\n+\tGIT_CONFIG=other-config git config --list >output &&\n \ttest_cmp expect output\n '\n \n test_expect_success 'alternative GIT_CONFIG (--file)' '\n-\tgit config --file other-config -l > output &&\n+\tgit config --file other-config --list >output &&\n \ttest_cmp expect output\n '\n \n+test_expect_success 'alternative GIT_CONFIG (--file=-)' '\n+\tgit config --file - --list <other-config >output &&\n+\ttest_cmp expect output\n+'\n+\n+test_expect_success 'setting a value in stdin is an error' '\n+\ttest_must_fail git config --file - some.value foo\n+'\n+\n+test_expect_success 'editing stdin is an error' '\n+\ttest_must_fail git config --file - --edit\n+'\n+\n test_expect_success 'refer config from subdirectory' '\n \tmkdir x &&\n \t(\ndiff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh\nindex 6edd38b..9ba2ba1 100755\n--- a/t/t1305-config-include.sh\n+++ b/t/t1305-config-include.sh\n@@ -113,7 +113,7 @@ test_expect_success 'missing include files are ignored' '\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+\tgit -c include.path=\"$(pwd)/one\" config test.one >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -138,6 +138,20 @@ test_expect_success 'relative includes from blobs fail' '\n \ttest_must_fail git config --blob=$blob test.one\n '\n \n+test_expect_success 'absolute includes from stdin work' '\n+\techo \"[test]one = 1\" >one &&\n+\techo 1 >expect &&\n+\techo \"[include]path=\\\"$(pwd)/one\\\"\" |\n+\tgit config --file - test.one >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'relative includes from stdin line fail' '\n+\techo \"[test]one = 1\" >one &&\n+\techo \"[include]path=one\" |\n+\ttest_must_fail git config --file - test.one\n+'\n+\n test_expect_success 'include cycles are detected' '\n \tcat >.gitconfig <<-\\EOF &&\n \t[test]value = gitconfig\n"}]}