All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/4] config: add '--sources' option to print the source of a config value
@ 2016-02-19  9:15 larsxschneider
  2016-02-19  9:15 ` [PATCH v6 1/4] t: do not hide Git's exit code in tests using 'nul_to_q' larsxschneider
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: larsxschneider @ 2016-02-19  9:15 UTC (permalink / raw)
  To: git; +Cc: peff, ramsay, gitster, Johannes.Schindelin, Lars Schneider

From: Lars Schneider <larsxschneider@gmail.com>

diff to v5:
* rename 'type' to 'origin_type' as 'type' is too broad a word in the context
  of configuration file (Thanks to the reviewers Junio and Dscho)
* add dedicated patch to rename git_config_from_buf to git_config_from_mem
  (this change was part of the series since v4 as suggested by Junio but
  hidden in the "config: add 'origin_type'" patch)

Thanks,
Lars

Lars Schneider (4):
  t: do not hide Git's exit code in tests using 'nul_to_q'
  rename git_config_from_buf to git_config_from_mem
  config: add 'origin_type' to config_source struct
  config: add '--show-origin' option to print the origin of a config
    value

 Documentation/git-config.txt |  15 ++--
 builtin/config.c             |  33 +++++++++
 cache.h                      |   6 +-
 config.c                     |  36 +++++++---
 submodule-config.c           |   4 +-
 t/t1300-repo-config.sh       | 161 ++++++++++++++++++++++++++++++++++++++++++-
 t/t1308-config-set.sh        |   4 +-
 t/t7008-grep-binary.sh       |   3 +-
 8 files changed, 236 insertions(+), 26 deletions(-)

--
2.5.1

^ permalink raw reply	[flat|nested] 17+ messages in thread
* [PATCH v6 1/4] t: do not hide Git's exit code in tests using 'nul_to_q'
  2016-02-19  9:15 [PATCH v6 0/4] config: add '--sources' option to print the source of a config value larsxschneider
@ 2016-02-19  9:15 ` larsxschneider
  2016-02-19  9:16 ` [PATCH v6 2/4] rename git_config_from_buf to git_config_from_mem larsxschneider
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 17+ messages in thread
From: larsxschneider @ 2016-02-19  9:15 UTC (permalink / raw)
  To: git; +Cc: peff, ramsay, gitster, Johannes.Schindelin, Lars Schneider

From: Lars Schneider <larsxschneider@gmail.com>

Git should not be on the left-hand side of a pipe, because it hides the exit
code, and we want to make sure git does not fail.

Fix all invocations of 'nul_to_q' (defined in /t/test-lib-functions.sh) using
this pattern. There is one more occurrence of the pattern in t9010-svn-fe.sh
which is too evolved to change it easily.

All remaining test code that does not adhere to the pattern can be found with
the following command:
git grep -E 'git.*[^|]\|($|[^|])'

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
 t/t1300-repo-config.sh | 6 ++++--
 t/t7008-grep-binary.sh | 3 ++-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 52678e7..1782add 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -957,13 +957,15 @@ Qsection.sub=section.val4
 Qsection.sub=section.val5Q
 EOF
 test_expect_success '--null --list' '
-	git config --null --list | nul_to_q >result &&
+	git config --null --list >result.raw &&
+	nul_to_q <result.raw >result &&
 	echo >>result &&
 	test_cmp expect result
 '
 
 test_expect_success '--null --get-regexp' '
-	git config --null --get-regexp "val[0-9]" | nul_to_q >result &&
+	git config --null --get-regexp "val[0-9]" >result.raw &&
+	nul_to_q <result.raw >result &&
 	echo >>result &&
 	test_cmp expect result
 '
diff --git a/t/t7008-grep-binary.sh b/t/t7008-grep-binary.sh
index b146406..9c9c378 100755
--- a/t/t7008-grep-binary.sh
+++ b/t/t7008-grep-binary.sh
@@ -141,7 +141,8 @@ test_expect_success 'grep respects not-binary diff attribute' '
 	test_cmp expect actual &&
 	echo "b diff" >.gitattributes &&
 	echo "b:binQary" >expect &&
-	git grep bin b | nul_to_q >actual &&
+	git grep bin b >actual.raw &&
+	nul_to_q <actual.raw >actual &&
 	test_cmp expect actual
 '
 
-- 
2.5.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH v6 2/4] rename git_config_from_buf to git_config_from_mem
  2016-02-19  9:15 [PATCH v6 0/4] config: add '--sources' option to print the source of a config value larsxschneider
  2016-02-19  9:15 ` [PATCH v6 1/4] t: do not hide Git's exit code in tests using 'nul_to_q' larsxschneider
@ 2016-02-19  9:16 ` larsxschneider
  2016-02-19  9:16 ` [PATCH v6 3/4] config: add 'origin_type' to config_source struct larsxschneider
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 17+ messages in thread
From: larsxschneider @ 2016-02-19  9:16 UTC (permalink / raw)
  To: git; +Cc: peff, ramsay, gitster, Johannes.Schindelin, Lars Schneider

From: Lars Schneider <larsxschneider@gmail.com>

This matches the naming used in the index_{fd,mem,...} functions.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
 cache.h            | 2 +-
 config.c           | 4 ++--
 submodule-config.c | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/cache.h b/cache.h
index c63fcc1..6679bb4 100644
--- a/cache.h
+++ b/cache.h
@@ -1485,7 +1485,7 @@ struct git_config_source {
 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 *);
-extern int git_config_from_buf(config_fn_t fn, const char *name,
+extern int git_config_from_mem(config_fn_t fn, const char *name,
 			       const char *buf, size_t len, void *data);
 extern void git_config_push_parameter(const char *text);
 extern int git_config_from_parameters(config_fn_t fn, void *data);
diff --git a/config.c b/config.c
index 86a5eb2..36b0ddb 100644
--- a/config.c
+++ b/config.c
@@ -1096,7 +1096,7 @@ int git_config_from_file(config_fn_t fn, const char *filename, void *data)
 	return ret;
 }
 
-int git_config_from_buf(config_fn_t fn, const char *name, const char *buf,
+int git_config_from_mem(config_fn_t fn, const char *name, const char *buf,
 			size_t len, void *data)
 {
 	struct config_source top;
@@ -1132,7 +1132,7 @@ static int git_config_from_blob_sha1(config_fn_t fn,
 		return error("reference '%s' does not point to a blob", name);
 	}
 
-	ret = git_config_from_buf(fn, name, buf, size, data);
+	ret = git_config_from_mem(fn, name, buf, size, data);
 	free(buf);
 
 	return ret;
diff --git a/submodule-config.c b/submodule-config.c
index fe8ceab..b85a937 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -427,7 +427,7 @@ static const struct submodule *config_from(struct submodule_cache *cache,
 	parameter.commit_sha1 = commit_sha1;
 	parameter.gitmodules_sha1 = sha1;
 	parameter.overwrite = 0;
-	git_config_from_buf(parse_config, rev.buf, config, config_size,
+	git_config_from_mem(parse_config, rev.buf, config, config_size,
 			&parameter);
 	free(config);
 
-- 
2.5.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH v6 3/4] config: add 'origin_type' to config_source struct
  2016-02-19  9:15 [PATCH v6 0/4] config: add '--sources' option to print the source of a config value larsxschneider
  2016-02-19  9:15 ` [PATCH v6 1/4] t: do not hide Git's exit code in tests using 'nul_to_q' larsxschneider
  2016-02-19  9:16 ` [PATCH v6 2/4] rename git_config_from_buf to git_config_from_mem larsxschneider
@ 2016-02-19  9:16 ` larsxschneider
  2016-02-19  9:16 ` [PATCH v6 4/4] config: add '--show-origin' option to print the origin of a config value larsxschneider
  2016-02-19 18:26 ` [PATCH v6 0/4] config: add '--sources' option to print the source " Junio C Hamano
  4 siblings, 0 replies; 17+ messages in thread
From: larsxschneider @ 2016-02-19  9:16 UTC (permalink / raw)
  To: git; +Cc: peff, ramsay, gitster, Johannes.Schindelin, Lars Schneider

From: Lars Schneider <larsxschneider@gmail.com>

Use the config origin_type to print more detailed error messages that
inform the user about the origin of a config error (file, stdin, blob).

Helped-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
Acked-by: Jeff King <peff@peff.net>
---
 cache.h                |  6 ++++--
 config.c               | 36 +++++++++++++++++++++++++-----------
 submodule-config.c     |  4 ++--
 t/t1300-repo-config.sh |  8 +++++++-
 t/t1308-config-set.sh  |  4 ++--
 5 files changed, 40 insertions(+), 18 deletions(-)

diff --git a/cache.h b/cache.h
index 6679bb4..ad7fcfc 100644
--- a/cache.h
+++ b/cache.h
@@ -1485,8 +1485,8 @@ struct git_config_source {
 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 *);
-extern int git_config_from_mem(config_fn_t fn, const char *name,
-			       const char *buf, size_t len, void *data);
+extern int git_config_from_mem(config_fn_t fn, const char *origin_type,
+					const char *name, const char *buf, size_t len, void *data);
 extern void git_config_push_parameter(const char *text);
 extern int git_config_from_parameters(config_fn_t fn, void *data);
 extern void git_config(config_fn_t fn, void *);
@@ -1525,6 +1525,8 @@ extern const char *get_log_output_encoding(void);
 extern const char *get_commit_output_encoding(void);
 
 extern int git_config_parse_parameter(const char *, config_fn_t fn, void *data);
+extern const char *current_config_origin_type(void);
+extern const char *current_config_name(void);
 
 struct config_include_data {
 	int depth;
diff --git a/config.c b/config.c
index 36b0ddb..3be2cbc 100644
--- a/config.c
+++ b/config.c
@@ -24,6 +24,7 @@ struct config_source {
 			size_t pos;
 		} buf;
 	} u;
+	const char *origin_type;
 	const char *name;
 	const char *path;
 	int die_on_error;
@@ -471,9 +472,9 @@ static int git_parse_source(config_fn_t fn, void *data)
 			break;
 	}
 	if (cf->die_on_error)
-		die(_("bad config file line %d in %s"), cf->linenr, cf->name);
+		die(_("bad config line %d in %s %s"), cf->linenr, cf->origin_type, cf->name);
 	else
-		return error(_("bad config file line %d in %s"), cf->linenr, cf->name);
+		return error(_("bad config line %d in %s %s"), cf->linenr, cf->origin_type, cf->name);
 }
 
 static int parse_unit_factor(const char *end, uintmax_t *val)
@@ -588,9 +589,9 @@ static void die_bad_number(const char *name, const char *value)
 	if (!value)
 		value = "";
 
-	if (cf && cf->name)
-		die(_("bad numeric config value '%s' for '%s' in %s: %s"),
-		    value, name, cf->name, reason);
+	if (cf && cf->origin_type && cf->name)
+		die(_("bad numeric config value '%s' for '%s' in %s %s: %s"),
+		    value, name, cf->origin_type, cf->name, reason);
 	die(_("bad numeric config value '%s' for '%s': %s"), value, name, reason);
 }
 
@@ -1061,11 +1062,13 @@ static int do_config_from(struct config_source *top, config_fn_t fn, void *data)
 }
 
 static int do_config_from_file(config_fn_t fn,
-		const char *name, const char *path, FILE *f, void *data)
+		const char *origin_type, const char *name, const char *path, FILE *f,
+		void *data)
 {
 	struct config_source top;
 
 	top.u.file = f;
+	top.origin_type = origin_type;
 	top.name = name;
 	top.path = path;
 	top.die_on_error = 1;
@@ -1078,7 +1081,7 @@ static int do_config_from_file(config_fn_t fn,
 
 static int git_config_from_stdin(config_fn_t fn, void *data)
 {
-	return do_config_from_file(fn, "<stdin>", NULL, stdin, data);
+	return do_config_from_file(fn, "stdin", "", NULL, stdin, data);
 }
 
 int git_config_from_file(config_fn_t fn, const char *filename, void *data)
@@ -1089,21 +1092,22 @@ int git_config_from_file(config_fn_t fn, const char *filename, void *data)
 	f = fopen(filename, "r");
 	if (f) {
 		flockfile(f);
-		ret = do_config_from_file(fn, filename, filename, f, data);
+		ret = do_config_from_file(fn, "file", filename, filename, f, data);
 		funlockfile(f);
 		fclose(f);
 	}
 	return ret;
 }
 
-int git_config_from_mem(config_fn_t fn, const char *name, const char *buf,
-			size_t len, void *data)
+int git_config_from_mem(config_fn_t fn, const char *origin_type,
+			const char *name, const char *buf, size_t len, void *data)
 {
 	struct config_source top;
 
 	top.u.buf.buf = buf;
 	top.u.buf.len = len;
 	top.u.buf.pos = 0;
+	top.origin_type = origin_type;
 	top.name = name;
 	top.path = NULL;
 	top.die_on_error = 0;
@@ -1132,7 +1136,7 @@ static int git_config_from_blob_sha1(config_fn_t fn,
 		return error("reference '%s' does not point to a blob", name);
 	}
 
-	ret = git_config_from_mem(fn, name, buf, size, data);
+	ret = git_config_from_mem(fn, "blob", name, buf, size, data);
 	free(buf);
 
 	return ret;
@@ -2385,3 +2389,13 @@ int parse_config_key(const char *var,
 
 	return 0;
 }
+
+const char *current_config_origin_type(void)
+{
+	return cf && cf->origin_type ? cf->origin_type : "cmdline";
+}
+
+const char *current_config_name(void)
+{
+	return cf && cf->name ? cf->name : "";
+}
diff --git a/submodule-config.c b/submodule-config.c
index b85a937..92502b5 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -427,8 +427,8 @@ static const struct submodule *config_from(struct submodule_cache *cache,
 	parameter.commit_sha1 = commit_sha1;
 	parameter.gitmodules_sha1 = sha1;
 	parameter.overwrite = 0;
-	git_config_from_mem(parse_config, rev.buf, config, config_size,
-			&parameter);
+	git_config_from_mem(parse_config, "submodule-blob", rev.buf,
+			config, config_size, &parameter);
 	free(config);
 
 	switch (lookup_type) {
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 1782add..42ed5cc 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -700,12 +700,18 @@ test_expect_success 'invalid unit' '
 	git config aninvalid.unit >actual &&
 	test_cmp expect actual &&
 	cat >expect <<-\EOF &&
-	fatal: bad numeric config value '\''1auto'\'' for '\''aninvalid.unit'\'' in .git/config: invalid unit
+	fatal: bad numeric config value '\''1auto'\'' for '\''aninvalid.unit'\'' in file .git/config: invalid unit
 	EOF
 	test_must_fail git config --int --get aninvalid.unit 2>actual &&
 	test_i18ncmp expect actual
 '
 
+test_expect_success 'invalid stdin config' '
+	echo "fatal: bad config line 1 in stdin " >expect &&
+	echo "[broken" | test_must_fail git config --list --file - >output 2>&1 &&
+	test_cmp expect output
+'
+
 cat > expect << EOF
 true
 false
diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh
index 91235b7..82f82a1 100755
--- a/t/t1308-config-set.sh
+++ b/t/t1308-config-set.sh
@@ -195,14 +195,14 @@ test_expect_success 'proper error on error in default config files' '
 	cp .git/config .git/config.old &&
 	test_when_finished "mv .git/config.old .git/config" &&
 	echo "[" >>.git/config &&
-	echo "fatal: bad config file line 34 in .git/config" >expect &&
+	echo "fatal: bad config line 34 in file .git/config" >expect &&
 	test_expect_code 128 test-config get_value foo.bar 2>actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'proper error on error in custom config files' '
 	echo "[" >>syntax-error &&
-	echo "fatal: bad config file line 1 in syntax-error" >expect &&
+	echo "fatal: bad config line 1 in file syntax-error" >expect &&
 	test_expect_code 128 test-config configset_get_value foo.bar syntax-error 2>actual &&
 	test_cmp expect actual
 '
-- 
2.5.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH v6 4/4] config: add '--show-origin' option to print the origin of a config value
  2016-02-19  9:15 [PATCH v6 0/4] config: add '--sources' option to print the source of a config value larsxschneider
                   ` (2 preceding siblings ...)
  2016-02-19  9:16 ` [PATCH v6 3/4] config: add 'origin_type' to config_source struct larsxschneider
@ 2016-02-19  9:16 ` larsxschneider
  2016-02-22 17:43   ` Junio C Hamano
  2016-03-02 17:33   ` Johannes Sixt
  2016-02-19 18:26 ` [PATCH v6 0/4] config: add '--sources' option to print the source " Junio C Hamano
  4 siblings, 2 replies; 17+ messages in thread
From: larsxschneider @ 2016-02-19  9:16 UTC (permalink / raw)
  To: git; +Cc: peff, ramsay, gitster, Johannes.Schindelin, Lars Schneider

From: Lars Schneider <larsxschneider@gmail.com>

If config values are queried using 'git config' (e.g. via --get,
--get-all, --get-regexp, or --list flag) then it is sometimes hard to
find the configuration file where the values were defined.

Teach 'git config' the '--show-origin' option to print the source
configuration file for every printed value.

Based-on-patch-by: Jeff King <peff@peff.net>
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
 Documentation/git-config.txt |  15 +++--
 builtin/config.c             |  33 ++++++++++
 t/t1300-repo-config.sh       | 147 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 190 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 2608ca7..6374997 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -9,18 +9,18 @@ git-config - Get and set repository or global options
 SYNOPSIS
 --------
 [verse]
-'git config' [<file-option>] [type] [-z|--null] name [value [value_regex]]
+'git config' [<file-option>] [type] [--show-origin] [-z|--null] name [value [value_regex]]
 'git config' [<file-option>] [type] --add name value
 'git config' [<file-option>] [type] --replace-all name value [value_regex]
-'git config' [<file-option>] [type] [-z|--null] --get name [value_regex]
-'git config' [<file-option>] [type] [-z|--null] --get-all name [value_regex]
-'git config' [<file-option>] [type] [-z|--null] [--name-only] --get-regexp name_regex [value_regex]
+'git config' [<file-option>] [type] [--show-origin] [-z|--null] --get name [value_regex]
+'git config' [<file-option>] [type] [--show-origin] [-z|--null] --get-all name [value_regex]
+'git config' [<file-option>] [type] [--show-origin] [-z|--null] [--name-only] --get-regexp name_regex [value_regex]
 'git config' [<file-option>] [type] [-z|--null] --get-urlmatch name URL
 'git config' [<file-option>] --unset name [value_regex]
 'git config' [<file-option>] --unset-all name [value_regex]
 'git config' [<file-option>] --rename-section old_name new_name
 'git config' [<file-option>] --remove-section name
-'git config' [<file-option>] [-z|--null] [--name-only] -l | --list
+'git config' [<file-option>] [--show-origin] [-z|--null] [--name-only] -l | --list
 'git config' [<file-option>] --get-color name [default]
 'git config' [<file-option>] --get-colorbool name [stdout-is-tty]
 'git config' [<file-option>] -e | --edit
@@ -194,6 +194,11 @@ See also <<FILES>>.
 	Output only the names of config variables for `--list` or
 	`--get-regexp`.

+--show-origin::
+	Augment the output of all queried config options with the
+	origin type (file, stdin, blob, cmdline) and the actual origin
+	(config file path, ref, or blob id if applicable).
+
 --get-colorbool name [stdout-is-tty]::

 	Find the color setting for `name` (e.g. `color.diff`) and output
diff --git a/builtin/config.c b/builtin/config.c
index adc7727..a1a9b9a 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -3,6 +3,7 @@
 #include "color.h"
 #include "parse-options.h"
 #include "urlmatch.h"
+#include "quote.h"

 static const char *const builtin_config_usage[] = {
 	N_("git config [<options>]"),
@@ -27,6 +28,7 @@ static int actions, types;
 static const char *get_color_slot, *get_colorbool_slot;
 static int end_null;
 static int respect_includes = -1;
+static int show_origin;

 #define ACTION_GET (1<<0)
 #define ACTION_GET_ALL (1<<1)
@@ -81,6 +83,7 @@ static struct option builtin_config_options[] = {
 	OPT_BOOL('z', "null", &end_null, N_("terminate values with NUL byte")),
 	OPT_BOOL(0, "name-only", &omit_values, N_("show variable names only")),
 	OPT_BOOL(0, "includes", &respect_includes, N_("respect include directives on lookup")),
+	OPT_BOOL(0, "show-origin", &show_origin, N_("show origin of config (file, stdin, blob, cmdline)")),
 	OPT_END(),
 };

@@ -91,8 +94,28 @@ static void check_argc(int argc, int min, int max) {
 	usage_with_options(builtin_config_usage, builtin_config_options);
 }

+static void show_config_origin(struct strbuf *buf)
+{
+	const char term = end_null ? '\0' : '\t';
+
+	strbuf_addstr(buf, current_config_origin_type());
+	strbuf_addch(buf, ':');
+	if (end_null)
+		strbuf_addstr(buf, current_config_name());
+	else
+		quote_c_style(current_config_name(), buf, NULL, 0);
+	strbuf_addch(buf, term);
+}
+
 static int show_all_config(const char *key_, const char *value_, void *cb)
 {
+	if (show_origin) {
+		struct strbuf buf = STRBUF_INIT;
+		show_config_origin(&buf);
+		/* Use fwrite as "buf" can contain \0's if "end_null" is set. */
+		fwrite(buf.buf, 1, buf.len, stdout);
+		strbuf_release(&buf);
+	}
 	if (!omit_values && value_)
 		printf("%s%c%s%c", key_, delim, value_, term);
 	else
@@ -108,6 +131,8 @@ struct strbuf_list {

 static int format_config(struct strbuf *buf, const char *key_, const char *value_)
 {
+	if (show_origin)
+		show_config_origin(buf);
 	if (show_keys)
 		strbuf_addstr(buf, key_);
 	if (!omit_values) {
@@ -538,6 +563,14 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		error("--name-only is only applicable to --list or --get-regexp");
 		usage_with_options(builtin_config_usage, builtin_config_options);
 	}
+
+	if (show_origin && !(actions &
+		(ACTION_GET|ACTION_GET_ALL|ACTION_GET_REGEXP|ACTION_LIST))) {
+		error("--show-origin is only applicable to --get, --get-all, "
+			  "--get-regexp, and --list.");
+		usage_with_options(builtin_config_usage, builtin_config_options);
+	}
+
 	if (actions == ACTION_LIST) {
 		check_argc(argc, 0, 0);
 		if (git_config_with_options(show_all_config, NULL,
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 42ed5cc..e54f6d5 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1209,4 +1209,151 @@ test_expect_success POSIXPERM,PERL 'preserves existing permissions' '
 	  "die q(badrename) if ((stat(q(.git/config)))[2] & 07777) != 0600"
 '

+test_expect_success 'set up --show-origin tests' '
+	INCLUDE_DIR="$HOME/include" &&
+	mkdir -p "$INCLUDE_DIR" &&
+	cat >"$INCLUDE_DIR"/absolute.include <<-\EOF &&
+		[user]
+			absolute = include
+	EOF
+	cat >"$INCLUDE_DIR"/relative.include <<-\EOF &&
+		[user]
+			relative = include
+	EOF
+	cat >"$HOME"/.gitconfig <<-EOF &&
+		[user]
+			global = true
+			override = global
+		[include]
+			path = "$INCLUDE_DIR/absolute.include"
+	EOF
+	cat >.git/config <<-\EOF
+		[user]
+			local = true
+			override = local
+		[include]
+			path = ../include/relative.include
+	EOF
+'
+
+test_expect_success '--show-origin with --list' '
+	cat >expect <<-EOF &&
+		file:$HOME/.gitconfig	user.global=true
+		file:$HOME/.gitconfig	user.override=global
+		file:$HOME/.gitconfig	include.path=$INCLUDE_DIR/absolute.include
+		file:$INCLUDE_DIR/absolute.include	user.absolute=include
+		file:.git/config	user.local=true
+		file:.git/config	user.override=local
+		file:.git/config	include.path=../include/relative.include
+		file:.git/../include/relative.include	user.relative=include
+		cmdline:	user.cmdline=true
+	EOF
+	git -c user.cmdline=true config --list --show-origin >output &&
+	test_cmp expect output
+'
+
+test_expect_success '--show-origin with --list --null' '
+	cat >expect <<-EOF &&
+		file:$HOME/.gitconfigQuser.global
+		trueQfile:$HOME/.gitconfigQuser.override
+		globalQfile:$HOME/.gitconfigQinclude.path
+		$INCLUDE_DIR/absolute.includeQfile:$INCLUDE_DIR/absolute.includeQuser.absolute
+		includeQfile:.git/configQuser.local
+		trueQfile:.git/configQuser.override
+		localQfile:.git/configQinclude.path
+		../include/relative.includeQfile:.git/../include/relative.includeQuser.relative
+		includeQcmdline:Quser.cmdline
+		trueQ
+	EOF
+	git -c user.cmdline=true config --null --list --show-origin >output.raw &&
+	nul_to_q <output.raw >output &&
+	# The here-doc above adds a newline that the --null output would not
+	# include. Add it here to make the two comparable.
+	echo >>output &&
+	test_cmp expect output
+'
+
+test_expect_success '--show-origin with single file' '
+	cat >expect <<-\EOF &&
+		file:.git/config	user.local=true
+		file:.git/config	user.override=local
+		file:.git/config	include.path=../include/relative.include
+	EOF
+	git config --local --list --show-origin >output &&
+	test_cmp expect output
+'
+
+test_expect_success '--show-origin with --get-regexp' '
+	cat >expect <<-EOF &&
+		file:$HOME/.gitconfig	user.global true
+		file:.git/config	user.local true
+	EOF
+	git config --show-origin --get-regexp "user\.[g|l].*" >output &&
+	test_cmp expect output
+'
+
+test_expect_success '--show-origin getting a single key' '
+	cat >expect <<-\EOF &&
+		file:.git/config	local
+	EOF
+	git config --show-origin user.override >output &&
+	test_cmp expect output
+'
+
+test_expect_success 'set up custom config file' '
+	CUSTOM_CONFIG_FILE="file\" (dq) and spaces.conf" &&
+	cat >"$CUSTOM_CONFIG_FILE" <<-\EOF
+		[user]
+			custom = true
+	EOF
+'
+
+test_expect_success '--show-origin escape special file name characters' '
+	cat >expect <<-\EOF &&
+		file:"file\" (dq) and spaces.conf"	user.custom=true
+	EOF
+	git config --file "$CUSTOM_CONFIG_FILE" --show-origin --list >output &&
+	test_cmp expect output
+'
+
+test_expect_success '--show-origin stdin' '
+	cat >expect <<-\EOF &&
+		stdin:	user.custom=true
+	EOF
+	git config --file - --show-origin --list <"$CUSTOM_CONFIG_FILE" >output &&
+	test_cmp expect output
+'
+
+test_expect_success '--show-origin stdin with file include' '
+	cat >"$INCLUDE_DIR"/stdin.include <<-EOF &&
+		[user]
+			stdin = include
+	EOF
+	cat >expect <<-EOF &&
+		file:$INCLUDE_DIR/stdin.include	include
+	EOF
+	echo "[include]path=\"$INCLUDE_DIR\"/stdin.include" \
+		| git config --show-origin --includes --file - user.stdin >output &&
+	test_cmp expect output
+'
+
+test_expect_success '--show-origin blob' '
+	cat >expect <<-\EOF &&
+		blob:a9d9f9e555b5c6f07cbe09d3f06fe3df11e09c08	user.custom=true
+	EOF
+	blob=$(git hash-object -w "$CUSTOM_CONFIG_FILE") &&
+	git config --blob=$blob --show-origin --list >output &&
+	test_cmp expect output
+'
+
+test_expect_success '--show-origin blob ref' '
+	cat >expect <<-\EOF &&
+		blob:"master:file\" (dq) and spaces.conf"	user.custom=true
+	EOF
+	git add "$CUSTOM_CONFIG_FILE" &&
+	git commit -m "new config file" &&
+	git config --blob=master:"$CUSTOM_CONFIG_FILE" --show-origin --list >output &&
+	test_cmp expect output
+'
+
 test_done
--
2.5.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* Re: [PATCH v6 0/4] config: add '--sources' option to print the source of a config value
  2016-02-19  9:15 [PATCH v6 0/4] config: add '--sources' option to print the source of a config value larsxschneider
                   ` (3 preceding siblings ...)
  2016-02-19  9:16 ` [PATCH v6 4/4] config: add '--show-origin' option to print the origin of a config value larsxschneider
@ 2016-02-19 18:26 ` Junio C Hamano
  2016-02-22  9:23   ` [PATCH v6 squash 0/2] " larsxschneider
  4 siblings, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2016-02-19 18:26 UTC (permalink / raw)
  To: larsxschneider; +Cc: git, peff, ramsay, Johannes.Schindelin

larsxschneider@gmail.com writes:

> From: Lars Schneider <larsxschneider@gmail.com>
>
> diff to v5:
> * rename 'type' to 'origin_type' as 'type' is too broad a word in the context
>   of configuration file (Thanks to the reviewers Junio and Dscho)
> * add dedicated patch to rename git_config_from_buf to git_config_from_mem
>   (this change was part of the series since v4 as suggested by Junio but
>   hidden in the "config: add 'origin_type'" patch)
>
> Thanks,
> Lars

Overall this round looks good.

I personally prefer 'stdin' to be spelled out as '(the) standard
input' in end-user facing messages, e.g. the expected message in
this piece

    test_expect_success 'invalid stdin config' '
            echo "fatal: bad config line 1 in stdin " >expect &&

to say "line 1 in the standard input", but that is a quite minor
point and just a preference.

Thanks.

Let's queue this version and move it forward to 'next' soonish.

^ permalink raw reply	[flat|nested] 17+ messages in thread
* [PATCH v6 squash 0/2] config: add '--sources' option to print the source of a config value
  2016-02-19 18:26 ` [PATCH v6 0/4] config: add '--sources' option to print the source " Junio C Hamano
@ 2016-02-22  9:23   ` larsxschneider
  2016-02-22  9:23     ` [PATCH v7 1/2] fixup: config: add 'origin_type' to config_source struct larsxschneider
  2016-02-22  9:23     ` [PATCH v7 2/2] fixup: config: add '--show-origin' option to print the origin of a config value larsxschneider
  0 siblings, 2 replies; 17+ messages in thread
From: larsxschneider @ 2016-02-22  9:23 UTC (permalink / raw)
  To: git; +Cc: gitster, Lars Schneider

From: Lars Schneider <larsxschneider@gmail.com>

Hi Junio,

please squash these two patches into the corresponding patches from my v6
to spell out end user facing messages (response to [1], sorry gmane didn't
work for me).

I am not sure about the right way an answer with a "two patch" squash on
the mailing list. Please let me know if I should reroll the topic with
the squashed patches included.

Thanks,
Lars

[1] http://www.spinics.net/lists/git/msg268424.html

Lars Schneider (2):
  fixup: config: add 'origin_type' to config_source struct
  fixup: config: add '--show-origin' option to print the origin of a
    config value

 Documentation/git-config.txt | 5 +++--
 builtin/config.c             | 2 +-
 config.c                     | 4 ++--
 t/t1300-repo-config.sh       | 8 ++++----
 4 files changed, 10 insertions(+), 9 deletions(-)

--
2.5.1

^ permalink raw reply	[flat|nested] 17+ messages in thread
* [PATCH v7 1/2] fixup: config: add 'origin_type' to config_source struct
  2016-02-22  9:23   ` [PATCH v6 squash 0/2] " larsxschneider
@ 2016-02-22  9:23     ` larsxschneider
  2016-02-22  9:23     ` [PATCH v7 2/2] fixup: config: add '--show-origin' option to print the origin of a config value larsxschneider
  1 sibling, 0 replies; 17+ messages in thread
From: larsxschneider @ 2016-02-22  9:23 UTC (permalink / raw)
  To: git; +Cc: gitster, Lars Schneider

From: Lars Schneider <larsxschneider@gmail.com>

---
 config.c               | 4 ++--
 t/t1300-repo-config.sh | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/config.c b/config.c
index 3be2cbc..0fce371 100644
--- a/config.c
+++ b/config.c
@@ -1081,7 +1081,7 @@ static int do_config_from_file(config_fn_t fn,
 
 static int git_config_from_stdin(config_fn_t fn, void *data)
 {
-	return do_config_from_file(fn, "stdin", "", NULL, stdin, data);
+	return do_config_from_file(fn, "standard input", "", NULL, stdin, data);
 }
 
 int git_config_from_file(config_fn_t fn, const char *filename, void *data)
@@ -2392,7 +2392,7 @@ int parse_config_key(const char *var,
 
 const char *current_config_origin_type(void)
 {
-	return cf && cf->origin_type ? cf->origin_type : "cmdline";
+	return cf && cf->origin_type ? cf->origin_type : "command line";
 }
 
 const char *current_config_name(void)
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index e54f6d5..254643a 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -707,7 +707,7 @@ test_expect_success 'invalid unit' '
 '
 
 test_expect_success 'invalid stdin config' '
-	echo "fatal: bad config line 1 in stdin " >expect &&
+	echo "fatal: bad config line 1 in standard input " >expect &&
 	echo "[broken" | test_must_fail git config --list --file - >output 2>&1 &&
 	test_cmp expect output
 '
-- 
2.5.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH v7 2/2] fixup: config: add '--show-origin' option to print the origin of a config value
  2016-02-22  9:23   ` [PATCH v6 squash 0/2] " larsxschneider
  2016-02-22  9:23     ` [PATCH v7 1/2] fixup: config: add 'origin_type' to config_source struct larsxschneider
@ 2016-02-22  9:23     ` larsxschneider
  1 sibling, 0 replies; 17+ messages in thread
From: larsxschneider @ 2016-02-22  9:23 UTC (permalink / raw)
  To: git; +Cc: gitster, Lars Schneider

From: Lars Schneider <larsxschneider@gmail.com>

---
 Documentation/git-config.txt | 5 +++--
 builtin/config.c             | 2 +-
 t/t1300-repo-config.sh       | 6 +++---
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 6374997..499fc0f 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -196,8 +196,9 @@ See also <<FILES>>.
 
 --show-origin::
 	Augment the output of all queried config options with the
-	origin type (file, stdin, blob, cmdline) and the actual origin
-	(config file path, ref, or blob id if applicable).
+	origin type (file, standard input, blob, command line) and
+	the actual origin (config file path, ref, or blob id if
+	applicable).
 
 --get-colorbool name [stdout-is-tty]::
 
diff --git a/builtin/config.c b/builtin/config.c
index a1a9b9a..0f14220 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -83,7 +83,7 @@ static struct option builtin_config_options[] = {
 	OPT_BOOL('z', "null", &end_null, N_("terminate values with NUL byte")),
 	OPT_BOOL(0, "name-only", &omit_values, N_("show variable names only")),
 	OPT_BOOL(0, "includes", &respect_includes, N_("respect include directives on lookup")),
-	OPT_BOOL(0, "show-origin", &show_origin, N_("show origin of config (file, stdin, blob, cmdline)")),
+	OPT_BOOL(0, "show-origin", &show_origin, N_("show origin of config (file, standard input, blob, command line)")),
 	OPT_END(),
 };
 
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 254643a..8867ce1 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1246,7 +1246,7 @@ test_expect_success '--show-origin with --list' '
 		file:.git/config	user.override=local
 		file:.git/config	include.path=../include/relative.include
 		file:.git/../include/relative.include	user.relative=include
-		cmdline:	user.cmdline=true
+		command line:	user.cmdline=true
 	EOF
 	git -c user.cmdline=true config --list --show-origin >output &&
 	test_cmp expect output
@@ -1262,7 +1262,7 @@ test_expect_success '--show-origin with --list --null' '
 		trueQfile:.git/configQuser.override
 		localQfile:.git/configQinclude.path
 		../include/relative.includeQfile:.git/../include/relative.includeQuser.relative
-		includeQcmdline:Quser.cmdline
+		includeQcommand line:Quser.cmdline
 		trueQ
 	EOF
 	git -c user.cmdline=true config --null --list --show-origin >output.raw &&
@@ -1318,7 +1318,7 @@ test_expect_success '--show-origin escape special file name characters' '
 
 test_expect_success '--show-origin stdin' '
 	cat >expect <<-\EOF &&
-		stdin:	user.custom=true
+		standard input:	user.custom=true
 	EOF
 	git config --file - --show-origin --list <"$CUSTOM_CONFIG_FILE" >output &&
 	test_cmp expect output
-- 
2.5.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* Re: [PATCH v6 4/4] config: add '--show-origin' option to print the origin of a config value
  2016-02-19  9:16 ` [PATCH v6 4/4] config: add '--show-origin' option to print the origin of a config value larsxschneider
@ 2016-02-22 17:43   ` Junio C Hamano
  2016-02-22 17:58     ` Jeff King
  2016-03-02 17:33   ` Johannes Sixt
  1 sibling, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2016-02-22 17:43 UTC (permalink / raw)
  To: larsxschneider; +Cc: git, peff, ramsay, Johannes.Schindelin

larsxschneider@gmail.com writes:

> +test_expect_success '--show-origin with --list' '
> +	cat >expect <<-EOF &&
> +		file:$HOME/.gitconfig	user.global=true
> +		file:$HOME/.gitconfig	user.override=global
> +		file:$HOME/.gitconfig	include.path=$INCLUDE_DIR/absolute.include
> +		file:$INCLUDE_DIR/absolute.include	user.absolute=include
> +		file:.git/config	user.local=true
> +		file:.git/config	user.override=local
> +		file:.git/config	include.path=../include/relative.include
> +		file:.git/../include/relative.include	user.relative=include
> +		cmdline:	user.cmdline=true
> +	EOF
> ...
> +test_expect_success '--show-origin stdin' '
> +	cat >expect <<-\EOF &&
> +		stdin:	user.custom=true
> +	EOF

I do recall there was some bikeshedding^Wdesigning discussion, in
which I chose not to participate, on the output format, how
origin-type and origin-value are given in the output in an
unambiguous way that is easy to understand by the end users.

Does the above reflect the concensus from the discussion?  Just
double checking.

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH v6 4/4] config: add '--show-origin' option to print the origin of a config value
  2016-02-22 17:43   ` Junio C Hamano
@ 2016-02-22 17:58     ` Jeff King
  2016-02-22 18:37       ` Junio C Hamano
  0 siblings, 1 reply; 17+ messages in thread
From: Jeff King @ 2016-02-22 17:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: larsxschneider, git, ramsay, Johannes.Schindelin

On Mon, Feb 22, 2016 at 09:43:25AM -0800, Junio C Hamano wrote:

> larsxschneider@gmail.com writes:
> 
> > +test_expect_success '--show-origin with --list' '
> > +	cat >expect <<-EOF &&
> > +		file:$HOME/.gitconfig	user.global=true
> > +		file:$HOME/.gitconfig	user.override=global
> > +		file:$HOME/.gitconfig	include.path=$INCLUDE_DIR/absolute.include
> > +		file:$INCLUDE_DIR/absolute.include	user.absolute=include
> > +		file:.git/config	user.local=true
> > +		file:.git/config	user.override=local
> > +		file:.git/config	include.path=../include/relative.include
> > +		file:.git/../include/relative.include	user.relative=include
> > +		cmdline:	user.cmdline=true
> > +	EOF
> > ...
> > +test_expect_success '--show-origin stdin' '
> > +	cat >expect <<-\EOF &&
> > +		stdin:	user.custom=true
> > +	EOF
> 
> I do recall there was some bikeshedding^Wdesigning discussion, in
> which I chose not to participate, on the output format, how
> origin-type and origin-value are given in the output in an
> unambiguous way that is easy to understand by the end users.
> 
> Does the above reflect the concensus from the discussion?  Just
> double checking.

Yes, I think so.

-Peff

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH v6 4/4] config: add '--show-origin' option to print the origin of a config value
  2016-02-22 17:58     ` Jeff King
@ 2016-02-22 18:37       ` Junio C Hamano
  0 siblings, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2016-02-22 18:37 UTC (permalink / raw)
  To: Jeff King; +Cc: larsxschneider, git, ramsay, Johannes.Schindelin

Jeff King <peff@peff.net> writes:

> On Mon, Feb 22, 2016 at 09:43:25AM -0800, Junio C Hamano wrote:
>
>> larsxschneider@gmail.com writes:
>> 
>> > +test_expect_success '--show-origin with --list' '
>> > +	cat >expect <<-EOF &&
>> > +		file:$HOME/.gitconfig	user.global=true
>> > +		file:$HOME/.gitconfig	user.override=global
>> > +		file:$HOME/.gitconfig	include.path=$INCLUDE_DIR/absolute.include
>> > +		file:$INCLUDE_DIR/absolute.include	user.absolute=include
>> > +		file:.git/config	user.local=true
>> > +		file:.git/config	user.override=local
>> > +		file:.git/config	include.path=../include/relative.include
>> > +		file:.git/../include/relative.include	user.relative=include
>> > +		cmdline:	user.cmdline=true
>> > +	EOF
>> > ...
>> > +test_expect_success '--show-origin stdin' '
>> > +	cat >expect <<-\EOF &&
>> > +		stdin:	user.custom=true
>> > +	EOF
>> 
>> I do recall there was some bikeshedding^Wdesigning discussion, in
>> which I chose not to participate, on the output format, how
>> origin-type and origin-value are given in the output in an
>> unambiguous way that is easy to understand by the end users.
>> 
>> Does the above reflect the concensus from the discussion?  Just
>> double checking.
>
> Yes, I think so.

OK, thanks.

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH v6 4/4] config: add '--show-origin' option to print the origin of a config value
  2016-02-19  9:16 ` [PATCH v6 4/4] config: add '--show-origin' option to print the origin of a config value larsxschneider
  2016-02-22 17:43   ` Junio C Hamano
@ 2016-03-02 17:33   ` Johannes Sixt
  2016-03-03  7:38     ` Lars Schneider
  2016-03-21  8:53     ` Lars Schneider
  1 sibling, 2 replies; 17+ messages in thread
From: Johannes Sixt @ 2016-03-02 17:33 UTC (permalink / raw)
  To: larsxschneider; +Cc: git, peff, ramsay, gitster, Johannes.Schindelin

Am 19.02.2016 um 10:16 schrieb larsxschneider@gmail.com:
> +test_expect_success '--show-origin with --list' '
> +	cat >expect <<-EOF &&
> +		file:$HOME/.gitconfig	user.global=true
> +		file:$HOME/.gitconfig	user.override=global
> +		file:$HOME/.gitconfig	include.path=$INCLUDE_DIR/absolute.include

On Windows, this injects POSIX-style paths in the expected output, but 
git.exe produces mangled paths (with a drive letter). The pattern I use 
to fix this is:

		file:$(pwd)/.gitconfig	user.override=global

> +		file:$INCLUDE_DIR/absolute.include	user.absolute=include
> +		file:.git/config	user.local=true
> +		file:.git/config	user.override=local
> +		file:.git/config	include.path=../include/relative.include
> +		file:.git/../include/relative.include	user.relative=include
> +		cmdline:	user.cmdline=true
> +	EOF
> +	git -c user.cmdline=true config --list --show-origin >output &&
> +	test_cmp expect output
> +'
...
> +test_expect_success 'set up custom config file' '
> +	CUSTOM_CONFIG_FILE="file\" (dq) and spaces.conf" &&
> +	cat >"$CUSTOM_CONFIG_FILE" <<-\EOF
> +		[user]
> +			custom = true
> +	EOF

This fails on Windows, because the shell cannot create a file containing 
a double-quote character.

IIUC, the test serves two purposes: (1) to test C-style quoting of the 
output and (2) non-standard configuration files. We'll have to separate 
that so that we can test at least (2) on Windows with "regular" file 
name. We cannot test (1) because the only case where quoting is used is 
when the file name contains a double-quote character.

> +'
> +
> +test_expect_success '--show-origin escape special file name characters' '
> +	cat >expect <<-\EOF &&
> +		file:"file\" (dq) and spaces.conf"	user.custom=true
> +	EOF
> +	git config --file "$CUSTOM_CONFIG_FILE" --show-origin --list >output &&
> +	test_cmp expect output
> +'
...
> +test_expect_success '--show-origin blob ref' '
> +	cat >expect <<-\EOF &&
> +		blob:"master:file\" (dq) and spaces.conf"	user.custom=true
> +	EOF
> +	git add "$CUSTOM_CONFIG_FILE" &&

Is this dual-purpose as well or just re-using the files established 
earlier in the test suite?

> +	git commit -m "new config file" &&
> +	git config --blob=master:"$CUSTOM_CONFIG_FILE" --show-origin --list >output &&
> +	test_cmp expect output
> +'
> +
>   test_done
> --
> 2.5.1
>

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH v6 4/4] config: add '--show-origin' option to print the origin of a config value
  2016-03-02 17:33   ` Johannes Sixt
@ 2016-03-03  7:38     ` Lars Schneider
  2016-03-03 18:36       ` Johannes Sixt
  2016-03-22 14:44       ` Johannes Schindelin
  2016-03-21  8:53     ` Lars Schneider
  1 sibling, 2 replies; 17+ messages in thread
From: Lars Schneider @ 2016-03-03  7:38 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: Git List, Jeff King, Ramsay Jones, Junio C Hamano,
	Johannes.Schindelin, Duy Nguyen


> On 02 Mar 2016, at 18:33, Johannes Sixt <j6t@kdbg.org> wrote:
> 
> Am 19.02.2016 um 10:16 schrieb larsxschneider@gmail.com:
>> +test_expect_success '--show-origin with --list' '
>> +	cat >expect <<-EOF &&
>> +		file:$HOME/.gitconfig	user.global=true
>> +		file:$HOME/.gitconfig	user.override=global
>> +		file:$HOME/.gitconfig	include.path=$INCLUDE_DIR/absolute.include
> 
> On Windows, this injects POSIX-style paths in the expected output, but git.exe produces mangled paths (with a drive letter). The pattern I use to fix this is:
> 
> 		file:$(pwd)/.gitconfig	user.override=global
OK, I try to fix it that way.

> 
>> +		file:$INCLUDE_DIR/absolute.include	user.absolute=include
>> +		file:.git/config	user.local=true
>> +		file:.git/config	user.override=local
>> +		file:.git/config	include.path=../include/relative.include
>> +		file:.git/../include/relative.include	user.relative=include
>> +		cmdline:	user.cmdline=true
>> +	EOF
>> +	git -c user.cmdline=true config --list --show-origin >output &&
>> +	test_cmp expect output
>> +'
> ...
>> +test_expect_success 'set up custom config file' '
>> +	CUSTOM_CONFIG_FILE="file\" (dq) and spaces.conf" &&
>> +	cat >"$CUSTOM_CONFIG_FILE" <<-\EOF
>> +		[user]
>> +			custom = true
>> +	EOF
> 
> This fails on Windows, because the shell cannot create a file containing a double-quote character.
> 
> IIUC, the test serves two purposes: (1) to test C-style quoting of the output and (2) non-standard configuration files. We'll have to separate that so that we can test at least (2) on Windows with "regular" file name. We cannot test (1) because the only case where quoting is used is when the file name contains a double-quote character.
OK, I will try to separate this and disable (1) for Windows.

> 
>> +'
>> +
>> +test_expect_success '--show-origin escape special file name characters' '
>> +	cat >expect <<-\EOF &&
>> +		file:"file\" (dq) and spaces.conf"	user.custom=true
>> +	EOF
>> +	git config --file "$CUSTOM_CONFIG_FILE" --show-origin --list >output &&
>> +	test_cmp expect output
>> +'
> ...
>> +test_expect_success '--show-origin blob ref' '
>> +	cat >expect <<-\EOF &&
>> +		blob:"master:file\" (dq) and spaces.conf"	user.custom=true
>> +	EOF
>> +	git add "$CUSTOM_CONFIG_FILE" &&
> 
> Is this dual-purpose as well or just re-using the files established earlier in the test suite?
Just re-using files.

Thanks for making me aware of the Windows problems. I can reproduce them with the Git for Windows SDK (super easy to setup, great work Dscho!) and I will try to fix them.

I am fairly new to the Git for Windows SDK (+ an inexperienced Windows user) and therefore I wonder if you can help me with the following questions:

(1) If I have a Git core branch with a some changes that builds and tests clean on Linux and OSX. How do I apply all the necessary Git for Windows specific changes to this branch?

(2) During my testing with Windows I noticed that the git config paths look funny by adding  ("\\" and "/"). I mentioned the problem in the Git for Windows forum:
https://groups.google.com/forum/#!topic/git-for-windows/zTv60HhfnYk
Duy suggested a solution in that thread. Is this the default way to deal with the paths? Would the list accept this solution?

(3) The tests on Windows seemed very slow to me. Are there tricks to speed them up? Did you try a RAM disk? If yes, how do you do it?

Thanks,
Lars

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH v6 4/4] config: add '--show-origin' option to print the origin of a config value
  2016-03-03  7:38     ` Lars Schneider
@ 2016-03-03 18:36       ` Johannes Sixt
  2016-03-22 14:44       ` Johannes Schindelin
  1 sibling, 0 replies; 17+ messages in thread
From: Johannes Sixt @ 2016-03-03 18:36 UTC (permalink / raw)
  To: Lars Schneider
  Cc: Git List, Jeff King, Ramsay Jones, Junio C Hamano,
	Johannes.Schindelin, Duy Nguyen

Am 03.03.2016 um 08:38 schrieb Lars Schneider:
> (1) If I have a Git core branch with a some changes that builds and
> tests clean on Linux and OSX. How do I apply all the necessary Git for
> Windows specific changes to this branch?

How do you do it when you make a patch on Linux and want to test it on 
OSX, or the other way around? It's the same with Windows, I would guess.

*I* would export the Linux directory for Windows using Samba and then 
fetch and push from the Windows side. I would *not* develop on Windows 
in the exported Samba directory directly. If Samba is too hairy, 
exchange git bundles on a USB stick.

> (2) During my testing with Windows I noticed that the git config
> paths look funny by adding ("\\" and "/"). I mentioned the problem in
> the Gitfor Windows forum:
> https://groups.google.com/forum/#!topic/git-for-windows/zTv60HhfnYk
> Duy suggested a solution in that thread. Is this the default way
> todeal with the paths? Would the list accept this solution?

IMHO, the solution is misguided. Either --show-origin is plumbing, then 
we need the quoting. Or it is porcelain, then the quoting can be removed 
(and it is not necessary to "prettify" the file names on Windows). I 
tend to categorize --show-origin as procelain.

> (3) The tests on Windows seemed very slow to me. Are there tricks to
> speed them up? Did you try a RAM disk? If yes, how do you do it?

Run on SSD (and be prepared to swap it out for a new one within a year 
or two) ;-)

Really, there doesn't seem to be much you can do. Run the tests 
--with-dashes to save a shell wrapper around git.exe.

Unfortunately, Windows does not have RAM disks built in. I would 
appreciate any hints in this direction as well.

-- Hannes

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH v6 4/4] config: add '--show-origin' option to print the origin of a config value
  2016-03-02 17:33   ` Johannes Sixt
  2016-03-03  7:38     ` Lars Schneider
@ 2016-03-21  8:53     ` Lars Schneider
  1 sibling, 0 replies; 17+ messages in thread
From: Lars Schneider @ 2016-03-21  8:53 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git, peff, ramsay, gitster, Johannes.Schindelin


On 02 Mar 2016, at 18:33, Johannes Sixt <j6t@kdbg.org> wrote:

> Am 19.02.2016 um 10:16 schrieb larsxschneider@gmail.com:
>> +test_expect_success '--show-origin with --list' '
>> +	cat >expect <<-EOF &&
>> +		file:$HOME/.gitconfig	user.global=true
>> +		file:$HOME/.gitconfig	user.override=global
>> +		file:$HOME/.gitconfig	include.path=$INCLUDE_DIR/absolute.include
> 
> On Windows, this injects POSIX-style paths in the expected output, but git.exe produces mangled paths (with a drive letter). The pattern I use to fix this is:
> 
> 		file:$(pwd)/.gitconfig	user.override=global

I tried that. But then I get this (notice the quotation marks):

-file:C:/git-sdk-64/usr/src/git/t/trash directory.t1300-repo-config/.gitconfig  user.global=true
+file:"C:\\git-sdk-64\\usr\\src\\git\\t\\trash directory.t1300-repo-config/.gitconfig"  user.global=true

I am struggling to find a solution that works on all platforms. I see the following options:

(1) I detect MINGW in the test run and check for another string
(2) I detect MINGW in the test run and change the output of 'git config --show-origin' with a regex (e.g. replace \\ with / and remote quotation marks)
(3) I change the implementation of 'git config --show-origin' similar to [1]

If I get your comment ($gmane/288203) correctly then (3) wouldn't be a good idea.
I think (1) would be the cleanest way. Do you have some pointers for me how
Git for Windows solved these kind of problems in the past?

Thanks,
Lars

[1] https://groups.google.com/forum/#!topic/git-for-windows/zTv60HhfnYk

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH v6 4/4] config: add '--show-origin' option to print the origin of a config value
  2016-03-03  7:38     ` Lars Schneider
  2016-03-03 18:36       ` Johannes Sixt
@ 2016-03-22 14:44       ` Johannes Schindelin
  1 sibling, 0 replies; 17+ messages in thread
From: Johannes Schindelin @ 2016-03-22 14:44 UTC (permalink / raw)
  To: Lars Schneider
  Cc: Johannes Sixt, Git List, Jeff King, Ramsay Jones, Junio C Hamano,
	Duy Nguyen

Hi Lars,

On Thu, 3 Mar 2016, Lars Schneider wrote:

> > On 02 Mar 2016, at 18:33, Johannes Sixt <j6t@kdbg.org> wrote:
> > 
> > Am 19.02.2016 um 10:16 schrieb larsxschneider@gmail.com:
> >> +test_expect_success '--show-origin with --list' '
> >> +	cat >expect <<-EOF &&
> >> +		file:$HOME/.gitconfig	user.global=true
> >> +		file:$HOME/.gitconfig	user.override=global
> >> +		file:$HOME/.gitconfig	include.path=$INCLUDE_DIR/absolute.include
> > 
> > On Windows, this injects POSIX-style paths in the expected output, but
> > git.exe produces mangled paths (with a drive letter). The pattern I
> > use to fix this is:
> > 
> > 		file:$(pwd)/.gitconfig	user.override=global
>
> OK, I try to fix it that way.

I am trying to make the upgrade of Git for Windows to 2.8.0 less painful
by rebasing already to -rc4 (and hopefully catching most, if not all,
problems early). In the course, I encountered this problem and see that it
is still there.

However, I also see that the `file:$(pwd)` trick does not work: for some
reason, my Git's output has quotes around the path, probably because the
path contains *back*slashes.

BTW this points to a larger problem with this test: it will fail if
anybody is cloning git.git into a directory whose path somehow requires
quoting.

So I worked around this with two ugly patches, one that makes sure that
the backslashes in the values of the environment variables HOME and
PROGRAMDATA are converted to forward slashes before being used by the
config machinery, and the other patch being this:

-- snip --
[PATCH] t1300: fix the new --show-origin tests on Windows

On Windows, we have that funny situation where the test script can refer
to POSIX paths because it runs in a shell that uses a POSIX emulation
layer ("MSYS2 runtime"). Yet, git.exe does *not* understand POSIX paths
at all but only pure Windows paths.

So let's just convert the POSIX paths to Windows paths before passing
them on to Git, using MSYS2's `cygpath` utility.

While fixing the new tests on Windows, we also have to exclude the tests
that want to write a file with a name that is illegal on Windows
(unfortunately, there is more than one test trying to make use of that
file).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t1300-repo-config.sh | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 73afbf7..93f9065 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1232,6 +1232,14 @@ test_expect_success 'set up --show-origin tests' '
 	EOF
 '
 
+if test_have_prereq MINGW
+then
+	HOME="$(cygpath -m "$HOME")"
+	INCLUDE_DIR="$(cygpath -m "$INCLUDE_DIR")"
+	export HOME INCLUDE_DIR
+	git config -f .gitconfig include.path
"$INCLUDE_DIR/absolute.include"
+fi
+
 test_expect_success '--show-origin with --list' '
 	cat >expect <<-EOF &&
 		file:$HOME/.gitconfig	user.global=true
@@ -1304,7 +1312,7 @@ test_expect_success 'set up custom config file' '
 	EOF
 '
 
-test_expect_success '--show-origin escape special file name characters' '
+test_expect_success !MINGW '--show-origin escape special file name
characters' '
 	cat >expect <<-\EOF &&
 		file:"file\" (dq) and spaces.conf"	user.custom=true
 	EOF
@@ -1333,7 +1341,7 @@ test_expect_success '--show-origin stdin with file
include' '
 	test_cmp expect output
 '
 
-test_expect_success '--show-origin blob' '
+test_expect_success !MINGW '--show-origin blob' '
 	cat >expect <<-\EOF &&
 		blob:a9d9f9e555b5c6f07cbe09d3f06fe3df11e09c08
user.custom=true
 	EOF
@@ -1342,7 +1350,7 @@ test_expect_success '--show-origin blob' '
 	test_cmp expect output
 '
 
-test_expect_success '--show-origin blob ref' '
+test_expect_success !MINGW '--show-origin blob ref' '
 	cat >expect <<-\EOF &&
 		blob:"master:file\" (dq) and spaces.conf"
user.custom=true
 	EOF
-- 
2.7.4.windows.1
-- snap --

> Thanks for making me aware of the Windows problems. I can reproduce them
> with the Git for Windows SDK (super easy to setup, great work Dscho!)
> and I will try to fix them.

Good that you find it easy to set up the SDK. I poured quite the effort
into making it easy.

I fixed them here and will push the changes out shortly. I imagine that
Hannes will scream murder when he finds out that I used the `cygpath`
utility that his MSys environment lacks. Maybe I can prod him into fixing
my patch in a way that is compatible with that environment.

> I am fairly new to the Git for Windows SDK (+ an inexperienced Windows
> user) and therefore I wonder if you can help me with the following
> questions:
> 
> (1) If I have a Git core branch with a some changes that builds and
> tests clean on Linux and OSX. How do I apply all the necessary Git for
> Windows specific changes to this branch?

Happily, my patch series that introduce support for building Git in the
Git for Windows SDK (or for that matter, building MINGW Git with MSYS2)
made it into 2.8.0-rc0. So there is not really any need any longer to
apply any patches to make the test suite pass.

> (2) During my testing with Windows I noticed that the git config paths
> look funny by adding  ("\\" and "/"). I mentioned the problem in the Git
> for Windows forum:
> https://groups.google.com/forum/#!topic/git-for-windows/zTv60HhfnYk
> Duy suggested a solution in that thread. Is this the default way to deal
> with the paths? Would the list accept this solution?

The proposed solution looks very klunky to me. I went with a much less
intrusive version (the first patch I mentioned above):

-- snip --
[PATCH] config --show-origin: report paths with forward slashes

On Windows, the backslash is the native directory separator, but all
supported Windows versions also accept the forward slash in most
circumstances.

Our tests expect forward slashes.

Relative paths are generated by Git using forward slashes.

So let's try to be consistent and use forward slashes in the $HOME and
the $PROGRAMDATA part of the paths reported by `git config
--show-origin`, too.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 compat/mingw.c | 4 +++-
 compat/mingw.h | 6 ++++++
 path.c         | 3 +++
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index d64f41c..74f7c99 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -2808,8 +2808,10 @@ const char *program_data_config(void)
 			env = mingw_getenv("ALLUSERSPROFILE");
 			extra = "/Application Data";
 		}
-		if (env)
+		if (env) {
 			strbuf_addf(&path, "%s%s/Git/config", env, extra);
+			convert_slashes(path.buf);
+		}
 		initialized = 1;
 	}
 	return *path.buf ? path.buf : NULL;
diff --git a/compat/mingw.h b/compat/mingw.h
index 7e33b83..aa989e3 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -433,6 +433,12 @@ static inline char *mingw_find_last_dir_sep(const
char *path)
 			ret = (char *)path;
 	return ret;
 }
+static inline void convert_slashes(char *path)
+{
+	for (; *path; path++)
+		if (*path == '\\')
+			*path = '/';
+}
 #define find_last_dir_sep mingw_find_last_dir_sep
 int mingw_offset_1st_component(const char *path);
 #define offset_1st_component mingw_offset_1st_component
diff --git a/path.c b/path.c
index c4d8d21..a1ce940 100644
--- a/path.c
+++ b/path.c
@@ -589,6 +589,9 @@ char *expand_user_path(const char *path)
 			if (!home)
 				goto return_null;
 			strbuf_addstr(&user_path, home);
+#ifdef GIT_WINDOWS_NATIVE
+			convert_slashes(user_path.buf);
+#endif
 		} else {
 			struct passwd *pw = getpw_str(username,
username_len);
 			if (!pw)
-- 
2.7.4.windows.1
-- snap --

> (3) The tests on Windows seemed very slow to me. Are there tricks to
> speed them up? Did you try a RAM disk? If yes, how do you do it?

The tests *do* run very slowly on Windows. The reason is that the test
suite makes extensive use of POSIX tools, and they all have to run through
the POSIX emulation layer.

To make it utterly clear how much of a difference this makes: on my work
machine, the test suite requires about 45 minutes to run in up to 15
parallel processes on Windows. On the same machine, in a Linux VM, with
only up to 5 parallel processes, the same Git version's test suite passes
in less than 4 minutes.

So yes, running the test suite on Windows is unbearably slow.

What can you do to speed them up?

Hannes mentioned a couple of tricks.

Another trick is: avoid running the tests you do not even need. For
example, if you comment out all tests in t1300 from the second one until
the one before the one that sets up the show-origin tests, you can run the
relevant tests much, much faster.

Also, testing with GIT_TEST_INSTALLED is faster because it avoids one
(POSIX emulation layer incurring) redirection.

I still want to find time to make Git for Windows work with dash instead
of Bash (if I am not mistaken, dash has a much more native way to execute
programs, avoiding the POSIX emulation layer altogether).

And of course all of the work to convert the scripts into truly portable C
code also will help.

Ciao,
Dscho

^ permalink raw reply related	[flat|nested] 17+ messages in thread
end of thread, other threads:[~2016-03-22 14:44 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-19  9:15 [PATCH v6 0/4] config: add '--sources' option to print the source of a config value larsxschneider
2016-02-19  9:15 ` [PATCH v6 1/4] t: do not hide Git's exit code in tests using 'nul_to_q' larsxschneider
2016-02-19  9:16 ` [PATCH v6 2/4] rename git_config_from_buf to git_config_from_mem larsxschneider
2016-02-19  9:16 ` [PATCH v6 3/4] config: add 'origin_type' to config_source struct larsxschneider
2016-02-19  9:16 ` [PATCH v6 4/4] config: add '--show-origin' option to print the origin of a config value larsxschneider
2016-02-22 17:43   ` Junio C Hamano
2016-02-22 17:58     ` Jeff King
2016-02-22 18:37       ` Junio C Hamano
2016-03-02 17:33   ` Johannes Sixt
2016-03-03  7:38     ` Lars Schneider
2016-03-03 18:36       ` Johannes Sixt
2016-03-22 14:44       ` Johannes Schindelin
2016-03-21  8:53     ` Lars Schneider
2016-02-19 18:26 ` [PATCH v6 0/4] config: add '--sources' option to print the source " Junio C Hamano
2016-02-22  9:23   ` [PATCH v6 squash 0/2] " larsxschneider
2016-02-22  9:23     ` [PATCH v7 1/2] fixup: config: add 'origin_type' to config_source struct larsxschneider
2016-02-22  9:23     ` [PATCH v7 2/2] fixup: config: add '--show-origin' option to print the origin of a config value larsxschneider

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.