git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* git config --help not functional on bad config
@ 2017-07-11 14:49 Peter Krefting
  2017-07-11 17:53 ` Jeff King
  0 siblings, 1 reply; 6+ messages in thread
From: Peter Krefting @ 2017-07-11 14:49 UTC (permalink / raw)
  To: Git Mailing List

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/

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: git config --help not functional on bad config
  2017-07-11 14:49 git config --help not functional on bad config Peter Krefting
@ 2017-07-11 17:53 ` Jeff King
  2017-07-11 19:05   ` Stefan Beller
  0 siblings, 1 reply; 6+ messages in thread
From: Jeff King @ 2017-07-11 17:53 UTC (permalink / raw)
  To: Peter Krefting; +Cc: Git Mailing List

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: git config --help not functional on bad config
  2017-07-11 17:53 ` Jeff King
@ 2017-07-11 19:05   ` Stefan Beller
  2017-07-11 19:08     ` Jeff King
  0 siblings, 1 reply; 6+ messages in thread
From: Stefan Beller @ 2017-07-11 19:05 UTC (permalink / raw)
  To: Jeff King; +Cc: Peter Krefting, Git Mailing List

On Tue, Jul 11, 2017 at 10:53 AM, Jeff King <peff@peff.net> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: git config --help not functional on bad config
  2017-07-11 19:05   ` Stefan Beller
@ 2017-07-11 19:08     ` Jeff King
  2017-07-11 19:13       ` Stefan Beller
  0 siblings, 1 reply; 6+ messages in thread
From: Jeff King @ 2017-07-11 19:08 UTC (permalink / raw)
  To: Stefan Beller; +Cc: Peter Krefting, Git Mailing List

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: git config --help not functional on bad config
  2017-07-11 19:08     ` Jeff King
@ 2017-07-11 19:13       ` Stefan Beller
  2017-07-11 19:19         ` Jeff King
  0 siblings, 1 reply; 6+ messages in thread
From: Stefan Beller @ 2017-07-11 19:13 UTC (permalink / raw)
  To: Jeff King; +Cc: Peter Krefting, Git Mailing List

On Tue, Jul 11, 2017 at 12:08 PM, Jeff King <peff@peff.net> 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 :/

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: git config --help not functional on bad config
  2017-07-11 19:13       ` Stefan Beller
@ 2017-07-11 19:19         ` Jeff King
  0 siblings, 0 replies; 6+ messages in thread
From: Jeff King @ 2017-07-11 19:19 UTC (permalink / raw)
  To: Stefan Beller; +Cc: Peter Krefting, Git Mailing List

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-07-11 19:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-11 14:49 git config --help not functional on bad config Peter Krefting
2017-07-11 17:53 ` Jeff King
2017-07-11 19:05   ` Stefan Beller
2017-07-11 19:08     ` Jeff King
2017-07-11 19:13       ` Stefan Beller
2017-07-11 19:19         ` Jeff King

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).