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

From: Jeff King <peff@peff.net>
Date: May 15, 2012, 02:32 UTC
Message-ID: <20120515023220.GA22947@sigill.intra.peff.net>
In-Reply-To: <20120515015437.GA13833@sigill.intra.peff.net>

On Mon, May 14, 2012 at 09:54:37PM -0400, Jeff King wrote:

Show 15 quoted lines

> Of course we've still polluted this crappy fake name into
> git_default_name, so that later calls with error_on_no_name will see it
> and not error. I think so far it hasn't mattered because the only user
> of this "warn" code is format-patch, which otherwise does not care about
> ident (and doesn't even end up using the name at all!). And I doubt this
> code path gets triggered much anyway; do people really run
> "GIT_COMMITTER_NAME= git format-patch"?
> 
> I can just leave it as it's not really hurting anybody, I think. But I
> was refactoring in the area and it just seemed flaky and questionable. I
> wonder if we can simply get rid of the IDENT_WARN_ON_NO_NAME code path
> entirely. The use here is grabbing the email address to use as part of a
> message id. Could we just call setup_ident and then read from
> git_default_email directly? There's no need to respect
> GIT_COMMITTER_EMAIL here at all.

Hmm, I was mistaken. This code path also gets followed whenever IDENT_ERROR_ON_NO_NAME is not set (regardless of IDENT_WARN_ON_NO_NAME). So other programs may accidentally get this pollution of git_default_name and show a username when we _could_ have shown the name from config. I can see the pollution in a debugger in "git commit", but I don't think you can actually trigger a commit with it, because later calls to fmt_ident use ERROR_ON_NO_NAME.

I really wonder if we can just get rid of all of the calls which do not use ERROR_ON_NO_NAME. As far as I can tell, they are all part of programs which later end up using ERROR_ON_NO_NAME anyway.

-Peff