Re: PATCH: Less fragile lookup of gpg key

[Grant Olson <kgo@grant-olson.net>]

[-- Attachment #1: Type: text/plain, Size: 5217 bytes --]

On 05/01/2010 03:54 PM, Junio C Hamano wrote:
> Grant Olson <kgo@grant-olson.net> writes:
>>
>> Unless I'm mis-understanding you, the does the opposite of that.  It
>> finds your gpg key based on your git email, ignoring your git name, so
>> that different spellings of the name between gpg and git become irrelevant.
> 
> If I have two keys like these:
> 
>     Junio C Hamano <gitster@pobox.com>
>     Junio Hamano <gitster@pobox.com>
> 
> and I have the latter set in .git/config to use for the project I am
> working on, your patch picks one at random, making the process less
> reliable.
> 
> AFAIU, ALASCM's suggestion was to first try the current method (which
> reliably picks what I told git to use by specifying user.name), and only
> if that fails, i.e. if I do not have neither of the above two keys but
> only have a key named like e.g.
> 
>     Git Panda <gitster@pobox.com>
> 
> then use only the e-mail as you wanted to, but do so purely as a
> fallback.
> 
> Which I found quite reasonable.

Fair enough.  This version of the patch will try to gpg sign by email
address only, if (and only if) you try to sign a tag without explicitly
providing a key id (-s) and the lookup by "user.name <user.email>" fails.

From 791a110dc4d362b2cd11b19ae25a86bf91710e34 Mon Sep 17 00:00:00 2001
From: Grant Olson <kgo@grant-olson.net>
Date: Sun, 2 May 2010 19:33:41 -0400
Subject: [PATCH] Lookup gpg key by email address if user+email lookup
fails with -s

---
 builtin/tag.c |   67
+++++++++++++++++++++++++++++++++++++++++++++------------
 cache.h       |    1 +
 ident.c       |    9 +++++++
 3 files changed, 63 insertions(+), 14 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index d311491..45dd43d 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -156,22 +156,13 @@ static int verify_tag(const char *name, const char
*ref,
 	return 0;
 }

-static int do_sign(struct strbuf *buffer)
+static int do_gpg(struct strbuf *buffer)
 {
+	/* retval can be standard -1 for error, 0 for ok, or 1 for a warning
+	 * so that we can attempt to recover by running gpg again. */
 	struct child_process gpg;
 	const char *args[4];
-	char *bracket;
 	int len;
-	int i, j;
-
-	if (!*signingkey) {
-		if (strlcpy(signingkey, git_committer_info(IDENT_ERROR_ON_NO_NAME),
-				sizeof(signingkey)) > sizeof(signingkey) - 1)
-			return error("committer info too long.");
-		bracket = strchr(signingkey, '>');
-		if (bracket)
-			bracket[1] = '\0';
-	}

 	/* When the username signingkey is bad, program could be terminated
 	 * because gpg exits without reading and then write gets SIGPIPE. */
@@ -199,8 +190,56 @@ static int do_sign(struct strbuf *buffer)
 	len = strbuf_read(buffer, gpg.out, 1024);
 	close(gpg.out);

-	if (finish_command(&gpg) || !len || len < 0)
-		return error("gpg failed to sign the tag");
+	if(finish_command(&gpg))
+		{
+			warning("gpg failed to sign the tag");
+			return 1;
+		}
+
+	return 0;
+}
+
+static int do_sign(struct strbuf *buffer)
+{
+	char *bracket;
+	int i, j;
+	int uid_for_key = 0;
+	int err_ok_warning;
+
+	if (!*signingkey) {
+		if (strlcpy(signingkey, git_committer_info(IDENT_ERROR_ON_NO_NAME),
+				sizeof(signingkey)) > sizeof(signingkey) - 1)
+			return error("committer info too long.");
+		bracket = strchr(signingkey, '>');
+		if (bracket)
+			bracket[1] = '\0';
+		uid_for_key = 1;
+	}
+
+	err_ok_warning = do_gpg(buffer);
+
+	if(!err_ok_warning || err_ok_warning != 1)
+		return -1;
+
+	if (err_ok_warning == 1 || !buffer->len || buffer->len < 0)
+	{
+		if (uid_for_key)
+		{
+			warning("couldn't find key for '%s'...", signingkey);
+			warning("Trying key lookup by email address only.");
+
+			if (strlcpy(signingkey, git_committer_email(),
+				sizeof(signingkey)) > sizeof(signingkey) - 1)
+					return error("committer info too long.");
+
+				err_ok_warning = do_gpg(buffer);
+
+			if (err_ok_warning || !buffer->len || buffer-> len < 0)
+				return error("gpg failed to sign the tag");
+		} else {
+			return error("gpg failed to sign the tag");
+		}
+	}

 	/* Strip CR from the line endings, in case we are on Windows. */
 	for (i = j = 0; i < buffer->len; i++)
diff --git a/cache.h b/cache.h
index 5eb0573..90a3067 100644
--- a/cache.h
+++ b/cache.h
@@ -789,6 +789,7 @@ enum date_mode parse_date_format(const char *format);
 #define IDENT_NO_DATE	       4
 extern const char *git_author_info(int);
 extern const char *git_committer_info(int);
+extern const char *git_committer_email();
 extern const char *fmt_ident(const char *name, const char *email, const
char *date_str, int);
 extern const char *fmt_name(const char *name, const char *email);
 extern const char *git_editor(void);
diff --git a/ident.c b/ident.c
index 9e24388..0e8b78a 100644
--- a/ident.c
+++ b/ident.c
@@ -260,6 +260,15 @@ const char *git_committer_info(int flag)
 			 flag);
 }

+const char *git_committer_email(void)
+{
+	const char *email = getenv("GIT_COMMITTER_EMAIL");
+	if(!email)
+		email = git_default_email;
+
+	return email;
+}
+
 int user_ident_sufficiently_given(void)
 {
 #ifndef WINDOWS
-- 
1.7.0.4


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 554 bytes --]