git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Angus Hammond <angusgh@gmail.com>, git@vger.kernel.org
Subject: [PATCH 0/13] ident cleanups and bugfixes
Date: Fri, 18 May 2012 19:05:28 -0400	[thread overview]
Message-ID: <20120518230528.GA30510@sigill.intra.peff.net> (raw)
In-Reply-To: <7vsjf1e2n7.fsf@alter.siamese.dyndns.org>

On Tue, May 15, 2012 at 11:10:36AM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> 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

  reply	other threads:[~2012-05-18 23:21 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-10 19:06 [PATCH 1/2] Change error messages in ident.c Make error messages caused by failed reads of the /etc/passwd file easier to understand. Signed-off-by: Angus Hammond <angusgh@gmail.com> Angus Hammond
2012-05-10 19:06 ` [PATCH 2/2] Remove diagnostics section from commit-tree and var man pages New error messages shouldn't need explaining like the old ones did so just delete the diagnostics section of the man pages. " Angus Hammond
2012-05-10 19:21   ` Angus Hammond
2012-05-10 19:23 ` [PATCH 1/2] Change error messages in ident.c Jeff King
2012-05-10 19:56   ` Jeff King
2012-05-11 22:53     ` Junio C Hamano
2012-05-11 23:13       ` Jeff King
2012-05-14 16:28         ` [PATCH 1/2] drop length limitations on gecos-derived names and emails Jeff King
2012-05-14 17:05           ` Jeff King
2012-05-14 21:02           ` Jeff King
2012-05-14 21:13             ` Jeff King
2012-05-15  1:54               ` Jeff King
2012-05-15  2:32                 ` Jeff King
2012-05-15 15:03                 ` Junio C Hamano
2012-05-15 17:47                   ` Jeff King
2012-05-15 18:10                     ` Junio C Hamano
2012-05-18 23:05                       ` Jeff King [this message]
2012-05-18 23:07                         ` [PATCH 01/13] ident: split setup_ident into separate functions Jeff King
2012-05-18 23:09                         ` [PATCH 02/13] http-push: do not access git_default_email directly Jeff King
2012-05-18 23:10                         ` [PATCH 03/13] fmt-merge-msg: don't use static buffer in record_person Jeff King
2012-05-18 23:11                         ` [PATCH 04/13] move identity config parsing to ident.c Jeff King
2012-05-18 23:11                         ` [PATCH 05/13] move git_default_* variables " Jeff King
2012-05-21  4:07                           ` Junio C Hamano
2012-05-21  5:41                             ` Jeff King
2012-05-21  6:41                               ` Jeff King
2012-05-18 23:13                         ` [PATCH 06/13] format-patch: use default email for generating message ids Jeff King
2012-05-21  2:58                           ` Junio C Hamano
2012-05-21  6:36                             ` Jeff King
2012-05-18 23:14                         ` [PATCH 07/13] fmt_ident: drop IDENT_WARN_ON_NO_NAME code Jeff King
2012-05-18 23:19                         ` [PATCH 08/13] ident: don't write fallback username into git_default_name Jeff King
2012-05-21  2:54                           ` Junio C Hamano
2012-05-21  6:31                             ` Jeff King
2012-05-21  9:11                               ` Junio C Hamano
2012-05-21 23:09                                 ` [PATCHv2 0/15] ident cleanups git_default_name Jeff King
2012-05-21 23:09                                   ` [PATCHv2 01/15] ident: split setup_ident into separate functions Jeff King
2012-05-21 23:09                                   ` [PATCHv2 02/15] http-push: do not access git_default_email directly Jeff King
2012-05-21 23:09                                   ` [PATCHv2 03/15] fmt-merge-msg: don't use static buffer in record_person Jeff King
2012-05-21 23:09                                   ` [PATCHv2 04/15] move identity config parsing to ident.c Jeff King
2012-05-21 23:09                                   ` [PATCHv2 05/15] move git_default_* variables " Jeff King
2012-05-21 23:10                                   ` [PATCHv2 06/15] ident: trim trailing newline from /etc/mailname Jeff King
2012-05-21 23:10                                   ` [PATCHv2 07/15] format-patch: use default email for generating message ids Jeff King
2012-05-21 23:10                                   ` [PATCHv2 08/15] fmt_ident: drop IDENT_WARN_ON_NO_NAME code Jeff King
2012-05-21 23:10                                   ` [PATCHv2 09/15] ident: don't write fallback username into git_default_name Jeff King
2012-05-21 23:10                                   ` [PATCHv2 10/15] drop length limitations on gecos-derived names and emails Jeff King
2013-01-24 23:21                                     ` [regression] " Jonathan Nieder
2013-01-25  1:05                                       ` Jeff King
2013-01-25 18:46                                         ` Junio C Hamano
2013-01-25 22:10                                           ` Jeff King
2012-05-21 23:10                                   ` [PATCHv2 11/15] ident: report passwd errors with a more friendly message Jeff King
2012-05-21 23:10                                   ` [PATCHv2 12/15] ident: use full dns names to generate email addresses Jeff King
2012-05-21 23:10                                   ` [PATCHv2 13/15] ident: use a dynamic strbuf in fmt_ident Jeff King
2012-05-21 23:10                                   ` [PATCHv2 14/15] ident: trim whitespace from default name/email Jeff King
2012-05-22 16:55                                     ` Junio C Hamano
2012-05-22 17:12                                       ` Jeff King
2012-05-22 17:21                                         ` Junio C Hamano
2012-05-21 23:10                                   ` [PATCHv2 15/15] format-patch: refactor get_patch_filename Jeff King
2012-05-18 23:20                         ` [PATCH 09/13] drop length limitations on gecos-derived names and emails Jeff King
2012-05-18 23:21                         ` [PATCH 10/13] ident: report passwd errors with a more friendly message Jeff King
2012-05-18 23:22                         ` [PATCH 11/13] ident: use full dns names to generate email addresses Jeff King
2012-05-18 23:23                         ` [PATCH 12/13] ident: use a dynamic strbuf in fmt_ident Jeff King
2012-05-18 23:24                         ` [PATCH 13/13] format-patch: refactor get_patch_filename Jeff King
2012-05-14 16:36         ` [PATCH 2/2] ident: report passwd errors with a more friendly message Jeff King
2012-05-10 20:04   ` [PATCH 1/2] Change error messages in ident.c Junio C Hamano
2012-05-10 20:22     ` Jeff King
2012-05-10 20:28       ` Junio C Hamano
2012-05-10 19:43 ` [PATCH 1/2] Change error messages in ident.c Make error messages caused by failed reads of the /etc/passwd file easier to understand. Signed-off-by: Angus Hammond <angusgh@gmail.com> Junio C Hamano
2012-05-10 19:57   ` Angus Hammond
2012-05-11 11:35 ` Nguyen Thai Ngoc Duy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120518230528.GA30510@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=angusgh@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).