{"thread":{"id":"28555","subject":"[PATCH/RFC] ident: check /etc/mailname if email is unknown","startedAt":"2011-10-03T04:57:46Z","lastAt":"2011-10-06T18:48:23Z","messageCount":11,"participants":["Jonathan Nieder","Junio C Hamano","Johannes Sixt","Ian Jackson"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"176703","messageId":"20111003045745.GA17604@elie","threadId":"28555","inReplyTo":null,"subject":"[PATCH/RFC] ident: check /etc/mailname if email is unknown","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-10-03T04:57:46Z","receivedAt":"2011-10-03T04:57:46Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Gerrit Pape <pape@smarden.org>\n\nBefore falling back to gethostname(), check /etc/mailname if\nGIT_AUTHOR_EMAIL is not set in the environment or through config\nfiles.  Only fall back if /etc/mailname cannot be opened or read.\n\nThe /etc/mailname convention comes from Debian policy section 11.6\n(\"mail transport, delivery and user agents\"), though it seems more\nwidely useful.  The lack of this support was noticed by various people\nin different ways:\n\n - Ian observed that git was choosing the address\n   'ian@anarres.relativity.greenend.org.uk' rather than\n   'ian@davenant.greenend.org.uk' as it should have done.\n\n - Jonathan noticed that operations like \"git commit\" were needlessly\n   slow when the using a resolver that was slow to handle reverse DNS\n   lookups.\n\nOn the other hand, this means that if /etc/mailname is set up and the\n[user] name and email configuration aren't, the committer email will\nnot provide a charming reminder of which machine commits were made on\nany more.  I think that cost is worth it.\n\nRequested-by: Ian Jackson <ijackson@chiark.greenend.org.uk>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\nHi,\n\nDebian has been using something like this patch for years now (see\n<http://bugs.debian.org/448655>) but I thought it might be useful\nto others, too.  A similar patch visited the list three years ago:\n<http://thread.gmane.org/gmane.comp.version-control.git/51800>.\n\nThoughts of all kinds welcome, of course.\n\nThanks,\nJonathan\n\n Documentation/git-commit-tree.txt |    8 +++++++-\n ident.c                           |   23 +++++++++++++++++++++++\n 2 files changed, 30 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-commit-tree.txt b/Documentation/git-commit-tree.txt\nindex 0fdb82ee..02133d5f 100644\n--- a/Documentation/git-commit-tree.txt\n+++ b/Documentation/git-commit-tree.txt\n@@ -68,7 +68,9 @@ if set:\n \n In case (some of) these environment variables are not set, the information\n is taken from the configuration items user.name and user.email, or, if not\n-present, system user name and fully qualified hostname.\n+present, system user name and the hostname used for outgoing mail (taken\n+from `/etc/mailname` and falling back to the fully qualified hostname when\n+that file does not exist).\n \n A commit comment is read from stdin. If a changelog\n entry is not provided via \"<\" redirection, 'git commit-tree' will just wait\n@@ -90,6 +92,10 @@ Discussion\n \n include::i18n.txt[]\n \n+FILES\n+-----\n+/etc/mailname\n+\n SEE ALSO\n --------\n linkgit:git-write-tree[1]\ndiff --git a/ident.c b/ident.c\nindex 35a6f264..87f0a37e 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -52,6 +52,8 @@ static void copy_gecos(const struct passwd *w, char *name, size_t sz)\n \n static void copy_email(const struct passwd *pw)\n {\n+\tFILE *mailname;\n+\n \t/*\n \t * Make up a fake email address\n \t * (name + '@' + hostname [+ '.' + domainname])\n@@ -61,6 +63,27 @@ static void copy_email(const struct passwd *pw)\n \t\tdie(\"Your sysadmin must hate you!\");\n \tmemcpy(git_default_email, pw->pw_name, len);\n \tgit_default_email[len++] = '@';\n+\n+\t/*\n+\t * The domain part comes from /etc/mailname if it is readable,\n+\t * or the current hostname and domain name otherwise.\n+\t */\n+\tmailname = fopen(\"/etc/mailname\", \"r\");\n+\tif (!mailname) {\n+\t\tif (errno != ENOENT)\n+\t\t\twarning(\"cannot open /etc/mailname: %s\",\n+\t\t\t\tstrerror(errno));\n+\t} else if (fgets(git_default_email + len,\n+\t\t\t sizeof(git_default_email) - len, mailname)) {\n+\t\t/* success! */\n+\t\tfclose(mailname);\n+\t\treturn;\n+\t} else {\n+\t\tif (ferror(mailname))\n+\t\t\twarning(\"cannot read /etc/mailname: %s\",\n+\t\t\t\tstrerror(errno));\n+\t\tfclose(mailname);\n+\t}\n \tgethostname(git_default_email + len, sizeof(git_default_email) - len);\n \tif (!strchr(git_default_email+len, '.')) {\n \t\tstruct hostent *he = gethostbyname(git_default_email + len);\n-- \n1.7.7.rc1\n"},{"id":"176705","messageId":"7v8vp2iqvc.fsf@alter.siamese.dyndns.org","threadId":"28555","inReplyTo":"20111003045745.GA17604@elie","subject":"Re: [PATCH/RFC] ident: check /etc/mailname if email is unknown","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-03T05:30:31Z","receivedAt":"2011-10-03T05:30:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> @@ -52,6 +52,8 @@ static void copy_gecos(const struct passwd *w, char *name, size_t sz)\n>  \n>  static void copy_email(const struct passwd *pw)\n>  {\n> +\tFILE *mailname;\n> +\n>  \t/*\n>  \t * Make up a fake email address\n>  \t * (name + '@' + hostname [+ '.' + domainname])\n> @@ -61,6 +63,27 @@ static void copy_email(const struct passwd *pw)\n>  \t\tdie(\"Your sysadmin must hate you!\");\n>  \tmemcpy(git_default_email, pw->pw_name, len);\n>  \tgit_default_email[len++] = '@';\n> +\n> +\t/*\n> +\t * The domain part comes from /etc/mailname if it is readable,\n> +\t * or the current hostname and domain name otherwise.\n> +\t */\n> +\tmailname = fopen(\"/etc/mailname\", \"r\");\n> +\tif (!mailname) {\n> +\t\tif (errno != ENOENT)\n> +\t\t\twarning(\"cannot open /etc/mailname: %s\",\n> +\t\t\t\tstrerror(errno));\n> +\t} else if (fgets(git_default_email + len,\n> +\t\t\t sizeof(git_default_email) - len, mailname)) {\n> +\t\t/* success! */\n> +\t\tfclose(mailname);\n> +\t\treturn;\n> +\t} else {\n> +\t\tif (ferror(mailname))\n> +\t\t\twarning(\"cannot read /etc/mailname: %s\",\n> +\t\t\t\tstrerror(errno));\n> +\t\tfclose(mailname);\n> +\t}\n>  \tgethostname(git_default_email + len, sizeof(git_default_email) - len);\n>  \tif (!strchr(git_default_email+len, '.')) {\n>  \t\tstruct hostent *he = gethostbyname(git_default_email + len);\n\nI do not think this would hurt, even though I see /etc/mailname on only\none of my boxes (i.e. Debian). For maintainability for the future,\nhowever, I would prefer to see the above hunk separated into a helper\nfunction to keep addition to copy_email() to the minimum, e.g.\n\n\tmemcpy(git_default_email, pw->pw_name, len);\n        git_default_email[len++] = '@';\n\n+\tif (add_mailname_host(git_default_email, len, sizeof(git_default_email)))\n+\t\treturn; /* read from \"/etc/mailname\" (Debian) */\n\n\tgethostname(git_default_email + len, sizeof(git_default_email) - len);\n        ...\n\nSo that people who care about other distros can more easily add a single\nimplementation to a similar location without making copy_email() too long\nto lose clarity. The fallback default logic that does gethostname() might\nalso want to become a separate helper function as well.\n"},{"id":"176706","messageId":"20111003061633.GB17289@elie","threadId":"28555","inReplyTo":"7v8vp2iqvc.fsf@alter.siamese.dyndns.org","subject":"[PATCH v2] ident: check /etc/mailname if email is unknown","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-10-03T06:16:33Z","receivedAt":"2011-10-03T06:16:33Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Before falling back to gethostname(), check /etc/mailname if\nGIT_AUTHOR_EMAIL is not set in the environment or through config\nfiles.  Only fall back if /etc/mailname cannot be opened or read.\n\nThe /etc/mailname convention comes from Debian policy section 11.6\n(\"mail transport, delivery and user agents\"), though maybe it could be\nuseful sometimes on other machines, too.  The lack of this support was\nnoticed by various people in different ways:\n\n - Ian observed that git was choosing the address\n   'ian@anarres.relativity.greenend.org.uk' rather than\n   'ian@davenant.greenend.org.uk' as it should have done.\n\n - Jonathan noticed that operations like \"git commit\" were needlessly\n   slow when using a resolver that was slow to handle reverse DNS\n   lookups.\n\nAlas, after this patch, if /etc/mailname is set up and the [user] name\nand email configuration aren't, the committer email will not provide a\ncharming reminder of which machine commits were made on any more.  But\nI think it's worth it.\n\nMechanics: the functionality of reading mailname goes in its own\nfunction, so people who care about other distros can easily add an\nimplementation to a similar location without making copy_email() too\nlong and losing clarity.  While at it, we split out the fallback\ndefault logic that does gethostname(), too (rearranging it a little\nand adding a check for errors from gethostname while at it).\n\nBased on a patch by Gerrit Pape <pape@smarden.org>.\n\nRequested-by: Ian Jackson <ijackson@chiark.greenend.org.uk>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\nImproved-by: Junio C Hamano <gitster@pobox.com>\n---\nJunio C Hamano wrote:\n\n> I do not think this would hurt, even though I see /etc/mailname on only\n> one of my boxes (i.e. Debian). For maintainability for the future,\n> however, I would prefer to see the above hunk separated into a helper\n> function to keep addition to copy_email() to the minimum, e.g.\n\nMakes sense.  How about this?\n\n Documentation/git-commit-tree.txt |    8 ++++-\n ident.c                           |   66 +++++++++++++++++++++++++++++-------\n 2 files changed, 60 insertions(+), 14 deletions(-)\n\ndiff --git a/Documentation/git-commit-tree.txt b/Documentation/git-commit-tree.txt\nindex 0fdb82ee..02133d5f 100644\n--- a/Documentation/git-commit-tree.txt\n+++ b/Documentation/git-commit-tree.txt\n@@ -68,7 +68,9 @@ if set:\n \n In case (some of) these environment variables are not set, the information\n is taken from the configuration items user.name and user.email, or, if not\n-present, system user name and fully qualified hostname.\n+present, system user name and the hostname used for outgoing mail (taken\n+from `/etc/mailname` and falling back to the fully qualified hostname when\n+that file does not exist).\n \n A commit comment is read from stdin. If a changelog\n entry is not provided via \"<\" redirection, 'git commit-tree' will just wait\n@@ -90,6 +92,10 @@ Discussion\n \n include::i18n.txt[]\n \n+FILES\n+-----\n+/etc/mailname\n+\n SEE ALSO\n --------\n linkgit:git-write-tree[1]\ndiff --git a/ident.c b/ident.c\nindex 35a6f264..edb43144 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -50,6 +50,54 @@ static void copy_gecos(const struct passwd *w, char *name, size_t sz)\n \n }\n \n+static int add_mailname_host(char *buf, size_t len)\n+{\n+\tFILE *mailname;\n+\n+\tmailname = fopen(\"/etc/mailname\", \"r\");\n+\tif (!mailname) {\n+\t\tif (errno != ENOENT)\n+\t\t\twarning(\"cannot open /etc/mailname: %s\",\n+\t\t\t\tstrerror(errno));\n+\t\treturn -1;\n+\t}\n+\tif (!fgets(buf, len, mailname)) {\n+\t\tif (ferror(mailname))\n+\t\t\twarning(\"cannot read /etc/mailname: %s\",\n+\t\t\t\tstrerror(errno));\n+\t\tfclose(mailname);\n+\t\treturn -1;\n+\t}\n+\t/* success! */\n+\tfclose(mailname);\n+\treturn 0;\n+}\n+\n+static void add_domainname(char *buf, size_t len)\n+{\n+\tstruct hostent *he;\n+\tsize_t namelen;\n+\tconst char *domainname;\n+\n+\tif (gethostname(buf, len)) {\n+\t\twarning(\"cannot get host name: %s\", strerror(errno));\n+\t\tstrlcpy(buf, \"(none)\", len);\n+\t\treturn;\n+\t}\n+\tnamelen = strlen(buf);\n+\tif (memchr(buf, '.', namelen))\n+\t\treturn;\n+\n+\the = gethostbyname(buf);\n+\tbuf[namelen++] = '.';\n+\tbuf += namelen;\n+\tlen -= namelen;\n+\tif (he && (domainname = strchr(he->h_name, '.')))\n+\t\tstrlcpy(buf, domainname + 1, len);\n+\telse\n+\t\tstrlcpy(buf, \"(none)\", len);\n+}\n+\n static void copy_email(const struct passwd *pw)\n {\n \t/*\n@@ -61,20 +109,12 @@ static void copy_email(const struct passwd *pw)\n \t\tdie(\"Your sysadmin must hate you!\");\n \tmemcpy(git_default_email, pw->pw_name, len);\n \tgit_default_email[len++] = '@';\n-\tgethostname(git_default_email + len, sizeof(git_default_email) - len);\n-\tif (!strchr(git_default_email+len, '.')) {\n-\t\tstruct hostent *he = gethostbyname(git_default_email + len);\n-\t\tchar *domainname;\n \n-\t\tlen = strlen(git_default_email);\n-\t\tgit_default_email[len++] = '.';\n-\t\tif (he && (domainname = strchr(he->h_name, '.')))\n-\t\t\tstrlcpy(git_default_email + len, domainname + 1,\n-\t\t\t\tsizeof(git_default_email) - len);\n-\t\telse\n-\t\t\tstrlcpy(git_default_email + len, \"(none)\",\n-\t\t\t\tsizeof(git_default_email) - len);\n-\t}\n+\tif (!add_mailname_host(git_default_email + len,\n+\t\t\t\tsizeof(git_default_email) - len))\n+\t\treturn;\t/* read from \"/etc/mailname\" (Debian) */\n+\tadd_domainname(git_default_email + len,\n+\t\t\tsizeof(git_default_email) - len);\n }\n \n static void setup_ident(void)\n-- \n1.7.7.rc1\n"},{"id":"176709","messageId":"4E895FBD.8020904@viscovery.net","threadId":"28555","inReplyTo":"20111003061633.GB17289@elie","subject":"Re: [PATCH v2] ident: check /etc/mailname if email is unknown","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2011-10-03T07:09:49Z","receivedAt":"2011-10-03T07:09:49Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 10/3/2011 8:16, schrieb Jonathan Nieder:\n> +static int add_mailname_host(char *buf, size_t len)\n> +{\n> +\tFILE *mailname;\n> +\n> +\tmailname = fopen(\"/etc/mailname\", \"r\");\n> +\tif (!mailname) {\n> +\t\tif (errno != ENOENT)\n> +\t\t\twarning(\"cannot open /etc/mailname: %s\",\n> +\t\t\t\tstrerror(errno));\n\nThis warns on EACCES. Is that OK? (Just asking, I have no opinion.)\n\n-- Hannes\n"},{"id":"176718","messageId":"20111003074433.GD17289@elie","threadId":"28555","inReplyTo":"4E895FBD.8020904@viscovery.net","subject":"Re: [PATCH v2] ident: check /etc/mailname if email is unknown","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-10-03T07:44:34Z","receivedAt":"2011-10-03T07:44:34Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Johannes Sixt wrote:\n\n> This warns on EACCES. Is that OK? (Just asking, I have no opinion.)\n\nGood catch.  I was worried for a moment: could some command call\ncopy_email() in a loop, producing an annoyingly redundant stream of\nwarnings?\n\nLuckily setup_ident() checks if git_default_email has been set to save\nthe trouble of computing it again.  So the behavior is to warn exactly\nonce when using a command that uses an ident string, which is still\nmore often than ideal.  It would be better to only warn if the\npermissions are creating an actual problem, so the user can (1)\ncomplain to the sysadmin and then (2) set an email address in\n~/.gitconfig and move on.\n\nWe _could_ add parameters to setup_ident() to only warn if\n/etc/mailname was going to be used to produce the committer ident (or\nwhichever ident is checked first).  That would be confusing if\nGIT_COMMITTER_EMAIL is set and GIT_AUTHOR_EMAIL is not.\n\nIn the long term it would be nice to find a way to warn when the\nmailname we tried to retrieve was actually going to be used, but short\nof that, the least confusing behavior is to just not warn at all.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n ident.c |   10 +---------\n 1 files changed, 1 insertions(+), 9 deletions(-)\n\ndiff --git a/ident.c b/ident.c\nindex edb43144..6f5c885d 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -55,16 +55,9 @@ static int add_mailname_host(char *buf, size_t len)\n \tFILE *mailname;\n \n \tmailname = fopen(\"/etc/mailname\", \"r\");\n-\tif (!mailname) {\n-\t\tif (errno != ENOENT)\n-\t\t\twarning(\"cannot open /etc/mailname: %s\",\n-\t\t\t\tstrerror(errno));\n+\tif (!mailname)\n \t\treturn -1;\n-\t}\n \tif (!fgets(buf, len, mailname)) {\n-\t\tif (ferror(mailname))\n-\t\t\twarning(\"cannot read /etc/mailname: %s\",\n-\t\t\t\tstrerror(errno));\n \t\tfclose(mailname);\n \t\treturn -1;\n \t}\n@@ -80,7 +73,6 @@ static void add_domainname(char *buf, size_t len)\n \tconst char *domainname;\n \n \tif (gethostname(buf, len)) {\n-\t\twarning(\"cannot get host name: %s\", strerror(errno));\n \t\tstrlcpy(buf, \"(none)\", len);\n \t\treturn;\n \t}\n-- \n1.7.7.rc1\n"},{"id":"176738","messageId":"20105.40257.351258.425389@chiark.greenend.org.uk","threadId":"28555","inReplyTo":"4E895FBD.8020904@viscovery.net","subject":"Re: [PATCH v2] ident: check /etc/mailname if email is unknown","fromName":"Ian Jackson","fromEmail":"ijackson@chiark.greenend.org.uk","sentAt":"2011-10-03T11:32:17Z","receivedAt":"2011-10-03T11:32:17Z","isPatch":true,"sender":{"key":"ijackson@chiark.greenend.org.uk","avatar":null},"body":"Johannes Sixt writes (\"Re: [PATCH v2] ident: check /etc/mailname if email is unknown\"):\n> Am 10/3/2011 8:16, schrieb Jonathan Nieder:\n> > +static int add_mailname_host(char *buf, size_t len)\n> > +{\n> > +\tFILE *mailname;\n> > +\n> > +\tmailname = fopen(\"/etc/mailname\", \"r\");\n> > +\tif (!mailname) {\n> > +\t\tif (errno != ENOENT)\n> > +\t\t\twarning(\"cannot open /etc/mailname: %s\",\n> > +\t\t\t\tstrerror(errno));\n> \n> This warns on EACCES. Is that OK? (Just asking, I have no opinion.)\n\nI think that's correct.  Personally I'm a bit of an error handling\nfascist and I would have it crash on EACCES but that's probably a bit\nharsh.\n\nCertainly this file ought to be generally readable, if it exists.\n\nIan.\n"},{"id":"176774","messageId":"7vty7pga7y.fsf@alter.siamese.dyndns.org","threadId":"28555","inReplyTo":"20111003074433.GD17289@elie","subject":"Re: [PATCH v2] ident: check /etc/mailname if email is unknown","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-03T19:13:05Z","receivedAt":"2011-10-03T19:13:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> In the long term it would be nice to find a way to warn when the\n> mailname we tried to retrieve was actually going to be used, but short\n> of that, the least confusing behavior is to just not warn at all.\n\nI would think that if we widely advertise that we read from /etc/mailname\nif/when it exists, a user who wonders why it is not used would appreciate\nit if Git tells them why when it cannot use it.\n\nBut I agree that has to happen _after_ we flip the logic so that\nsetup_ident() does not trigger when the user gave us the necessary\ninformation.\n\nI wonder if that \"flipping the logic\" would be as simple as something like\nthis. It probably is about time for us to stop using the static array that\nis hardwired MAX_GITNAME bytes long and start using strbuf, so I didn't\nbother with strlcpy().\n\n\n ident.c |    4 ++++\n 1 files changed, 4 insertions(+), 0 deletions(-)\n\ndiff --git a/ident.c b/ident.c\nindex edb4314..9cc55f7 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -239,6 +239,10 @@ const char *fmt_ident(const char *name, const char *email,\n \tint warn_on_no_name = (flag & IDENT_WARN_ON_NO_NAME);\n \tint name_addr_only = (flag & IDENT_NO_DATE);\n \n+\tif (name && !git_default_name[0])\n+\t\tstrcpy(git_default_name, name);\n+\tif (email && !git_default_email[0])\n+\t\tstrcpy(git_default_email, email);\n \tsetup_ident();\n \tif (!name)\n \t\tname = git_default_name;\n"},{"id":"176776","messageId":"7vpqidga3q.fsf@alter.siamese.dyndns.org","threadId":"28555","inReplyTo":"20105.40257.351258.425389@chiark.greenend.org.uk","subject":"Re: [PATCH v2] ident: check /etc/mailname if email is unknown","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-03T19:15:37Z","receivedAt":"2011-10-03T19:15:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ian Jackson <ijackson@chiark.greenend.org.uk> writes:\n\n>> > +\tif (!mailname) {\n>> > +\t\tif (errno != ENOENT)\n>> > +\t\t\twarning(\"cannot open /etc/mailname: %s\",\n>> > +\t\t\t\tstrerror(errno));\n>> \n>> This warns on EACCES. Is that OK? (Just asking, I have no opinion.)\n>\n> I think that's correct.  Personally I'm a bit of an error handling\n> fascist and I would have it crash on EACCES but that's probably a bit\n> harsh.\n\nIt is not just harsh but is outright wrong if the platform is not Debian\nand the platform happens to use the file for other purposes that does not\nrequire normal users to be able to read the file.\n\nIt _might_ make sense to do\n\n        #ifndef DEBIAN\n        #define add_mailname_host(x,y) (-1)\n        #else\n        static int add_mailname_host(char *buf, size_t len)\n        {\n                ...\n        }\n        #endif\n"},{"id":"177062","messageId":"20111006171719.GB6946@elie","threadId":"28555","inReplyTo":"7vty7pga7y.fsf@alter.siamese.dyndns.org","subject":"[PATCH/RFC jn/ident-from-etc-mailname] ident: do not retrieve default ident when unnecessary","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-10-06T17:17:19Z","receivedAt":"2011-10-06T17:17:19Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Avoid a getpwuid() call (which contacts the network if the password\ndatabase is not local), read of /etc/mailname, gethostname() call, and\nreverse DNS lookup if the user has already chosen a name and email\nthrough configuration, the environment, or the command line.\n\nThis should slightly speed up commands like \"git commit\".  More\nimportantly, it improves error reporting when computation of the\ndefault ident string does not go smoothly.  For example, after\ndetecting a problem (e.g., \"warning: cannot open /etc/mailname:\nPermission denied\") in retrieving the default committer identity:\n\n\ttouch /etc/mailname;\t# as root\n\tchmod -r /etc/mailname;\t# as root\n\tgit commit -m 'test commit'\n\nyou can squelch the warning while waiting for your sysadmin to fix the\npermissions problem.\n\n\techo '[user] email = me@example.com' >>~/.gitconfig\n\nInspired-by: Johannes Sixt <j6t@kdgb.org>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\nJunio C Hamano wrote:\n\n> --- a/ident.c\n> +++ b/ident.c\n> @@ -239,6 +239,10 @@ const char *fmt_ident(const char *name, const char *email,\n>  \tint warn_on_no_name = (flag & IDENT_WARN_ON_NO_NAME);\n>  \tint name_addr_only = (flag & IDENT_NO_DATE);\n>  \n> +\tif (name && !git_default_name[0])\n> +\t\tstrcpy(git_default_name, name);\n> +\tif (email && !git_default_email[0])\n> +\t\tstrcpy(git_default_email, email);\n\nThat poisons the \"git_default_foo\" variables for the next fmt_ident\ncall.  But something similar should work.\n\n ident.c |   16 ++++++++--------\n 1 files changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/ident.c b/ident.c\nindex edb43144..f619619b 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -117,19 +117,21 @@ static void copy_email(const struct passwd *pw)\n \t\t\tsizeof(git_default_email) - len);\n }\n \n-static void setup_ident(void)\n+static void setup_ident(const char **name, const char **emailp)\n {\n \tstruct passwd *pw = NULL;\n \n \t/* Get the name (\"gecos\") */\n-\tif (!git_default_name[0]) {\n+\tif (!*name && !git_default_name[0]) {\n \t\tpw = getpwuid(getuid());\n \t\tif (!pw)\n \t\t\tdie(\"You don't exist. Go away!\");\n \t\tcopy_gecos(pw, git_default_name, sizeof(git_default_name));\n \t}\n+\tif (!*name)\n+\t\t*name = git_default_name;\n \n-\tif (!git_default_email[0]) {\n+\tif (!*emailp && !git_default_email[0]) {\n \t\tconst char *email = getenv(\"EMAIL\");\n \n \t\tif (email && email[0]) {\n@@ -144,6 +146,8 @@ static void setup_ident(void)\n \t\t\tcopy_email(pw);\n \t\t}\n \t}\n+\tif (!*emailp)\n+\t\t*emailp = git_default_email;\n \n \t/* And set the default date */\n \tif (!git_default_date[0])\n@@ -239,11 +243,7 @@ const char *fmt_ident(const char *name, const char *email,\n \tint warn_on_no_name = (flag & IDENT_WARN_ON_NO_NAME);\n \tint name_addr_only = (flag & IDENT_NO_DATE);\n \n-\tsetup_ident();\n-\tif (!name)\n-\t\tname = git_default_name;\n-\tif (!email)\n-\t\temail = git_default_email;\n+\tsetup_ident(&name, &email);\n \n \tif (!*name) {\n \t\tstruct passwd *pw;\n-- \n1.7.7.rc1\n"},{"id":"177081","messageId":"7vy5wyyod1.fsf@alter.siamese.dyndns.org","threadId":"28555","inReplyTo":"20111006171719.GB6946@elie","subject":"Re: [PATCH/RFC jn/ident-from-etc-mailname] ident: do not retrieve default ident when unnecessary","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-06T18:19:22Z","receivedAt":"2011-10-06T18:19:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Avoid a getpwuid() call (which contacts the network if the password\n> database is not local), read of /etc/mailname, gethostname() call, and\n> reverse DNS lookup if the user has already chosen a name and email\n> through configuration, the environment, or the command line.\n\nOh boy that is a hard to parse paragraph that took me three reads.\n\nIn any case, both the motivation of the patch and the change itself make\nsense to me. Thanks.\n"},{"id":"177091","messageId":"20111006184823.GB16942@elie","threadId":"28555","inReplyTo":"7vy5wyyod1.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC jn/ident-from-etc-mailname] ident: do not retrieve default ident when unnecessary","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-10-06T18:48:23Z","receivedAt":"2011-10-06T18:48:23Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n\n>> Avoid a getpwuid() call (which contacts the network if the password\n>> database is not local), read of /etc/mailname, gethostname() call, and\n>> reverse DNS lookup if the user has already chosen a name and email\n>> through configuration, the environment, or the command line.\n>\n> Oh boy that is a hard to parse paragraph that took me three reads.\n\nHere's a possible replacement:\n\n Avoid looking up the current user's password database entry (which\n might be on another machine) and the current machine's domain name for\n outgoing mail (which can involve a reverse DNS lookup) when the\n environment, configuration, or command line specifies a name and email\n that would override them.\n\nOr it might make sense to drop that paragraph altogether, since the\nsubject line already says as much.\n\nThanks for looking it over.\n\nSleepily,\nJonathan\n"}]}