Re: [PATCH 1/2] ident: check for useConfigOnly before auto-detection of name/email

From: Jeff King <peff@peff.net>
Date: Mar 31, 2016, 16:31 UTC
Message-ID: <20160331163156.GA1189@sigill.intra.peff.net>
In-Reply-To: <20160331150109.GA7235@zeno>

On Thu, Mar 31, 2016 at 06:01:09PM +0300, Marios Titas wrote:

Show 26 quoted lines

> On Thu, Mar 31, 2016 at 10:40:03AM -0400, Jeff King wrote:
> >On Wed, Mar 30, 2016 at 10:29:42PM +0300, Marios Titas wrote:
> >
> >>If user.useConfigOnly is set, it does not make sense to try to
> >>auto-detect the name and/or the email. So it's better to do the
> >>useConfigOnly checks first.
> >
> >It might be nice to explain how it is better here. I'd guess it is
> >because we may fail during xgetpwuid(), giving a message that is much
> >less informative?
> 
> Oops sorry, my bad, I should have included an example in the commit message.
> So with git 2.8.0, if you provide a name and set useConfigOnly to true in
> your ~/.gitconfig file, then if try to commit something in a new git repo,
> it will fail with the following message:
> 
>    *** Please tell me who you are.
>    Run
>      git config --global user.email "you@example.com"
>      git config --global user.name "Your Name"
>    to set your account's default identity.
>    Omit --global to set the identity only in this repository.
>    fatal: unable to auto-detect email address (got 'XXX@YYY.(none)')
> 
> (provided of course that auto-detection of email fails). This wrong, because
> auto-detection is disabled anyway.

Ah, right. We used to die in xgetpwuid, but now we just set default_name_is_bogus. So I think bumping the use_config_only check above the name_is_bogus check would be sufficient. Where you put it (above ident_default_name) is fine, though it would be a problem if we later lazily loaded the config in that function (I don't have any particular plans to do so, though).

-Peff