threads / discuss / 46357

git config --help not functional on bad config

Subject: git config --help not functional on bad config

## tl;dr

6 messages between Jul 11, 2017 and Jul 11, 2017.

replies: 5people: 3as markdown or json

Peter Krefting· Jul 11, 2017, 14:49 UTC · lore

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· Jul 11, 2017, 17:53 UTC · re: Peter Krefting · lore

Re: git config --help not functional on bad config

On Tue, Jul 11, 2017 at 03:49:21PM +0100, Peter Krefting wrote:

Show 8 quoted lines

> 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· Jul 11, 2017, 19:05 UTC · re: Jeff King · lore

Re: git config --help not functional on bad config

On Tue, Jul 11, 2017 at 10:53 AM, Jeff King <peff@peff.net> wrote:

Show 32 quoted lines

> 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· Jul 11, 2017, 19:08 UTC · re: Stefan Beller · lore

Re: git config --help not functional on bad config

On Tue, Jul 11, 2017 at 12:05:01PM -0700, Stefan Beller wrote:

Show 21 quoted lines

> > 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· Jul 11, 2017, 19:13 UTC · re: Jeff King · lore

Re: git config --help not functional on bad config

On Tue, Jul 11, 2017 at 12:08 PM, Jeff King <peff@peff.net> wrote:

Show 31 quoted lines

> 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· Jul 11, 2017, 19:19 UTC · re: Stefan Beller · lore

Re: git config --help not functional on bad config

On Tue, Jul 11, 2017 at 12:13:59PM -0700, Stefan Beller wrote:

Show 14 quoted lines

> > 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