Re: [PATCH v2] ident: check /etc/mailname if email is unknown

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Oct 3, 2011, 07:44 UTC
Message-ID: <20111003074433.GD17289@elie>
In-Reply-To: <4E895FBD.8020904@viscovery.net>

Johannes Sixt wrote:

> This warns on EACCES. Is that OK? (Just asking, I have no opinion.)

Good catch. I was worried for a moment: could some command call copy_email() in a loop, producing an annoyingly redundant stream of warnings?

Luckily setup_ident() checks if git_default_email has been set to save the trouble of computing it again. So the behavior is to warn exactly once when using a command that uses an ident string, which is still more often than ideal. It would be better to only warn if the permissions are creating an actual problem, so the user can (1) complain to the sysadmin and then (2) set an email address in ~/.gitconfig and move on.

We _could_ add parameters to setup_ident() to only warn if /etc/mailname was going to be used to produce the committer ident (or whichever ident is checked first). That would be confusing if GIT_COMMITTER_EMAIL is set and GIT_AUTHOR_EMAIL is not.

In the long term it would be nice to find a way to warn when the mailname we tried to retrieve was actually going to be used, but short of that, the least confusing behavior is to just not warn at all.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 ident.c |   10 +---------
 1 files changed, 1 insertions(+), 9 deletions(-)

diff --git a/ident.c b/ident.c
index edb43144..6f5c885d 100644
--- a/ident.c
+++ b/ident.c
@@ -55,16 +55,9 @@ static int add_mailname_host(char *buf, size_t len)
 	FILE *mailname;
 
 	mailname = fopen("/etc/mailname", "r");
-	if (!mailname) {
-		if (errno != ENOENT)
-			warning("cannot open /etc/mailname: %s",
-				strerror(errno));
+	if (!mailname)
 		return -1;
-	}
 	if (!fgets(buf, len, mailname)) {
-		if (ferror(mailname))
-			warning("cannot read /etc/mailname: %s",
-				strerror(errno));
 		fclose(mailname);
 		return -1;
 	}
@@ -80,7 +73,6 @@ static void add_domainname(char *buf, size_t len)
 	const char *domainname;
 
 	if (gethostname(buf, len)) {
-		warning("cannot get host name: %s", strerror(errno));
 		strlcpy(buf, "(none)", len);
 		return;
 	}

-- 
1.7.7.rc1