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: Taylor Braun-Jones <taylor@braun-jones.org>,
	Duy Nguyen <pclouds@gmail.com>,
	Git Mailing List <git@vger.kernel.org>
Subject: [PATCH 3/3] ident: loosen getpwuid error in non-strict mode
Date: Thu, 10 Dec 2015 16:41:29 -0500	[thread overview]
Message-ID: <20151210214129.GC8374@sigill.intra.peff.net> (raw)
In-Reply-To: <20151210213228.GB29055@sigill.intra.peff.net>

If the user has not specified an identity and we have to
turn to getpwuid() to find the username or gecos field, we
die immediately when getpwuid fails (e.g., because the user
does not exist). This is OK for making a commit, where we
have set IDENT_STRICT and would want to bail on bogus input.

But for something like a reflog, where the ident is "best
effort", it can be pain. For instance, even running "git
clone" with a UID that is not in /etc/passwd will result in
git barfing, just because we can't find an ident to put in
the reflog.

Instead of dying in xgetpwuid_self, we can instead return a
fallback value, and set a "bogus" flag. For the username in
an email, we already have a "default_email_is_bogus" flag.
For the name field, we introduce (and check) a matching
"default_name_is_bogus" flag. As a bonus, this means you now
get the usual "tell me who you are" advice instead of just a
"no such user" error.

No tests, as this is dependent on configuration outside of
git's control. However, I did confirm that it behaves
sensibly when I delete myself from the local /etc/passwd
(reflogs get written, and commits complain).

Signed-off-by: Jeff King <peff@peff.net>
---
I dislike #ifdefs in the middle of functions like the one below, but I
couldn't come up with an elegant way to avoid it. We're reusing "struct
passwd" for our fallback, so we need to know whether we have that field
to set or not.

An obvious alternative would be to always return our own

  struct git_passwd_struct {
	const char *name;
	const char *gecos;
  };

and translate the existing getpwuid() on the fly. That still needs an
#ifdef, but we could then get rid of the #ifdef for the get_gecos()
macro.

Yet another alternative is to simply zero the fallback passwd struct,
and pw->pw_gecos will either not exist, or will be NULL. But then the
callers have to do something sane with the NULL, and the whole point of
this exercise was to avoid callers having to deal with the error cases
directly, so...

 ident.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/ident.c b/ident.c
index 085cfbe..2d5b876 100644
--- a/ident.c
+++ b/ident.c
@@ -11,6 +11,7 @@ static struct strbuf git_default_name = STRBUF_INIT;
 static struct strbuf git_default_email = STRBUF_INIT;
 static struct strbuf git_default_date = STRBUF_INIT;
 static int default_email_is_bogus;
+static int default_name_is_bogus;
 
 #define IDENT_NAME_GIVEN 01
 #define IDENT_MAIL_GIVEN 02
@@ -24,15 +25,22 @@ static int author_ident_explicitly_given;
 #define get_gecos(struct_passwd) ((struct_passwd)->pw_gecos)
 #endif
 
-static struct passwd *xgetpwuid_self(void)
+static struct passwd *xgetpwuid_self(int *is_bogus)
 {
 	struct passwd *pw;
 
 	errno = 0;
 	pw = getpwuid(getuid());
-	if (!pw)
-		die(_("unable to look up current user in the passwd file: %s"),
-		    errno ? strerror(errno) : _("no such user"));
+	if (!pw) {
+		struct passwd fallback;
+		fallback.pw_name = "unknown";
+#ifndef NO_GECOS_IN_PWENT
+		fallback.pw_gecos = "Unknown";
+#endif
+		pw = &fallback;
+		if (is_bogus)
+			*is_bogus = 1;
+	}
 	return pw;
 }
 
@@ -122,7 +130,7 @@ static void copy_email(const struct passwd *pw, struct strbuf *email,
 const char *ident_default_name(void)
 {
 	if (!git_default_name.len) {
-		copy_gecos(xgetpwuid_self(), &git_default_name);
+		copy_gecos(xgetpwuid_self(&default_name_is_bogus), &git_default_name);
 		strbuf_trim(&git_default_name);
 	}
 	return git_default_name.buf;
@@ -138,8 +146,8 @@ const char *ident_default_email(void)
 			committer_ident_explicitly_given |= IDENT_MAIL_GIVEN;
 			author_ident_explicitly_given |= IDENT_MAIL_GIVEN;
 		} else
-			copy_email(xgetpwuid_self(), &git_default_email,
-				   &default_email_is_bogus);
+			copy_email(xgetpwuid_self(&default_email_is_bogus),
+				   &git_default_email, &default_email_is_bogus);
 		strbuf_trim(&git_default_email);
 	}
 	return git_default_email.buf;
@@ -327,10 +335,16 @@ const char *fmt_ident(const char *name, const char *email,
 				fputs(env_hint, stderr);
 			die("empty ident name (for <%s>) not allowed", email);
 		}
-		pw = xgetpwuid_self();
+		pw = xgetpwuid_self(NULL);
 		name = pw->pw_name;
 	}
 
+	if (want_name && strict &&
+	    name == git_default_name.buf && default_name_is_bogus) {
+		fputs(env_hint, stderr);
+		die("unable to auto-detect name (got '%s')", name);
+	}
+
 	if (strict && email == git_default_email.buf && default_email_is_bogus) {
 		fputs(env_hint, stderr);
 		die("unable to auto-detect email address (got '%s')", email);
-- 
2.6.3.636.g1460207

  parent reply	other threads:[~2015-12-10 21:41 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-02 20:10 git-clone fails when current user is not in /etc/passwd Taylor Braun-Jones
2015-12-09 15:23 ` Taylor Braun-Jones
2015-12-09 16:08 ` Duy Nguyen
2015-12-09 18:24   ` Duy Nguyen
2015-12-09 22:35     ` Taylor Braun-Jones
2015-12-10 18:33       ` Taylor Braun-Jones
2015-12-10 18:34       ` Jeff King
2015-12-10 19:57         ` Junio C Hamano
2015-12-10 20:40           ` Jeff King
2015-12-10 21:32             ` [PATCH 0/3] " Jeff King
2015-12-10 21:33               ` [PATCH 1/3] ident: make xgetpwuid_self() a static local helper Jeff King
2015-12-10 23:39                 ` Junio C Hamano
2015-12-10 21:35               ` [PATCH 2/3] ident: keep a flag for bogus default_email Jeff King
2015-12-10 22:54                 ` Jeff King
2015-12-10 21:41               ` Jeff King [this message]
2015-12-14 15:07                 ` [PATCH 3/3] ident: loosen getpwuid error in non-strict mode Jeff King
2015-12-10 23:44               ` [PATCH 0/3] git-clone fails when current user is not in /etc/passwd Junio C Hamano
2015-12-11  2:20               ` Taylor Braun-Jones
2015-12-10 18:43     ` Junio C Hamano
2015-12-10 18:52       ` Taylor Braun-Jones

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=20151210214129.GC8374@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    --cc=taylor@braun-jones.org \
    /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).