{"thread":{"id":"33301","subject":"[PATCH v2] log: Read gpg settings for signed commit verification","startedAt":"2013-03-27T15:13:39Z","lastAt":"2013-03-27T17:02:49Z","messageCount":4,"participants":["Hans Brigman","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"212383","messageId":"8C726954D36902459248B0627BF2E66F45D70C3E4E@AUSP01VMBX10.collaborationhost.net","threadId":"33301","inReplyTo":null,"subject":"[PATCH v2] log: Read gpg settings for signed commit verification","fromName":"Hans Brigman","fromEmail":"hbrigman@openspan.com","sentAt":"2013-03-27T15:13:39Z","receivedAt":"2013-03-27T15:13:39Z","isPatch":true,"sender":{"key":"hbrigman@openspan.com","avatar":null},"body":"From: Jacob Sarvis\n\nlog: Read gpg settings for signed commit verification\n\nCommit signature verification fails when alternative gpg.program\nsigns the commit, but gpg attempts to verify the signature.\n\"show --show-signature\" and \"log --show-signature\" do not read\nthe gpg.program setting from git config.\nCommit signing, tag signing, and tag verification use this setting\nproperly.\n\nMake log and show commands pass through settings to gpg interface.\n\nSigned-off-by: Hans Brigman \n---\n builtin/log.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 8f0b2e8..31f5a9e 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -23,6 +23,7 @@\n #include \"streaming.h\"\n #include \"version.h\"\n #include \"mailmap.h\"\n+#include \"gpg-interface.h\"\n \n /* Set a default date-time format for git log (\"log.date\" config variable) */\n static const char *default_date_mode = NULL;\n@@ -364,7 +365,8 @@ static int git_log_config(const char *var, const char *value, void *cb)\n \t\tuse_mailmap_config = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n-\n+\tif (git_gpg_config(var, value, cb) < 0)\n+\t\treturn -1;\n \tif (grep_config(var, value, cb) < 0)\n \t\treturn -1;\n \treturn git_diff_ui_config(var, value, cb);\n-- \n1.7.11.msysgit.0\n\n\nOn Mon, Mar 25, 2013 at 01:03:52PM -0500, Hans Brigman wrote:\n\n> \"show --show-signature\" doesn't currently use the gpg.program setting. Commit signing, tag signing, and tag verification currently use this setting properly, so the logic has been added to handle it here as well.\n\nPlease wrap your commit messages at something reasonable (70 is probably as high as you want to go, given that log output is often shown indented).\n\n> @@ -364,7 +365,8 @@ static int git_log_config(const char *var, const char *value, void *cb)\n> \t\tuse_mailmap_config = git_config_bool(var, value);\n> \t\treturn 0;\n> \t}\n> -\n> +\tif (!prefixcmp(var, \"gpg.\"))\n> +\t\treturn git_gpg_config(var, value, NULL);\n> \tif (grep_config(var, value, cb) < 0)\n> \t\treturn -1;\n\nThe gpg config can also be other places than \"gpg.*\". Right now it is just user.signingkey, which log would not care about, but it feels like we are depending an unnecessary detail here. We also don't know whether it would care about the callback data. Is there a reason not to do:\n\n if (git_gpg_config(var, value, cb) < 0)\n return -1;\n\njust like the grep_config call below?\n\n-Peff\n\n\nFrom 960175cb3326a12e038805121edbdc8119fbb104 Mon Sep 17 00:00:00 2001\nFrom: Jacob Sarvis \nDate: Fri, 22 Mar 2013 12:54:32 -0400\nSubject: [PATCH 1/2] log: Read gpg settings for signed commit verification\n\nCommit signature verification fails when alternative gpg.program\nsigns the commit, but gpg attempts to verify the signature.\n\"show --show-signature\" and \"log --show-signature\" do not read\nthe gpg.program setting from git config.\nCommit signing, tag signing, and tag verification use this setting\nproperly.\n\nMake log and show commands pass through settings to gpg interface.\n\nSigned-off-by: Jacob Sarvis \n---\n builtin/log.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 8f0b2e8..31f5a9e 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -23,6 +23,7 @@\n #include \"streaming.h\"\n #include \"version.h\"\n #include \"mailmap.h\"\n+#include \"gpg-interface.h\"\n \n /* Set a default date-time format for git log (\"log.date\" config variable) */\n static const char *default_date_mode = NULL;\n@@ -364,7 +365,8 @@ static int git_log_config(const char *var, const char *value, void *cb)\n \t\tuse_mailmap_config = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n-\n+\tif (git_gpg_config(var, value, cb) < 0)\n+\t\treturn -1;\n \tif (grep_config(var, value, cb) < 0)\n \t\treturn -1;\n \treturn git_diff_ui_config(var, value, cb);\n-- \n1.7.11.msysgit.0\n\n"},{"id":"212393","messageId":"7vip4com2p.fsf@alter.siamese.dyndns.org","threadId":"33301","inReplyTo":"8C726954D36902459248B0627BF2E66F45D70C3E4E@AUSP01VMBX10.collaborationhost.net","subject":"Re: [PATCH v2] log: Read gpg settings for signed commit verification","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-27T16:15:58Z","receivedAt":"2013-03-27T16:15:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Hans Brigman writes:\n\n> Content-Type: multipart/mixed; boundary=\"_002_8C726954D36902459248B0627BF2E66F45D70C3E4EAUSP01VMBX10c_\"\n\nNo multipart/anything please. We prefer to see text/plain.\n\nIn general, please follow Documentation/SubmittingPatches.\n\n> From: Jacob Sarvis\n\nMissing SP between name and e-mail.\n\n>\n> log: Read gpg settings for signed commit verification\n\nThat's the same as the e-mail subject; drop it.\n\n>\n> Commit signature verification fails when alternative gpg.program\n> signs the commit, but gpg attempts to verify the signature.\n> \"show --show-signature\" and \"log --show-signature\" do not read\n> the gpg.program setting from git config.\n> Commit signing, tag signing, and tag verification use this setting\n> properly.\n>\n> Make log and show commands pass through settings to gpg interface.\n>\n> Signed-off-by: Hans Brigman \n\nNeeds the author's Sign-off before yours.\n\n> ---\n> builtin/log.c | 4 +++-\n> 1 file changed, 3 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/log.c b/builtin/log.c\n> index 8f0b2e8..31f5a9e 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -23,6 +23,7 @@\n> #include \"streaming.h\"\n> #include \"version.h\"\n> #include \"mailmap.h\"\n> +#include \"gpg-interface.h\"\n> \n> /* Set a default date-time format for git log (\"log.date\" config variable) */\n> static const char *default_date_mode = NULL;\n> @@ -364,7 +365,8 @@ static int git_log_config(const char *var, const char *value, void *cb)\n> \t\tuse_mailmap_config = git_config_bool(var, value);\n> \t\treturn 0;\n> \t}\n> -\n> +\tif (git_gpg_config(var, value, cb) < 0)\n> +\t\treturn -1;\n> \tif (grep_config(var, value, cb) < 0)\n> \t\treturn -1;\n\nHmph. I do not particularly like the way the call to grep_config()\nloses information by not ignoring its return value and always\nreturning -1, but I'll let it pass for this patch.\n\n> \treturn git_diff_ui_config(var, value, cb);\n"},{"id":"212396","messageId":"20130327162232.GB22386@sigill.intra.peff.net","threadId":"33301","inReplyTo":"7vip4com2p.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] log: Read gpg settings for signed commit verification","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-27T16:22:32Z","receivedAt":"2013-03-27T16:22:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 27, 2013 at 09:15:58AM -0700, Junio C Hamano wrote:\n\n> > \t}\n> > -\n> > +\tif (git_gpg_config(var, value, cb) < 0)\n> > +\t\treturn -1;\n> > \tif (grep_config(var, value, cb) < 0)\n> > \t\treturn -1;\n> \n> Hmph. I do not particularly like the way the call to grep_config()\n> loses information by not ignoring its return value and always\n> returning -1, but I'll let it pass for this patch.\n\nThat's my fault for suggesting he follow the same style as grep here.\nBut I wonder if it is worth the effort. We have never cared about\nanything beyond \"was there an error\" in our config callbacks, and the\nvalue returned from the callbacks is lost in git_parse_file (i.e., we do\nnot propagate the actual return value, but only check that\n\"callback(var, value, data) < 0\", and die if so).\n\nExisting callbacks pass data out by writing into a struct pointed to by\nthe data pointer, which is more flexible, anyway.\n\nSo unless you want to overhaul the whole config system to propagate\nreturn codes back to the caller, I do not think there is any point in\nworrying about it.\n\n-Peff\n"},{"id":"212404","messageId":"7vwqssn5c6.fsf@alter.siamese.dyndns.org","threadId":"33301","inReplyTo":"20130327162232.GB22386@sigill.intra.peff.net","subject":"Re: [PATCH v2] log: Read gpg settings for signed commit verification","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-27T17:02:49Z","receivedAt":"2013-03-27T17:02:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, Mar 27, 2013 at 09:15:58AM -0700, Junio C Hamano wrote:\n>\n>> > \t}\n>> > -\n>> > +\tif (git_gpg_config(var, value, cb) < 0)\n>> > +\t\treturn -1;\n>> > \tif (grep_config(var, value, cb) < 0)\n>> > \t\treturn -1;\n>> \n>> Hmph. I do not particularly like the way the call to grep_config()\n>> loses information by not ignoring its return value and always\n>> returning -1, but I'll let it pass for this patch.\n>\n> That's my fault for suggesting he follow the same style as grep here.\n> But I wonder if it is worth the effort. We have never cared about\n> anything beyond \"was there an error\" in our config callbacks, and the\n> value returned from the callbacks is lost in git_parse_file (i.e., we do\n> not propagate the actual return value, but only check that\n> \"callback(var, value, data) < 0\", and die if so).\n>\n> Existing callbacks pass data out by writing into a struct pointed to by\n> the data pointer, which is more flexible, anyway.\n>\n> So unless you want to overhaul the whole config system to propagate\n> return codes back to the caller, I do not think there is any point in\n> worrying about it.\n\nYeah, that is where my conclusion in the message you are responding\ncomes from ;-)\n"}]}