{"thread":{"id":"25498","subject":"[PATCH] config: treat non-existent config files as empty","startedAt":"2010-10-21T14:45:44Z","lastAt":"2010-10-21T17:00:46Z","messageCount":3,"participants":["Jeff King","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"153967","messageId":"20101021144544.GA18901@sigill.intra.peff.net","threadId":"25498","inReplyTo":null,"subject":"[PATCH] config: treat non-existent config files as empty","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-10-21T14:45:44Z","receivedAt":"2010-10-21T14:45:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The git_config() function signals error by returning -1 in\ntwo instances:\n\n  1. An actual error occurs in opening a config file (parse\n     errors cause an immediate die).\n\n  2. Of the three possible config files, none was found.\n\nHowever, this second case is often not an error at all; it\nsimply means that the user has no configuration (they are\noutside a repo, and they have no ~/.gitconfig file). This\ncan lead to confusing errors, such as when the bash\ncompletion calls \"git config --list\" outside of a repo. If\nthe user has a ~/.gitconfig, the command completes\nsuccesfully; if they do not, it complains to stderr.\n\nThis patch allows callers of git_config to distinguish\nbetween the two cases. Error is signaled by -1, and\notherwise the return value is the number of files parsed.\nThis means that the traditional \"git_config(...) < 0\" check\nfor error should work, but callers who want to know whether\nwe parsed any files or not can still do so.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nAnother cleaned up re-post. Original discussion here:\n\n  http://article.gmane.org/gmane.comp.version-control.git/128206\n\nYour original questions/objections were:\n\n  1. Does it still error on GIT_CONFIG=/nonexistent? I said then I\n     hadn't tested. I just did, and it does still produce an error.\n\n  2. You mentioned some other issues in the config code. While I agree\n     they could be fixed, they are outside the scope of this patch,\n     which does fix something we both agreed was a bug.\n\nThe original also had to fix up a callsite for the new semantics, but\nthat callsite is now gone.\n\n config.c               |    4 +---\n t/t1300-repo-config.sh |    8 ++++++++\n 2 files changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex 4b0a820..c63d683 100644\n--- a/config.c\n+++ b/config.c\n@@ -871,9 +871,7 @@ int git_config(config_fn_t fn, void *data)\n \tif (config_parameters)\n \t\tfound += 1;\n \n-\tif (found == 0)\n-\t\treturn -1;\n-\treturn ret;\n+\treturn ret == 0 ? found : ret;\n }\n \n /*\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex 074f2f2..6f6ed2d 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -289,6 +289,14 @@ test_expect_success 'working --list' \\\n \t'git config --list > output && cmp output expect'\n \n cat > expect << EOF\n+EOF\n+\n+test_expect_success '--list without repo produces empty output' '\n+\tgit --git-dir=nonexistent config --list >output &&\n+\ttest_cmp expect output\n+'\n+\n+cat > expect << EOF\n beta.noindent sillyValue\n nextsection.nonewline wow2 for me\n EOF\n-- \n1.7.3.1.235.gdd6c0\n"},{"id":"153980","messageId":"20101021164708.GD3732@burratino","threadId":"25498","inReplyTo":"20101021144544.GA18901@sigill.intra.peff.net","subject":"Re: [PATCH] config: treat non-existent config files as empty","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-21T16:47:08Z","receivedAt":"2010-10-21T16:47:08Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n>   1. Does it still error on GIT_CONFIG=/nonexistent? I said then I\n>      hadn't tested. I just did, and it does still produce an error.\n\nMaybe it would make sense to squash in something like this, then.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex c703257..d0e5546 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -844,6 +844,27 @@ test_expect_success SYMLINKS 'symlinked configuration' '\n \n '\n \n+test_expect_success 'nonexistent configuration' '\n+\t(\n+\t\tGIT_CONFIG=doesnotexist &&\n+\t\texport GIT_CONFIG &&\n+\t\ttest_must_fail git config --list &&\n+\t\ttest_must_fail git config test.xyzzy\n+\t)\n+'\n+\n+test_expect_success SYMLINKS 'symlink to nonexistent configuration' '\n+\tln -s doesnotexist linktonada &&\n+\tln -s linktonada linktolinktonada &&\n+\t(\n+\t\tGIT_CONFIG=linktonada &&\n+\t\texport GIT_CONFIG &&\n+\t\ttest_must_fail git config --list &&\n+\t\tGIT_CONFIG=linktolinktonada &&\n+\t\ttest_must_fail git config --list\n+\t)\n+'\n+\n test_expect_success 'check split_cmdline return' \"\n \tgit config alias.split-cmdline-fix 'echo \\\"' &&\n \ttest_must_fail git split-cmdline-fix &&\n"},{"id":"153982","messageId":"20101021170046.GA15667@sigill.intra.peff.net","threadId":"25498","inReplyTo":"20101021164708.GD3732@burratino","subject":"Re: [PATCH] config: treat non-existent config files as empty","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-10-21T17:00:46Z","receivedAt":"2010-10-21T17:00:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 21, 2010 at 11:47:08AM -0500, Jonathan Nieder wrote:\n\n> Jeff King wrote:\n> \n> >   1. Does it still error on GIT_CONFIG=/nonexistent? I said then I\n> >      hadn't tested. I just did, and it does still produce an error.\n> \n> Maybe it would make sense to squash in something like this, then.\n\nYes, I think it's worth adding those tests. Thanks.\n\n-Peff\n"}]}