{"thread":{"id":"41755","subject":"[PATCH] Add the tag.gpgsign option to sign all created tags","startedAt":"2016-03-19T18:23:10Z","lastAt":"2016-03-22T20:41:26Z","messageCount":22,"participants":["Laurent Arnoud","Jeff King","Ramsay Jones","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"281246","messageId":"20160319182310.GA23124@spk-laptop","threadId":"41755","inReplyTo":null,"subject":"[PATCH] Add the tag.gpgsign option to sign all created tags","fromName":"Laurent Arnoud","fromEmail":"laurent@spkdev.net","sentAt":"2016-03-19T18:23:10Z","receivedAt":"2016-03-19T18:23:10Z","isPatch":true,"sender":{"key":"laurent@spkdev.net","avatar":"https://avatars.githubusercontent.com/u/98590?v=4"},"body":"The tag.gpgsign config option allows to sign all\ncommits automatically.\n\nSigned-off-by: Laurent Arnoud <laurent@spkdev.net>\n---\n Documentation/config.txt |  3 +++\n builtin/tag.c            | 19 ++++++++++++++++---\n 2 files changed, 19 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 2cd6bdd..076c68a 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2729,6 +2729,9 @@ submodule.<name>.ignore::\n \t\"--ignore-submodules\" option. The 'git submodule' commands are not\n \taffected by this setting.\n \n+tag.gpgSign::\n+\tA boolean to specify whether all tags created should be GPG signed.\n+\n tag.sort::\n \tThis variable controls the sort ordering of tags when displayed by\n \tlinkgit:git-tag[1]. Without the \"--sort=<value>\" option provided, the\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 1705c94..53cad28 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -29,6 +29,7 @@ static const char * const git_tag_usage[] = {\n };\n \n static unsigned int colopts;\n+static const char *sign_tag;\n \n static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, const char *format)\n {\n@@ -166,6 +167,11 @@ static int git_tag_config(const char *var, const char *value, void *cb)\n \tstatus = git_gpg_config(var, value, cb);\n \tif (status)\n \t\treturn status;\n+\tif (!strcmp(var, \"tag.gpgsign\")) {\n+\t\tsign_tag = git_config_bool(var, value) ? \"\" : NULL;\n+\t\treturn 0;\n+\t}\n+\n \tif (starts_with(var, \"column.\"))\n \t\treturn git_column_config(var, value, \"tag\", &colopts);\n \treturn git_default_config(var, value, cb);\n@@ -381,14 +387,21 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \n \targc = parse_options(argc, argv, prefix, options, git_tag_usage, 0);\n \n+\tif (argc == 0 && !cmdmode)\n+\t\tcmdmode = 'l';\n+\n+\t/* Remove config option when calling command other than create tag */\n+\tif (cmdmode != 0 && sign_tag)\n+\t\tsign_tag = NULL;\n+\n \tif (keyid) {\n \t\topt.sign = 1;\n \t\tset_signing_key(keyid);\n \t}\n-\tif (opt.sign)\n+\tif (opt.sign || sign_tag) {\n+\t\topt.sign = 1;\n \t\tannotate = 1;\n-\tif (argc == 0 && !cmdmode)\n-\t\tcmdmode = 'l';\n+\t}\n \n \tif ((annotate || msg.given || msgfile || force) && (cmdmode != 0))\n \t\tusage_with_options(git_tag_usage, options);\n-- \n2.7.0\n\n\n-- \nLaurent\n"},{"id":"281256","messageId":"20160320042912.GD18312@sigill.intra.peff.net","threadId":"41755","inReplyTo":"20160319182310.GA23124@spk-laptop","subject":"Re: [PATCH] Add the tag.gpgsign option to sign all created tags","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-20T04:29:12Z","receivedAt":"2016-03-20T04:29:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Mar 19, 2016 at 07:23:10PM +0100, Laurent Arnoud wrote:\n\n> The tag.gpgsign config option allows to sign all\n> commits automatically.\n\nWe have commit.gpgsign, so this makes some sense. Would you want to sign\n_all_ tags created with \"git tag\", including lightweight tags, or only\nthose that would already create a tag object (i.e., annotated tags)?\n\n> diff --git a/builtin/tag.c b/builtin/tag.c\n> index 1705c94..53cad28 100644\n> --- a/builtin/tag.c\n> +++ b/builtin/tag.c\n> @@ -29,6 +29,7 @@ static const char * const git_tag_usage[] = {\n>  };\n>  \n>  static unsigned int colopts;\n> +static const char *sign_tag;\n>  \n>  static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, const char *format)\n>  {\n> @@ -166,6 +167,11 @@ static int git_tag_config(const char *var, const char *value, void *cb)\n>  \tstatus = git_gpg_config(var, value, cb);\n>  \tif (status)\n>  \t\treturn status;\n> +\tif (!strcmp(var, \"tag.gpgsign\")) {\n> +\t\tsign_tag = git_config_bool(var, value) ? \"\" : NULL;\n> +\t\treturn 0;\n> +\t}\n\nWhy is \"sign_tag\" a pointer, and not simply an \"int\"?\n\nIf it is just representing the config, should it perhaps be given a more\nspecific name, like \"sign_tag_config\"?\n\n> @@ -381,14 +387,21 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n>  \n>  \targc = parse_options(argc, argv, prefix, options, git_tag_usage, 0);\n>  \n> +\tif (argc == 0 && !cmdmode)\n> +\t\tcmdmode = 'l';\n> +\n> +\t/* Remove config option when calling command other than create tag */\n> +\tif (cmdmode != 0 && sign_tag)\n> +\t\tsign_tag = NULL;\n> +\n\nPerhaps rather than rearranging the setup of cmdmode in this function,\nyou can just use have a conditional that makes explicit when this config\noption kicks in, like:\n\n  if (!cmdmode && sign_tag_config)\n\topt.sign = 1;\n\nThat seems easier to follow to me (and then we don't have to check\nsign_tag_config later; it acts as if the user gave \"-s\"). Although...\n\n>  \tif (keyid) {\n>  \t\topt.sign = 1;\n>  \t\tset_signing_key(keyid);\n>  \t}\n> -\tif (opt.sign)\n> +\tif (opt.sign || sign_tag) {\n> +\t\topt.sign = 1;\n>  \t\tannotate = 1;\n> -\tif (argc == 0 && !cmdmode)\n> -\t\tcmdmode = 'l';\n> +\t}\n>  \n>  \tif ((annotate || msg.given || msgfile || force) && (cmdmode != 0))\n>  \t\tusage_with_options(git_tag_usage, options);\n\nHow do I disable tag.gpgsign if I want an unsigned tag? I should be able\nto do:\n\n  git config tag.gpgsign true\n  git tag --no-sign foo\n\nbut I don't think that works with your patch. You'd need to tweak the\nhandling of opt.sign to be able to tell the difference between\n\"--no-sign was given\" and \"there was no signing-related option given\",\nand only kick in the config in the latter case.\n\n-Peff\n"},{"id":"281272","messageId":"20160320122023.GA5139@spk-laptop","threadId":"41755","inReplyTo":"20160320042912.GD18312@sigill.intra.peff.net","subject":"Re: [PATCH] Add the tag.gpgsign option to sign all created tags","fromName":"Laurent Arnoud","fromEmail":"laurent@spkdev.net","sentAt":"2016-03-20T12:20:23Z","receivedAt":"2016-03-20T12:20:23Z","isPatch":true,"sender":{"key":"laurent@spkdev.net","avatar":"https://avatars.githubusercontent.com/u/98590?v=4"},"body":"Hi Jeff,\n\nOn Sun, Mar 20, 2016 at 12:29:12AM -0400, Jeff King wrote:\n> We have commit.gpgsign, so this makes some sense. Would you want to sign\n> _all_ tags created with \"git tag\", including lightweight tags, or only\n> those that would already create a tag object (i.e., annotated tags)?\n\nYes those that create a tag object (annotated tags).\nAll you suggestions make sense and I will prepare a patch v2.\n\nCheers,\n\n-- \nLaurent\n"},{"id":"281279","messageId":"20160320150703.GB5139@spk-laptop","threadId":"41755","inReplyTo":"20160320042912.GD18312@sigill.intra.peff.net","subject":"[PATCH v2] Add the tag.gpgsign option to sign all created tags","fromName":"Laurent Arnoud","fromEmail":"laurent@spkdev.net","sentAt":"2016-03-20T15:07:03Z","receivedAt":"2016-03-20T15:07:03Z","isPatch":true,"sender":{"key":"laurent@spkdev.net","avatar":"https://avatars.githubusercontent.com/u/98590?v=4"},"body":"The `tag.gpgsign` config option allows to sign all\ncommits automatically.\n\nSupport `--no-sign` option to countermand configuration `tag.gpgsign`.\n\nSigned-off-by: Laurent Arnoud <laurent@spkdev.net>\nReviewed-by: Jeff King <peff@peff.net>\n---\n Documentation/config.txt  |  3 +++\n Documentation/git-tag.txt |  4 ++++\n builtin/tag.c             | 21 ++++++++++++++++-----\n 3 files changed, 23 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 2cd6bdd..076c68a 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2729,6 +2729,9 @@ submodule.<name>.ignore::\n \t\"--ignore-submodules\" option. The 'git submodule' commands are not\n \taffected by this setting.\n \n+tag.gpgSign::\n+\tA boolean to specify whether all tags created should be GPG signed.\n+\n tag.sort::\n \tThis variable controls the sort ordering of tags when displayed by\n \tlinkgit:git-tag[1]. Without the \"--sort=<value>\" option provided, the\ndiff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\nindex abab481..757baa1 100644\n--- a/Documentation/git-tag.txt\n+++ b/Documentation/git-tag.txt\n@@ -64,6 +64,10 @@ OPTIONS\n --sign::\n \tMake a GPG-signed tag, using the default e-mail address's key.\n \n+--no-sign::\n+\tCountermand `tag.gpgSign` configuration variable that is\n+\tset to force each and every tag to be signed.\n+\n -u <keyid>::\n --local-user=<keyid>::\n \tMake a GPG-signed tag, using the given key.\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 1705c94..2a7b2f2 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -29,6 +29,7 @@ static const char * const git_tag_usage[] = {\n };\n \n static unsigned int colopts;\n+static unsigned int sign_tag_config;\n \n static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, const char *format)\n {\n@@ -166,6 +167,11 @@ static int git_tag_config(const char *var, const char *value, void *cb)\n \tstatus = git_gpg_config(var, value, cb);\n \tif (status)\n \t\treturn status;\n+\tif (!strcmp(var, \"tag.gpgsign\")) {\n+\t\tsign_tag_config = git_config_bool(var, value) ? 1 : 0;\n+\t\treturn 0;\n+\t}\n+\n \tif (starts_with(var, \"column.\"))\n \t\treturn git_column_config(var, value, \"tag\", &colopts);\n \treturn git_default_config(var, value, cb);\n@@ -195,7 +201,7 @@ static void write_tag_body(int fd, const unsigned char *sha1)\n \n static int build_tag_object(struct strbuf *buf, int sign, unsigned char *result)\n {\n-\tif (sign && do_sign(buf) < 0)\n+\tif (sign > 0 && do_sign(buf) < 0)\n \t\treturn error(_(\"unable to sign the tag\"));\n \tif (write_sha1_file(buf->buf, buf->len, tag_type, result) < 0)\n \t\treturn error(_(\"unable to write tag file\"));\n@@ -204,7 +210,7 @@ static int build_tag_object(struct strbuf *buf, int sign, unsigned char *result)\n \n struct create_tag_options {\n \tunsigned int message_given:1;\n-\tunsigned int sign;\n+\tint sign;\n \tenum {\n \t\tCLEANUP_NONE,\n \t\tCLEANUP_SPACE,\n@@ -378,17 +384,22 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tmemset(&opt, 0, sizeof(opt));\n \tmemset(&filter, 0, sizeof(filter));\n \tfilter.lines = -1;\n+\topt.sign = -1;\n \n \targc = parse_options(argc, argv, prefix, options, git_tag_usage, 0);\n \n+\tif (argc == 0 && !cmdmode)\n+\t\tcmdmode = 'l';\n+\n+\tif (!cmdmode && sign_tag_config && opt.sign != 0)\n+\t\topt.sign = 1;\n+\n \tif (keyid) {\n \t\topt.sign = 1;\n \t\tset_signing_key(keyid);\n \t}\n-\tif (opt.sign)\n+\tif (opt.sign > 0)\n \t\tannotate = 1;\n-\tif (argc == 0 && !cmdmode)\n-\t\tcmdmode = 'l';\n \n \tif ((annotate || msg.given || msgfile || force) && (cmdmode != 0))\n \t\tusage_with_options(git_tag_usage, options);\n-- \n2.7.0\n"},{"id":"281290","messageId":"56EED20A.3000407@ramsayjones.plus.com","threadId":"41755","inReplyTo":"20160320150703.GB5139@spk-laptop","subject":"Re: [PATCH v2] Add the tag.gpgsign option to sign all created tags","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-03-20T16:38:34Z","receivedAt":"2016-03-20T16:38:34Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 20/03/16 15:07, Laurent Arnoud wrote:\n> The `tag.gpgsign` config option allows to sign all\n> commits automatically.\n  ^^^^^^^\npresumably you meant tags.\n\n> \n> Support `--no-sign` option to countermand configuration `tag.gpgsign`.\n> \n> Signed-off-by: Laurent Arnoud <laurent@spkdev.net>\n> Reviewed-by: Jeff King <peff@peff.net>\n> ---\n>  Documentation/config.txt  |  3 +++\n>  Documentation/git-tag.txt |  4 ++++\n>  builtin/tag.c             | 21 ++++++++++++++++-----\n>  3 files changed, 23 insertions(+), 5 deletions(-)\n> \n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 2cd6bdd..076c68a 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -2729,6 +2729,9 @@ submodule.<name>.ignore::\n>  \t\"--ignore-submodules\" option. The 'git submodule' commands are not\n>  \taffected by this setting.\n>  \n> +tag.gpgSign::\n> +\tA boolean to specify whether all tags created should be GPG signed.\n> +\n>  tag.sort::\n>  \tThis variable controls the sort ordering of tags when displayed by\n>  \tlinkgit:git-tag[1]. Without the \"--sort=<value>\" option provided, the\n> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\n> index abab481..757baa1 100644\n> --- a/Documentation/git-tag.txt\n> +++ b/Documentation/git-tag.txt\n> @@ -64,6 +64,10 @@ OPTIONS\n>  --sign::\n>  \tMake a GPG-signed tag, using the default e-mail address's key.\n>  \n> +--no-sign::\n> +\tCountermand `tag.gpgSign` configuration variable that is\n> +\tset to force each and every tag to be signed.\n> +\n\nI assume that, after setting tag.gpgsign in the config, this is the\nonly way to get a lightweight tag. Maybe here, or above in config.txt,\nthis could be stated more obviously? dunno.\n\n[Also, don't we normally describe --[no]-sign options together?]\n\nATB,\nRamsay Jones\n"},{"id":"281291","messageId":"20160320165216.GA5710@sigill.intra.peff.net","threadId":"41755","inReplyTo":"20160320122023.GA5139@spk-laptop","subject":"Re: [PATCH] Add the tag.gpgsign option to sign all created tags","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-20T16:52:17Z","receivedAt":"2016-03-20T16:52:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Mar 20, 2016 at 01:20:23PM +0100, Laurent Arnoud wrote:\n\n> Hi Jeff,\n> \n> On Sun, Mar 20, 2016 at 12:29:12AM -0400, Jeff King wrote:\n> > We have commit.gpgsign, so this makes some sense. Would you want to sign\n> > _all_ tags created with \"git tag\", including lightweight tags, or only\n> > those that would already create a tag object (i.e., annotated tags)?\n> \n> Yes those that create a tag object (annotated tags).\n> All you suggestions make sense and I will prepare a patch v2.\n\nThat behavior makes more sense to me, but I don't think it's what your\npatch does (v1 or v2). Perhaps it would make sense to add some tests,\nboth to verify that it is behaving as expected, and to protect that\nbehavior from future changes?\n\n-Peff\n"},{"id":"281295","messageId":"20160320174434.GB20083@spk-laptop","threadId":"41755","inReplyTo":"20160320165216.GA5710@sigill.intra.peff.net","subject":"Re: [PATCH] Add the tag.gpgsign option to sign all created tags","fromName":"Laurent Arnoud","fromEmail":"laurent@spkdev.net","sentAt":"2016-03-20T17:44:34Z","receivedAt":"2016-03-20T17:44:34Z","isPatch":true,"sender":{"key":"laurent@spkdev.net","avatar":"https://avatars.githubusercontent.com/u/98590?v=4"},"body":"On Sun, Mar 20, 2016 at 12:52:17PM -0400, Jeff King wrote:\n> That behavior makes more sense to me, but I don't think it's what your\n> patch does (v1 or v2). Perhaps it would make sense to add some tests,\n> both to verify that it is behaving as expected, and to protect that\n> behavior from future changes?\n\nYes, I will try to do so during the week.\n\nCheers,\n\n-- \nLaurent\n"},{"id":"281327","messageId":"xmqq7fgwnzuv.fsf@gitster.mtv.corp.google.com","threadId":"41755","inReplyTo":"20160320150703.GB5139@spk-laptop","subject":"Re: [PATCH v2] Add the tag.gpgsign option to sign all created tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-21T05:50:48Z","receivedAt":"2016-03-21T05:50:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Laurent Arnoud <laurent@spkdev.net> writes:\n\n> The `tag.gpgsign` config option allows to sign all\n> commits automatically.\n\nI presume that you meant \"all annotated tags\" here.  But I am not\nsure it this is sensible.\n\n> Support `--no-sign` option to countermand configuration `tag.gpgsign`.\n\nThat sound quite counter-intuitive.\n\n    $ git tag -s -m \"my message\" v1.0\n\nis an explicit request to create a signed tag, as opposed to\n\n\n    $ git tag -a -m \"my message\" v1.0\n\nis an explicit request to create an unsigned annotated tag.  So \n\nI think a short-hand\n\n    $ git tag -m \"my message\" v1.0\n\nfalls back to annotated and not signed tag, and I can understand\nif the patch is about allowing the user to tweak this fallback to\ncreate signed tag instead.\n\nSo I do not see why you need a new --no-sign option at all.  If\nyou have the configuration and you do want to create an unsigned\nannotated tag one-shot, all you need is to explicitly ask for \"-a\"\ni.e.\n\n    $ git tag -a -m \"my message\" v1.0\n\nisn't it?\n\nIf you are forcing users to always leave a message and then further\nforcing users to always sign with the single new configuration, i.e.\n\n    $ git tag v1.0\n    ... opens the editor to ask for a message ...\n    ... then makes the user sign with GPG ...\n\nthen I would first have to say that is a bad idea.\n\nI can sort-of understand (but do not necessarily agree that it is a\ngood idea) adding new two configurations, i.e.\n\n - \"even without -a/-s, force the user to annotate the tag\" is one\n   configuration, and\n\n - \"even when the user did not say -s, force the user to sign an\n   annotated tag\" is the other.\n\nAnd with such a system, I can see why you would need an option\n\"--lightweight\" to force creation of a light-weight tag (i.e. to\ncountermand the first one).  You can view this new option as\nsomething that sits next to existing -a/-s.  The current system lets\nuser choose among the three variants (lightweight, annotated and\nsigned) by not giving any, giving -a, and giving -s option\nrespectively, but with the \"--lightweight\" option, the user can ask\nfor one of the three explicitly, as opposed to using \"lack of either\n-a/-s\" as a signal to create lightweight one.\n\nAnd in the context of such a system, \"--no-sign\" may make sense to\noverride the second configuration (i.e. \"force the user to sign an\nannotated tag\").\n\nBut otherwise, adding only \"--no-sign\" does not make much sense to\nme, as it implies \"not signing always means annotated\", which is not\ntrue.  It is unclear between lightweight and annotated which one the\nuser who says \"--no-sign\" really wants.\n"},{"id":"281365","messageId":"20160321192904.GC20083@spk-laptop","threadId":"41755","inReplyTo":"xmqq7fgwnzuv.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] Add the tag.gpgsign option to sign all created tags","fromName":"Laurent Arnoud","fromEmail":"laurent@spkdev.net","sentAt":"2016-03-21T19:29:04Z","receivedAt":"2016-03-21T19:29:04Z","isPatch":true,"sender":{"key":"laurent@spkdev.net","avatar":"https://avatars.githubusercontent.com/u/98590?v=4"},"body":"On Sun, Mar 20, 2016 at 10:50:48PM -0700, Junio C Hamano wrote:\n> > The `tag.gpgsign` config option allows to sign all\n> > commits automatically.\n> \n> I presume that you meant \"all annotated tags\" here.  But I am not\n> sure it this is sensible.\n\nYes its a mistake.\n\n> > Support `--no-sign` option to countermand configuration `tag.gpgsign`.\n> So I do not see why you need a new --no-sign option at all.  If\n> you have the configuration and you do want to create an unsigned\n> annotated tag one-shot, all you need is to explicitly ask for \"-a\"\n> i.e.\n> \n>     $ git tag -a -m \"my message\" v1.0\n> \n> isn't it?\n\nYou know that when you have sign configuration enabled globally annotate is\nimplicite, so its difficult to join both world. I use same idea as in your\npatch `55ca3f99ae4895605a348322dd2fc50f2065f508`.\n\n> If you are forcing users to always leave a message and then further\n> forcing users to always sign with the single new configuration, i.e.\n> \n>     $ git tag v1.0\n>     ... opens the editor to ask for a message ...\n>     ... then makes the user sign with GPG ...\n\nI'm not forcing this type of user to enable global configuration, that will be\nannoying for them of course.\nI tried to fix a need I have currently and this is a good compromise for me.\n\n> then I would first have to say that is a bad idea.\n> \n> I can sort-of understand (but do not necessarily agree that it is a\n> good idea) adding new two configurations, i.e.\n> \n>  - \"even without -a/-s, force the user to annotate the tag\" is one\n>    configuration, and\n> \n>  - \"even when the user did not say -s, force the user to sign an\n>    annotated tag\" is the other.\n> \n> And with such a system, I can see why you would need an option\n> \"--lightweight\" to force creation of a light-weight tag (i.e. to\n> countermand the first one).  You can view this new option as\n> something that sits next to existing -a/-s.  The current system lets\n> user choose among the three variants (lightweight, annotated and\n> signed) by not giving any, giving -a, and giving -s option\n> respectively, but with the \"--lightweight\" option, the user can ask\n> for one of the three explicitly, as opposed to using \"lack of either\n> -a/-s\" as a signal to create lightweight one.\n> \n> And in the context of such a system, \"--no-sign\" may make sense to\n> override the second configuration (i.e. \"force the user to sign an\n> annotated tag\").\n> \n> But otherwise, adding only \"--no-sign\" does not make much sense to\n> me, as it implies \"not signing always means annotated\", which is not\n> true.  It is unclear between lightweight and annotated which one the\n> user who says \"--no-sign\" really wants.\n\nAs I said it's difficult to easily join both world, as you know with\nconfiguration and command line options. This is an override and if its really a\nno go for this patch without refactoring this I will stop my work on it.\n\nJust let me know I will send a patch v3 updated with tests after this.\n\nCheers,\n\n-- \nLaurent\n"},{"id":"281367","messageId":"20160321193207.GD20083@spk-laptop","threadId":"41755","inReplyTo":"xmqq7fgwnzuv.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v3] Add the tag.gpgsign option to sign annotated tags","fromName":"Laurent Arnoud","fromEmail":"laurent@spkdev.net","sentAt":"2016-03-21T19:32:07Z","receivedAt":"2016-03-21T19:32:07Z","isPatch":true,"sender":{"key":"laurent@spkdev.net","avatar":"https://avatars.githubusercontent.com/u/98590?v=4"},"body":"The `tag.gpgsign` config option allows to sign all\nannotated tags automatically.\n\nSupport `--no-sign` option to countermand configuration `tag.gpgsign`.\n\nSigned-off-by: Laurent Arnoud <laurent@spkdev.net>\nReviewed-by: Jeff King <peff@peff.net>\n---\n Documentation/config.txt  |  5 +++++\n Documentation/git-tag.txt |  6 +++++-\n builtin/tag.c             | 21 ++++++++++++++++-----\n t/t7004-tag.sh            | 45 +++++++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 71 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 2cd6bdd..ba9d4da 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2729,6 +2729,11 @@ submodule.<name>.ignore::\n \t\"--ignore-submodules\" option. The 'git submodule' commands are not\n \taffected by this setting.\n \n+tag.gpgSign::\n+\tA boolean to specify whether annotated tags created should be GPG signed.\n+\tIf `--no-sign` is specified on the command line, it takes\n+\tprecedence over this option.\n+\n tag.sort::\n \tThis variable controls the sort ordering of tags when displayed by\n \tlinkgit:git-tag[1]. Without the \"--sort=<value>\" option provided, the\ndiff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\nindex abab481..180edd2 100644\n--- a/Documentation/git-tag.txt\n+++ b/Documentation/git-tag.txt\n@@ -9,7 +9,7 @@ git-tag - Create, list, delete or verify a tag object signed with GPG\n SYNOPSIS\n --------\n [verse]\n-'git tag' [-a | -s | -u <keyid>] [-f] [-m <msg> | -F <file>]\n+'git tag' [-a | -s | --no-sign | -u <keyid>] [-f] [-m <msg> | -F <file>]\n \t<tagname> [<commit> | <object>]\n 'git tag' -d <tagname>...\n 'git tag' [-n[<num>]] -l [--contains <commit>] [--points-at <object>]\n@@ -64,6 +64,10 @@ OPTIONS\n --sign::\n \tMake a GPG-signed tag, using the default e-mail address's key.\n \n+--no-sign::\n+\tCountermand `tag.gpgSign` configuration variable that is\n+\tset to force annoted tags to be signed.\n+\n -u <keyid>::\n --local-user=<keyid>::\n \tMake a GPG-signed tag, using the given key.\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 1705c94..2a7b2f2 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -29,6 +29,7 @@ static const char * const git_tag_usage[] = {\n };\n \n static unsigned int colopts;\n+static unsigned int sign_tag_config;\n \n static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, const char *format)\n {\n@@ -166,6 +167,11 @@ static int git_tag_config(const char *var, const char *value, void *cb)\n \tstatus = git_gpg_config(var, value, cb);\n \tif (status)\n \t\treturn status;\n+\tif (!strcmp(var, \"tag.gpgsign\")) {\n+\t\tsign_tag_config = git_config_bool(var, value) ? 1 : 0;\n+\t\treturn 0;\n+\t}\n+\n \tif (starts_with(var, \"column.\"))\n \t\treturn git_column_config(var, value, \"tag\", &colopts);\n \treturn git_default_config(var, value, cb);\n@@ -195,7 +201,7 @@ static void write_tag_body(int fd, const unsigned char *sha1)\n \n static int build_tag_object(struct strbuf *buf, int sign, unsigned char *result)\n {\n-\tif (sign && do_sign(buf) < 0)\n+\tif (sign > 0 && do_sign(buf) < 0)\n \t\treturn error(_(\"unable to sign the tag\"));\n \tif (write_sha1_file(buf->buf, buf->len, tag_type, result) < 0)\n \t\treturn error(_(\"unable to write tag file\"));\n@@ -204,7 +210,7 @@ static int build_tag_object(struct strbuf *buf, int sign, unsigned char *result)\n \n struct create_tag_options {\n \tunsigned int message_given:1;\n-\tunsigned int sign;\n+\tint sign;\n \tenum {\n \t\tCLEANUP_NONE,\n \t\tCLEANUP_SPACE,\n@@ -378,17 +384,22 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tmemset(&opt, 0, sizeof(opt));\n \tmemset(&filter, 0, sizeof(filter));\n \tfilter.lines = -1;\n+\topt.sign = -1;\n \n \targc = parse_options(argc, argv, prefix, options, git_tag_usage, 0);\n \n+\tif (argc == 0 && !cmdmode)\n+\t\tcmdmode = 'l';\n+\n+\tif (!cmdmode && sign_tag_config && opt.sign != 0)\n+\t\topt.sign = 1;\n+\n \tif (keyid) {\n \t\topt.sign = 1;\n \t\tset_signing_key(keyid);\n \t}\n-\tif (opt.sign)\n+\tif (opt.sign > 0)\n \t\tannotate = 1;\n-\tif (argc == 0 && !cmdmode)\n-\t\tcmdmode = 'l';\n \n \tif ((annotate || msg.given || msgfile || force) && (cmdmode != 0))\n \t\tusage_with_options(git_tag_usage, options);\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex cf3469b..4e45361 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -775,6 +775,51 @@ test_expect_success GPG '-s implies annotated tag' '\n \ttest_cmp expect actual\n '\n \n+get_tag_header config-implied-annotate $commit commit $time >expect\n+./fakeeditor >>expect\n+echo '-----BEGIN PGP SIGNATURE-----' >>expect\n+git config tag.gpgsign true\n+test_expect_success GPG \\\n+\t'git tag -s implied if configured with tag.gpgsign' \\\n+\t'GIT_EDITOR=./fakeeditor git tag config-implied-annotate &&\n+\tget_tag_msg config-implied-annotate >actual &&\n+\ttest_cmp expect actual\n+'\n+git config --unset tag.gpgsign\n+\n+get_tag_header config-implied-annotate-disabled $commit commit $time >expect\n+echo \"A message\" >>expect\n+git config tag.gpgsign true\n+test_expect_success GPG \\\n+\t'git tag --no-sign disable configured tag.gpgsign' \\\n+\t'git tag --no-sign -m \"A message\" config-implied-annotate-disabled &&\n+\tget_tag_msg config-implied-annotate-disabled >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_must_fail git tag -v config-implied-annotate-disabled\n+'\n+git config --unset tag.gpgsign\n+\n+get_tag_header config-disabled-gpgsign $commit commit $time >expect\n+echo \"A message\" >>expect\n+echo '-----BEGIN PGP SIGNATURE-----' >>expect\n+git config tag.gpgsign false\n+test_expect_success GPG \\\n+\t'git tag --sign enable GPG sign' \\\n+\t'git tag --sign -m \"A message\" config-disabled-gpgsign &&\n+\tget_tag_msg config-disabled-gpgsign >actual &&\n+\ttest_cmp expect actual\n+'\n+git config --unset tag.gpgsign\n+\n+git config tag.gpgsign true\n+test_expect_success GPG \\\n+\t'git tag --no-sign disable configured tag.gpgsign and annotate' \\\n+\t'git tag --no-sign config-non-annotated-tag &&\n+\ttag_exists config-non-annotated-tag &&\n+\ttest_must_fail git tag -v config-non-annotated-tag\n+'\n+git config --unset tag.gpgsign\n+\n test_expect_success GPG \\\n \t'trying to create a signed tag with non-existing -F file should fail' '\n \t! test -f nonexistingfile &&\n-- \n2.7.0\n"},{"id":"281371","messageId":"20160321194237.GA28301@sigill.intra.peff.net","threadId":"41755","inReplyTo":"20160321193207.GD20083@spk-laptop","subject":"Re: [PATCH v3] Add the tag.gpgsign option to sign annotated tags","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-21T19:42:38Z","receivedAt":"2016-03-21T19:42:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 21, 2016 at 08:32:07PM +0100, Laurent Arnoud wrote:\n\n> The `tag.gpgsign` config option allows to sign all\n> annotated tags automatically.\n> \n> Support `--no-sign` option to countermand configuration `tag.gpgsign`.\n> \n> Signed-off-by: Laurent Arnoud <laurent@spkdev.net>\n> Reviewed-by: Jeff King <peff@peff.net>\n\nThe meaning of \"Reviewed-by\" in this project is generally that the\nmentioned person has read and approved of the change. But in this case,\nI have not seen v3 at all yet, and I am also not sure that the ones I\n_did_ review are ready for merging.\n\nSo you should probably drop that.\n\n> +tag.gpgSign::\n> +\tA boolean to specify whether annotated tags created should be GPG signed.\n> +\tIf `--no-sign` is specified on the command line, it takes\n> +\tprecedence over this option.\n\nI take this to mean that we _only_ kick in signing if the created tag\nwould otherwise be annotated (and I thought that's what you meant in\nyour other mail, too). But that's not what happens with this patch, and\nyour tests check for the opposite:\n\n> +get_tag_header config-implied-annotate $commit commit $time >expect\n> +./fakeeditor >>expect\n> +echo '-----BEGIN PGP SIGNATURE-----' >>expect\n> +git config tag.gpgsign true\n> +test_expect_success GPG \\\n> +\t'git tag -s implied if configured with tag.gpgsign' \\\n> +\t'GIT_EDITOR=./fakeeditor git tag config-implied-annotate &&\n> +\tget_tag_msg config-implied-annotate >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +git config --unset tag.gpgsign\n\nThat's a lightweight tag that becomes an annotated one due to the config\nvariable.\n\nSo I think there may be some design-level issues to work out here, but\nI'll comment on a few more code-specific things, in case that code does\nget carried through:\n\n> +\tif (!strcmp(var, \"tag.gpgsign\")) {\n> +\t\tsign_tag_config = git_config_bool(var, value) ? 1 : 0;\n> +\t\treturn 0;\n> +\t}\n\ngit_config_bool() already converts to 0/1, you can just say:\n\n  sign_tag_config = git_config_bool(var, value);\n\n> +get_tag_header config-implied-annotate-disabled $commit commit $time >expect\n> +echo \"A message\" >>expect\n> +git config tag.gpgsign true\n> +test_expect_success GPG \\\n> +\t'git tag --no-sign disable configured tag.gpgsign' \\\n> +\t'git tag --no-sign -m \"A message\" config-implied-annotate-disabled &&\n> +\tget_tag_msg config-implied-annotate-disabled >actual &&\n> +\ttest_cmp expect actual &&\n> +\ttest_must_fail git tag -v config-implied-annotate-disabled\n> +'\n> +git config --unset tag.gpgsign\n\nHere (and in the other tests), you can use:\n\n  test_config tag.gpgsign true &&\n  ...\n\ninside the test_expect_success block. That has two advantages:\n\n  1. If setting the config fails for some reason, we'll notice and the\n     test will fail.\n\n  2. At the end of the test block, it will automatically clean up the\n     variable for us.\n\n-Peff\n"},{"id":"281372","messageId":"xmqqvb4fliq6.fsf@gitster.mtv.corp.google.com","threadId":"41755","inReplyTo":"20160321192904.GC20083@spk-laptop","subject":"Re: [PATCH v2] Add the tag.gpgsign option to sign all created tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-21T19:43:45Z","receivedAt":"2016-03-21T19:43:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Laurent Arnoud <laurent@spkdev.net> writes:\n\n>> > Support `--no-sign` option to countermand configuration `tag.gpgsign`.\n>> So I do not see why you need a new --no-sign option at all.  If\n>> you have the configuration and you do want to create an unsigned\n>> annotated tag one-shot, all you need is to explicitly ask for \"-a\"\n>> i.e.\n>> \n>>     $ git tag -a -m \"my message\" v1.0\n>> \n>> isn't it?\n>\n> You know that when you have sign configuration enabled globally annotate is\n> implicite, so its difficult to join both world.\n\nSorry, I am not sure what you mean by that.  It is unclear what two\nworlds you are referring to.\n\n> I use same idea as in your patch\n> `55ca3f99ae4895605a348322dd2fc50f2065f508`.\n\nThat is not a good comparison.  55ca3f99 (commit-tree: add and\ndocument --no-gpg-sign, 2013-12-13) is about signed commit, and over\nthere there are only two choices, i.e. a commit that corresponds to\nan annotated tag, and a signed commit that corresponds to a signed\ntag.  There is no \"lightweight-tag\" equivalent.\n\n>> If you are forcing users to always leave a message and then further\n>> forcing users to always sign with the single new configuration, i.e.\n>> \n>>     $ git tag v1.0\n>>     ... opens the editor to ask for a message ...\n>>     ... then makes the user sign with GPG ...\n>\n> I'm not forcing this type of user to enable global configuration, that will be\n> annoying for them of course.\n\nGood.\n\nIf so, then the configuration is \"when the user gives us a message\nto create a tag without explicitly saying -a/-s, we create an\nannotated tag by default, but create a signed tag instead in such a\ncase\", I would think.  That is:\n\n    $ git tag -m 'foo' $tagname\n\nwould create signed tag under such a configuration option, and I\nthink such an option may make sense.  And the way to override it\nwould be\n\n    $ git tag -a -m 'foo' $tagname\n\nSo there is no need for --no-sign option.  When the user explicitly\nasks to create an annotated tag with\n\n    $ git tag -a -m 'foo' $tagname\n\nit is unreasonable to override that explicit wish with a\nconfiguration setting.\n"},{"id":"281373","messageId":"20160321195348.GA31650@sigill.intra.peff.net","threadId":"41755","inReplyTo":"xmqq7fgwnzuv.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] Add the tag.gpgsign option to sign all created tags","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-21T19:53:48Z","receivedAt":"2016-03-21T19:53:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Mar 20, 2016 at 10:50:48PM -0700, Junio C Hamano wrote:\n\n> > Support `--no-sign` option to countermand configuration `tag.gpgsign`.\n> \n> That sound quite counter-intuitive.\n> [...]\n\nI was the one who suggested --no-sign, as we usually like to have a way\nto countermand the config. But having read your message, I agree that is\nprobably not the right mental model.\n\nIn particular, this:\n\n> I can sort-of understand (but do not necessarily agree that it is a\n> good idea) adding new two configurations, i.e.\n> \n>  - \"even without -a/-s, force the user to annotate the tag\" is one\n>    configuration, and\n> \n>  - \"even when the user did not say -s, force the user to sign an\n>    annotated tag\" is the other.\n> \n> And with such a system, I can see why you would need an option\n> \"--lightweight\" to force creation of a light-weight tag (i.e. to\n> countermand the first one).  You can view this new option as\n> something that sits next to existing -a/-s.  The current system lets\n> user choose among the three variants (lightweight, annotated and\n> signed) by not giving any, giving -a, and giving -s option\n> respectively, but with the \"--lightweight\" option, the user can ask\n> for one of the three explicitly, as opposed to using \"lack of either\n> -a/-s\" as a signal to create lightweight one.\n\nmakes sense to me (though like you, I do not necessarily think it is a\ngood idea and would not use it myself).\n\nAnother similar approach would be to collapse this down to a single\nvariable that selects from the options. IOW:\n\n  1. Add --lightweight for explicitly adding a lightweight tag.\n\n  2. When we are creating a tag and none of \"-a\", \"-s\", or\n     \"--lightweight\" is given, use the default given in\n     tag.defaultTagType (or whatever we call it), which can\n     be \"lightweight\", \"annotated\", or \"signed\".\n\n  3. tag.defaultTagType defaults to \"lightweight\".\n\nThat is conceptually simpler to me, with the main differences being:\n\n  - in yours, the second config would mean that an explicit \"-a\" implies\n    \"-s\" (unless the user says --no-sign).\n\n  - in mine, there is no way to kick in the signing _only_ when we are\n    annotating. If you configure \"signed\", then you have to explicitly\n    say \"--lightweight\" for lightweight tags.\n\nI dunno. It sounds like Laurent would set it to \"signed\", and that would\ndo what he wants. But like I said, I would not plan to use the feature\nmyself, and I could see it ending up a little bit annoying when you _do_\nwant a lightweight tag.\n\n-Peff\n"},{"id":"281374","messageId":"20160321200101.GE20083@spk-laptop","threadId":"41755","inReplyTo":"xmqqvb4fliq6.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] Add the tag.gpgsign option to sign all created tags","fromName":"Laurent Arnoud","fromEmail":"laurent@spkdev.net","sentAt":"2016-03-21T20:01:01Z","receivedAt":"2016-03-21T20:01:01Z","isPatch":true,"sender":{"key":"laurent@spkdev.net","avatar":"https://avatars.githubusercontent.com/u/98590?v=4"},"body":"On Mon, Mar 21, 2016 at 12:43:45PM -0700, Junio C Hamano wrote:\n> > You know that when you have sign configuration enabled globally annotate is\n> > implicite, so its difficult to join both world.\n> \n> Sorry, I am not sure what you mean by that.  It is unclear what two\n> worlds you are referring to.\n\nCommand line options and configuration, but forget about it.\n\n> \n> > I use same idea as in your patch\n> > `55ca3f99ae4895605a348322dd2fc50f2065f508`.\n> \n> That is not a good comparison.  55ca3f99 (commit-tree: add and\n> document --no-gpg-sign, 2013-12-13) is about signed commit, and over\n> there there are only two choices, i.e. a commit that corresponds to\n> an annotated tag, and a signed commit that corresponds to a signed\n> tag.  There is no \"lightweight-tag\" equivalent.\n> \n> >> If you are forcing users to always leave a message and then further\n> >> forcing users to always sign with the single new configuration, i.e.\n> >> \n> >>     $ git tag v1.0\n> >>     ... opens the editor to ask for a message ...\n> >>     ... then makes the user sign with GPG ...\n> >\n> > I'm not forcing this type of user to enable global configuration, that will be\n> > annoying for them of course.\n> \n> Good.\n> \n> If so, then the configuration is \"when the user gives us a message\n> to create a tag without explicitly saying -a/-s, we create an\n> annotated tag by default, but create a signed tag instead in such a\n> case\", I would think.  That is:\n> \n>     $ git tag -m 'foo' $tagname\n> \n> would create signed tag under such a configuration option, and I\n> think such an option may make sense.  And the way to override it\n> would be\n> \n>     $ git tag -a -m 'foo' $tagname\n> \n> So there is no need for --no-sign option.  When the user explicitly\n> asks to create an annotated tag with\n> \n>     $ git tag -a -m 'foo' $tagname\n> \n> it is unreasonable to override that explicit wish with a\n> configuration setting.\n> \n\nAh, I think I understand now, I think this will not take to much effort to fix.\n\n-- \nLaurent\n"},{"id":"281376","messageId":"20160321200451.GA32071@sigill.intra.peff.net","threadId":"41755","inReplyTo":"xmqqvb4fliq6.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] Add the tag.gpgsign option to sign all created tags","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-21T20:04:51Z","receivedAt":"2016-03-21T20:04:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 21, 2016 at 12:43:45PM -0700, Junio C Hamano wrote:\n\n> If so, then the configuration is \"when the user gives us a message\n> to create a tag without explicitly saying -a/-s, we create an\n> annotated tag by default, but create a signed tag instead in such a\n> case\", I would think.  That is:\n> \n>     $ git tag -m 'foo' $tagname\n> \n> would create signed tag under such a configuration option, and I\n> think such an option may make sense.  And the way to override it\n> would be\n> \n>     $ git tag -a -m 'foo' $tagname\n> \n> So there is no need for --no-sign option.  When the user explicitly\n> asks to create an annotated tag with\n> \n>     $ git tag -a -m 'foo' $tagname\n> \n> it is unreasonable to override that explicit wish with a\n> configuration setting.\n\nFWIW, of the schemes discussed in this thread, this one makes the most\nsense to me (and is one I might actually use).\n\nI think you could shoe-horn it into my \"tag-type\" config scheme as a new\nmode (\"sign-if-message\" or something), but I think it is much clearer as\na standalone config variable.\n\nThanks for a dose of sanity.\n\n-Peff\n"},{"id":"281383","messageId":"20160321205015.GF20083@spk-laptop","threadId":"41755","inReplyTo":"xmqqvb4fliq6.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v4] Add the tag.gpgsign option to sign annotated tags","fromName":"Laurent Arnoud","fromEmail":"laurent@spkdev.net","sentAt":"2016-03-21T20:50:15Z","receivedAt":"2016-03-21T20:50:15Z","isPatch":true,"sender":{"key":"laurent@spkdev.net","avatar":"https://avatars.githubusercontent.com/u/98590?v=4"},"body":"The `tag.gpgsign` config option allows to sign annotated tags\nautomatically.\n\n`--annotate` command line option disable configuration `tag.gpgsign`.\n\nSigned-off-by: Laurent Arnoud <laurent@spkdev.net>\n---\n Documentation/config.txt |  5 +++++\n builtin/tag.c            | 20 +++++++++++++++-----\n t/t7004-tag.sh           | 33 +++++++++++++++++++++++++++++++++\n 3 files changed, 53 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 2cd6bdd..62f7d2a 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2729,6 +2729,11 @@ submodule.<name>.ignore::\n \t\"--ignore-submodules\" option. The 'git submodule' commands are not\n \taffected by this setting.\n \n+tag.gpgSign::\n+\tA boolean to specify whether annotated tags created should be GPG signed.\n+\tIf `--annotate` is specified on the command line, it takes\n+\tprecedence over this option.\n+\n tag.sort::\n \tThis variable controls the sort ordering of tags when displayed by\n \tlinkgit:git-tag[1]. Without the \"--sort=<value>\" option provided, the\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 1705c94..748aeac 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -29,6 +29,7 @@ static const char * const git_tag_usage[] = {\n };\n \n static unsigned int colopts;\n+static unsigned int sign_tag_config;\n \n static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, const char *format)\n {\n@@ -166,6 +167,11 @@ static int git_tag_config(const char *var, const char *value, void *cb)\n \tstatus = git_gpg_config(var, value, cb);\n \tif (status)\n \t\treturn status;\n+\tif (!strcmp(var, \"tag.gpgsign\")) {\n+\t\tsign_tag_config = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \tif (starts_with(var, \"column.\"))\n \t\treturn git_column_config(var, value, \"tag\", &colopts);\n \treturn git_default_config(var, value, cb);\n@@ -326,7 +332,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tstruct create_tag_options opt;\n \tchar *cleanup_arg = NULL;\n \tint create_reflog = 0;\n-\tint annotate = 0, force = 0;\n+\tint annotate = -1, force = 0;\n \tint cmdmode = 0;\n \tconst char *msgfile = NULL, *keyid = NULL;\n \tstruct msg_arg msg = { 0, STRBUF_INIT };\n@@ -381,16 +387,20 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \n \targc = parse_options(argc, argv, prefix, options, git_tag_usage, 0);\n \n+\tif (argc == 0 && !cmdmode)\n+\t\tcmdmode = 'l';\n+\n+\tif (!cmdmode && sign_tag_config && annotate < 1)\n+\t\topt.sign = 1;\n+\n \tif (keyid) {\n \t\topt.sign = 1;\n \t\tset_signing_key(keyid);\n \t}\n \tif (opt.sign)\n \t\tannotate = 1;\n-\tif (argc == 0 && !cmdmode)\n-\t\tcmdmode = 'l';\n \n-\tif ((annotate || msg.given || msgfile || force) && (cmdmode != 0))\n+\tif ((annotate > 0 || msg.given || msgfile || force) && (cmdmode != 0))\n \t\tusage_with_options(git_tag_usage, options);\n \n \tfinalize_colopts(&colopts, -1);\n@@ -474,7 +484,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \telse\n \t\tdie(_(\"Invalid cleanup mode %s\"), cleanup_arg);\n \n-\tif (annotate)\n+\tif (annotate > 0)\n \t\tcreate_tag(object, tag, &buf, &opt, prev, object);\n \n \ttransaction = ref_transaction_begin(&err);\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex cf3469b..7791d00 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -775,6 +775,39 @@ test_expect_success GPG '-s implies annotated tag' '\n \ttest_cmp expect actual\n '\n \n+get_tag_header config-implied-annotate $commit commit $time >expect\n+./fakeeditor >>expect\n+echo '-----BEGIN PGP SIGNATURE-----' >>expect\n+test_expect_success GPG \\\n+\t'git tag -s implied if configured with tag.gpgsign' \\\n+\t'test_config tag.gpgsign true &&\n+\tGIT_EDITOR=./fakeeditor git tag config-implied-annotate &&\n+\tget_tag_msg config-implied-annotate >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+get_tag_header config-implied-annotate-disabled $commit commit $time >expect\n+echo \"A message\" >>expect\n+test_expect_success GPG \\\n+\t'git tag -a disable configured tag.gpgsign' \\\n+\t'test_config tag.gpgsign true &&\n+\tgit tag -a -m \"A message\" config-implied-annotate-disabled &&\n+\tget_tag_msg config-implied-annotate-disabled >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_must_fail git tag -v config-implied-annotate-disabled\n+'\n+\n+get_tag_header config-disabled-gpgsign $commit commit $time >expect\n+echo \"A message\" >>expect\n+echo '-----BEGIN PGP SIGNATURE-----' >>expect\n+test_expect_success GPG \\\n+\t'git tag --sign enable GPG sign' \\\n+\t'test_config tag.gpgsign false &&\n+\tgit tag --sign -m \"A message\" config-disabled-gpgsign &&\n+\tget_tag_msg config-disabled-gpgsign >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success GPG \\\n \t'trying to create a signed tag with non-existing -F file should fail' '\n \t! test -f nonexistingfile &&\n-- \n2.7.0\n"},{"id":"281389","messageId":"xmqqa8lrldz4.fsf@gitster.mtv.corp.google.com","threadId":"41755","inReplyTo":"20160321205015.GF20083@spk-laptop","subject":"Re: [PATCH v4] Add the tag.gpgsign option to sign annotated tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-21T21:26:23Z","receivedAt":"2016-03-21T21:26:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Laurent Arnoud <laurent@spkdev.net> writes:\n\n> @@ -326,7 +332,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n>  \tstruct create_tag_options opt;\n>  \tchar *cleanup_arg = NULL;\n>  \tint create_reflog = 0;\n> -\tint annotate = 0, force = 0;\n> +\tint annotate = -1, force = 0;\n\nI really do not like this part of the change.  The original code is\nalready bad in the sense that it uses \"annotate\" to mean \"we need to\ncreate a tag object, not just a lightweight tag\" in some places\nwhich already needs careful reading, but with this it no longer is\nclear what this argument even means.  It sometimes means \"we are\ncreating a tag object\" and some other times means \"we did not see\n'-a' on the command line\" but only until the variable gets assigned\nby existing \"if opt.sign is there, set it to true\" tweaking.\n\nI wonder if an approach like the following would result in a cleaner\ncode that is easier to maintain:\n\n - Leave this to be initialized to 0, allow it to be toggled upon\n   seeing \"-a\" via the parse_options() mechanism, but redefine its\n   meaning to be \"-a option was passed\".  Make it never be touched\n   by anything else (e.g. \"if (opt.sign) annotate = 1;\" is wrong).\n\n - Introduce a new \"create_tag_object\" variable, initialized to 0.\n   Existing code that sets \"annotate = 1\" (e.g. \"-s\" is passed,\n   \"-m/-F\" is passed, etc.) will instead set this variable.  The\n   condition of if() statement to call create_tag() would also be\n   looking at this variable, not \"annotate\".\n\n - Before calling create_tag(), look at the value of your new\n   configuration and \"annotate\", and set opt.sign as needed.\n\nA patch to do so might look something like this (totally untested).\n\n\n builtin/tag.c | 20 ++++++++++++++------\n 1 file changed, 14 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 1705c94..d262f92 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -29,6 +29,7 @@ static const char * const git_tag_usage[] = {\n };\n \n static unsigned int colopts;\n+static int force_sign_annotate;\n \n static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, const char *format)\n {\n@@ -168,6 +169,10 @@ static int git_tag_config(const char *var, const char *value, void *cb)\n \t\treturn status;\n \tif (starts_with(var, \"column.\"))\n \t\treturn git_column_config(var, value, \"tag\", &colopts);\n+\tif (!strcmp(var, \"tag.forcesignannotated\")) {\n+\t\tforce_sign_annotate = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \treturn git_default_config(var, value, cb);\n }\n \n@@ -327,7 +332,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tchar *cleanup_arg = NULL;\n \tint create_reflog = 0;\n \tint annotate = 0, force = 0;\n-\tint cmdmode = 0;\n+\tint cmdmode = 0, create_tag_object = 0;\n \tconst char *msgfile = NULL, *keyid = NULL;\n \tstruct msg_arg msg = { 0, STRBUF_INIT };\n \tstruct ref_transaction *transaction;\n@@ -385,12 +390,12 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\topt.sign = 1;\n \t\tset_signing_key(keyid);\n \t}\n-\tif (opt.sign)\n-\t\tannotate = 1;\n+\tif (opt.sign || annotate)\n+\t\tcreate_tag_object = 1;\n \tif (argc == 0 && !cmdmode)\n \t\tcmdmode = 'l';\n \n-\tif ((annotate || msg.given || msgfile || force) && (cmdmode != 0))\n+\tif ((create_tag_object || msg.given || msgfile || force) && (cmdmode != 0))\n \t\tusage_with_options(git_tag_usage, options);\n \n \tfinalize_colopts(&colopts, -1);\n@@ -431,7 +436,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tif (msg.given || msgfile) {\n \t\tif (msg.given && msgfile)\n \t\t\tdie(_(\"only one -F or -m option is allowed.\"));\n-\t\tannotate = 1;\n+\t\tcreate_tag_object = 1;\n \t\tif (msg.given)\n \t\t\tstrbuf_addbuf(&buf, &(msg.buf));\n \t\telse {\n@@ -474,8 +479,11 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \telse\n \t\tdie(_(\"Invalid cleanup mode %s\"), cleanup_arg);\n \n-\tif (annotate)\n+\tif (create_tag_object) {\n+\t\tif (force_sign_annotate && !annotate)\n+\t\t\topt.sign = 1;\n \t\tcreate_tag(object, tag, &buf, &opt, prev, object);\n+\t}\n \n \ttransaction = ref_transaction_begin(&err);\n \tif (!transaction ||\n"},{"id":"281393","messageId":"xmqqwpovjxjk.fsf@gitster.mtv.corp.google.com","threadId":"41755","inReplyTo":"xmqqvb4fliq6.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] Add the tag.gpgsign option to sign all created tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-21T22:06:39Z","receivedAt":"2016-03-21T22:06:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>>> If you are forcing users to always leave a message and then further\n>>> forcing users to always sign with the single new configuration, i.e.\n>>> \n>>>     $ git tag v1.0\n>>>     ... opens the editor to ask for a message ...\n>>>     ... then makes the user sign with GPG ...\n>>\n>> I'm not forcing this type of user to enable global configuration, that will be\n>> annoying for them of course.\n>\n> Good.\n\nEh, it seems that your test actually expects this kind of\ninteraction, though.\n\nThis piece from your PATCH v4:\n\n    diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\n    index cf3469b..7791d00 100755\n    --- a/t/t7004-tag.sh\n    +++ b/t/t7004-tag.sh\n    @@ -775,6 +775,39 @@ test_expect_success GPG '-s implies annotated tag' '\n            test_cmp expect actual\n     '\n\n    +get_tag_header config-implied-annotate $commit commit $time >expect\n    +./fakeeditor >>expect\n    +echo '-----BEGIN PGP SIGNATURE-----' >>expect\n    +test_expect_success GPG \\\n    +\t'git tag -s implied if configured with tag.gpgsign' \\\n    +\t'test_config tag.gpgsign true &&\n    +\tGIT_EDITOR=./fakeeditor git tag config-implied-annotate &&\n    +\tget_tag_msg config-implied-annotate >actual &&\n    +\ttest_cmp expect actual\n    +'\n    +\n\nwants to see that \"git tag config-implied-annotate\" (which looks\nexactly like \"git tag v1.0\" we discussed above) runs the fakeeditor\nand makes a signed tag.\n"},{"id":"281473","messageId":"20160322193617.GG20083@spk-laptop","threadId":"41755","inReplyTo":"xmqqa8lrldz4.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v5] Add the option to force sign annotated tags","fromName":"Laurent Arnoud","fromEmail":"laurent@spkdev.net","sentAt":"2016-03-22T19:36:17Z","receivedAt":"2016-03-22T19:36:17Z","isPatch":true,"sender":{"key":"laurent@spkdev.net","avatar":"https://avatars.githubusercontent.com/u/98590?v=4"},"body":"The `tag.forcesignannotated` config option allows to sign annotated tags\nautomatically.\n\n`--annotate` command line option disable configuration\n`tag.forcesignannotated`.\n\nSigned-off-by: Laurent Arnoud <laurent@spkdev.net>\n---\n Documentation/config.txt |  5 +++++\n builtin/tag.c            | 22 ++++++++++++++++------\n t/t7004-tag.sh           | 33 +++++++++++++++++++++++++++++++++\n 3 files changed, 54 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 2cd6bdd..95d5c9d 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2729,6 +2729,11 @@ submodule.<name>.ignore::\n \t\"--ignore-submodules\" option. The 'git submodule' commands are not\n \taffected by this setting.\n \n+tag.forceSignAnnotated::\n+\tA boolean to specify whether annotated tags created should be GPG signed.\n+\tIf `--annotate` is specified on the command line, it takes\n+\tprecedence over this option.\n+\n tag.sort::\n \tThis variable controls the sort ordering of tags when displayed by\n \tlinkgit:git-tag[1]. Without the \"--sort=<value>\" option provided, the\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 1705c94..c447738 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -29,6 +29,7 @@ static const char * const git_tag_usage[] = {\n };\n \n static unsigned int colopts;\n+static int force_sign_annotate;\n \n static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, const char *format)\n {\n@@ -166,6 +167,11 @@ static int git_tag_config(const char *var, const char *value, void *cb)\n \tstatus = git_gpg_config(var, value, cb);\n \tif (status)\n \t\treturn status;\n+\tif (!strcmp(var, \"tag.forcesignannotated\")) {\n+\t\tforce_sign_annotate = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \tif (starts_with(var, \"column.\"))\n \t\treturn git_column_config(var, value, \"tag\", &colopts);\n \treturn git_default_config(var, value, cb);\n@@ -327,7 +333,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tchar *cleanup_arg = NULL;\n \tint create_reflog = 0;\n \tint annotate = 0, force = 0;\n-\tint cmdmode = 0;\n+\tint cmdmode = 0, create_tag_object = 0;\n \tconst char *msgfile = NULL, *keyid = NULL;\n \tstruct msg_arg msg = { 0, STRBUF_INIT };\n \tstruct ref_transaction *transaction;\n@@ -385,12 +391,13 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\topt.sign = 1;\n \t\tset_signing_key(keyid);\n \t}\n-\tif (opt.sign)\n-\t\tannotate = 1;\n+\tif (opt.sign || annotate || force_sign_annotate)\n+\t\tcreate_tag_object = 1;\n+\n \tif (argc == 0 && !cmdmode)\n \t\tcmdmode = 'l';\n \n-\tif ((annotate || msg.given || msgfile || force) && (cmdmode != 0))\n+\tif ((create_tag_object || msg.given || msgfile || force) && (cmdmode != 0))\n \t\tusage_with_options(git_tag_usage, options);\n \n \tfinalize_colopts(&colopts, -1);\n@@ -431,7 +438,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tif (msg.given || msgfile) {\n \t\tif (msg.given && msgfile)\n \t\t\tdie(_(\"only one -F or -m option is allowed.\"));\n-\t\tannotate = 1;\n+\t\tcreate_tag_object = 1;\n \t\tif (msg.given)\n \t\t\tstrbuf_addbuf(&buf, &(msg.buf));\n \t\telse {\n@@ -474,8 +481,11 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \telse\n \t\tdie(_(\"Invalid cleanup mode %s\"), cleanup_arg);\n \n-\tif (annotate)\n+\tif (create_tag_object) {\n+\t\tif (force_sign_annotate && !annotate)\n+\t\t\topt.sign = 1;\n \t\tcreate_tag(object, tag, &buf, &opt, prev, object);\n+    }\n \n \ttransaction = ref_transaction_begin(&err);\n \tif (!transaction ||\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex cf3469b..be95318 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -775,6 +775,39 @@ test_expect_success GPG '-s implies annotated tag' '\n \ttest_cmp expect actual\n '\n \n+get_tag_header config-implied-annotate $commit commit $time >expect\n+./fakeeditor >>expect\n+echo '-----BEGIN PGP SIGNATURE-----' >>expect\n+test_expect_success GPG \\\n+\t'git tag -s implied if configured with tag.forcesignannotated' \\\n+\t'test_config tag.forcesignannotated true &&\n+\tGIT_EDITOR=./fakeeditor git tag config-implied-annotate &&\n+\tget_tag_msg config-implied-annotate >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+get_tag_header config-implied-annotate-disabled $commit commit $time >expect\n+echo \"A message\" >>expect\n+test_expect_success GPG \\\n+\t'git tag -a disable configured tag.forcesignannotated' \\\n+\t'test_config tag.forcesignannotated true &&\n+\tgit tag -a -m \"A message\" config-implied-annotate-disabled &&\n+\tget_tag_msg config-implied-annotate-disabled >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_must_fail git tag -v config-implied-annotate-disabled\n+'\n+\n+get_tag_header config-disabled-forcesignannotated $commit commit $time >expect\n+echo \"A message\" >>expect\n+echo '-----BEGIN PGP SIGNATURE-----' >>expect\n+test_expect_success GPG \\\n+\t'git tag --sign enable GPG sign' \\\n+\t'test_config tag.forcesignannotated false &&\n+\tgit tag --sign -m \"A message\" config-disabled-forcesignannotated &&\n+\tget_tag_msg config-disabled-forcesignannotated >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success GPG \\\n \t'trying to create a signed tag with non-existing -F file should fail' '\n \t! test -f nonexistingfile &&\n-- \n2.7.0\n"},{"id":"281474","messageId":"xmqqshziguot.fsf@gitster.mtv.corp.google.com","threadId":"41755","inReplyTo":"20160322193617.GG20083@spk-laptop","subject":"Re: [PATCH v5] Add the option to force sign annotated tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-22T19:48:50Z","receivedAt":"2016-03-22T19:48:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Laurent Arnoud <laurent@spkdev.net> writes:\n\n> The `tag.forcesignannotated` config option allows to sign\n> annotated tags automatically.\n\nIt looks like it does a lot more than that to me, though.\n\n> @@ -327,7 +333,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n>  \tchar *cleanup_arg = NULL;\n>  \tint create_reflog = 0;\n>  \tint annotate = 0, force = 0;\n> -\tint cmdmode = 0;\n> +\tint cmdmode = 0, create_tag_object = 0;\n>  \tconst char *msgfile = NULL, *keyid = NULL;\n>  \tstruct msg_arg msg = { 0, STRBUF_INIT };\n>  \tstruct ref_transaction *transaction;\n> @@ -385,12 +391,13 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n>  \t\topt.sign = 1;\n>  \t\tset_signing_key(keyid);\n>  \t}\n> -\tif (opt.sign)\n> -\t\tannotate = 1;\n> +\tif (opt.sign || annotate || force_sign_annotate)\n> +\t\tcreate_tag_object = 1;\n\nThis means that create_tag_object is always on if the configuration\nis set and there is no way to override that from the command line,\ndoesn't it?  I cannot see how a user would create a lightweight tag\nif this configuration variable is set with this change.\n\nI think it makes sense to have this here instead of these two lines:\n\n\tcreate_tag_object = (opt.sign || annotate || msg.given || msgfile);\n\n>  \tif (argc == 0 && !cmdmode)\n>  \t\tcmdmode = 'l';\n>  \n> -\tif ((annotate || msg.given || msgfile || force) && (cmdmode != 0))\n> +\tif ((create_tag_object || msg.given || msgfile || force) && (cmdmode != 0))\n\nand then simplify this to\n\n\tif ((create_tag_object || force) && (cmdmode != 0))\n\nperhaps?  Then ...\n\n>  \t\tusage_with_options(git_tag_usage, options);\n>  \n>  \tfinalize_colopts(&colopts, -1);\n> @@ -431,7 +438,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n>  \tif (msg.given || msgfile) {\n>  \t\tif (msg.given && msgfile)\n>  \t\t\tdie(_(\"only one -F or -m option is allowed.\"));\n> -\t\tannotate = 1;\n> +\t\tcreate_tag_object = 1;\n\n... this line can just go, as we are taking the presense of various\nways to say \"I'll give this message to the resulting tag\" as the\nsign that the user wants to create a tag object much earlier.\n\n>  \t\tif (msg.given)\n>  \t\t\tstrbuf_addbuf(&buf, &(msg.buf));\n>  \t\telse {\n> @@ -474,8 +481,11 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n>  \telse\n>  \t\tdie(_(\"Invalid cleanup mode %s\"), cleanup_arg);\n>  \n> -\tif (annotate)\n> +\tif (create_tag_object) {\n> +\t\tif (force_sign_annotate && !annotate)\n> +\t\t\topt.sign = 1;\n>  \t\tcreate_tag(object, tag, &buf, &opt, prev, object);\n> +    }\n\nAnd this hunk is OK.\n\n> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\n> index cf3469b..be95318 100755\n> --- a/t/t7004-tag.sh\n> +++ b/t/t7004-tag.sh\n> @@ -775,6 +775,39 @@ test_expect_success GPG '-s implies annotated tag' '\n>  \ttest_cmp expect actual\n>  '\n>  \n> +get_tag_header config-implied-annotate $commit commit $time >expect\n> +./fakeeditor >>expect\n> +echo '-----BEGIN PGP SIGNATURE-----' >>expect\n> +test_expect_success GPG \\\n> +\t'git tag -s implied if configured with tag.forcesignannotated' \\\n> +\t'test_config tag.forcesignannotated true &&\n> +\tGIT_EDITOR=./fakeeditor git tag config-implied-annotate &&\n\nThis contradicts with what you said earlier in your\n<20160321192904.GC20083@spk-laptop> aka\nhttp://thread.gmane.org/gmane.comp.version-control.git/289322/focus=289441\n\n    > If you are forcing users to always leave a message and then further\n    > forcing users to always sign with the single new configuration, i.e.\n    > \n    >     $ git tag v1.0\n    >     ... opens the editor to ask for a message ...\n    >     ... then makes the user sign with GPG ...\n\n    I'm not forcing this type of user to enable global\n    configuration, that will be annoying for them of course.\n\nBut this test expects that this invocation of \"git tag $tagname\"\nforces the user to give a message with editor and sign it, instead\nof creating a lightweight tag.\n"},{"id":"281478","messageId":"20160322200724.GH20083@spk-laptop","threadId":"41755","inReplyTo":"xmqqshziguot.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v5] Add the option to force sign annotated tags","fromName":"Laurent Arnoud","fromEmail":"laurent@spkdev.net","sentAt":"2016-03-22T20:07:24Z","receivedAt":"2016-03-22T20:07:24Z","isPatch":true,"sender":{"key":"laurent@spkdev.net","avatar":"https://avatars.githubusercontent.com/u/98590?v=4"},"body":"On Tue, Mar 22, 2016 at 12:48:50PM -0700, Junio C Hamano wrote:\n> Laurent Arnoud <laurent@spkdev.net> writes:\n> \n> > The `tag.forcesignannotated` config option allows to sign\n> > annotated tags automatically.\n> \n> It looks like it does a lot more than that to me, though.\n> \n> > @@ -327,7 +333,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n> >  \tchar *cleanup_arg = NULL;\n> >  \tint create_reflog = 0;\n> >  \tint annotate = 0, force = 0;\n> > -\tint cmdmode = 0;\n> > +\tint cmdmode = 0, create_tag_object = 0;\n> >  \tconst char *msgfile = NULL, *keyid = NULL;\n> >  \tstruct msg_arg msg = { 0, STRBUF_INIT };\n> >  \tstruct ref_transaction *transaction;\n> > @@ -385,12 +391,13 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n> >  \t\topt.sign = 1;\n> >  \t\tset_signing_key(keyid);\n> >  \t}\n> > -\tif (opt.sign)\n> > -\t\tannotate = 1;\n> > +\tif (opt.sign || annotate || force_sign_annotate)\n> > +\t\tcreate_tag_object = 1;\n> \n> This means that create_tag_object is always on if the configuration\n> is set and there is no way to override that from the command line,\n> doesn't it?  I cannot see how a user would create a lightweight tag\n> if this configuration variable is set with this change.\n> \n> I think it makes sense to have this here instead of these two lines:\n> \n> \tcreate_tag_object = (opt.sign || annotate || msg.given || msgfile);\n> \n> >  \tif (argc == 0 && !cmdmode)\n> >  \t\tcmdmode = 'l';\n> >  \n> > -\tif ((annotate || msg.given || msgfile || force) && (cmdmode != 0))\n> > +\tif ((create_tag_object || msg.given || msgfile || force) && (cmdmode != 0))\n> \n> and then simplify this to\n> \n> \tif ((create_tag_object || force) && (cmdmode != 0))\n> \n> perhaps?  Then ...\n> \n> >  \t\tusage_with_options(git_tag_usage, options);\n> >  \n> >  \tfinalize_colopts(&colopts, -1);\n> > @@ -431,7 +438,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n> >  \tif (msg.given || msgfile) {\n> >  \t\tif (msg.given && msgfile)\n> >  \t\t\tdie(_(\"only one -F or -m option is allowed.\"));\n> > -\t\tannotate = 1;\n> > +\t\tcreate_tag_object = 1;\n> \n> ... this line can just go, as we are taking the presense of various\n> ways to say \"I'll give this message to the resulting tag\" as the\n> sign that the user wants to create a tag object much earlier.\n> \n> >  \t\tif (msg.given)\n> >  \t\t\tstrbuf_addbuf(&buf, &(msg.buf));\n> >  \t\telse {\n> > @@ -474,8 +481,11 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n> >  \telse\n> >  \t\tdie(_(\"Invalid cleanup mode %s\"), cleanup_arg);\n> >  \n> > -\tif (annotate)\n> > +\tif (create_tag_object) {\n> > +\t\tif (force_sign_annotate && !annotate)\n> > +\t\t\topt.sign = 1;\n> >  \t\tcreate_tag(object, tag, &buf, &opt, prev, object);\n> > +    }\n> \n> And this hunk is OK.\n> \n> > diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\n> > index cf3469b..be95318 100755\n> > --- a/t/t7004-tag.sh\n> > +++ b/t/t7004-tag.sh\n> > @@ -775,6 +775,39 @@ test_expect_success GPG '-s implies annotated tag' '\n> >  \ttest_cmp expect actual\n> >  '\n> >  \n> > +get_tag_header config-implied-annotate $commit commit $time >expect\n> > +./fakeeditor >>expect\n> > +echo '-----BEGIN PGP SIGNATURE-----' >>expect\n> > +test_expect_success GPG \\\n> > +\t'git tag -s implied if configured with tag.forcesignannotated' \\\n> > +\t'test_config tag.forcesignannotated true &&\n> > +\tGIT_EDITOR=./fakeeditor git tag config-implied-annotate &&\n> \n> This contradicts with what you said earlier in your\n> <20160321192904.GC20083@spk-laptop> aka\n> http://thread.gmane.org/gmane.comp.version-control.git/289322/focus=289441\n> \n>     > If you are forcing users to always leave a message and then further\n>     > forcing users to always sign with the single new configuration, i.e.\n>     > \n>     >     $ git tag v1.0\n>     >     ... opens the editor to ask for a message ...\n>     >     ... then makes the user sign with GPG ...\n> \n>     I'm not forcing this type of user to enable global\n>     configuration, that will be annoying for them of course.\n> \n> But this test expects that this invocation of \"git tag $tagname\"\n> forces the user to give a message with editor and sign it, instead\n> of creating a lightweight tag.\n\nYes you're right, I will fix that...\n\n-- \nLaurent\n"},{"id":"281481","messageId":"20160322204126.GI20083@spk-laptop","threadId":"41755","inReplyTo":"xmqqshziguot.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v6] Add the option to force sign annotated tags","fromName":"Laurent Arnoud","fromEmail":"laurent@spkdev.net","sentAt":"2016-03-22T20:41:26Z","receivedAt":"2016-03-22T20:41:26Z","isPatch":true,"sender":{"key":"laurent@spkdev.net","avatar":"https://avatars.githubusercontent.com/u/98590?v=4"},"body":"The `tag.forcesignannotated` config option allows to sign annotated tags\nautomatically.\n\n`--annotate` command line option disable configuration\n`tag.forcesignannotated`.\n\nSigned-off-by: Laurent Arnoud <laurent@spkdev.net>\n---\n Documentation/config.txt |  5 +++++\n builtin/tag.c            | 20 ++++++++++++++------\n t/t7004-tag.sh           | 41 +++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 60 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 2cd6bdd..95d5c9d 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2729,6 +2729,11 @@ submodule.<name>.ignore::\n \t\"--ignore-submodules\" option. The 'git submodule' commands are not\n \taffected by this setting.\n \n+tag.forceSignAnnotated::\n+\tA boolean to specify whether annotated tags created should be GPG signed.\n+\tIf `--annotate` is specified on the command line, it takes\n+\tprecedence over this option.\n+\n tag.sort::\n \tThis variable controls the sort ordering of tags when displayed by\n \tlinkgit:git-tag[1]. Without the \"--sort=<value>\" option provided, the\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 1705c94..528a1ba 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -29,6 +29,7 @@ static const char * const git_tag_usage[] = {\n };\n \n static unsigned int colopts;\n+static int force_sign_annotate;\n \n static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, const char *format)\n {\n@@ -166,6 +167,11 @@ static int git_tag_config(const char *var, const char *value, void *cb)\n \tstatus = git_gpg_config(var, value, cb);\n \tif (status)\n \t\treturn status;\n+\tif (!strcmp(var, \"tag.forcesignannotated\")) {\n+\t\tforce_sign_annotate = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \tif (starts_with(var, \"column.\"))\n \t\treturn git_column_config(var, value, \"tag\", &colopts);\n \treturn git_default_config(var, value, cb);\n@@ -327,7 +333,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tchar *cleanup_arg = NULL;\n \tint create_reflog = 0;\n \tint annotate = 0, force = 0;\n-\tint cmdmode = 0;\n+\tint cmdmode = 0, create_tag_object = 0;\n \tconst char *msgfile = NULL, *keyid = NULL;\n \tstruct msg_arg msg = { 0, STRBUF_INIT };\n \tstruct ref_transaction *transaction;\n@@ -385,12 +391,12 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\topt.sign = 1;\n \t\tset_signing_key(keyid);\n \t}\n-\tif (opt.sign)\n-\t\tannotate = 1;\n+\tcreate_tag_object = (opt.sign || annotate || msg.given || msgfile);\n+\n \tif (argc == 0 && !cmdmode)\n \t\tcmdmode = 'l';\n \n-\tif ((annotate || msg.given || msgfile || force) && (cmdmode != 0))\n+\tif ((create_tag_object || force) && (cmdmode != 0))\n \t\tusage_with_options(git_tag_usage, options);\n \n \tfinalize_colopts(&colopts, -1);\n@@ -431,7 +437,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tif (msg.given || msgfile) {\n \t\tif (msg.given && msgfile)\n \t\t\tdie(_(\"only one -F or -m option is allowed.\"));\n-\t\tannotate = 1;\n \t\tif (msg.given)\n \t\t\tstrbuf_addbuf(&buf, &(msg.buf));\n \t\telse {\n@@ -474,8 +479,11 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \telse\n \t\tdie(_(\"Invalid cleanup mode %s\"), cleanup_arg);\n \n-\tif (annotate)\n+\tif (create_tag_object) {\n+\t\tif (force_sign_annotate && !annotate)\n+\t\t\topt.sign = 1;\n \t\tcreate_tag(object, tag, &buf, &opt, prev, object);\n+\t}\n \n \ttransaction = ref_transaction_begin(&err);\n \tif (!transaction ||\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex cf3469b..f9b7d79 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -775,6 +775,47 @@ test_expect_success GPG '-s implies annotated tag' '\n \ttest_cmp expect actual\n '\n \n+get_tag_header forcesignannotated-implied-sign $commit commit $time >expect\n+echo \"A message\" >>expect\n+echo '-----BEGIN PGP SIGNATURE-----' >>expect\n+test_expect_success GPG \\\n+\t'git tag -s implied if configured with tag.forcesignannotated' \\\n+\t'test_config tag.forcesignannotated true &&\n+\tgit tag -m \"A message\" forcesignannotated-implied-sign &&\n+\tget_tag_msg forcesignannotated-implied-sign >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG \\\n+\t'lightweight with no message when configured with tag.forcesignannotated' \\\n+\t'test_config tag.forcesignannotated true &&\n+\tgit tag forcesignannotated-lightweight &&\n+\ttag_exists forcesignannotated-lightweight &&\n+\ttest_must_fail git tag -v forcesignannotated-no-message\n+'\n+\n+get_tag_header forcesignannotated-annotate $commit commit $time >expect\n+echo \"A message\" >>expect\n+test_expect_success GPG \\\n+\t'git tag -a disable configured tag.forcesignannotated' \\\n+\t'test_config tag.forcesignannotated true &&\n+\tgit tag -a -m \"A message\" forcesignannotated-annotate &&\n+\tget_tag_msg forcesignannotated-annotate >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_must_fail git tag -v forcesignannotated-annotate\n+'\n+\n+get_tag_header forcesignannotated-disabled $commit commit $time >expect\n+echo \"A message\" >>expect\n+echo '-----BEGIN PGP SIGNATURE-----' >>expect\n+test_expect_success GPG \\\n+\t'git tag --sign enable GPG sign' \\\n+\t'test_config tag.forcesignannotated false &&\n+\tgit tag --sign -m \"A message\" forcesignannotated-disabled &&\n+\tget_tag_msg forcesignannotated-disabled >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success GPG \\\n \t'trying to create a signed tag with non-existing -F file should fail' '\n \t! test -f nonexistingfile &&\n-- \n2.7.0\n"}]}