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

[PATCH 03/10] config: die on error in command-line config

From
Jeff King <peff@peff.net>
Date
Jun 9, 2011, 15:52 UTC
Message-ID
<20110609155232.GC25507@sigill.intra.peff.net>
In-Reply-To
<20110609155001.GA14969@sigill.intra.peff.net>

The error handling for git_config is somewhat confusing. We collect errors from running git_config_from_file on the various config files and carefully pass them back up. But the two odd things are:

  1. We actually die on most errors in git_config_from_file.
     In fact, the only error we actually pass back up is if
     fopen() fails on the file.
  2. Most callers of git_config do not check the error
     return at all, but will continue if git_config reports
     an error.

When the code for "git -c core.foo=bar" was added, it dutifully passed errors up the call stack, only for them to be eventually ignored. This makes it inconsistent with the file-parsing code, which will die when it sees malformed config. And it's somewhat unsafe, because it means an error in parsing a typo like:

  git -c clean.requireforce=ture clean

will continue the command, ignoring the config the user tried to give.

Signed-off-by: Jeff King <peff@peff.net>
---
Another option would be to just make git_config call die() on error
instead of returning the error code that is ignored everywhere. But I
wasn't sure if anybody was relying on the "fopen failure is silently
ignored" behavior.
 config.c               |    2 +-
 t/t1300-repo-config.sh |    8 ++++++++
 2 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/config.c b/config.c
index aa5eb78..ebd404a 100644
--- a/config.c
+++ b/config.c
@@ -856,7 +856,7 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
 
 	switch (git_config_from_parameters(fn, data)) {
 	case -1: /* error */
-		ret--;
+		die("unable to parse command-line config");
 		break;
 	case 0: /* found nothing */
 		break;
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index ca5058e..584e956 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -910,4 +910,12 @@ test_expect_success 'git -c does not split values on equals' '
 	test_cmp expect actual
 '
 
+test_expect_success 'git -c dies on bogus config' '
+	test_must_fail git -c core.bare=foo rev-parse
+'
+
+test_expect_success 'git -c complains about empty key' '
+	test_must_fail git -c "=foo" rev-parse
+'
+
 test_done
-- 
1.7.6.rc1.36.g91167
Previous: Jeff KingNext: Jeff King
Message 10 of 25 in “Git-Mediawiki : cloning a set of pages”
  1. Claire FousseJun 8, 2011
  2. Jeff KingJun 8, 2011
  3. Sverre RabbelierJun 8, 2011
  4. Jeff KingJun 8, 2011
  5. Jeff KingJun 9, 2011
  6. 01/10 strbuf_split: add a max parameterJeff King, Jun 9, 2011
  7. Junio C HamanoJun 13, 2011
  8. Jeff KingJun 13, 2011
  9. 02/10 fix "git -c" parsing of values with equals signsJeff King, Jun 9, 2011
  10. 03/10 config: die on error in command-line configJeff King, Jun 9, 2011
  11. 04/10 config: avoid segfault when parsing command-line configJeff King, Jun 9, 2011
  12. Junio C HamanoJun 13, 2011
  13. Jeff KingJun 13, 2011
  14. 05/10 strbuf: allow strbuf_split to work on non-strbufsJeff King, Jun 9, 2011
  15. 06/10 config: use strbuf_split_str instead of a temporary strbufJeff King, Jun 9, 2011
  16. 07/10 parse-options: add OPT_STRING_LIST helperJeff King, Jun 9, 2011
  17. 08/10 remote: use new OPT_STRING_LISTJeff King, Jun 9, 2011
  18. 09/10 config: make git_config_parse_parameter a public functionJeff King, Jun 9, 2011
  19. 10/10 clone: accept config options on the command lineJeff King, Jun 9, 2011
  20. Bert WesargJun 9, 2011
  21. Jeff KingJun 9, 2011
  22. Jeff KingJun 9, 2011
  23. Matthieu MoyJun 9, 2011
  24. Jakub NarebskiJun 8, 2011
  25. Claire FousseJun 9, 2011

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.