{"thread":{"id":"46357","subject":"git config --help not functional on bad config","startedAt":"2017-07-11T15:00:15Z","lastAt":"2017-07-11T19:19:20Z","messageCount":6,"participants":["Peter Krefting","Jeff King","Stefan Beller"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"324195","messageId":"alpine.DEB.2.00.1707111541150.18163@ds9.cixit.se","threadId":"46357","inReplyTo":null,"subject":"git config --help not functional on bad config","fromName":"Peter Krefting","fromEmail":"peter@softwolves.pp.se","sentAt":"2017-07-11T14:49:21Z","receivedAt":"2017-07-11T15:00:15Z","isPatch":false,"sender":{"key":"peter@softwolves.pp.se","avatar":"https://avatars.githubusercontent.com/u/990764?v=4"},"body":"So, I set the wrong value for a configuration option, and git tells me:\n\n   $ git config branch.autoSetupRebase false\n   $ git log\n   error: malformed value for branch.autosetuprebase\n   fatal: bad config variable 'branch.autosetuprebase' in file '.git/config' at line 24\n\nThat's fine. However, when trying to look for help, it is not that \nuseful:\n\n   $ git config --help\n   error: malformed value for branch.autosetuprebase\n   fatal: bad config variable 'branch.autosetuprebase' in file '.git/config' at line 24\n\nPerhaps it should allow \"--help\" to go through even if the \nconfiguration is bad?\n\nAfter a \"git config --unset branch.autosetuprebase\" everything works \nagain, but I had to look that up manually calling \"man git-config\" \n(afterwards I realized I could go out of the repo to be unaffected \nby the config, but that probably wouldn't have helped if I had put \nthis in my global config).\n\n-- \n\\\\// Peter - http://www.softwolves.pp.se/\n"},{"id":"324209","messageId":"20170711175342.mllx7efdsqlzv5lo@sigill.intra.peff.net","threadId":"46357","inReplyTo":"alpine.DEB.2.00.1707111541150.18163@ds9.cixit.se","subject":"Re: git config --help not functional on bad config","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-07-11T17:53:42Z","receivedAt":"2017-07-11T17:53:49Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jul 11, 2017 at 03:49:21PM +0100, Peter Krefting wrote:\n\n> That's fine. However, when trying to look for help, it is not that useful:\n> \n>   $ git config --help\n>   error: malformed value for branch.autosetuprebase\n>   fatal: bad config variable 'branch.autosetuprebase' in file '.git/config' at line 24\n> \n> Perhaps it should allow \"--help\" to go through even if the configuration is\n> bad?\n\nYes, I agree the current behavior is poor. What's happening under the\nhood is that \"--help\" for any command runs \"git help config\", which in\nturn looks at the config to pick up things like help.format.\n\nBut it also loads git_default_config(), which I suspect isn't actually\nuseful. It goes all the way back to 70087cdbd (git-help: add\n\"help.format\" config variable., 2007-12-15), and it looks like it was\nprobably added just to match other config callbacks.\n\nSo I think we could probably just do this:\n\ndiff --git a/builtin/help.c b/builtin/help.c\nindex 334a8494a..c42dfc9e9 100644\n--- a/builtin/help.c\n+++ b/builtin/help.c\n@@ -273,7 +273,7 @@ static int git_help_config(const char *var, const char *value, void *cb)\n \tif (starts_with(var, \"man.\"))\n \t\treturn add_man_viewer_info(var, value);\n \n-\treturn git_default_config(var, value, cb);\n+\treturn 0;\n }\n \n static struct cmdnames main_cmds, other_cmds;\n\nwhich makes your case work.\n\n-Peff\n"},{"id":"324219","messageId":"CAGZ79kbQsRQaHjRccs-0jkYkfhBwU-gX-M3A9sP4hqTtqqpFYw@mail.gmail.com","threadId":"46357","inReplyTo":"20170711175342.mllx7efdsqlzv5lo@sigill.intra.peff.net","subject":"Re: git config --help not functional on bad config","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-11T19:05:01Z","receivedAt":"2017-07-11T19:05:23Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Jul 11, 2017 at 10:53 AM, Jeff King <peff@peff.net> wrote:\n> On Tue, Jul 11, 2017 at 03:49:21PM +0100, Peter Krefting wrote:\n>\n>> That's fine. However, when trying to look for help, it is not that useful:\n>>\n>>   $ git config --help\n>>   error: malformed value for branch.autosetuprebase\n>>   fatal: bad config variable 'branch.autosetuprebase' in file '.git/config' at line 24\n>>\n>> Perhaps it should allow \"--help\" to go through even if the configuration is\n>> bad?\n>\n> Yes, I agree the current behavior is poor. What's happening under the\n> hood is that \"--help\" for any command runs \"git help config\", which in\n> turn looks at the config to pick up things like help.format.\n>\n> But it also loads git_default_config(), which I suspect isn't actually\n> useful. It goes all the way back to 70087cdbd (git-help: add\n> \"help.format\" config variable., 2007-12-15), and it looks like it was\n> probably added just to match other config callbacks.\n>\n> So I think we could probably just do this:\n>\n> diff --git a/builtin/help.c b/builtin/help.c\n> index 334a8494a..c42dfc9e9 100644\n> --- a/builtin/help.c\n> +++ b/builtin/help.c\n> @@ -273,7 +273,7 @@ static int git_help_config(const char *var, const char *value, void *cb)\n>         if (starts_with(var, \"man.\"))\n>                 return add_man_viewer_info(var, value);\n>\n> -       return git_default_config(var, value, cb);\n> +       return 0;\n\nInstead of ignoring any default config, we could do\n\n    git_default_config(var, value, cb);\n    /* ignore broken config, we may be the help call for config */\n    return 0;\n\nI looked through git_default_config which seems to only die\nwith useful error messages for compression related settings,\nbut these we may want to convert to errors instead of dies,\nwhen going this way.\n\nThanks,\nStefan\n"},{"id":"324220","messageId":"20170711190846.jr53xhwa42uz4ky2@sigill.intra.peff.net","threadId":"46357","inReplyTo":"CAGZ79kbQsRQaHjRccs-0jkYkfhBwU-gX-M3A9sP4hqTtqqpFYw@mail.gmail.com","subject":"Re: git config --help not functional on bad config","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-07-11T19:08:46Z","receivedAt":"2017-07-11T19:08:52Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jul 11, 2017 at 12:05:01PM -0700, Stefan Beller wrote:\n\n> > diff --git a/builtin/help.c b/builtin/help.c\n> > index 334a8494a..c42dfc9e9 100644\n> > --- a/builtin/help.c\n> > +++ b/builtin/help.c\n> > @@ -273,7 +273,7 @@ static int git_help_config(const char *var, const char *value, void *cb)\n> >         if (starts_with(var, \"man.\"))\n> >                 return add_man_viewer_info(var, value);\n> >\n> > -       return git_default_config(var, value, cb);\n> > +       return 0;\n> \n> Instead of ignoring any default config, we could do\n> \n>     git_default_config(var, value, cb);\n>     /* ignore broken config, we may be the help call for config */\n>     return 0;\n> \n> I looked through git_default_config which seems to only die\n> with useful error messages for compression related settings,\n> but these we may want to convert to errors instead of dies,\n> when going this way.\n\nThere are other die calls hidden deeper. For instance:\n\n  $ git -c core.ignorecase=foo help config\n  fatal: bad numeric config value 'foo' for 'core.ignorecase': invalid unit\n\nThose ones are hard to fix without changing the call signature of\ngit_config_bool().\n\n-Peff\n"},{"id":"324221","messageId":"CAGZ79kbL=Ry_6L9iJw1GQA39gdPsS=twoJFpKPc3ZP+M2saoWA@mail.gmail.com","threadId":"46357","inReplyTo":"20170711190846.jr53xhwa42uz4ky2@sigill.intra.peff.net","subject":"Re: git config --help not functional on bad config","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-11T19:13:59Z","receivedAt":"2017-07-11T19:14:10Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Jul 11, 2017 at 12:08 PM, Jeff King <peff@peff.net> wrote:\n> On Tue, Jul 11, 2017 at 12:05:01PM -0700, Stefan Beller wrote:\n>\n>> > diff --git a/builtin/help.c b/builtin/help.c\n>> > index 334a8494a..c42dfc9e9 100644\n>> > --- a/builtin/help.c\n>> > +++ b/builtin/help.c\n>> > @@ -273,7 +273,7 @@ static int git_help_config(const char *var, const char *value, void *cb)\n>> >         if (starts_with(var, \"man.\"))\n>> >                 return add_man_viewer_info(var, value);\n>> >\n>> > -       return git_default_config(var, value, cb);\n>> > +       return 0;\n>>\n>> Instead of ignoring any default config, we could do\n>>\n>>     git_default_config(var, value, cb);\n>>     /* ignore broken config, we may be the help call for config */\n>>     return 0;\n>>\n>> I looked through git_default_config which seems to only die\n>> with useful error messages for compression related settings,\n>> but these we may want to convert to errors instead of dies,\n>> when going this way.\n>\n> There are other die calls hidden deeper. For instance:\n>\n>   $ git -c core.ignorecase=foo help config\n>   fatal: bad numeric config value 'foo' for 'core.ignorecase': invalid unit\n>\n> Those ones are hard to fix without changing the call signature of\n> git_config_bool().\n\nGood point. While looking at it, parse_help_format can also die,\nso building a safe git help config is hard:\n\n    git config --global help.format foo\n    # everything is broken, how do I fix it?\n    git help config # breaks, too, for the same reason as you outlined :/\n"},{"id":"324222","messageId":"20170711191913.jjvw6kqtywyeyf7a@sigill.intra.peff.net","threadId":"46357","inReplyTo":"CAGZ79kbL=Ry_6L9iJw1GQA39gdPsS=twoJFpKPc3ZP+M2saoWA@mail.gmail.com","subject":"Re: git config --help not functional on bad config","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-07-11T19:19:13Z","receivedAt":"2017-07-11T19:19:20Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jul 11, 2017 at 12:13:59PM -0700, Stefan Beller wrote:\n\n> > There are other die calls hidden deeper. For instance:\n> >\n> >   $ git -c core.ignorecase=foo help config\n> >   fatal: bad numeric config value 'foo' for 'core.ignorecase': invalid unit\n> >\n> > Those ones are hard to fix without changing the call signature of\n> > git_config_bool().\n> \n> Good point. While looking at it, parse_help_format can also die,\n> so building a safe git help config is hard:\n> \n>     git config --global help.format foo\n>     # everything is broken, how do I fix it?\n>     git help config # breaks, too, for the same reason as you outlined :/\n\nYeah, I think that should be switched to return an error, and probably\nall errors from git_help_config() should issue a warning and still\nreturn 0.\n\n-Peff\n"}]}