git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v4 2/3] config: add 'type' to config_source struct that identifies config type

From
Ramsay Jones <ramsay@ramsayjones.plus.com>
Date
Feb 15, 2016, 21:30 UTC
Message-ID
<56C24375.4080007@ramsayjones.plus.com>
In-Reply-To
<1455531466-16617-3-git-send-email-larsxschneider@gmail.com>
On 15/02/16 10:17, larsxschneider@gmail.com wrote:
Show 96 quoted lines
> From: Lars Schneider <larsxschneider@gmail.com>
> 
> Use the config type to print more detailed error messages that inform
> the user about the origin of a config error (file, stdin, blob).
> 
> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
> ---
>  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 c63fcc1..8d86e5c 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_buf(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 *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_type();
> +extern const char *current_config_name();
>  
>  struct config_include_data {
>  	int depth;
> diff --git a/config.c b/config.c
> index 86a5eb2..5cf11b5 100644
> --- a/config.c
> +++ b/config.c
> @@ -24,6 +24,7 @@ struct config_source {
>  			size_t pos;
>  		} buf;
>  	} u;
> +	const char *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->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->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->type && cf->name)
> +		die(_("bad numeric config value '%s' for '%s' in %s %s: %s"),
> +		    value, name, cf->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 *type, const char *name, const char *path, FILE *f,
> +		void *data)
>  {
>  	struct config_source top;
>  
>  	top.u.file = f;
> +	top.type = 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);
I think this should be:
	return do_config_from_file(fn, "file", "<stdin>", NULL, stdin, data);

ie. <stdin> is not a separate type, but a file that does not exist in the filesystem and, thus, has no name. (what you use internally is a separate issue, but <file,NULL> works for me.)

Also, I'm so used to '<stdin>' as the 'name' (token/handle) of that file, that it looks very odd spelt otherwise. :-D

Show 84 quoted lines
>  }
>  
>  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_buf(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 *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.type = 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_buf(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_type()
> +{
> +	return cf && cf->type ? cf->type : "cmdline";
> +}
> +
> +const char *current_config_name()
> +{
> +	return cf && cf->name ? cf->name : "";
> +}
> diff --git a/submodule-config.c b/submodule-config.c
> index fe8ceab..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_buf(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 &&
So, this looks odd. Using the above would give:
	fatal: bad config line 1 in file <stdin>
Show 29 quoted lines
> +	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
>  '
>

ATB, Ramsay Jones

Previous: Ramsay JonesNext: Johannes Schindelin
Message 8 of 32 in “config: add '--sources' option to print the source of a config value”
  1. 0/3 config: add '--sources' option to print the source of a config valuelarsxschneider@gmail.com, Feb 15, 2016
  2. 1/3 t: do not hide Git's exit code in testslarsxschneider@gmail.com, Feb 15, 2016
  3. Jeff KingFeb 15, 2016
  4. Lars SchneiderFeb 16, 2016
  5. 2/3 config: add 'type' to config_source struct that identifies config typelarsxschneider@gmail.com, Feb 15, 2016
  6. Jeff KingFeb 15, 2016
  7. Ramsay JonesFeb 16, 2016
  8. Ramsay JonesFeb 15, 2016
  9. Johannes SchindelinFeb 17, 2016
  10. Lars SchneiderFeb 18, 2016
  11. 3/3 config: add '--show-origin' option to print the origin of a config valuelarsxschneider@gmail.com, Feb 15, 2016
  12. Jeff KingFeb 15, 2016
  13. Eric SunshineFeb 15, 2016
  14. Jeff KingFeb 15, 2016
  15. Lars SchneiderFeb 17, 2016
  16. Junio C HamanoFeb 17, 2016
  17. Ramsay JonesFeb 15, 2016
  18. Jeff KingFeb 15, 2016
  19. Ramsay JonesFeb 15, 2016
  20. Lars SchneiderFeb 16, 2016
  21. Ramsay JonesFeb 16, 2016
  22. Jeff KingFeb 16, 2016
  23. Ramsay JonesFeb 16, 2016
  24. Jeff KingFeb 16, 2016
  25. Junio C HamanoFeb 15, 2016
  26. Lars SchneiderFeb 16, 2016
  27. Junio C HamanoFeb 15, 2016
  28. Jeff KingFeb 15, 2016
  29. Junio C HamanoFeb 15, 2016
  30. Lars SchneiderFeb 16, 2016
  31. Jeff KingFeb 15, 2016
  32. Lars SchneiderFeb 16, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.