{"thread":{"id":"51237","subject":"config option to force all tags be GPG-signed - comeback","startedAt":"2019-06-04T11:53:55Z","lastAt":"2019-06-05T21:33:31Z","messageCount":13,"participants":["Tigran Mkrtchyan","Johannes Schindelin","Mkrtchyan, Tigran","Todd Zullinger","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"376649","messageId":"20190604114356.11042-1-tigran.mkrtchyan@desy.de","threadId":"51237","inReplyTo":"60741736.3439901.1509090074292.JavaMail.zimbra@desy.de","subject":"config option to force all tags be GPG-signed - comeback","fromName":"Tigran Mkrtchyan","fromEmail":"tigran.mkrtchyan@desy.de","sentAt":"2019-06-04T11:43:55Z","receivedAt":"2019-06-04T11:53:55Z","isPatch":false,"sender":{"key":"tigran.mkrtchyan@desy.de","avatar":"https://avatars.githubusercontent.com/u/575965?v=4"},"body":"As there was some interest on my original pull request on github[1], \nI decided to rebase, update the commit message and re-post this patch.\n\n\n[1] https://github.com/git/git/pull/419\n\n"},{"id":"376650","messageId":"20190604114356.11042-2-tigran.mkrtchyan@desy.de","threadId":"51237","inReplyTo":"20190604114356.11042-1-tigran.mkrtchyan@desy.de","subject":"[PATCH v2] tag: add tag.gpgSign config option to force all tags be GPG-signed","fromName":"Tigran Mkrtchyan","fromEmail":"tigran.mkrtchyan@desy.de","sentAt":"2019-06-04T11:43:56Z","receivedAt":"2019-06-04T11:53:56Z","isPatch":true,"sender":{"key":"tigran.mkrtchyan@desy.de","avatar":"https://avatars.githubusercontent.com/u/575965?v=4"},"body":"As may CI/CD tools don't allow to control command line options when\nexecuting `git tag` command, a default value in the configuration file\nwill allow to enforce tag signing if required.\n\nThe new config-file option tag.gpgSign enforces signed tags. Additional\ncommand line option --no-gpg-sign is added to disable such behavior if\nneeded. E.g.:\n\n    $ git tag -m \"commit message\"\n\nwill generate a GPG signed tag if tag.gpgSign option is true, while\n\n    $ git tag --no-gpg-sign -m \"commit message\"\n\nwill skip the signing step.\n\nSigned-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>\n---\n Documentation/git-tag.txt |  7 +++++++\n builtin/tag.c             | 18 +++++++++++++++---\n t/t7004-tag.sh            | 21 +++++++++++++++++++++\n 3 files changed, 43 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\nindex a74e7b926d..d9dbfb4e37 100644\n--- a/Documentation/git-tag.txt\n+++ b/Documentation/git-tag.txt\n@@ -64,6 +64,9 @@ OPTIONS\n -s::\n --sign::\n \tMake a GPG-signed tag, using the default e-mail address's key.\n+\tThe default behabior of tag GPG-signing controlled by `tag.gpgSign`\n+\tconfiguration variable if it exists, or disabled oder otherwise.\n+\tSee linkgit:git-config[1].\n \n -u <keyid>::\n --local-user=<keyid>::\n@@ -193,6 +196,10 @@ This option is only applicable when listing tags without annotation lines.\n \tthat of linkgit:git-for-each-ref[1].  When unspecified,\n \tdefaults to `%(refname:strip=2)`.\n \n+--no-gpg-sign::\n+\tCountermand `tag.gpgSign` configuration variable that is\n+\tset to force each and every tag to be signed.\n+\n <tagname>::\n \tThe name of the tag to create, delete, or describe.\n \tThe new tag name must pass all checks defined by\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex ef37dccf86..7f9aef4840 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -33,6 +33,7 @@ static const char * const git_tag_usage[] = {\n \n static unsigned int colopts;\n static int force_sign_annotate;\n+static int sign_tag;\n \n static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting,\n \t\t     struct ref_format *format)\n@@ -144,6 +145,11 @@ static int git_tag_config(const char *var, const char *value, void *cb)\n \tint status;\n \tstruct ref_sorting **sorting_tail = (struct ref_sorting **)cb;\n \n+\tif (!strcmp(var, \"tag.gpgsign\")) {\n+\t\tsign_tag = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \tif (!strcmp(var, \"tag.sort\")) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n@@ -392,6 +398,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tstruct ref_format format = REF_FORMAT_INIT;\n \tint icase = 0;\n \tint edit_flag = 0;\n+\tint no_gpg_sign = 0;\n \tstruct option options[] = {\n \t\tOPT_CMDMODE('l', \"list\", &cmdmode, N_(\"list tag names\"), 'l'),\n \t\t{ OPTION_INTEGER, 'n', NULL, &filter.lines, N_(\"n\"),\n@@ -413,6 +420,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\t\t\t\tN_(\"use another key to sign the tag\")),\n \t\tOPT__FORCE(&force, N_(\"replace the tag if exists\"), 0),\n \t\tOPT_BOOL(0, \"create-reflog\", &create_reflog, N_(\"create a reflog\")),\n+\t\tOPT_BOOL(0, \"no-gpg-sign\", &no_gpg_sign, N_(\"do not GPG-sign tag\")),\n \n \t\tOPT_GROUP(N_(\"Tag listing options\")),\n \t\tOPT_COLUMN(0, \"column\", &colopts, N_(\"show tag list in columns\")),\n@@ -445,6 +453,10 @@ 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 (no_gpg_sign) {\n+\t\tsign_tag = 0;\n+\t}\n+\n \tif (keyid) {\n \t\topt.sign = 1;\n \t\tset_signing_key(keyid);\n@@ -463,7 +475,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tif (cmdmode == 'l')\n \t\tsetup_auto_pager(\"tag\", 1);\n \n-\tif ((create_tag_object || force) && (cmdmode != 0))\n+\tif ((create_tag_object || force || no_gpg_sign) && (cmdmode != 0))\n \t\tusage_with_options(git_tag_usage, options);\n \n \tfinalize_colopts(&colopts, -1);\n@@ -556,8 +568,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \n \tcreate_reflog_msg(&object, &reflog_msg);\n \n-\tif (create_tag_object) {\n-\t\tif (force_sign_annotate && !annotate)\n+\tif (create_tag_object || sign_tag) {\n+\t\tif (sign_tag || (force_sign_annotate && !annotate))\n \t\t\topt.sign = 1;\n \t\tcreate_tag(&object, object_ref, tag, &buf, &opt, &prev, &object);\n \t}\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 6aeeb279a0..98a07a29d2 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -932,6 +932,27 @@ test_expect_success GPG \\\n \ttest_cmp expect actual\n '\n \n+get_tag_header gpgsign-enabled $commit commit $time >expect\n+echo \"A message\" >>expect\n+echo '-----BEGIN PGP SIGNATURE-----' >>expect\n+test_expect_success GPG \\\n+\t'git tag configured tag.gpgsign enables GPG sign' \\\n+\t'test_config tag.gpgsign true &&\n+\tgit tag -m \"A message\" gpgsign-enabled &&\n+\tget_tag_msg gpgsign-enabled>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+get_tag_header no-gpg-sign $commit commit $time >expect\n+echo \"A message\" >>expect\n+test_expect_success GPG \\\n+\t'git tag --no-gpg-sign configured tag.gpgsign skip GPG sign' \\\n+\t'test_config tag.gpgsign true &&\n+\tgit tag -a --no-gpg-sign -m \"A message\" no-gpg-sign &&\n+\tget_tag_msg no-gpg-sign>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.21.0\n\n"},{"id":"376663","messageId":"nycvar.QRO.7.76.6.1906041608570.1775@tvgsbejvaqbjf.bet","threadId":"51237","inReplyTo":"20190604114356.11042-2-tigran.mkrtchyan@desy.de","subject":"Re: [PATCH v2] tag: add tag.gpgSign config option to force all tags be GPG-signed","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-06-04T14:33:08Z","receivedAt":"2019-06-04T14:33:29Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Tigran,\n\nOn Tue, 4 Jun 2019, Tigran Mkrtchyan wrote:\n\n> As may CI/CD tools don't allow to control command line options when\n\ns/may/many/, maybe?\n\n> executing `git tag` command, a default value in the configuration file\n> will allow to enforce tag signing if required.\n>\n> The new config-file option tag.gpgSign enforces signed tags. Additional\n> command line option --no-gpg-sign is added to disable such behavior if\n> needed. E.g.:\n>\n>     $ git tag -m \"commit message\"\n>\n> will generate a GPG signed tag if tag.gpgSign option is true, while\n>\n>     $ git tag --no-gpg-sign -m \"commit message\"\n>\n> will skip the signing step.\n>\n> Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>\n> ---\n>  Documentation/git-tag.txt |  7 +++++++\n>  builtin/tag.c             | 18 +++++++++++++++---\n\nHow about adding a section to Documentation/config/tag.txt as well?\n\nEven better: it could be modeled after the description of commit.gpgsign\nto which Stefan Beller linked in his review of your earlier patch (see\nhttps://public-inbox.org/git/20131105112840.GZ4589@mars-attacks.org/):\n\n  tag.gpgsign::\n\tA boolean to specify whether all tags should be GPG signed.\n\tUse of this option when running in an automated script can\n\tresult in a large number of tags being signed. It is therefore\n\tconvenient to use an agent to avoid typing your gpg passphrase\n\tseveral times.\n\n>  t/t7004-tag.sh            | 21 +++++++++++++++++++++\n>  3 files changed, 43 insertions(+), 3 deletions(-)\n>\n> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\n> index a74e7b926d..d9dbfb4e37 100644\n> --- a/Documentation/git-tag.txt\n> +++ b/Documentation/git-tag.txt\n> @@ -64,6 +64,9 @@ OPTIONS\n>  -s::\n>  --sign::\n>  \tMake a GPG-signed tag, using the default e-mail address's key.\n> +\tThe default behabior of tag GPG-signing controlled by `tag.gpgSign`\n\ns/behabior/behavior/ maybe?\n\nAnd I would also insert an \"is\" before \"controlled\".\n\n> +\tconfiguration variable if it exists, or disabled oder otherwise.\n> +\tSee linkgit:git-config[1].\n>\n>  -u <keyid>::\n>  --local-user=<keyid>::\n> @@ -193,6 +196,10 @@ This option is only applicable when listing tags without annotation lines.\n>  \tthat of linkgit:git-for-each-ref[1].  When unspecified,\n>  \tdefaults to `%(refname:strip=2)`.\n>\n> +--no-gpg-sign::\n\nShould this not be `--no-sign`? There is already a `--sign` option, and I\nwould wager a guess that it automagically handles `--no-sign`...\n\n> +\tCountermand `tag.gpgSign` configuration variable that is\n> +\tset to force each and every tag to be signed.\n\nIf you replace \"Countermand\" by \"Override\", you could simply merge this\ninto the section talking about `--sign`...\n\n\n> diff --git a/builtin/tag.c b/builtin/tag.c\n> index ef37dccf86..7f9aef4840 100644\n> --- a/builtin/tag.c\n> +++ b/builtin/tag.c\n> @@ -33,6 +33,7 @@ static const char * const git_tag_usage[] = {\n>\n>  static unsigned int colopts;\n>  static int force_sign_annotate;\n> +static int sign_tag;\n\nSince this holds the value parsed from the config, I would like to see\nsome indication of that in the name. Maybe something like\n`config_sign_tag`?\n\nAlso, I would recommend to initialize it with `-1` to be able to discern\nbetween the three states `true`, `false` and `unspecified`.\n\n>  static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting,\n>  \t\t     struct ref_format *format)\n> @@ -144,6 +145,11 @@ static int git_tag_config(const char *var, const char *value, void *cb)\n>  \tint status;\n>  \tstruct ref_sorting **sorting_tail = (struct ref_sorting **)cb;\n>\n> +\tif (!strcmp(var, \"tag.gpgsign\")) {\n> +\t\tsign_tag = git_config_bool(var, value);\n> +\t\treturn 0;\n> +\t}\n> +\n>  \tif (!strcmp(var, \"tag.sort\")) {\n>  \t\tif (!value)\n>  \t\t\treturn config_error_nonbool(var);\n> @@ -392,6 +398,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n>  \tstruct ref_format format = REF_FORMAT_INIT;\n>  \tint icase = 0;\n>  \tint edit_flag = 0;\n> +\tint no_gpg_sign = 0;\n>  \tstruct option options[] = {\n>  \t\tOPT_CMDMODE('l', \"list\", &cmdmode, N_(\"list tag names\"), 'l'),\n>  \t\t{ OPTION_INTEGER, 'n', NULL, &filter.lines, N_(\"n\"),\n> @@ -413,6 +420,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n>  \t\t\t\t\tN_(\"use another key to sign the tag\")),\n>  \t\tOPT__FORCE(&force, N_(\"replace the tag if exists\"), 0),\n>  \t\tOPT_BOOL(0, \"create-reflog\", &create_reflog, N_(\"create a reflog\")),\n> +\t\tOPT_BOOL(0, \"no-gpg-sign\", &no_gpg_sign, N_(\"do not GPG-sign tag\")),\n>\n>  \t\tOPT_GROUP(N_(\"Tag listing options\")),\n>  \t\tOPT_COLUMN(0, \"column\", &colopts, N_(\"show tag list in columns\")),\n> @@ -445,6 +453,10 @@ 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 (no_gpg_sign) {\n> +\t\tsign_tag = 0;\n> +\t}\n> +\n\nHmm. I'd rather like to see this folded into the `--no-sign` option\nimplied by the `OPT_BOOL('s', \"sign\", ...)` part. The way this would work\nwould be to initialize the struct like this:\n\n\t\tstruct create_tag_options opt = { .sign = -1 };\n\nthen leave\n\n\t\tOPT_BOOL('s', \"sign\", &opt.sign, N_(\"annotated and GPG-signed tag\")),\n\nas-is, and later do this:\n\n\tif (opt.sign < 0)\n\t\topt.sign = config_sign_tag > 0;\n\n> @@ -463,7 +475,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n>  \tif (cmdmode == 'l')\n>  \t\tsetup_auto_pager(\"tag\", 1);\n>\n> -\tif ((create_tag_object || force) && (cmdmode != 0))\n> +\tif ((create_tag_object || force || no_gpg_sign) && (cmdmode != 0))\n>  \t\tusage_with_options(git_tag_usage, options);\n\nShould we really try to be that strict? If so, we would have to test for\n`opt.sign > 0` here and make sure that above-mentioned `if (opt.sign < 0)`\nblock comes *after* this block.\n\n> @@ -556,8 +568,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n>\n>  \tcreate_reflog_msg(&object, &reflog_msg);\n>\n> -\tif (create_tag_object) {\n> -\t\tif (force_sign_annotate && !annotate)\n> +\tif (create_tag_object || sign_tag) {\n> +\t\tif (sign_tag || (force_sign_annotate && !annotate))\n>  \t\t\topt.sign = 1;\n\nThis would probably be better handled via\n\n-\tcreate_tag_object = (opt.sign || annotate || msg.given || msgfile);\n+\tcreate_tag_object = (opt.sign > 0 || config_sign_tag > 0 ||\n+\t\tannotate || msg.given || msgfile);\n\nearlier. After all, this assignment of `create_tag_object` suggests that\nit is a Boolean that catches *all* cases where a tag should be created.\n\nBut then, we would have to be very careful about this, as we don't want to\nerror out when a user calls `git tag -l` while `tag.gpgSign = true`. So\nmaybe instead:\n\n-\tcreate_tag_object = (opt.sign || annotate || msg.given || msgfile);\n+\tcreate_tag_object = (opt.sign > 0 || annotate || msg.given || msgfile);\n\nand later, after that test whether `(create_tag_object || force) &&\ncmd_mode != 0` do:\n\n\tif (opt.sign < 0) {\n\t\topt.sign = config_sign_tag > 0;\n\t\tcreate_tag_object ||= opt.sign;\n\n> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\n> index 6aeeb279a0..98a07a29d2 100755\n> --- a/t/t7004-tag.sh\n> +++ b/t/t7004-tag.sh\n> @@ -932,6 +932,27 @@ test_expect_success GPG \\\n>  \ttest_cmp expect actual\n>  '\n>\n> +get_tag_header gpgsign-enabled $commit commit $time >expect\n> +echo \"A message\" >>expect\n> +echo '-----BEGIN PGP SIGNATURE-----' >>expect\n> +test_expect_success GPG \\\n> +\t'git tag configured tag.gpgsign enables GPG sign' \\\n> +\t'test_config tag.gpgsign true &&\n> +\tgit tag -m \"A message\" gpgsign-enabled &&\n> +\tget_tag_msg gpgsign-enabled>actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +get_tag_header no-gpg-sign $commit commit $time >expect\n> +echo \"A message\" >>expect\n> +test_expect_success GPG \\\n> +\t'git tag --no-gpg-sign configured tag.gpgsign skip GPG sign' \\\n> +\t'test_config tag.gpgsign true &&\n> +\tgit tag -a --no-gpg-sign -m \"A message\" no-gpg-sign &&\n\nWith my idea above, this would of course become `--no-sign`.\n\n> +\tget_tag_msg no-gpg-sign>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\nApart from the things I suggested in the hope to help you improve the\npatch, this looks good to me so far.\n\nThanks,\nJohannes\n"},{"id":"376670","messageId":"139144069.10140489.1559664262817.JavaMail.zimbra@desy.de","threadId":"51237","inReplyTo":"nycvar.QRO.7.76.6.1906041608570.1775@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2] tag: add tag.gpgSign config option to force all tags be GPG-signed","fromName":"Mkrtchyan, Tigran","fromEmail":"tigran.mkrtchyan@desy.de","sentAt":"2019-06-04T16:04:22Z","receivedAt":"2019-06-04T16:04:28Z","isPatch":true,"sender":{"key":"tigran.mkrtchyan@desy.de","avatar":"https://avatars.githubusercontent.com/u/575965?v=4"},"body":"\n\nHi Johannes,\n\nThanks for the comments. I update the patch ant re-post.\n\nTigran.\n\n----- Original Message -----\n> From: \"Johannes Schindelin\" <Johannes.Schindelin@gmx.de>\n> To: \"Tigran Mkrtchyan\" <tigran.mkrtchyan@desy.de>\n> Cc: \"git\" <git@vger.kernel.org>, \"Jonathan Nieder\" <jrnieder@gmail.com>\n> Sent: Tuesday, June 4, 2019 4:33:08 PM\n> Subject: Re: [PATCH v2] tag: add tag.gpgSign config option to force all tags be GPG-signed\n\n> Hi Tigran,\n> \n> On Tue, 4 Jun 2019, Tigran Mkrtchyan wrote:\n> \n>> As may CI/CD tools don't allow to control command line options when\n> \n> s/may/many/, maybe?\n> \n>> executing `git tag` command, a default value in the configuration file\n>> will allow to enforce tag signing if required.\n>>\n>> The new config-file option tag.gpgSign enforces signed tags. Additional\n>> command line option --no-gpg-sign is added to disable such behavior if\n>> needed. E.g.:\n>>\n>>     $ git tag -m \"commit message\"\n>>\n>> will generate a GPG signed tag if tag.gpgSign option is true, while\n>>\n>>     $ git tag --no-gpg-sign -m \"commit message\"\n>>\n>> will skip the signing step.\n>>\n>> Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>\n>> ---\n>>  Documentation/git-tag.txt |  7 +++++++\n>>  builtin/tag.c             | 18 +++++++++++++++---\n> \n> How about adding a section to Documentation/config/tag.txt as well?\n> \n> Even better: it could be modeled after the description of commit.gpgsign\n> to which Stefan Beller linked in his review of your earlier patch (see\n> https://public-inbox.org/git/20131105112840.GZ4589@mars-attacks.org/):\n> \n>  tag.gpgsign::\n>\tA boolean to specify whether all tags should be GPG signed.\n>\tUse of this option when running in an automated script can\n>\tresult in a large number of tags being signed. It is therefore\n>\tconvenient to use an agent to avoid typing your gpg passphrase\n>\tseveral times.\n> \n>>  t/t7004-tag.sh            | 21 +++++++++++++++++++++\n>>  3 files changed, 43 insertions(+), 3 deletions(-)\n>>\n>> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\n>> index a74e7b926d..d9dbfb4e37 100644\n>> --- a/Documentation/git-tag.txt\n>> +++ b/Documentation/git-tag.txt\n>> @@ -64,6 +64,9 @@ OPTIONS\n>>  -s::\n>>  --sign::\n>>  \tMake a GPG-signed tag, using the default e-mail address's key.\n>> +\tThe default behabior of tag GPG-signing controlled by `tag.gpgSign`\n> \n> s/behabior/behavior/ maybe?\n> \n> And I would also insert an \"is\" before \"controlled\".\n> \n>> +\tconfiguration variable if it exists, or disabled oder otherwise.\n>> +\tSee linkgit:git-config[1].\n>>\n>>  -u <keyid>::\n>>  --local-user=<keyid>::\n>> @@ -193,6 +196,10 @@ This option is only applicable when listing tags without\n>> annotation lines.\n>>  \tthat of linkgit:git-for-each-ref[1].  When unspecified,\n>>  \tdefaults to `%(refname:strip=2)`.\n>>\n>> +--no-gpg-sign::\n> \n> Should this not be `--no-sign`? There is already a `--sign` option, and I\n> would wager a guess that it automagically handles `--no-sign`...\n> \n>> +\tCountermand `tag.gpgSign` configuration variable that is\n>> +\tset to force each and every tag to be signed.\n> \n> If you replace \"Countermand\" by \"Override\", you could simply merge this\n> into the section talking about `--sign`...\n> \n> \n>> diff --git a/builtin/tag.c b/builtin/tag.c\n>> index ef37dccf86..7f9aef4840 100644\n>> --- a/builtin/tag.c\n>> +++ b/builtin/tag.c\n>> @@ -33,6 +33,7 @@ static const char * const git_tag_usage[] = {\n>>\n>>  static unsigned int colopts;\n>>  static int force_sign_annotate;\n>> +static int sign_tag;\n> \n> Since this holds the value parsed from the config, I would like to see\n> some indication of that in the name. Maybe something like\n> `config_sign_tag`?\n> \n> Also, I would recommend to initialize it with `-1` to be able to discern\n> between the three states `true`, `false` and `unspecified`.\n> \n>>  static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting,\n>>  \t\t     struct ref_format *format)\n>> @@ -144,6 +145,11 @@ static int git_tag_config(const char *var, const char\n>> *value, void *cb)\n>>  \tint status;\n>>  \tstruct ref_sorting **sorting_tail = (struct ref_sorting **)cb;\n>>\n>> +\tif (!strcmp(var, \"tag.gpgsign\")) {\n>> +\t\tsign_tag = git_config_bool(var, value);\n>> +\t\treturn 0;\n>> +\t}\n>> +\n>>  \tif (!strcmp(var, \"tag.sort\")) {\n>>  \t\tif (!value)\n>>  \t\t\treturn config_error_nonbool(var);\n>> @@ -392,6 +398,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n>>  \tstruct ref_format format = REF_FORMAT_INIT;\n>>  \tint icase = 0;\n>>  \tint edit_flag = 0;\n>> +\tint no_gpg_sign = 0;\n>>  \tstruct option options[] = {\n>>  \t\tOPT_CMDMODE('l', \"list\", &cmdmode, N_(\"list tag names\"), 'l'),\n>>  \t\t{ OPTION_INTEGER, 'n', NULL, &filter.lines, N_(\"n\"),\n>> @@ -413,6 +420,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n>>  \t\t\t\t\tN_(\"use another key to sign the tag\")),\n>>  \t\tOPT__FORCE(&force, N_(\"replace the tag if exists\"), 0),\n>>  \t\tOPT_BOOL(0, \"create-reflog\", &create_reflog, N_(\"create a reflog\")),\n>> +\t\tOPT_BOOL(0, \"no-gpg-sign\", &no_gpg_sign, N_(\"do not GPG-sign tag\")),\n>>\n>>  \t\tOPT_GROUP(N_(\"Tag listing options\")),\n>>  \t\tOPT_COLUMN(0, \"column\", &colopts, N_(\"show tag list in columns\")),\n>> @@ -445,6 +453,10 @@ int cmd_tag(int argc, const char **argv, const char\n>> *prefix)\n>>\n>>  \targc = parse_options(argc, argv, prefix, options, git_tag_usage, 0);\n>>\n>> +\tif (no_gpg_sign) {\n>> +\t\tsign_tag = 0;\n>> +\t}\n>> +\n> \n> Hmm. I'd rather like to see this folded into the `--no-sign` option\n> implied by the `OPT_BOOL('s', \"sign\", ...)` part. The way this would work\n> would be to initialize the struct like this:\n> \n>\t\tstruct create_tag_options opt = { .sign = -1 };\n> \n> then leave\n> \n>\t\tOPT_BOOL('s', \"sign\", &opt.sign, N_(\"annotated and GPG-signed tag\")),\n> \n> as-is, and later do this:\n> \n>\tif (opt.sign < 0)\n>\t\topt.sign = config_sign_tag > 0;\n> \n>> @@ -463,7 +475,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n>>  \tif (cmdmode == 'l')\n>>  \t\tsetup_auto_pager(\"tag\", 1);\n>>\n>> -\tif ((create_tag_object || force) && (cmdmode != 0))\n>> +\tif ((create_tag_object || force || no_gpg_sign) && (cmdmode != 0))\n>>  \t\tusage_with_options(git_tag_usage, options);\n> \n> Should we really try to be that strict? If so, we would have to test for\n> `opt.sign > 0` here and make sure that above-mentioned `if (opt.sign < 0)`\n> block comes *after* this block.\n> \n>> @@ -556,8 +568,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n>>\n>>  \tcreate_reflog_msg(&object, &reflog_msg);\n>>\n>> -\tif (create_tag_object) {\n>> -\t\tif (force_sign_annotate && !annotate)\n>> +\tif (create_tag_object || sign_tag) {\n>> +\t\tif (sign_tag || (force_sign_annotate && !annotate))\n>>  \t\t\topt.sign = 1;\n> \n> This would probably be better handled via\n> \n> -\tcreate_tag_object = (opt.sign || annotate || msg.given || msgfile);\n> +\tcreate_tag_object = (opt.sign > 0 || config_sign_tag > 0 ||\n> +\t\tannotate || msg.given || msgfile);\n> \n> earlier. After all, this assignment of `create_tag_object` suggests that\n> it is a Boolean that catches *all* cases where a tag should be created.\n> \n> But then, we would have to be very careful about this, as we don't want to\n> error out when a user calls `git tag -l` while `tag.gpgSign = true`. So\n> maybe instead:\n> \n> -\tcreate_tag_object = (opt.sign || annotate || msg.given || msgfile);\n> +\tcreate_tag_object = (opt.sign > 0 || annotate || msg.given || msgfile);\n> \n> and later, after that test whether `(create_tag_object || force) &&\n> cmd_mode != 0` do:\n> \n>\tif (opt.sign < 0) {\n>\t\topt.sign = config_sign_tag > 0;\n>\t\tcreate_tag_object ||= opt.sign;\n> \n>> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\n>> index 6aeeb279a0..98a07a29d2 100755\n>> --- a/t/t7004-tag.sh\n>> +++ b/t/t7004-tag.sh\n>> @@ -932,6 +932,27 @@ test_expect_success GPG \\\n>>  \ttest_cmp expect actual\n>>  '\n>>\n>> +get_tag_header gpgsign-enabled $commit commit $time >expect\n>> +echo \"A message\" >>expect\n>> +echo '-----BEGIN PGP SIGNATURE-----' >>expect\n>> +test_expect_success GPG \\\n>> +\t'git tag configured tag.gpgsign enables GPG sign' \\\n>> +\t'test_config tag.gpgsign true &&\n>> +\tgit tag -m \"A message\" gpgsign-enabled &&\n>> +\tget_tag_msg gpgsign-enabled>actual &&\n>> +\ttest_cmp expect actual\n>> +'\n>> +\n>> +get_tag_header no-gpg-sign $commit commit $time >expect\n>> +echo \"A message\" >>expect\n>> +test_expect_success GPG \\\n>> +\t'git tag --no-gpg-sign configured tag.gpgsign skip GPG sign' \\\n>> +\t'test_config tag.gpgsign true &&\n>> +\tgit tag -a --no-gpg-sign -m \"A message\" no-gpg-sign &&\n> \n> With my idea above, this would of course become `--no-sign`.\n> \n>> +\tget_tag_msg no-gpg-sign>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> \n> Apart from the things I suggested in the hope to help you improve the\n> patch, this looks good to me so far.\n> \n> Thanks,\n> Johannes\n"},{"id":"376716","messageId":"20190605155300.26506-1-tigran.mkrtchyan@desy.de","threadId":"51237","inReplyTo":"139144069.10140489.1559664262817.JavaMail.zimbra@desy.de","subject":"[PATCH v3] tag: add tag.gpgSign config option to force all tags be GPG-signed","fromName":"Tigran Mkrtchyan","fromEmail":"tigran.mkrtchyan@desy.de","sentAt":"2019-06-05T15:53:00Z","receivedAt":"2019-06-05T15:53:18Z","isPatch":true,"sender":{"key":"tigran.mkrtchyan@desy.de","avatar":"https://avatars.githubusercontent.com/u/575965?v=4"},"body":"As many CI/CD tools don't allow to control command line options when\nexecuting `git tag` command, a default value in the configuration file\nwill allow to enforce tag signing if required.\n\nThe new config-file option tag.gpgSign enforces signed tags. Additional\ncommand line option --no-sign is added to disable such behavior if\nneeded. E.g.:\n\n    $ git tag -m \"commit message\"\n\nwill generate a GPG signed tag if tag.gpgSign option is true, while\n\n    $ git tag --no-sign -m \"commit message\"\n\nwill skip the signing step.\n\nThe combination of -u <key-id> and --no-sign not allowed.\n\nSigned-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>\n---\n Documentation/config/tag.txt |  7 +++++++\n Documentation/git-tag.txt    |  7 +++++++\n builtin/tag.c                | 23 ++++++++++++++++++-----\n t/t7004-tag.sh               | 21 +++++++++++++++++++++\n 4 files changed, 53 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/config/tag.txt b/Documentation/config/tag.txt\nindex 663663bdec..675483c3c3 100644\n--- a/Documentation/config/tag.txt\n+++ b/Documentation/config/tag.txt\n@@ -8,6 +8,13 @@ tag.sort::\n \tlinkgit:git-tag[1]. Without the \"--sort=<value>\" option provided, the\n \tvalue of this variable will be used as the default.\n \n+tag.gpgsign::\n+\tA boolean to specify whether all tags should be GPG signed.\n+\tUse of this option when running in an automated script can\n+\tresult in a large number of tags being signed. It is therefore\n+\tconvenient to use an agent to avoid typing your gpg passphrase\n+\tseveral times.\n+\n tar.umask::\n \tThis variable can be used to restrict the permission bits of\n \ttar archive entries.  The default is 0002, which turns off the\ndiff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\nindex a74e7b926d..2e5599a67f 100644\n--- a/Documentation/git-tag.txt\n+++ b/Documentation/git-tag.txt\n@@ -64,6 +64,13 @@ OPTIONS\n -s::\n --sign::\n \tMake a GPG-signed tag, using the default e-mail address's key.\n+\tThe default behavior of tag GPG-signing is controlled by `tag.gpgSign`\n+\tconfiguration variable if it exists, or disabled oder otherwise.\n+\tSee linkgit:git-config[1].\n+\n+--no-sign::\n+\tOverride `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>::\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex ef37dccf86..ec5fd1dcc0 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -33,6 +33,7 @@ static const char * const git_tag_usage[] = {\n \n static unsigned int colopts;\n static int force_sign_annotate;\n+static int config_sign_tag = -1; /* unspecified */\n \n static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting,\n \t\t     struct ref_format *format)\n@@ -144,6 +145,11 @@ static int git_tag_config(const char *var, const char *value, void *cb)\n \tint status;\n \tstruct ref_sorting **sorting_tail = (struct ref_sorting **)cb;\n \n+\tif (!strcmp(var, \"tag.gpgsign\")) {\n+\t\tconfig_sign_tag = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \tif (!strcmp(var, \"tag.sort\")) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n@@ -442,14 +448,12 @@ 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 (keyid) {\n-\t\topt.sign = 1;\n-\t\tset_signing_key(keyid);\n-\t}\n-\tcreate_tag_object = (opt.sign || annotate || msg.given || msgfile);\n+\tif (keyid && !opt.sign)\n+\t\tdie(_(\"user key can't be specified with --no-sign option\"));\n \n \tif (!cmdmode) {\n \t\tif (argc == 0)\n@@ -463,6 +467,15 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tif (cmdmode == 'l')\n \t\tsetup_auto_pager(\"tag\", 1);\n \n+\tif (opt.sign == -1)\n+\t\topt.sign = cmdmode ? 0 : config_sign_tag > 0;\n+\n+\tif (keyid) {\n+\t\topt.sign = 1;\n+\t\tset_signing_key(keyid);\n+\t}\n+\tcreate_tag_object = (opt.sign || annotate || msg.given || msgfile);\n+\n \tif ((create_tag_object || force) && (cmdmode != 0))\n \t\tusage_with_options(git_tag_usage, options);\n \ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 6aeeb279a0..80eb13d94e 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -932,6 +932,27 @@ test_expect_success GPG \\\n \ttest_cmp expect actual\n '\n \n+get_tag_header gpgsign-enabled $commit commit $time >expect\n+echo \"A message\" >>expect\n+echo '-----BEGIN PGP SIGNATURE-----' >>expect\n+test_expect_success GPG \\\n+\t'git tag configured tag.gpgsign enables GPG sign' \\\n+\t'test_config tag.gpgsign true &&\n+\tgit tag -m \"A message\" gpgsign-enabled &&\n+\tget_tag_msg gpgsign-enabled>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+get_tag_header no-sign $commit commit $time >expect\n+echo \"A message\" >>expect\n+test_expect_success GPG \\\n+\t'git tag --no-sign configured tag.gpgsign skip GPG sign' \\\n+\t'test_config tag.gpgsign true &&\n+\tgit tag -a --no-sign -m \"A message\" no-sign &&\n+\tget_tag_msg no-sign>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.22.0.rc3.1.gdcc2db3530.dirty\n\n"},{"id":"376718","messageId":"20190605162101.GX3654@pobox.com","threadId":"51237","inReplyTo":"20190605155300.26506-1-tigran.mkrtchyan@desy.de","subject":"Re: [PATCH v3] tag: add tag.gpgSign config option to force all tags be GPG-signed","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2019-06-05T16:21:01Z","receivedAt":"2019-06-05T16:21:11Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Hi,\n\nTigran Mkrtchyan wrote:\n> diff --git a/Documentation/config/tag.txt b/Documentation/config/tag.txt\n> index 663663bdec..675483c3c3 100644\n> --- a/Documentation/config/tag.txt\n> +++ b/Documentation/config/tag.txt\n> @@ -8,6 +8,13 @@ tag.sort::\n>  \tlinkgit:git-tag[1]. Without the \"--sort=<value>\" option provided, the\n>  \tvalue of this variable will be used as the default.\n>  \n> +tag.gpgsign::\n> +\tA boolean to specify whether all tags should be GPG signed.\n> +\tUse of this option when running in an automated script can\n> +\tresult in a large number of tags being signed. It is therefore\n> +\tconvenient to use an agent to avoid typing your gpg passphrase\n> +\tseveral times.\n\nI think the variable should be camelCased, i.e. tag.gpgSign,\nfor consistency with other documentation.\n\n-- \nTodd\n"},{"id":"376719","messageId":"xmqqv9xkroc5.fsf@gitster-ct.c.googlers.com","threadId":"51237","inReplyTo":"20190605155300.26506-1-tigran.mkrtchyan@desy.de","subject":"Re: [PATCH v3] tag: add tag.gpgSign config option to force all tags be GPG-signed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-06-05T16:25:46Z","receivedAt":"2019-06-05T16:25:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tigran Mkrtchyan <tigran.mkrtchyan@desy.de> writes:\n\n> As many CI/CD tools don't allow to control command line options when\n> executing `git tag` command, a default value in the configuration file\n> will allow to enforce tag signing if required.\n\nHmm.  Would these \"many\" tools still allow arbigrary configuration\nset to affect their operation?  It sounds like a bigger issue but it\nis a separate one.\n\n> The new config-file option tag.gpgSign enforces signed tags. Additional\n> ...\n> will skip the signing step.\n\nThis paragraph is well written.\n\n> The combination of -u <key-id> and --no-sign not allowed.\n\nThis sentence lacks a verb.  Perhaps s/not allowed/is &/.  \n\nBut more importantly, I think we should justify why this \"not\nallowed\" makes sense as the design of the feature. A plausible\nalternative design would simply follow the \"last one wins\" paradigm,\nwhere\n\n    git tag -u key # \"-u key\" implies \"-s\"\n\n    git tag -u key --no-sign # \"--no-sign' trumps the implied \"-s\"\n\n    git tag --no-sign -u key # \"-u key\"'s implication of \"-s\" trumps the\n\t\t\t     # earlier \"--no-sign\"\n\nand having \"[tag] gpgsign\" simply adds to the implication early in\nthe chain to be overridden by later command line options.\n\nLet's explain why \"you cannot give -u <key> and --no-sign at the\nsame time\" is better than \"the last one wins\".\n\n> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\n> index a74e7b926d..2e5599a67f 100644\n> --- a/Documentation/git-tag.txt\n> +++ b/Documentation/git-tag.txt\n> @@ -64,6 +64,13 @@ OPTIONS\n>  -s::\n>  --sign::\n>  \tMake a GPG-signed tag, using the default e-mail address's key.\n> +\tThe default behavior of tag GPG-signing is controlled by `tag.gpgSign`\n> +\tconfiguration variable if it exists, or disabled oder otherwise.\n> +\tSee linkgit:git-config[1].\n> +\n> +--no-sign::\n> +\tOverride `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\nIf we justify \"-u and --no-sign do not mix\", that design needs to be\nexplained to the end users in the documentation, not just in the\nproposed log messsage.\n\n"},{"id":"376731","messageId":"1933659713.10490718.1559765529240.JavaMail.zimbra@desy.de","threadId":"51237","inReplyTo":"xmqqv9xkroc5.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3] tag: add tag.gpgSign config option to force all tags be GPG-signed","fromName":"Mkrtchyan, Tigran","fromEmail":"tigran.mkrtchyan@desy.de","sentAt":"2019-06-05T20:12:09Z","receivedAt":"2019-06-05T20:12:13Z","isPatch":true,"sender":{"key":"tigran.mkrtchyan@desy.de","avatar":"https://avatars.githubusercontent.com/u/575965?v=4"},"body":"\n\n----- Original Message -----\n> From: \"Junio C Hamano\" <gitster@pobox.com>\n> To: \"Tigran Mkrtchyan\" <tigran.mkrtchyan@desy.de>\n> Cc: \"git\" <git@vger.kernel.org>, \"Jonathan Nieder\" <jrnieder@gmail.com>, \"Johannes Schindelin\"\n> <Johannes.Schindelin@gmx.de>\n> Sent: Wednesday, June 5, 2019 6:25:46 PM\n> Subject: Re: [PATCH v3] tag: add tag.gpgSign config option to force all tags be GPG-signed\n\n> Tigran Mkrtchyan <tigran.mkrtchyan@desy.de> writes:\n> \n>> As many CI/CD tools don't allow to control command line options when\n>> executing `git tag` command, a default value in the configuration file\n>> will allow to enforce tag signing if required.\n\nMust of them blindly execute git commands with some hard-coded combination of\noptions. It's clear to me, that they are the source of the problem, but\ngit can be the solution.\n\n> \n> Hmm.  Would these \"many\" tools still allow arbigrary configuration\n> set to affect their operation?  It sounds like a bigger issue but it\n> is a separate one.\n> \n>> The new config-file option tag.gpgSign enforces signed tags. Additional\n>> ...\n>> will skip the signing step.\n> \n> This paragraph is well written.\n> \n>> The combination of -u <key-id> and --no-sign not allowed.\n> \n> This sentence lacks a verb.  Perhaps s/not allowed/is &/.\n> \n\nJup. Sorry.\n\n> But more importantly, I think we should justify why this \"not\n> allowed\" makes sense as the design of the feature. A plausible\n> alternative design would simply follow the \"last one wins\" paradigm,\n> where\n> \n>    git tag -u key # \"-u key\" implies \"-s\"\n> \n>    git tag -u key --no-sign # \"--no-sign' trumps the implied \"-s\"\n> \n>    git tag --no-sign -u key # \"-u key\"'s implication of \"-s\" trumps the\n>\t\t\t     # earlier \"--no-sign\"\n> \n> and having \"[tag] gpgsign\" simply adds to the implication early in\n> the chain to be overridden by later command line options.\n> \n> Let's explain why \"you cannot give -u <key> and --no-sign at the\n> same time\" is better than \"the last one wins\".\n\nThis is matter of convention. I never pay attention to the order of options\non the git command line, but I don't put conflicting options either, I hope.\nDoes git already have position depended options? If yes, then fine with me.\nOtherwise, I don't want to introduce ambiguity. Yes, less ambiguity is my\nanswer to why it's better than \"the last one wins\".\n\n\n> \n>> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\n>> index a74e7b926d..2e5599a67f 100644\n>> --- a/Documentation/git-tag.txt\n>> +++ b/Documentation/git-tag.txt\n>> @@ -64,6 +64,13 @@ OPTIONS\n>>  -s::\n>>  --sign::\n>>  \tMake a GPG-signed tag, using the default e-mail address's key.\n>> +\tThe default behavior of tag GPG-signing is controlled by `tag.gpgSign`\n>> +\tconfiguration variable if it exists, or disabled oder otherwise.\n>> +\tSee linkgit:git-config[1].\n>> +\n>> +--no-sign::\n>> +\tOverride `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> \n> If we justify \"-u and --no-sign do not mix\", that design needs to be\n> explained to the end users in the documentation, not just in the\n> proposed log messsage.\n\nMake sense.\n\nThanks,\n   Tigran.\n"},{"id":"376734","messageId":"xmqqblzbsquq.fsf@gitster-ct.c.googlers.com","threadId":"51237","inReplyTo":"1933659713.10490718.1559765529240.JavaMail.zimbra@desy.de","subject":"Re: [PATCH v3] tag: add tag.gpgSign config option to force all tags be GPG-signed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-06-05T20:46:05Z","receivedAt":"2019-06-05T20:46:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Mkrtchyan, Tigran\" <tigran.mkrtchyan@desy.de> writes:\n\n>> But more importantly, I think we should justify why this \"not\n>> allowed\" makes sense as the design of the feature. A plausible\n>> alternative design would simply follow the \"last one wins\" paradigm,\n>> ...\n> This is matter of convention.\n\nOh, if you put it that way, it is quite clear that we should redo\nthis part, as the rest of Git command line processing all pretty\nmuch follow \"last one wins\".  I was mostly curious if there was any\nstrong reason why this combination has to be different.\n"},{"id":"376735","messageId":"327924049.10494298.1559767809461.JavaMail.zimbra@desy.de","threadId":"51237","inReplyTo":"xmqqblzbsquq.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3] tag: add tag.gpgSign config option to force all tags be GPG-signed","fromName":"Mkrtchyan, Tigran","fromEmail":"tigran.mkrtchyan@desy.de","sentAt":"2019-06-05T20:50:09Z","receivedAt":"2019-06-05T20:50:12Z","isPatch":true,"sender":{"key":"tigran.mkrtchyan@desy.de","avatar":"https://avatars.githubusercontent.com/u/575965?v=4"},"body":"Thanks for clarification. I will update the patch and re-send it.\n\nTigran.\n\n----- Original Message -----\n> From: \"Junio C Hamano\" <gitster@pobox.com>\n> To: \"Tigran Mkrtchyan\" <tigran.mkrtchyan@desy.de>\n> Cc: \"git\" <git@vger.kernel.org>, \"Jonathan Nieder\" <jrnieder@gmail.com>, \"Johannes Schindelin\"\n> <Johannes.Schindelin@gmx.de>\n> Sent: Wednesday, June 5, 2019 10:46:05 PM\n> Subject: Re: [PATCH v3] tag: add tag.gpgSign config option to force all tags be GPG-signed\n\n> \"Mkrtchyan, Tigran\" <tigran.mkrtchyan@desy.de> writes:\n> \n>>> But more importantly, I think we should justify why this \"not\n>>> allowed\" makes sense as the design of the feature. A plausible\n>>> alternative design would simply follow the \"last one wins\" paradigm,\n>>> ...\n>> This is matter of convention.\n> \n> Oh, if you put it that way, it is quite clear that we should redo\n> this part, as the rest of Git command line processing all pretty\n> much follow \"last one wins\".  I was mostly curious if there was any\n> strong reason why this combination has to be different.\n"},{"id":"376736","messageId":"xmqqzhmvrbr5.fsf@gitster-ct.c.googlers.com","threadId":"51237","inReplyTo":"xmqqblzbsquq.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3] tag: add tag.gpgSign config option to force all tags be GPG-signed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-06-05T20:57:34Z","receivedAt":"2019-06-05T20:57:42Z","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> \"Mkrtchyan, Tigran\" <tigran.mkrtchyan@desy.de> writes:\n>\n>>> But more importantly, I think we should justify why this \"not\n>>> allowed\" makes sense as the design of the feature. A plausible\n>>> alternative design would simply follow the \"last one wins\" paradigm,\n>>> ...\n>> This is matter of convention.\n>\n> Oh, if you put it that way, it is quite clear that we should redo\n> this part, as the rest of Git command line processing all pretty\n> much follow \"last one wins\".  I was mostly curious if there was any\n> strong reason why this combination has to be different.\n\nOne reason why \"last one wins\" aka \"you have a chance to correct\nyourself\" is so useful is because people can then use alias to\ndefine leading options, e.g. with\n\n    [alias] myfoo = foo --option1 --option2 --option3\n\na user can say \"git myfoo\" to save typing, but in a rare occasion\nwhere only options 2 and 3 (but not option 1) are desired, can use\n\"git myfoo --no-option1\" to countermand it.\n\nUnfortunately, I do not think we can use the same technique for\noptions that is given to the \"git\" potty, e.g. with\n\n    [alias] paged-status = --paginate status\n\nyou can say \"git paged-status\" to have your \"git status\" output sent\nto the pager, but you cannot say \"git paged-status --no-pager\" to\ndefeat that leading option.  We may want to find a way to correct\nit, but it is rather low on the priority scale ;-)\n\n\n"},{"id":"376737","messageId":"2146612842.10496277.1559768996738.JavaMail.zimbra@desy.de","threadId":"51237","inReplyTo":"xmqqzhmvrbr5.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3] tag: add tag.gpgSign config option to force all tags be GPG-signed","fromName":"Mkrtchyan, Tigran","fromEmail":"tigran.mkrtchyan@desy.de","sentAt":"2019-06-05T21:09:56Z","receivedAt":"2019-06-05T21:10:01Z","isPatch":true,"sender":{"key":"tigran.mkrtchyan@desy.de","avatar":"https://avatars.githubusercontent.com/u/575965?v=4"},"body":"\n\n----- Original Message -----\n> From: \"Junio C Hamano\" <gitster@pobox.com>\n> To: \"Tigran Mkrtchyan\" <tigran.mkrtchyan@desy.de>\n> Cc: \"git\" <git@vger.kernel.org>, \"Jonathan Nieder\" <jrnieder@gmail.com>, \"Johannes Schindelin\"\n> <Johannes.Schindelin@gmx.de>\n> Sent: Wednesday, June 5, 2019 10:57:34 PM\n> Subject: Re: [PATCH v3] tag: add tag.gpgSign config option to force all tags be GPG-signed\n\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n>> \"Mkrtchyan, Tigran\" <tigran.mkrtchyan@desy.de> writes:\n>>\n>>>> But more importantly, I think we should justify why this \"not\n>>>> allowed\" makes sense as the design of the feature. A plausible\n>>>> alternative design would simply follow the \"last one wins\" paradigm,\n>>>> ...\n>>> This is matter of convention.\n>>\n>> Oh, if you put it that way, it is quite clear that we should redo\n>> this part, as the rest of Git command line processing all pretty\n>> much follow \"last one wins\".  I was mostly curious if there was any\n>> strong reason why this combination has to be different.\n> \n> One reason why \"last one wins\" aka \"you have a chance to correct\n> yourself\" is so useful is because people can then use alias to\n> define leading options, e.g. with\n> \n>    [alias] myfoo = foo --option1 --option2 --option3\n> \n> a user can say \"git myfoo\" to save typing, but in a rare occasion\n> where only options 2 and 3 (but not option 1) are desired, can use\n> \"git myfoo --no-option1\" to countermand it.\n> \n> Unfortunately, I do not think we can use the same technique for\n> options that is given to the \"git\" potty, e.g. with\n> \n>    [alias] paged-status = --paginate status\n> \n> you can say \"git paged-status\" to have your \"git status\" output sent\n> to the pager, but you cannot say \"git paged-status --no-pager\" to\n> defeat that leading option.  We may want to find a way to correct\n> it, but it is rather low on the priority scale ;-)\n\n\nMakes sense. Thanks for that use case, which I actually use myself :).\n\nBTW, `--no-sign` currently doesn't affects `-u <key>` in any order:\n\n```\n    if (keyid) {\n        opt.sign = 1;\n        set_signing_key(keyid);\n    }\n```\n\nIOW, `-u <key>` is another way to enable signing, what is, actually,\nconsistent with man page:\n\n```\n       -u <keyid>, --local-user=<keyid>\n           Make a GPG-signed tag, using the given key.\n```\n\nI will leave it as-is, I will just describe that tag.gpgSign options\ndoesn't affect '-u' behavior.\n\nTigran.\n"},{"id":"376739","messageId":"20190605213321.6577-1-tigran.mkrtchyan@desy.de","threadId":"51237","inReplyTo":"2146612842.10496277.1559768996738.JavaMail.zimbra@desy.de","subject":"[PATCH v4] tag: add tag.gpgSign config option to force all tags be GPG-signed","fromName":"Tigran Mkrtchyan","fromEmail":"tigran.mkrtchyan@desy.de","sentAt":"2019-06-05T21:33:21Z","receivedAt":"2019-06-05T21:33:31Z","isPatch":true,"sender":{"key":"tigran.mkrtchyan@desy.de","avatar":"https://avatars.githubusercontent.com/u/575965?v=4"},"body":"As many CI/CD tools don't allow to control command line options when\nexecuting `git tag` command, a default value in the configuration file\nwill allow to enforce tag signing if required.\n\nThe new config-file option tag.gpgSign is added to define default behavior\nof tag signings. To override default behavior the command line option -s,\n--sign and --no-sign can be used:\n\n    $ git tag -m \"commit message\"\n\nwill generate a GPG signed tag if tag.gpgSign option is true, while\n\n    $ git tag --no-sign -m \"commit message\"\n\nwill skip the signing step.\n\nSigned-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>\n---\n Documentation/config/tag.txt |  8 ++++++++\n Documentation/git-tag.txt    |  7 +++++++\n builtin/tag.c                | 22 ++++++++++++++++------\n t/t7004-tag.sh               | 21 +++++++++++++++++++++\n 4 files changed, 52 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/config/tag.txt b/Documentation/config/tag.txt\nindex 663663bdec..ef5adb3f42 100644\n--- a/Documentation/config/tag.txt\n+++ b/Documentation/config/tag.txt\n@@ -8,6 +8,14 @@ tag.sort::\n \tlinkgit:git-tag[1]. Without the \"--sort=<value>\" option provided, the\n \tvalue of this variable will be used as the default.\n \n+tag.gpgSign::\n+\tA boolean to specify whether all tags should be GPG signed.\n+\tUse of this option when running in an automated script can\n+\tresult in a large number of tags being signed. It is therefore\n+\tconvenient to use an agent to avoid typing your gpg passphrase\n+\tseveral times. Note that this option doesn't affects tag signing\n+\tbehavior enabled by \"-u <keyid>\" or \"--local-user=<keyid>\" options.\n+\n tar.umask::\n \tThis variable can be used to restrict the permission bits of\n \ttar archive entries.  The default is 0002, which turns off the\ndiff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\nindex a74e7b926d..2e5599a67f 100644\n--- a/Documentation/git-tag.txt\n+++ b/Documentation/git-tag.txt\n@@ -64,6 +64,13 @@ OPTIONS\n -s::\n --sign::\n \tMake a GPG-signed tag, using the default e-mail address's key.\n+\tThe default behavior of tag GPG-signing is controlled by `tag.gpgSign`\n+\tconfiguration variable if it exists, or disabled oder otherwise.\n+\tSee linkgit:git-config[1].\n+\n+--no-sign::\n+\tOverride `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>::\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex ef37dccf86..e0a4c25382 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -33,6 +33,7 @@ static const char * const git_tag_usage[] = {\n \n static unsigned int colopts;\n static int force_sign_annotate;\n+static int config_sign_tag = -1; /* unspecified */\n \n static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting,\n \t\t     struct ref_format *format)\n@@ -144,6 +145,11 @@ static int git_tag_config(const char *var, const char *value, void *cb)\n \tint status;\n \tstruct ref_sorting **sorting_tail = (struct ref_sorting **)cb;\n \n+\tif (!strcmp(var, \"tag.gpgsign\")) {\n+\t\tconfig_sign_tag = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \tif (!strcmp(var, \"tag.sort\")) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n@@ -442,15 +448,10 @@ 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 (keyid) {\n-\t\topt.sign = 1;\n-\t\tset_signing_key(keyid);\n-\t}\n-\tcreate_tag_object = (opt.sign || annotate || msg.given || msgfile);\n-\n \tif (!cmdmode) {\n \t\tif (argc == 0)\n \t\t\tcmdmode = 'l';\n@@ -463,6 +464,15 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tif (cmdmode == 'l')\n \t\tsetup_auto_pager(\"tag\", 1);\n \n+\tif (opt.sign == -1)\n+\t\topt.sign = cmdmode ? 0 : config_sign_tag > 0;\n+\n+\tif (keyid) {\n+\t\topt.sign = 1;\n+\t\tset_signing_key(keyid);\n+\t}\n+\tcreate_tag_object = (opt.sign || annotate || msg.given || msgfile);\n+\n \tif ((create_tag_object || force) && (cmdmode != 0))\n \t\tusage_with_options(git_tag_usage, options);\n \ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 6aeeb279a0..80eb13d94e 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -932,6 +932,27 @@ test_expect_success GPG \\\n \ttest_cmp expect actual\n '\n \n+get_tag_header gpgsign-enabled $commit commit $time >expect\n+echo \"A message\" >>expect\n+echo '-----BEGIN PGP SIGNATURE-----' >>expect\n+test_expect_success GPG \\\n+\t'git tag configured tag.gpgsign enables GPG sign' \\\n+\t'test_config tag.gpgsign true &&\n+\tgit tag -m \"A message\" gpgsign-enabled &&\n+\tget_tag_msg gpgsign-enabled>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+get_tag_header no-sign $commit commit $time >expect\n+echo \"A message\" >>expect\n+test_expect_success GPG \\\n+\t'git tag --no-sign configured tag.gpgsign skip GPG sign' \\\n+\t'test_config tag.gpgsign true &&\n+\tgit tag -a --no-sign -m \"A message\" no-sign &&\n+\tget_tag_msg no-sign>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.22.0.rc3.1.gdbadaa0575\n\n"}]}