# git config --help not functional on bad config 6 messages from 2017-07-11 to 2017-07-11. Participants: Peter Krefting, Jeff King, Stefan Beller. Thread: https://gitlist.dev/t/46357 ## Peter Krefting, 2017-07-11 14:49 Subject: git config --help not functional on bad config Message-ID: URL: https://gitlist.dev/e/alpine.DEB.2.00.1707111541150.18163%40ds9.cixit.se ``` So, I set the wrong value for a configuration option, and git tells me: $ git config branch.autoSetupRebase false $ git log error: malformed value for branch.autosetuprebase fatal: bad config variable 'branch.autosetuprebase' in file '.git/config' at line 24 That's fine. However, when trying to look for help, it is not that useful: $ git config --help error: malformed value for branch.autosetuprebase fatal: bad config variable 'branch.autosetuprebase' in file '.git/config' at line 24 Perhaps it should allow "--help" to go through even if the configuration is bad? After a "git config --unset branch.autosetuprebase" everything works again, but I had to look that up manually calling "man git-config" (afterwards I realized I could go out of the repo to be unaffected by the config, but that probably wouldn't have helped if I had put this in my global config). -- \\// Peter - http://www.softwolves.pp.se/ ``` ## Jeff King, 2017-07-11 17:53 Subject: Re: git config --help not functional on bad config Message-ID: <20170711175342.mllx7efdsqlzv5lo@sigill.intra.peff.net> URL: https://gitlist.dev/e/20170711175342.mllx7efdsqlzv5lo%40sigill.intra.peff.net In-Reply-To: ``` On Tue, Jul 11, 2017 at 03:49:21PM +0100, Peter Krefting wrote: > That's fine. However, when trying to look for help, it is not that useful: > > $ git config --help > error: malformed value for branch.autosetuprebase > fatal: bad config variable 'branch.autosetuprebase' in file '.git/config' at line 24 > > Perhaps it should allow "--help" to go through even if the configuration is > bad? Yes, I agree the current behavior is poor. What's happening under the hood is that "--help" for any command runs "git help config", which in turn looks at the config to pick up things like help.format. But it also loads git_default_config(), which I suspect isn't actually useful. It goes all the way back to 70087cdbd (git-help: add "help.format" config variable., 2007-12-15), and it looks like it was probably added just to match other config callbacks. So I think we could probably just do this: diff --git a/builtin/help.c b/builtin/help.c index 334a8494a..c42dfc9e9 100644 --- a/builtin/help.c +++ b/builtin/help.c @@ -273,7 +273,7 @@ static int git_help_config(const char *var, const char *value, void *cb) if (starts_with(var, "man.")) return add_man_viewer_info(var, value); - return git_default_config(var, value, cb); + return 0; } static struct cmdnames main_cmds, other_cmds; which makes your case work. -Peff ``` ## Stefan Beller, 2017-07-11 19:05 Subject: Re: git config --help not functional on bad config Message-ID: URL: https://gitlist.dev/e/CAGZ79kbQsRQaHjRccs-0jkYkfhBwU-gX-M3A9sP4hqTtqqpFYw%40mail.gmail.com In-Reply-To: <20170711175342.mllx7efdsqlzv5lo@sigill.intra.peff.net> ``` On Tue, Jul 11, 2017 at 10:53 AM, Jeff King wrote: > On Tue, Jul 11, 2017 at 03:49:21PM +0100, Peter Krefting wrote: > >> That's fine. However, when trying to look for help, it is not that useful: >> >> $ git config --help >> error: malformed value for branch.autosetuprebase >> fatal: bad config variable 'branch.autosetuprebase' in file '.git/config' at line 24 >> >> Perhaps it should allow "--help" to go through even if the configuration is >> bad? > > Yes, I agree the current behavior is poor. What's happening under the > hood is that "--help" for any command runs "git help config", which in > turn looks at the config to pick up things like help.format. > > But it also loads git_default_config(), which I suspect isn't actually > useful. It goes all the way back to 70087cdbd (git-help: add > "help.format" config variable., 2007-12-15), and it looks like it was > probably added just to match other config callbacks. > > So I think we could probably just do this: > > diff --git a/builtin/help.c b/builtin/help.c > index 334a8494a..c42dfc9e9 100644 > --- a/builtin/help.c > +++ b/builtin/help.c > @@ -273,7 +273,7 @@ static int git_help_config(const char *var, const char *value, void *cb) > if (starts_with(var, "man.")) > return add_man_viewer_info(var, value); > > - return git_default_config(var, value, cb); > + return 0; Instead of ignoring any default config, we could do git_default_config(var, value, cb); /* ignore broken config, we may be the help call for config */ return 0; I looked through git_default_config which seems to only die with useful error messages for compression related settings, but these we may want to convert to errors instead of dies, when going this way. Thanks, Stefan ``` ## Jeff King, 2017-07-11 19:08 Subject: Re: git config --help not functional on bad config Message-ID: <20170711190846.jr53xhwa42uz4ky2@sigill.intra.peff.net> URL: https://gitlist.dev/e/20170711190846.jr53xhwa42uz4ky2%40sigill.intra.peff.net In-Reply-To: ``` On Tue, Jul 11, 2017 at 12:05:01PM -0700, Stefan Beller wrote: > > diff --git a/builtin/help.c b/builtin/help.c > > index 334a8494a..c42dfc9e9 100644 > > --- a/builtin/help.c > > +++ b/builtin/help.c > > @@ -273,7 +273,7 @@ static int git_help_config(const char *var, const char *value, void *cb) > > if (starts_with(var, "man.")) > > return add_man_viewer_info(var, value); > > > > - return git_default_config(var, value, cb); > > + return 0; > > Instead of ignoring any default config, we could do > > git_default_config(var, value, cb); > /* ignore broken config, we may be the help call for config */ > return 0; > > I looked through git_default_config which seems to only die > with useful error messages for compression related settings, > but these we may want to convert to errors instead of dies, > when going this way. There are other die calls hidden deeper. For instance: $ git -c core.ignorecase=foo help config fatal: bad numeric config value 'foo' for 'core.ignorecase': invalid unit Those ones are hard to fix without changing the call signature of git_config_bool(). -Peff ``` ## Stefan Beller, 2017-07-11 19:13 Subject: Re: git config --help not functional on bad config Message-ID: URL: https://gitlist.dev/e/CAGZ79kbL%3DRy_6L9iJw1GQA39gdPsS%3DtwoJFpKPc3ZP%2BM2saoWA%40mail.gmail.com In-Reply-To: <20170711190846.jr53xhwa42uz4ky2@sigill.intra.peff.net> ``` On Tue, Jul 11, 2017 at 12:08 PM, Jeff King wrote: > On Tue, Jul 11, 2017 at 12:05:01PM -0700, Stefan Beller wrote: > >> > diff --git a/builtin/help.c b/builtin/help.c >> > index 334a8494a..c42dfc9e9 100644 >> > --- a/builtin/help.c >> > +++ b/builtin/help.c >> > @@ -273,7 +273,7 @@ static int git_help_config(const char *var, const char *value, void *cb) >> > if (starts_with(var, "man.")) >> > return add_man_viewer_info(var, value); >> > >> > - return git_default_config(var, value, cb); >> > + return 0; >> >> Instead of ignoring any default config, we could do >> >> git_default_config(var, value, cb); >> /* ignore broken config, we may be the help call for config */ >> return 0; >> >> I looked through git_default_config which seems to only die >> with useful error messages for compression related settings, >> but these we may want to convert to errors instead of dies, >> when going this way. > > There are other die calls hidden deeper. For instance: > > $ git -c core.ignorecase=foo help config > fatal: bad numeric config value 'foo' for 'core.ignorecase': invalid unit > > Those ones are hard to fix without changing the call signature of > git_config_bool(). Good point. While looking at it, parse_help_format can also die, so building a safe git help config is hard: git config --global help.format foo # everything is broken, how do I fix it? git help config # breaks, too, for the same reason as you outlined :/ ``` ## Jeff King, 2017-07-11 19:19 Subject: Re: git config --help not functional on bad config Message-ID: <20170711191913.jjvw6kqtywyeyf7a@sigill.intra.peff.net> URL: https://gitlist.dev/e/20170711191913.jjvw6kqtywyeyf7a%40sigill.intra.peff.net In-Reply-To: ``` On Tue, Jul 11, 2017 at 12:13:59PM -0700, Stefan Beller wrote: > > There are other die calls hidden deeper. For instance: > > > > $ git -c core.ignorecase=foo help config > > fatal: bad numeric config value 'foo' for 'core.ignorecase': invalid unit > > > > Those ones are hard to fix without changing the call signature of > > git_config_bool(). > > Good point. While looking at it, parse_help_format can also die, > so building a safe git help config is hard: > > git config --global help.format foo > # everything is broken, how do I fix it? > git help config # breaks, too, for the same reason as you outlined :/ Yeah, I think that should be switched to return an error, and probably all errors from git_help_config() should issue a warning and still return 0. -Peff ```