On Tue, May 15, 2012 at 11:10:36AM -0700, Junio C Hamano wrote: > Jeff King <peff@xxxxxxxx> writes: > > > So it seems to me like a much simpler set of rules would be: > > > > 1. When reading gecos, always fall back to the username if the gecos > > field is unavailable or blank. > > > > 2. Always die when the name field is blank. That means we will die > > when you pass in a bogus empty GIT_COMMITTER_NAME (or an empty > > config name), which makes a lot more sense to me than falling back; > > those are bogus requests, not system config problems. And we won't > > ever have a blank gecos name, because we'll always fall back on the > > username. > > That certainly sounds very simple to explain and understand, and I do not > offhand think of anything *sane* that would break ;-) Actually, it does end up breaking. The "error_on_no_name" check wants to see the value _before_ the fallback, because it is not just about "do not hand out an ident with an empty name", but rather about "do not hand out this crappy fallback name when the gecos field is empty". So we cannot do an "always fallback" behavior without breaking that check. I ended up avoiding the issue in a different way, as you'll see in patch 8 below. Here's the series: [01/13]: ident: split setup_ident into separate functions [02/13]: http-push: do not access git_default_email directly [03/13]: fmt-merge-msg: don't use static buffer in record_person [04/13]: move identity config parsing to ident.c [05/13]: move git_default_* variables to ident.c [06/13]: format-patch: use default email for generating message ids [07/13]: fmt_ident: drop IDENT_WARN_ON_NO_NAME code [08/13]: ident: don't write fallback username into git_default_name [09/13]: drop length limitations on gecos-derived names and emails [10/13]: ident: report passwd errors with a more friendly message [11/13]: ident: use full dns names to generate email addresses [12/13]: ident: use a dynamic strbuf in fmt_ident [13/13]: format-patch: refactor get_patch_filename Patches 2, 8, and 11 fix actual bugs. The rest of it is refactoring and cleanup; here's the diffstat: Documentation/git-commit-tree.txt | 9 -- Documentation/git-var.txt | 9 -- builtin/fmt-merge-msg.c | 8 +- builtin/log.c | 45 ++------ cache.h | 12 +- config.c | 24 +---- environment.c | 3 - git-compat-util.h | 3 + http-push.c | 2 +- ident.c | 213 ++++++++++++++++--------------------- log-tree.c | 19 ++-- log-tree.h | 4 +- wrapper.c | 12 ++ 13 files changed, 142 insertions(+), 221 deletions(-) -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html