{"thread":{"id":"11234","subject":"\"git tag -u keyname\" broken","startedAt":"2007-12-11T04:08:06Z","lastAt":"2007-12-11T05:22:57Z","messageCount":4,"participants":["Linus Torvalds","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"62645","messageId":"alpine.LFD.0.9999.0712101950110.28293@woody.linux-foundation.org","threadId":"11234","inReplyTo":null,"subject":"\"git tag -u keyname\" broken","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-12-11T04:08:06Z","receivedAt":"2007-12-11T04:08:06Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nCommit 396865859918e9c7bf8ce74aae137c57da134610 broke signed tags using \nthe \"-u\" flag when it made builtin-tag.c use parse_options() to parse its\narguments (but it quite possibly was broken even before that, by the \nbuiltin rewrite).\n\nIt used to be that passing the signing ID with the -u parameter also \n(obviously!) implied that you wanted to sign and annotate the tag, but \nthat logic got dropped. It also totally ignored the actual key ID that was \npassed in.\n\nThis reinstates it all.\n\nSigned-off-by: Linus Torvalds \n---\n\nThis has gotten only very minimal testing. Somebody should double-check it \nall, and adding a test would probably be great too.\n\n builtin-tag.c | 14 +++++++++++---\n 1 files changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-tag.c b/builtin-tag.c\nindex 517419f..274901a 100644\n--- a/builtin-tag.c\n+++ b/builtin-tag.c\n@@ -236,14 +236,18 @@ static const char tag_template[] =\n \t\"# Write a tag message\\n\"\n \t\"#\\n\";\n \n+static void set_signingkey(const char *value)\n+{\n+\tif (strlcpy(signingkey, value, sizeof(signingkey)) >= sizeof(signingkey))\n+\t\tdie(\"signing key value too long (%.10s...)\", value);\n+}\n+\n static int git_tag_config(const char *var, const char *value)\n {\n \tif (!strcmp(var, \"user.signingkey\")) {\n \t\tif (!value)\n \t\t\tdie(\"user.signingkey without value\");\n-\t\tif (strlcpy(signingkey, value, sizeof(signingkey))\n-\t\t\t\t\t\t>= sizeof(signingkey))\n-\t\t\tdie(\"user.signingkey value too long\");\n+\t\tset_signingkey(value);\n \t\treturn 0;\n \t}\n \n@@ -396,6 +400,10 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \n \targc = parse_options(argc, argv, options, git_tag_usage, 0);\n \n+\tif (keyid) {\n+\t\tsign = 1;\n+\t\tset_signingkey(keyid);\n+\t}\n \tif (sign)\n \t\tannotate = 1;\n \n"},{"id":"62646","messageId":"20071211042436.GA23805@coredump.intra.peff.net","threadId":"11234","inReplyTo":"alpine.LFD.0.9999.0712101950110.28293@woody.linux-foundation.org","subject":"[PATCH] t7004: test that \"git-tag -u\" implies \"-s\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-12-11T04:24:36Z","receivedAt":"2007-12-11T04:24:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This was lost in the C conversion, but recently fixed by\nLinus.\n\nSigned-off-by: Jeff King \n---\nOn Mon, Dec 10, 2007 at 08:08:06PM -0800, Linus Torvalds wrote:\n\n> It used to be that passing the signing ID with the -u parameter also \n> (obviously!) implied that you wanted to sign and annotate the tag, but \n> that logic got dropped. It also totally ignored the actual key ID that was \n> passed in.\n> [...]\n> This has gotten only very minimal testing. Somebody should double-check it \n> all, and adding a test would probably be great too.\n\nLooks good to me, and here's a test. It was trivial to whip up, since I\nwrote a test for almost the identical bug a few days ago (that one was\n\"-s implies -a\").\n\n t/t7004-tag.sh | 8 ++++++++\n 1 files changed, 8 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex c7130c4..5393d35 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -682,6 +682,14 @@ test_expect_success '-s implies annotated tag' '\n \tget_tag_msg implied-annotate >actual &&\n \tgit diff expect actual\n '\n+get_tag_header implied-sign $commit commit $time >expect\n+./fakeeditor >>expect\n+echo '-----BEGIN PGP SIGNATURE-----' >>expect\n+test_expect_success '-u implies signed tag' '\n+\tGIT_EDITOR=./fakeeditor git-tag -u CDDE430D implied-sign &&\n+\tget_tag_msg implied-sign >actual &&\n+\tgit diff expect actual\n+'\n \n test_expect_success \\\n \t'trying to create a signed tag with non-existing -F file should fail' '\n-- \n1.5.3.7.2229.gd040e-dirty\n"},{"id":"62647","messageId":"7v63z5j16g.fsf@gitster.siamese.dyndns.org","threadId":"11234","inReplyTo":"20071211042436.GA23805@coredump.intra.peff.net","subject":"Re: [PATCH] t7004: test that \"git-tag -u\" implies \"-s\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-11T04:38:31Z","receivedAt":"2007-12-11T04:38:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Looks good to me, and here's a test. It was trivial to whip up, since I\n> wrote a test for almost the identical bug a few days ago (that one was\n> \"-s implies -a\").\n\nLooks good to me. We should verify both common forms of key-id and also\ncheck a negative case. This squashes a few of mine on top of yours.\n\n---\n\n t/t7004-tag.sh | 26 ++++++++++++++++++++++++++\n 1 files changed, 26 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex c7130c4..a502286 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -640,6 +640,32 @@ test_expect_success 'creating a signed tag with -m message should succeed' '\n \tgit diff expect actual\n '\n \n+get_tag_header implied-sign $commit commit $time >expect\n+echo 'Another signed tag' >>expect\n+echo '-----BEGIN PGP SIGNATURE-----' >>expect\n+test_expect_success '-u implies signed tag' '\n+\tgit-tag -u CDDE430D -m \"Another signed tag\" implied-sign &&\n+\tget_tag_msg implied-sign >actual &&\n+\tgit diff expect actual\n+'\n+\n+get_tag_header u-signed-tag $commit commit $time >expect\n+echo 'Another message' >>expect\n+echo '-----BEGIN PGP SIGNATURE-----' >>expect\n+test_expect_success 'sign with a given key id' '\n+\n+\tgit tag -u committer@example.com -m \"Another message\" u-signed-tag &&\n+\tget_tag_msg u-signed-tag >actual &&\n+\tgit diff expect actual\n+\n+'\n+\n+test_expect_success 'sign with an unknown id' '\n+\n+\t! git tag -u author@example.com -m \"Another message\" o-signed-tag\n+\n+'\n+\n cat >sigmsgfile < +get_tag_header implied-sign $commit commit $time >expect\n> +echo 'Another signed tag' >>expect\n> +echo '-----BEGIN PGP SIGNATURE-----' >>expect\n> +test_expect_success '-u implies signed tag' '\n> +\tgit-tag -u CDDE430D -m \"Another signed tag\" implied-sign &&\n> +\tget_tag_msg implied-sign >actual &&\n> +\tgit diff expect actual\n> +'\n\nOne thing that my original did test but this version doesn't: not only\ndoes -u imply -s, but -u implies -s which implies -a. IOW, using \"-u\"\nwill put you in an editor (and a poorly done fix might have gotten this\nwrong, but apparently Linus is a super-genius).\n\nYour other two tests look good.\n\n-Peff\n"}]}