All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] builtin/config.c: rename check_blob_write() -> check_write()
@ 2014-02-14 23:37 Kirill A. Shutemov
  2014-02-14 23:37 ` [PATCH 2/3] config: change git_config_with_options() interface Kirill A. Shutemov
  2014-02-14 23:37 ` [PATCH 3/3] config: teach "git config --file -" to read from the standard input Kirill A. Shutemov
  0 siblings, 2 replies; 8+ messages in thread
From: Kirill A. Shutemov @ 2014-02-14 23:37 UTC (permalink / raw)
  To: git; +Cc: Kirill A. Shutemov

The function will be reused to check for other conditions which prevent
write.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 builtin/config.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 92ebf23f0a9a..a7c55e68883c 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -362,7 +362,7 @@ static int get_colorbool(int print)
 		return get_colorbool_found ? 0 : 1;
 }
 
-static void check_blob_write(void)
+static void check_write(void)
 {
 	if (given_config_blob)
 		die("writing config blobs is not supported");
@@ -572,7 +572,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 	}
 	else if (actions == ACTION_SET) {
 		int ret;
-		check_blob_write();
+		check_write();
 		check_argc(argc, 2, 2);
 		value = normalize_value(argv[0], argv[1]);
 		ret = git_config_set_in_file(given_config_file, argv[0], value);
@@ -582,21 +582,21 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		return ret;
 	}
 	else if (actions == ACTION_SET_ALL) {
-		check_blob_write();
+		check_write();
 		check_argc(argc, 2, 3);
 		value = normalize_value(argv[0], argv[1]);
 		return git_config_set_multivar_in_file(given_config_file,
 						       argv[0], value, argv[2], 0);
 	}
 	else if (actions == ACTION_ADD) {
-		check_blob_write();
+		check_write();
 		check_argc(argc, 2, 2);
 		value = normalize_value(argv[0], argv[1]);
 		return git_config_set_multivar_in_file(given_config_file,
 						       argv[0], value, "^$", 0);
 	}
 	else if (actions == ACTION_REPLACE_ALL) {
-		check_blob_write();
+		check_write();
 		check_argc(argc, 2, 3);
 		value = normalize_value(argv[0], argv[1]);
 		return git_config_set_multivar_in_file(given_config_file,
@@ -623,7 +623,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		return get_urlmatch(argv[0], argv[1]);
 	}
 	else if (actions == ACTION_UNSET) {
-		check_blob_write();
+		check_write();
 		check_argc(argc, 1, 2);
 		if (argc == 2)
 			return git_config_set_multivar_in_file(given_config_file,
@@ -633,14 +633,14 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 						      argv[0], NULL);
 	}
 	else if (actions == ACTION_UNSET_ALL) {
-		check_blob_write();
+		check_write();
 		check_argc(argc, 1, 2);
 		return git_config_set_multivar_in_file(given_config_file,
 						       argv[0], NULL, argv[1], 1);
 	}
 	else if (actions == ACTION_RENAME_SECTION) {
 		int ret;
-		check_blob_write();
+		check_write();
 		check_argc(argc, 2, 2);
 		ret = git_config_rename_section_in_file(given_config_file,
 							argv[0], argv[1]);
@@ -651,7 +651,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 	}
 	else if (actions == ACTION_REMOVE_SECTION) {
 		int ret;
-		check_blob_write();
+		check_write();
 		check_argc(argc, 1, 1);
 		ret = git_config_rename_section_in_file(given_config_file,
 							argv[0], NULL);
-- 
1.8.5.2

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3] config: change git_config_with_options() interface
  2014-02-14 23:37 [PATCH 1/3] builtin/config.c: rename check_blob_write() -> check_write() Kirill A. Shutemov
@ 2014-02-14 23:37 ` Kirill A. Shutemov
  2014-02-14 23:37 ` [PATCH 3/3] config: teach "git config --file -" to read from the standard input Kirill A. Shutemov
  1 sibling, 0 replies; 8+ messages in thread
From: Kirill A. Shutemov @ 2014-02-14 23:37 UTC (permalink / raw)
  To: git; +Cc: Kirill A. Shutemov

We're going to have more options for config source.

Let's alter git_config_with_options() interface to accept struct with
all source options.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 builtin/config.c | 75 ++++++++++++++++++++++++++------------------------------
 cache.h          |  8 ++++--
 config.c         | 13 +++++-----
 3 files changed, 47 insertions(+), 49 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index a7c55e68883c..de41ba50e9ca 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -21,8 +21,7 @@ static char key_delim = ' ';
 static char term = '\n';
 
 static int use_global_config, use_system_config, use_local_config;
-static const char *given_config_file;
-static const char *given_config_blob;
+static struct git_config_source given_config_source;
 static int actions, types;
 static const char *get_color_slot, *get_colorbool_slot;
 static int end_null;
@@ -55,8 +54,8 @@ static struct option builtin_config_options[] = {
 	OPT_BOOL(0, "global", &use_global_config, N_("use global config file")),
 	OPT_BOOL(0, "system", &use_system_config, N_("use system config file")),
 	OPT_BOOL(0, "local", &use_local_config, N_("use repository config file")),
-	OPT_STRING('f', "file", &given_config_file, N_("file"), N_("use given config file")),
-	OPT_STRING(0, "blob", &given_config_blob, N_("blob-id"), N_("read config from given blob object")),
+	OPT_STRING('f', "file", &given_config_source.file, N_("file"), N_("use given config file")),
+	OPT_STRING(0, "blob", &given_config_source.blob, N_("blob-id"), N_("read config from given blob object")),
 	OPT_GROUP(N_("Action")),
 	OPT_BIT(0, "get", &actions, N_("get value: name [value-regex]"), ACTION_GET),
 	OPT_BIT(0, "get-all", &actions, N_("get all values: key [value-regex]"), ACTION_GET_ALL),
@@ -224,8 +223,7 @@ static int get_value(const char *key_, const char *regex_)
 	}
 
 	git_config_with_options(collect_config, &values,
-				given_config_file, given_config_blob,
-				respect_includes);
+				&given_config_source, respect_includes);
 
 	ret = !values.nr;
 
@@ -309,8 +307,7 @@ static void get_color(const char *def_color)
 	get_color_found = 0;
 	parsed_color[0] = '\0';
 	git_config_with_options(git_get_color_config, NULL,
-				given_config_file, given_config_blob,
-				respect_includes);
+				&given_config_source, respect_includes);
 
 	if (!get_color_found && def_color)
 		color_parse(def_color, "command line", parsed_color);
@@ -339,8 +336,7 @@ static int get_colorbool(int print)
 	get_diff_color_found = -1;
 	get_color_ui_found = -1;
 	git_config_with_options(git_get_colorbool_config, NULL,
-				given_config_file, given_config_blob,
-				respect_includes);
+				&given_config_source, respect_includes);
 
 	if (get_colorbool_found < 0) {
 		if (!strcmp(get_colorbool_slot, "color.diff"))
@@ -364,7 +360,7 @@ static int get_colorbool(int print)
 
 static void check_write(void)
 {
-	if (given_config_blob)
+	if (given_config_source.blob)
 		die("writing config blobs is not supported");
 }
 
@@ -435,7 +431,7 @@ static int get_urlmatch(const char *var, const char *url)
 	}
 
 	git_config_with_options(urlmatch_config_entry, &config,
-				given_config_file, NULL, respect_includes);
+				&given_config_source, respect_includes);
 
 	for_each_string_list_item(item, &values) {
 		struct urlmatch_current_candidate_value *matched = item->util;
@@ -464,14 +460,14 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 	int nongit = !startup_info->have_repository;
 	char *value;
 
-	given_config_file = getenv(CONFIG_ENVIRONMENT);
+	given_config_source.file = getenv(CONFIG_ENVIRONMENT);
 
 	argc = parse_options(argc, argv, prefix, builtin_config_options,
 			     builtin_config_usage,
 			     PARSE_OPT_STOP_AT_NON_OPTION);
 
 	if (use_global_config + use_system_config + use_local_config +
-	    !!given_config_file + !!given_config_blob > 1) {
+	    !!given_config_source.file + !!given_config_source.blob > 1) {
 		error("only one config file at a time.");
 		usage_with_options(builtin_config_usage, builtin_config_options);
 	}
@@ -493,24 +489,24 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 
 		if (access_or_warn(user_config, R_OK, 0) &&
 		    xdg_config && !access_or_warn(xdg_config, R_OK, 0))
-			given_config_file = xdg_config;
+			given_config_source.file = xdg_config;
 		else
-			given_config_file = user_config;
+			given_config_source.file = user_config;
 	}
 	else if (use_system_config)
-		given_config_file = git_etc_gitconfig();
+		given_config_source.file = git_etc_gitconfig();
 	else if (use_local_config)
-		given_config_file = git_pathdup("config");
-	else if (given_config_file) {
-		if (!is_absolute_path(given_config_file) && prefix)
-			given_config_file =
+		given_config_source.file = git_pathdup("config");
+	else if (given_config_source.file) {
+		if (!is_absolute_path(given_config_source.file) && prefix)
+			given_config_source.file =
 				xstrdup(prefix_filename(prefix,
 							strlen(prefix),
-							given_config_file));
+							given_config_source.file));
 	}
 
 	if (respect_includes == -1)
-		respect_includes = !given_config_file;
+		respect_includes = !given_config_source.file;
 
 	if (end_null) {
 		term = '\0';
@@ -549,25 +545,24 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 	if (actions == ACTION_LIST) {
 		check_argc(argc, 0, 0);
 		if (git_config_with_options(show_all_config, NULL,
-					    given_config_file,
-					    given_config_blob,
+					    &given_config_source,
 					    respect_includes) < 0) {
-			if (given_config_file)
+			if (given_config_source.file)
 				die_errno("unable to read config file '%s'",
-					  given_config_file);
+					  given_config_source.file);
 			else
 				die("error processing config file(s)");
 		}
 	}
 	else if (actions == ACTION_EDIT) {
 		check_argc(argc, 0, 0);
-		if (!given_config_file && nongit)
+		if (!given_config_source.file && nongit)
 			die("not in a git directory");
-		if (given_config_blob)
+		if (given_config_source.blob)
 			die("editing blobs is not supported");
 		git_config(git_default_config, NULL);
-		launch_editor(given_config_file ?
-			      given_config_file : git_path("config"),
+		launch_editor(given_config_source.file ?
+			      given_config_source.file : git_path("config"),
 			      NULL, NULL);
 	}
 	else if (actions == ACTION_SET) {
@@ -575,7 +570,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		check_write();
 		check_argc(argc, 2, 2);
 		value = normalize_value(argv[0], argv[1]);
-		ret = git_config_set_in_file(given_config_file, argv[0], value);
+		ret = git_config_set_in_file(given_config_source.file, argv[0], value);
 		if (ret == CONFIG_NOTHING_SET)
 			error("cannot overwrite multiple values with a single value\n"
 			"       Use a regexp, --add or --replace-all to change %s.", argv[0]);
@@ -585,21 +580,21 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		check_write();
 		check_argc(argc, 2, 3);
 		value = normalize_value(argv[0], argv[1]);
-		return git_config_set_multivar_in_file(given_config_file,
+		return git_config_set_multivar_in_file(given_config_source.file,
 						       argv[0], value, argv[2], 0);
 	}
 	else if (actions == ACTION_ADD) {
 		check_write();
 		check_argc(argc, 2, 2);
 		value = normalize_value(argv[0], argv[1]);
-		return git_config_set_multivar_in_file(given_config_file,
+		return git_config_set_multivar_in_file(given_config_source.file,
 						       argv[0], value, "^$", 0);
 	}
 	else if (actions == ACTION_REPLACE_ALL) {
 		check_write();
 		check_argc(argc, 2, 3);
 		value = normalize_value(argv[0], argv[1]);
-		return git_config_set_multivar_in_file(given_config_file,
+		return git_config_set_multivar_in_file(given_config_source.file,
 						       argv[0], value, argv[2], 1);
 	}
 	else if (actions == ACTION_GET) {
@@ -626,23 +621,23 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		check_write();
 		check_argc(argc, 1, 2);
 		if (argc == 2)
-			return git_config_set_multivar_in_file(given_config_file,
+			return git_config_set_multivar_in_file(given_config_source.file,
 							       argv[0], NULL, argv[1], 0);
 		else
-			return git_config_set_in_file(given_config_file,
+			return git_config_set_in_file(given_config_source.file,
 						      argv[0], NULL);
 	}
 	else if (actions == ACTION_UNSET_ALL) {
 		check_write();
 		check_argc(argc, 1, 2);
-		return git_config_set_multivar_in_file(given_config_file,
+		return git_config_set_multivar_in_file(given_config_source.file,
 						       argv[0], NULL, argv[1], 1);
 	}
 	else if (actions == ACTION_RENAME_SECTION) {
 		int ret;
 		check_write();
 		check_argc(argc, 2, 2);
-		ret = git_config_rename_section_in_file(given_config_file,
+		ret = git_config_rename_section_in_file(given_config_source.file,
 							argv[0], argv[1]);
 		if (ret < 0)
 			return ret;
@@ -653,7 +648,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		int ret;
 		check_write();
 		check_argc(argc, 1, 1);
-		ret = git_config_rename_section_in_file(given_config_file,
+		ret = git_config_rename_section_in_file(given_config_source.file,
 							argv[0], NULL);
 		if (ret < 0)
 			return ret;
diff --git a/cache.h b/cache.h
index dc040fb1aa99..9d94bd69f7db 100644
--- a/cache.h
+++ b/cache.h
@@ -1146,6 +1146,11 @@ extern int update_server_info(int);
 #define CONFIG_INVALID_PATTERN 6
 #define CONFIG_GENERIC_ERROR 7
 
+struct git_config_source {
+	const char *file;
+	const char *blob;
+};
+
 typedef int (*config_fn_t)(const char *, const char *, void *);
 extern int git_default_config(const char *, const char *, void *);
 extern int git_config_from_file(config_fn_t fn, const char *, void *);
@@ -1155,8 +1160,7 @@ extern void git_config_push_parameter(const char *text);
 extern int git_config_from_parameters(config_fn_t fn, void *data);
 extern int git_config(config_fn_t fn, void *);
 extern int git_config_with_options(config_fn_t fn, void *,
-				   const char *filename,
-				   const char *blob_ref,
+				   struct git_config_source *config_source,
 				   int respect_includes);
 extern int git_config_early(config_fn_t fn, void *, const char *repo_config);
 extern int git_parse_ulong(const char *, unsigned long *);
diff --git a/config.c b/config.c
index d969a5aefc2b..6269da907964 100644
--- a/config.c
+++ b/config.c
@@ -1170,8 +1170,7 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
 }
 
 int git_config_with_options(config_fn_t fn, void *data,
-			    const char *filename,
-			    const char *blob_ref,
+			    struct git_config_source *config_source,
 			    int respect_includes)
 {
 	char *repo_config = NULL;
@@ -1189,10 +1188,10 @@ int git_config_with_options(config_fn_t fn, void *data,
 	 * If we have a specific filename, use it. Otherwise, follow the
 	 * regular lookup sequence.
 	 */
-	if (filename)
-		return git_config_from_file(fn, filename, data);
-	else if (blob_ref)
-		return git_config_from_blob_ref(fn, blob_ref, data);
+	if (config_source && config_source->file)
+		return git_config_from_file(fn, config_source->file, data);
+	else if (config_source && config_source->blob)
+		return git_config_from_blob_ref(fn, config_source->blob, data);
 
 	repo_config = git_pathdup("config");
 	ret = git_config_early(fn, data, repo_config);
@@ -1203,7 +1202,7 @@ int git_config_with_options(config_fn_t fn, void *data,
 
 int git_config(config_fn_t fn, void *data)
 {
-	return git_config_with_options(fn, data, NULL, NULL, 1);
+	return git_config_with_options(fn, data, NULL, 1);
 }
 
 /*
-- 
1.8.5.2

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3] config: teach "git config --file -" to read from the standard input
  2014-02-14 23:37 [PATCH 1/3] builtin/config.c: rename check_blob_write() -> check_write() Kirill A. Shutemov
  2014-02-14 23:37 ` [PATCH 2/3] config: change git_config_with_options() interface Kirill A. Shutemov
@ 2014-02-14 23:37 ` Kirill A. Shutemov
  2014-02-16  2:12   ` Eric Sunshine
  1 sibling, 1 reply; 8+ messages in thread
From: Kirill A. Shutemov @ 2014-02-14 23:37 UTC (permalink / raw)
  To: git; +Cc: Kirill A. Shutemov

The patch extends git config --file interface to allow read config from
stdin.

Editing stdin or setting value in stdin is an error.

Include by absolute path is allowed in stdin config, but not by relative
path.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 builtin/config.c          | 11 +++++++++
 cache.h                   |  1 +
 config.c                  | 58 ++++++++++++++++++++++++++++++++---------------
 t/t1300-repo-config.sh    | 17 ++++++++++++--
 t/t1305-config-include.sh | 14 ++++++++++++
 5 files changed, 81 insertions(+), 20 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index de41ba50e9ca..5677c942b693 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -360,6 +360,9 @@ static int get_colorbool(int print)
 
 static void check_write(void)
 {
+	if (given_config_source.use_stdin)
+		die("writing to stdin is not supported");
+
 	if (given_config_source.blob)
 		die("writing config blobs is not supported");
 }
@@ -472,6 +475,12 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		usage_with_options(builtin_config_usage, builtin_config_options);
 	}
 
+	if (given_config_source.file &&
+			!strcmp(given_config_source.file, "-")) {
+		given_config_source.file = NULL;
+		given_config_source.use_stdin = 1;
+	}
+
 	if (use_global_config) {
 		char *user_config = NULL;
 		char *xdg_config = NULL;
@@ -558,6 +567,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		check_argc(argc, 0, 0);
 		if (!given_config_source.file && nongit)
 			die("not in a git directory");
+		if (given_config_source.use_stdin)
+			die("editing stdin is not supported");
 		if (given_config_source.blob)
 			die("editing blobs is not supported");
 		git_config(git_default_config, NULL);
diff --git a/cache.h b/cache.h
index 9d94bd69f7db..4db19b537059 100644
--- a/cache.h
+++ b/cache.h
@@ -1147,6 +1147,7 @@ extern int update_server_info(int);
 #define CONFIG_GENERIC_ERROR 7
 
 struct git_config_source {
+	unsigned int use_stdin:1;
 	const char *file;
 	const char *blob;
 };
diff --git a/config.c b/config.c
index 6269da907964..7b42608f5c89 100644
--- a/config.c
+++ b/config.c
@@ -443,10 +443,20 @@ static int git_parse_source(config_fn_t fn, void *data)
 		if (get_value(fn, data, var) < 0)
 			break;
 	}
-	if (cf->die_on_error)
-		die("bad config file line %d in %s", cf->linenr, cf->name);
-	else
-		return error("bad config file line %d in %s", cf->linenr, cf->name);
+	if (cf->die_on_error) {
+		if (cf->name)
+			die("bad config file line %d in %s",
+					cf->linenr, cf->name);
+		else
+			die("bad config file line %d", cf->linenr);
+
+	} else {
+		if (cf->name)
+			return error("bad config file line %d in %s",
+					cf->linenr, cf->name);
+		else
+			return error("bad config file line %d", cf->linenr);
+	}
 }
 
 static int parse_unit_factor(const char *end, uintmax_t *val)
@@ -1030,24 +1040,34 @@ static int do_config_from(struct config_source *top, config_fn_t fn, void *data)
 	return ret;
 }
 
-int git_config_from_file(config_fn_t fn, const char *filename, void *data)
+static int do_config_from_file(config_fn_t fn, const char *filename, FILE *f,
+			       void *data)
 {
-	int ret;
-	FILE *f = fopen(filename, "r");
+	struct config_source top;
 
-	ret = -1;
-	if (f) {
-		struct config_source top;
+	top.u.file = f;
+	top.name = filename;
+	top.die_on_error = 1;
+	top.do_fgetc = config_file_fgetc;
+	top.do_ungetc = config_file_ungetc;
+	top.do_ftell = config_file_ftell;
 
-		top.u.file = f;
-		top.name = filename;
-		top.die_on_error = 1;
-		top.do_fgetc = config_file_fgetc;
-		top.do_ungetc = config_file_ungetc;
-		top.do_ftell = config_file_ftell;
+	return do_config_from(&top, fn, data);
+}
+
+static int git_config_from_stdin(config_fn_t fn, void *data)
+{
+	return do_config_from_file(fn, NULL, stdin, data);
+}
 
-		ret = do_config_from(&top, fn, data);
+int git_config_from_file(config_fn_t fn, const char *filename, void *data)
+{
+	int ret = -1;
+	FILE *f;
 
+	f = fopen(filename, "r");
+	if (f) {
+		ret = do_config_from_file(fn, filename, f, data);
 		fclose(f);
 	}
 	return ret;
@@ -1188,7 +1208,9 @@ int git_config_with_options(config_fn_t fn, void *data,
 	 * If we have a specific filename, use it. Otherwise, follow the
 	 * regular lookup sequence.
 	 */
-	if (config_source && config_source->file)
+	if (config_source && config_source->use_stdin)
+		return git_config_from_stdin(fn, data);
+	else if (config_source && config_source->file)
 		return git_config_from_file(fn, config_source->file, data);
 	else if (config_source && config_source->blob)
 		return git_config_from_blob_ref(fn, config_source->blob, data);
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 967359344dab..c9c426c273e5 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -475,15 +475,28 @@ ein.bahn=strasse
 EOF
 
 test_expect_success 'alternative GIT_CONFIG' '
-	GIT_CONFIG=other-config git config -l >output &&
+	GIT_CONFIG=other-config git config --list >output &&
 	test_cmp expect output
 '
 
 test_expect_success 'alternative GIT_CONFIG (--file)' '
-	git config --file other-config -l > output &&
+	git config --file other-config --list >output &&
 	test_cmp expect output
 '
 
+test_expect_success 'alternative GIT_CONFIG (--file=-)' '
+	git config --file - --list <other-config >output &&
+	test_cmp expect output
+'
+
+test_expect_success 'setting a value in stdin is an error' '
+	test_must_fail git config --file - some.value foo
+'
+
+test_expect_success 'editing stdin is an error' '
+	test_must_fail git config --file - --edit
+'
+
 test_expect_success 'refer config from subdirectory' '
 	mkdir x &&
 	(
diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh
index a70707620f14..fda6555024c5 100755
--- a/t/t1305-config-include.sh
+++ b/t/t1305-config-include.sh
@@ -122,6 +122,20 @@ test_expect_success 'relative includes from command line fail' '
 	test_must_fail git -c include.path=one config test.one
 '
 
+test_expect_success 'absolute includes from stdin work' '
+	echo "[test]one = 1" >one &&
+	echo 1 >expect &&
+	echo "[include]path=\"$PWD/one\"" |
+	git config --file - test.one >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'relative includes from stdin line fail' '
+	echo "[test]one = 1" >one &&
+	echo "[include]path=one" |
+	test_must_fail git config --file - test.one
+'
+
 test_expect_success 'include cycles are detected' '
 	cat >.gitconfig <<-\EOF &&
 	[test]value = gitconfig
-- 
1.8.5.2

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] config: teach "git config --file -" to read from the standard input
  2014-02-14 23:37 ` [PATCH 3/3] config: teach "git config --file -" to read from the standard input Kirill A. Shutemov
@ 2014-02-16  2:12   ` Eric Sunshine
  2014-02-16 12:13     ` [PATCH] " Kirill A. Shutemov
  0 siblings, 1 reply; 8+ messages in thread
From: Eric Sunshine @ 2014-02-16  2:12 UTC (permalink / raw)
  To: Kirill A. Shutemov; +Cc: Git List

On Fri, Feb 14, 2014 at 6:37 PM, Kirill A. Shutemov
<kirill@shutemov.name> wrote:
> The patch extends git config --file interface to allow read config from
> stdin.
>
> Editing stdin or setting value in stdin is an error.
>
> Include by absolute path is allowed in stdin config, but not by relative
> path.
>
> Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
> ---
> diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh
> index a70707620f14..fda6555024c5 100755
> --- a/t/t1305-config-include.sh
> +++ b/t/t1305-config-include.sh
> @@ -122,6 +122,20 @@ test_expect_success 'relative includes from command line fail' '
>         test_must_fail git -c include.path=one config test.one
>  '
>
> +test_expect_success 'absolute includes from stdin work' '
> +       echo "[test]one = 1" >one &&
> +       echo 1 >expect &&
> +       echo "[include]path=\"$PWD/one\"" |

To be Windows-friendly, you may want to use $(pwd). Quoting from t/README:

   When a test checks for an absolute path that a git command generated,
   construct the expected value using $(pwd) rather than $PWD,
   $TEST_DIRECTORY, or $TRASH_DIRECTORY. It makes a difference on
   Windows, where the shell (MSYS bash) mangles absolute path names.
   For details, see the commit message of 4114156ae9.

> +       git config --file - test.one >actual &&
> +       test_cmp expect actual
> +'
> +
> +test_expect_success 'relative includes from stdin line fail' '
> +       echo "[test]one = 1" >one &&
> +       echo "[include]path=one" |
> +       test_must_fail git config --file - test.one
> +'
> +
>  test_expect_success 'include cycles are detected' '
>         cat >.gitconfig <<-\EOF &&
>         [test]value = gitconfig
> --
> 1.8.5.2

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] config: teach "git config --file -" to read from the standard input
  2014-02-16  2:12   ` Eric Sunshine
@ 2014-02-16 12:13     ` Kirill A. Shutemov
  2014-02-18  8:41       ` Jeff King
  0 siblings, 1 reply; 8+ messages in thread
From: Kirill A. Shutemov @ 2014-02-16 12:13 UTC (permalink / raw)
  To: git, Eric Sunshine; +Cc: Kirill A. Shutemov

The patch extends git config --file interface to allow read config from
stdin.

Editing stdin or setting value in stdin is an error.

Include by absolute path is allowed in stdin config, but not by relative
path.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 builtin/config.c          | 11 +++++++++
 cache.h                   |  1 +
 config.c                  | 58 ++++++++++++++++++++++++++++++++---------------
 t/t1300-repo-config.sh    | 17 ++++++++++++--
 t/t1305-config-include.sh | 16 ++++++++++++-
 5 files changed, 82 insertions(+), 21 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index de41ba50e9ca..5677c942b693 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -360,6 +360,9 @@ static int get_colorbool(int print)
 
 static void check_write(void)
 {
+	if (given_config_source.use_stdin)
+		die("writing to stdin is not supported");
+
 	if (given_config_source.blob)
 		die("writing config blobs is not supported");
 }
@@ -472,6 +475,12 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		usage_with_options(builtin_config_usage, builtin_config_options);
 	}
 
+	if (given_config_source.file &&
+			!strcmp(given_config_source.file, "-")) {
+		given_config_source.file = NULL;
+		given_config_source.use_stdin = 1;
+	}
+
 	if (use_global_config) {
 		char *user_config = NULL;
 		char *xdg_config = NULL;
@@ -558,6 +567,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		check_argc(argc, 0, 0);
 		if (!given_config_source.file && nongit)
 			die("not in a git directory");
+		if (given_config_source.use_stdin)
+			die("editing stdin is not supported");
 		if (given_config_source.blob)
 			die("editing blobs is not supported");
 		git_config(git_default_config, NULL);
diff --git a/cache.h b/cache.h
index 9d94bd69f7db..4db19b537059 100644
--- a/cache.h
+++ b/cache.h
@@ -1147,6 +1147,7 @@ extern int update_server_info(int);
 #define CONFIG_GENERIC_ERROR 7
 
 struct git_config_source {
+	unsigned int use_stdin:1;
 	const char *file;
 	const char *blob;
 };
diff --git a/config.c b/config.c
index 6269da907964..7b42608f5c89 100644
--- a/config.c
+++ b/config.c
@@ -443,10 +443,20 @@ static int git_parse_source(config_fn_t fn, void *data)
 		if (get_value(fn, data, var) < 0)
 			break;
 	}
-	if (cf->die_on_error)
-		die("bad config file line %d in %s", cf->linenr, cf->name);
-	else
-		return error("bad config file line %d in %s", cf->linenr, cf->name);
+	if (cf->die_on_error) {
+		if (cf->name)
+			die("bad config file line %d in %s",
+					cf->linenr, cf->name);
+		else
+			die("bad config file line %d", cf->linenr);
+
+	} else {
+		if (cf->name)
+			return error("bad config file line %d in %s",
+					cf->linenr, cf->name);
+		else
+			return error("bad config file line %d", cf->linenr);
+	}
 }
 
 static int parse_unit_factor(const char *end, uintmax_t *val)
@@ -1030,24 +1040,34 @@ static int do_config_from(struct config_source *top, config_fn_t fn, void *data)
 	return ret;
 }
 
-int git_config_from_file(config_fn_t fn, const char *filename, void *data)
+static int do_config_from_file(config_fn_t fn, const char *filename, FILE *f,
+			       void *data)
 {
-	int ret;
-	FILE *f = fopen(filename, "r");
+	struct config_source top;
 
-	ret = -1;
-	if (f) {
-		struct config_source top;
+	top.u.file = f;
+	top.name = filename;
+	top.die_on_error = 1;
+	top.do_fgetc = config_file_fgetc;
+	top.do_ungetc = config_file_ungetc;
+	top.do_ftell = config_file_ftell;
 
-		top.u.file = f;
-		top.name = filename;
-		top.die_on_error = 1;
-		top.do_fgetc = config_file_fgetc;
-		top.do_ungetc = config_file_ungetc;
-		top.do_ftell = config_file_ftell;
+	return do_config_from(&top, fn, data);
+}
+
+static int git_config_from_stdin(config_fn_t fn, void *data)
+{
+	return do_config_from_file(fn, NULL, stdin, data);
+}
 
-		ret = do_config_from(&top, fn, data);
+int git_config_from_file(config_fn_t fn, const char *filename, void *data)
+{
+	int ret = -1;
+	FILE *f;
 
+	f = fopen(filename, "r");
+	if (f) {
+		ret = do_config_from_file(fn, filename, f, data);
 		fclose(f);
 	}
 	return ret;
@@ -1188,7 +1208,9 @@ int git_config_with_options(config_fn_t fn, void *data,
 	 * If we have a specific filename, use it. Otherwise, follow the
 	 * regular lookup sequence.
 	 */
-	if (config_source && config_source->file)
+	if (config_source && config_source->use_stdin)
+		return git_config_from_stdin(fn, data);
+	else if (config_source && config_source->file)
 		return git_config_from_file(fn, config_source->file, data);
 	else if (config_source && config_source->blob)
 		return git_config_from_blob_ref(fn, config_source->blob, data);
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 967359344dab..c9c426c273e5 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -475,15 +475,28 @@ ein.bahn=strasse
 EOF
 
 test_expect_success 'alternative GIT_CONFIG' '
-	GIT_CONFIG=other-config git config -l >output &&
+	GIT_CONFIG=other-config git config --list >output &&
 	test_cmp expect output
 '
 
 test_expect_success 'alternative GIT_CONFIG (--file)' '
-	git config --file other-config -l > output &&
+	git config --file other-config --list >output &&
 	test_cmp expect output
 '
 
+test_expect_success 'alternative GIT_CONFIG (--file=-)' '
+	git config --file - --list <other-config >output &&
+	test_cmp expect output
+'
+
+test_expect_success 'setting a value in stdin is an error' '
+	test_must_fail git config --file - some.value foo
+'
+
+test_expect_success 'editing stdin is an error' '
+	test_must_fail git config --file - --edit
+'
+
 test_expect_success 'refer config from subdirectory' '
 	mkdir x &&
 	(
diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh
index a70707620f14..01e50c5882d0 100755
--- a/t/t1305-config-include.sh
+++ b/t/t1305-config-include.sh
@@ -113,7 +113,7 @@ test_expect_success 'missing include files are ignored' '
 test_expect_success 'absolute includes from command line work' '
 	echo "[test]one = 1" >one &&
 	echo 1 >expect &&
-	git -c include.path="$PWD/one" config test.one >actual &&
+	git -c include.path="$(pwd)/one" config test.one >actual &&
 	test_cmp expect actual
 '
 
@@ -122,6 +122,20 @@ test_expect_success 'relative includes from command line fail' '
 	test_must_fail git -c include.path=one config test.one
 '
 
+test_expect_success 'absolute includes from stdin work' '
+	echo "[test]one = 1" >one &&
+	echo 1 >expect &&
+	echo "[include]path=\"$(pwd)/one\"" |
+	git config --file - test.one >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'relative includes from stdin line fail' '
+	echo "[test]one = 1" >one &&
+	echo "[include]path=one" |
+	test_must_fail git config --file - test.one
+'
+
 test_expect_success 'include cycles are detected' '
 	cat >.gitconfig <<-\EOF &&
 	[test]value = gitconfig
-- 
1.8.5.2

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] config: teach "git config --file -" to read from the standard input
  2014-02-16 12:13     ` [PATCH] " Kirill A. Shutemov
@ 2014-02-18  8:41       ` Jeff King
  2014-02-18 19:15         ` Junio C Hamano
  0 siblings, 1 reply; 8+ messages in thread
From: Jeff King @ 2014-02-18  8:41 UTC (permalink / raw)
  To: Kirill A. Shutemov; +Cc: git, Eric Sunshine

On Sun, Feb 16, 2014 at 02:13:01PM +0200, Kirill A. Shutemov wrote:

> The patch extends git config --file interface to allow read config from
> stdin.
> 
> Editing stdin or setting value in stdin is an error.
> 
> Include by absolute path is allowed in stdin config, but not by relative
> path.

Thanks, this looks very cleanly done.

One nit:

> diff --git a/config.c b/config.c
> index 6269da907964..7b42608f5c89 100644
> --- a/config.c
> +++ b/config.c
> @@ -443,10 +443,20 @@ static int git_parse_source(config_fn_t fn, void *data)
>  		if (get_value(fn, data, var) < 0)
>  			break;
>  	}
> -	if (cf->die_on_error)
> -		die("bad config file line %d in %s", cf->linenr, cf->name);
> -	else
> -		return error("bad config file line %d in %s", cf->linenr, cf->name);
> +	if (cf->die_on_error) {
> +		if (cf->name)
> +			die("bad config file line %d in %s",
> +					cf->linenr, cf->name);
> +		else
> +			die("bad config file line %d", cf->linenr);
> +
> +	} else {
> +		if (cf->name)
> +			return error("bad config file line %d in %s",
> +					cf->linenr, cf->name);
> +		else
> +			return error("bad config file line %d", cf->linenr);
> +	}

I think I preferred the earlier version where you had "<stdin>" in the
name field, and this hunk could just go away. I know you switched it to
NULL here to avoid making bogus relative filenames in includes.

But that would be pretty straightforward to fix by splitting the "name"
field into an additional "path" field. It looks like "git config --blob"
has the same problem (it will try relative lookups in the filesystem,
even though that is nonsensical from the blob). So we could fix the
existing bug at the same time. :)

Perhaps this could go at the start of your series?

-- >8 --
Subject: config: disallow relative include paths from blobs

When we see a relative config include like:

  [include]
  path = foo

we make it relative to the containing directory of the file
that contains the snippet. This makes no sense for config
read from a blob, as it is not on the filesystem.  Something
like "HEAD:some/path" could have a relative path within the
tree, but:

  1. It would not be part of include.path, which explicitly
     refers to the filesystem.

  2. It would need different parsing rules anyway to
     determine that it is a tree path.

The current code just uses the "name" field, which is wrong.
Let's split that into "name" and "path" fields, use the
latter for relative includes, and fill in only the former
for blobs.

Signed-off-by: Jeff King <peff@peff.net>
---
I don't think we considered includes at all when adding --blob. I cannot
think of any good reason to have even an absolute filesystem include
from a blob. And I wonder if it is possible to cause security mischief
by convincing git to look at /etc/passwd or similar (it would not parse,
of course, but you might be able to glean information from the error
messages or something).

It's probably OK, though, because you would generally not read real
config from an untrusted source (there are many bad things they could
set), and other code which uses the config reader explicitly does not
turn on includes.

 config.c                  | 10 ++++++----
 t/t1305-config-include.sh | 16 ++++++++++++++++
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/config.c b/config.c
index d969a5a..b295310 100644
--- a/config.c
+++ b/config.c
@@ -21,6 +21,7 @@ struct config_source {
 		} buf;
 	} u;
 	const char *name;
+	const char *path;
 	int die_on_error;
 	int linenr;
 	int eof;
@@ -97,12 +98,12 @@ static int handle_path_include(const char *path, struct config_include_data *inc
 	if (!is_absolute_path(path)) {
 		char *slash;
 
-		if (!cf || !cf->name)
+		if (!cf || !cf->path)
 			return error("relative config includes must come from files");
 
-		slash = find_last_dir_sep(cf->name);
+		slash = find_last_dir_sep(cf->path);
 		if (slash)
-			strbuf_add(&buf, cf->name, slash - cf->name + 1);
+			strbuf_add(&buf, cf->path, slash - cf->path + 1);
 		strbuf_addstr(&buf, path);
 		path = buf.buf;
 	}
@@ -1040,7 +1041,7 @@ int git_config_from_file(config_fn_t fn, const char *filename, void *data)
 		struct config_source top;
 
 		top.u.file = f;
-		top.name = filename;
+		top.name = top.path = filename;
 		top.die_on_error = 1;
 		top.do_fgetc = config_file_fgetc;
 		top.do_ungetc = config_file_ungetc;
@@ -1062,6 +1063,7 @@ int git_config_from_buf(config_fn_t fn, const char *name, const char *buf,
 	top.u.buf.len = len;
 	top.u.buf.pos = 0;
 	top.name = name;
+	top.path = NULL;
 	top.die_on_error = 0;
 	top.do_fgetc = config_buf_fgetc;
 	top.do_ungetc = config_buf_ungetc;
diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh
index a707076..6edd38b 100755
--- a/t/t1305-config-include.sh
+++ b/t/t1305-config-include.sh
@@ -122,6 +122,22 @@ test_expect_success 'relative includes from command line fail' '
 	test_must_fail git -c include.path=one config test.one
 '
 
+test_expect_success 'absolute includes from blobs work' '
+	echo "[test]one = 1" >one &&
+	echo "[include]path=$(pwd)/one" >blob &&
+	blob=$(git hash-object -w blob) &&
+	echo 1 >expect &&
+	git config --blob=$blob test.one >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'relative includes from blobs fail' '
+	echo "[test]one = 1" >one &&
+	echo "[include]path=one" >blob &&
+	blob=$(git hash-object -w blob) &&
+	test_must_fail git config --blob=$blob test.one
+'
+
 test_expect_success 'include cycles are detected' '
 	cat >.gitconfig <<-\EOF &&
 	[test]value = gitconfig
-- 
1.8.5.2.500.g8060133

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] config: teach "git config --file -" to read from the standard input
  2014-02-18  8:41       ` Jeff King
@ 2014-02-18 19:15         ` Junio C Hamano
  2014-02-18 23:02           ` Junio C Hamano
  0 siblings, 1 reply; 8+ messages in thread
From: Junio C Hamano @ 2014-02-18 19:15 UTC (permalink / raw)
  To: Jeff King; +Cc: Kirill A. Shutemov, git, Eric Sunshine

Jeff King <peff@peff.net> writes:

>> +	} else {
>> +		if (cf->name)
>> +			return error("bad config file line %d in %s",
>> +					cf->linenr, cf->name);
>> +		else
>> +			return error("bad config file line %d", cf->linenr);
>> +	}
>
> I think I preferred the earlier version where you had "<stdin>" in the
> name field, and this hunk could just go away. I know you switched it to
> NULL here to avoid making bogus relative filenames in includes.

Exactly the same comment here.  I really like the way how this
series becomes more polished every time we see it.

> But that would be pretty straightforward to fix by splitting the "name"
> field into an additional "path" field. It looks like "git config --blob"
> has the same problem (it will try relative lookups in the filesystem,
> even though that is nonsensical from the blob). So we could fix the
> existing bug at the same time. :)
>
> Perhaps this could go at the start of your series?

Sounds like the right thing to do.

Thanks.

> -- >8 --
> Subject: config: disallow relative include paths from blobs
>
> When we see a relative config include like:
>
>   [include]
>   path = foo
>
> we make it relative to the containing directory of the file
> that contains the snippet. This makes no sense for config
> read from a blob, as it is not on the filesystem.  Something
> like "HEAD:some/path" could have a relative path within the
> tree, but:
>
>   1. It would not be part of include.path, which explicitly
>      refers to the filesystem.
>
>   2. It would need different parsing rules anyway to
>      determine that it is a tree path.
>
> The current code just uses the "name" field, which is wrong.
> Let's split that into "name" and "path" fields, use the
> latter for relative includes, and fill in only the former
> for blobs.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> I don't think we considered includes at all when adding --blob. I cannot
> think of any good reason to have even an absolute filesystem include
> from a blob. And I wonder if it is possible to cause security mischief
> by convincing git to look at /etc/passwd or similar (it would not parse,
> of course, but you might be able to glean information from the error
> messages or something).
>
> It's probably OK, though, because you would generally not read real
> config from an untrusted source (there are many bad things they could
> set), and other code which uses the config reader explicitly does not
> turn on includes.
>
>  config.c                  | 10 ++++++----
>  t/t1305-config-include.sh | 16 ++++++++++++++++
>  2 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/config.c b/config.c
> index d969a5a..b295310 100644
> --- a/config.c
> +++ b/config.c
> @@ -21,6 +21,7 @@ struct config_source {
>  		} buf;
>  	} u;
>  	const char *name;
> +	const char *path;
>  	int die_on_error;
>  	int linenr;
>  	int eof;
> @@ -97,12 +98,12 @@ static int handle_path_include(const char *path, struct config_include_data *inc
>  	if (!is_absolute_path(path)) {
>  		char *slash;
>  
> -		if (!cf || !cf->name)
> +		if (!cf || !cf->path)
>  			return error("relative config includes must come from files");
>  
> -		slash = find_last_dir_sep(cf->name);
> +		slash = find_last_dir_sep(cf->path);
>  		if (slash)
> -			strbuf_add(&buf, cf->name, slash - cf->name + 1);
> +			strbuf_add(&buf, cf->path, slash - cf->path + 1);
>  		strbuf_addstr(&buf, path);
>  		path = buf.buf;
>  	}
> @@ -1040,7 +1041,7 @@ int git_config_from_file(config_fn_t fn, const char *filename, void *data)
>  		struct config_source top;
>  
>  		top.u.file = f;
> -		top.name = filename;
> +		top.name = top.path = filename;
>  		top.die_on_error = 1;
>  		top.do_fgetc = config_file_fgetc;
>  		top.do_ungetc = config_file_ungetc;
> @@ -1062,6 +1063,7 @@ int git_config_from_buf(config_fn_t fn, const char *name, const char *buf,
>  	top.u.buf.len = len;
>  	top.u.buf.pos = 0;
>  	top.name = name;
> +	top.path = NULL;
>  	top.die_on_error = 0;
>  	top.do_fgetc = config_buf_fgetc;
>  	top.do_ungetc = config_buf_ungetc;
> diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh
> index a707076..6edd38b 100755
> --- a/t/t1305-config-include.sh
> +++ b/t/t1305-config-include.sh
> @@ -122,6 +122,22 @@ test_expect_success 'relative includes from command line fail' '
>  	test_must_fail git -c include.path=one config test.one
>  '
>  
> +test_expect_success 'absolute includes from blobs work' '
> +	echo "[test]one = 1" >one &&
> +	echo "[include]path=$(pwd)/one" >blob &&
> +	blob=$(git hash-object -w blob) &&
> +	echo 1 >expect &&
> +	git config --blob=$blob test.one >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'relative includes from blobs fail' '
> +	echo "[test]one = 1" >one &&
> +	echo "[include]path=one" >blob &&
> +	blob=$(git hash-object -w blob) &&
> +	test_must_fail git config --blob=$blob test.one
> +'
> +
>  test_expect_success 'include cycles are detected' '
>  	cat >.gitconfig <<-\EOF &&
>  	[test]value = gitconfig

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] config: teach "git config --file -" to read from the standard input
  2014-02-18 19:15         ` Junio C Hamano
@ 2014-02-18 23:02           ` Junio C Hamano
  0 siblings, 0 replies; 8+ messages in thread
From: Junio C Hamano @ 2014-02-18 23:02 UTC (permalink / raw)
  To: Jeff King; +Cc: Kirill A. Shutemov, git, Eric Sunshine

Junio C Hamano <gitster@pobox.com> writes:

>> I think I preferred the earlier version where you had "<stdin>" in the
>> name field, and this hunk could just go away. I know you switched it to
>> NULL here to avoid making bogus relative filenames in includes.
>
> Exactly the same comment here.  I really like the way how this
> series becomes more polished every time we see it.
>
>> But that would be pretty straightforward to fix by splitting the "name"
>> field into an additional "path" field. It looks like "git config --blob"
>> has the same problem (it will try relative lookups in the filesystem,
>> even though that is nonsensical from the blob). So we could fix the
>> existing bug at the same time. :)
>>
>> Perhaps this could go at the start of your series?
>
> Sounds like the right thing to do.
>
> Thanks.

And [PATCH 3/3] rebased on the others may look like this.

 builtin/config.c          | 11 +++++++++++
 cache.h                   |  1 +
 config.c                  | 42 ++++++++++++++++++++++++++++--------------
 t/t1300-repo-config.sh    | 17 +++++++++++++++--
 t/t1305-config-include.sh | 16 +++++++++++++++-
 5 files changed, 70 insertions(+), 17 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index de41ba5..5677c94 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -360,6 +360,9 @@ static int get_colorbool(int print)
 
 static void check_write(void)
 {
+	if (given_config_source.use_stdin)
+		die("writing to stdin is not supported");
+
 	if (given_config_source.blob)
 		die("writing config blobs is not supported");
 }
@@ -472,6 +475,12 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		usage_with_options(builtin_config_usage, builtin_config_options);
 	}
 
+	if (given_config_source.file &&
+			!strcmp(given_config_source.file, "-")) {
+		given_config_source.file = NULL;
+		given_config_source.use_stdin = 1;
+	}
+
 	if (use_global_config) {
 		char *user_config = NULL;
 		char *xdg_config = NULL;
@@ -558,6 +567,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		check_argc(argc, 0, 0);
 		if (!given_config_source.file && nongit)
 			die("not in a git directory");
+		if (given_config_source.use_stdin)
+			die("editing stdin is not supported");
 		if (given_config_source.blob)
 			die("editing blobs is not supported");
 		git_config(git_default_config, NULL);
diff --git a/cache.h b/cache.h
index 9d94bd6..4db19b5 100644
--- a/cache.h
+++ b/cache.h
@@ -1147,6 +1147,7 @@ extern int update_server_info(int);
 #define CONFIG_GENERIC_ERROR 7
 
 struct git_config_source {
+	unsigned int use_stdin:1;
 	const char *file;
 	const char *blob;
 };
diff --git a/config.c b/config.c
index a21b0dd..9dd0bdb 100644
--- a/config.c
+++ b/config.c
@@ -1031,24 +1031,36 @@ static int do_config_from(struct config_source *top, config_fn_t fn, void *data)
 	return ret;
 }
 
-int git_config_from_file(config_fn_t fn, const char *filename, void *data)
+static int do_config_from_file(config_fn_t fn,
+			       const char *filename, FILE *f,
+			       const char *label, void *data)
 {
-	int ret;
-	FILE *f = fopen(filename, "r");
+	struct config_source top;
 
-	ret = -1;
-	if (f) {
-		struct config_source top;
+	top.u.file = f;
+	top.name = label;
+	top.path = filename;
+	top.die_on_error = 1;
+	top.do_fgetc = config_file_fgetc;
+	top.do_ungetc = config_file_ungetc;
+	top.do_ftell = config_file_ftell;
+
+	return do_config_from(&top, fn, data);
+}
 
-		top.u.file = f;
-		top.name = top.path = filename;
-		top.die_on_error = 1;
-		top.do_fgetc = config_file_fgetc;
-		top.do_ungetc = config_file_ungetc;
-		top.do_ftell = config_file_ftell;
+static int git_config_from_stdin(config_fn_t fn, void *data)
+{
+	return do_config_from_file(fn, NULL, stdin, "<stdin>", data);
+}
 
-		ret = do_config_from(&top, fn, data);
+int git_config_from_file(config_fn_t fn, const char *filename, void *data)
+{
+	int ret = -1;
+	FILE *f;
 
+	f = fopen(filename, "r");
+	if (f) {
+		ret = do_config_from_file(fn, filename, f, filename, data);
 		fclose(f);
 	}
 	return ret;
@@ -1190,7 +1202,9 @@ int git_config_with_options(config_fn_t fn, void *data,
 	 * If we have a specific filename, use it. Otherwise, follow the
 	 * regular lookup sequence.
 	 */
-	if (config_source && config_source->file)
+	if (config_source && config_source->use_stdin)
+		return git_config_from_stdin(fn, data);
+	else if (config_source && config_source->file)
 		return git_config_from_file(fn, config_source->file, data);
 	else if (config_source && config_source->blob)
 		return git_config_from_blob_ref(fn, config_source->blob, data);
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 9673593..c9c426c 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -475,15 +475,28 @@ ein.bahn=strasse
 EOF
 
 test_expect_success 'alternative GIT_CONFIG' '
-	GIT_CONFIG=other-config git config -l >output &&
+	GIT_CONFIG=other-config git config --list >output &&
 	test_cmp expect output
 '
 
 test_expect_success 'alternative GIT_CONFIG (--file)' '
-	git config --file other-config -l > output &&
+	git config --file other-config --list >output &&
 	test_cmp expect output
 '
 
+test_expect_success 'alternative GIT_CONFIG (--file=-)' '
+	git config --file - --list <other-config >output &&
+	test_cmp expect output
+'
+
+test_expect_success 'setting a value in stdin is an error' '
+	test_must_fail git config --file - some.value foo
+'
+
+test_expect_success 'editing stdin is an error' '
+	test_must_fail git config --file - --edit
+'
+
 test_expect_success 'refer config from subdirectory' '
 	mkdir x &&
 	(
diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh
index 6edd38b..9ba2ba1 100755
--- a/t/t1305-config-include.sh
+++ b/t/t1305-config-include.sh
@@ -113,7 +113,7 @@ test_expect_success 'missing include files are ignored' '
 test_expect_success 'absolute includes from command line work' '
 	echo "[test]one = 1" >one &&
 	echo 1 >expect &&
-	git -c include.path="$PWD/one" config test.one >actual &&
+	git -c include.path="$(pwd)/one" config test.one >actual &&
 	test_cmp expect actual
 '
 
@@ -138,6 +138,20 @@ test_expect_success 'relative includes from blobs fail' '
 	test_must_fail git config --blob=$blob test.one
 '
 
+test_expect_success 'absolute includes from stdin work' '
+	echo "[test]one = 1" >one &&
+	echo 1 >expect &&
+	echo "[include]path=\"$(pwd)/one\"" |
+	git config --file - test.one >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'relative includes from stdin line fail' '
+	echo "[test]one = 1" >one &&
+	echo "[include]path=one" |
+	test_must_fail git config --file - test.one
+'
+
 test_expect_success 'include cycles are detected' '
 	cat >.gitconfig <<-\EOF &&
 	[test]value = gitconfig

^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-02-18 23:03 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-14 23:37 [PATCH 1/3] builtin/config.c: rename check_blob_write() -> check_write() Kirill A. Shutemov
2014-02-14 23:37 ` [PATCH 2/3] config: change git_config_with_options() interface Kirill A. Shutemov
2014-02-14 23:37 ` [PATCH 3/3] config: teach "git config --file -" to read from the standard input Kirill A. Shutemov
2014-02-16  2:12   ` Eric Sunshine
2014-02-16 12:13     ` [PATCH] " Kirill A. Shutemov
2014-02-18  8:41       ` Jeff King
2014-02-18 19:15         ` Junio C Hamano
2014-02-18 23:02           ` Junio C Hamano

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.