Re: [PATCH] config: complain about --local outside of a git repo

[Josh Hagins <hagins.josh@gmail.com>]

Hi Jeff,

For context, the "user.name" bit was purely notional; I just wanted to
give a sample reproduction. Where I've actually been running into this
in real life is with oh-my-git, a GitHub-themed Powerline bash prompt:
https://github.com/arialdomartini/oh-my-git/blob/42c11f08540949b75bd513e6880a4ce6824a2bcc/base.sh#L52.

Since this command is invoked every time the prompt is build, I see
the error message after every single command I run outside of a Git
repository. While this is more indicative of a code smell in
oh-my-git, I figured that, as you said, BUG assertions should never
get hit in the wild. Thanks for the patch!

Cheers,
Josh

On Fri, May 12, 2017 at 4:34 PM, Jeff King <peff@peff.net> wrote:
> On Fri, May 12, 2017 at 10:19:59AM -0400, Josh Hagins wrote:
>
>> Since upgrading to Git 2.13.0 I'm seeing this error message whenever
>> `git config --local <whatever>` is called outside a Git repository.
>> For example, note the difference in behavior between Git 2.13 and
>> Apple Git:
>>
>>     $ pwd
>>     /Users/jhagins
>>     $ /usr/bin/git --version
>>     git version 2.11.0 (Apple Git-81)
>>     $ /usr/bin/git config --local --get user.name
>>     $ /usr/local/bin/git --version
>>     git version 2.13.0
>>     $ /usr/local/bin/git config --local --get user.name
>>     fatal: BUG: setup_git_env called without repository
>>
>> Apple Git outputs nothing, as expected. The summarized release notes
>> published by GitHub specifically mentioned that instances of this
>> error message should be reported, so here you go!
>
> Thanks for reporting. All the developers have been running with this
> change for months, but I knew as soon as it was released into the wild
> that somebody would find a new corner case. :)
>
> I think dying is the right thing here; you are asking for "--local" but
> there is no local repository. But we should never hit a BUG assertion.
> Patch is below.
>
> I'm not sure exactly what you wanted to accomplish with --local. If you
> just want to know if user.name is set anywhere (and you may or may not
> be in a git repo), then just "git config --get user.name" would work. If
> you want to know if you're in a local repo and if so whether the
> variable is set, you'd need to use two commands, like:
>
>   git rev-parse --git-dir >/dev/null 2>&1 &&
>   git config --local --get user.name
>
> -- >8 --
> Subject: [PATCH] config: complain about --local outside of a git repo
>
> The "--local" option instructs git-config to read or modify
> the repository-level config. This doesn't make any sense if
> you're not actually in a repository.
>
> Older versions of Git would blindly try to read or write
> ".git/config". For reading, this would result in a quiet
> failure, since there was no config to read (and thus no
> matching config value). Writing would generally fail
> noisily, since ".git" was unlikely to exist. But since
> b1ef400ee (setup_git_env: avoid blind fall-back to ".git",
> 2016-10-20), we catch this in the call to git_pathdup() and
> die("BUG").
>
> Dying is the right thing to do, but we should catch the
> problem early and give a more human-friendly error message.
>
> Note that even without --local, git-config will sometimes
> default to using local repository config. These cases are
> already protected by a similar check.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  builtin/config.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/builtin/config.c b/builtin/config.c
> index 3a554ad50..ad7c6a19c 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -496,6 +496,9 @@ int cmd_config(int argc, const char **argv, const char *prefix)
>                 usage_with_options(builtin_config_usage, builtin_config_options);
>         }
>
> +       if (use_local_config && nongit)
> +               die(_("--local only be used inside a git repository"));
> +
>         if (given_config_source.file &&
>                         !strcmp(given_config_source.file, "-")) {
>                 given_config_source.file = NULL;
> --
> 2.13.0.452.g0afc8e12b
>



-- 
Josh Hagins