Re: [PATCH 1/2] drop length limitations on gecos-derived names and emails

Jeff King [off-list ref] writes:

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.