Re: [PATCH 1/2] drop length limitations on gecos-derived names and emails
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 15, 2012, 15:03 UTC
- Message-ID
- <7vtxzhfpv9.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20120515015437.GA13833@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 14 quoted lines
> On Mon, May 14, 2012 at 05:13:24PM -0400, Jeff King wrote:
>
> We call setup_ident with our name pointer, which usually comes from
> getenv("GIT_*_NAME"), although could also come from something like "git
> commit -c $commit". We feed that to setup_ident. If name is NULL, then
> setup_ident will use git_default_name (filling it in from gecos or
> config). If it's not NULL, then we use it literally. And then we check
> _that_ result to see if it's empty. If it is, we either die or warn,
> depending on the flags. In the latter case, we fallback to using the
> username as the name.
>
> And that's what confuses me. Depending on what was passed in, we may
> have checked that GIT_COMMITTER_NAME is an empty string, or we may have
> checked that the config or gecos field yielded an empty string. Sounds quite sensible to me, though.
> In the > latter case, it makes sense to fall back to the username.
I agree that we should use something like "Sorry, Mr. McDonald" codepath when the GECOS field returns an empty string---after all that is what we do when we are built with NO_GECOS_IN_PWENT.
> But in the > former case, it doesn't; we should fall back to the config name or the > gecos name.
If the user said GIT_COMMITTER_NAME is empty with "GIT_COMMITTER_NAME=", that is different from saying with "unset GIT_COMMITTER_NAME" that the user does not want the environment to take effect, no? So I do not think falling back to configured or gecos in the former case is the right thing to do, even though that would mean explicitly giving an empty string in that configuration variable is asking only for an error without any recourse, which is not useful at all.