config option to force all tags be GPG-signed - comeback

config option to force all tags be GPG-signed - comeback

[Tigran Mkrtchyan]

As there was some interest on my original pull request on github[1], 
I decided to rebase, update the commit message and re-post this patch.


[1] https://github.com/git/git/pull/419

[PATCH v2] tag: add tag.gpgSign config option to force all tags be GPG-signed

[Tigran Mkrtchyan]

As may CI/CD tools don't allow to control command line options when
executing `git tag` command, a default value in the configuration file
will allow to enforce tag signing if required.

The new config-file option tag.gpgSign enforces signed tags. Additional
command line option --no-gpg-sign is added to disable such behavior if
needed. E.g.:

    $ git tag -m "commit message"

will generate a GPG signed tag if tag.gpgSign option is true, while

    $ git tag --no-gpg-sign -m "commit message"

will skip the signing step.

Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
---
 Documentation/git-tag.txt |  7 +++++++
 builtin/tag.c             | 18 +++++++++++++++---
 t/t7004-tag.sh            | 21 +++++++++++++++++++++
 3 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index a74e7b926d..d9dbfb4e37 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -64,6 +64,9 @@ OPTIONS
 -s::
 --sign::
 	Make a GPG-signed tag, using the default e-mail address's key.
+	The default behabior of tag GPG-signing controlled by `tag.gpgSign`
+	configuration variable if it exists, or disabled oder otherwise.
+	See linkgit:git-config[1].
 
 -u <keyid>::
 --local-user=<keyid>::
@@ -193,6 +196,10 @@ This option is only applicable when listing tags without annotation lines.
 	that of linkgit:git-for-each-ref[1].  When unspecified,
 	defaults to `%(refname:strip=2)`.
 
+--no-gpg-sign::
+	Countermand `tag.gpgSign` configuration variable that is
+	set to force each and every tag to be signed.
+
 <tagname>::
 	The name of the tag to create, delete, or describe.
 	The new tag name must pass all checks defined by
diff --git a/builtin/tag.c b/builtin/tag.c
index ef37dccf86..7f9aef4840 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -33,6 +33,7 @@ static const char * const git_tag_usage[] = {
 
 static unsigned int colopts;
 static int force_sign_annotate;
+static int sign_tag;
 
 static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting,
 		     struct ref_format *format)
@@ -144,6 +145,11 @@ static int git_tag_config(const char *var, const char *value, void *cb)
 	int status;
 	struct ref_sorting **sorting_tail = (struct ref_sorting **)cb;
 
+	if (!strcmp(var, "tag.gpgsign")) {
+		sign_tag = git_config_bool(var, value);
+		return 0;
+	}
+
 	if (!strcmp(var, "tag.sort")) {
 		if (!value)
 			return config_error_nonbool(var);
@@ -392,6 +398,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	struct ref_format format = REF_FORMAT_INIT;
 	int icase = 0;
 	int edit_flag = 0;
+	int no_gpg_sign = 0;
 	struct option options[] = {
 		OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'),
 		{ OPTION_INTEGER, 'n', NULL, &filter.lines, N_("n"),
@@ -413,6 +420,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 					N_("use another key to sign the tag")),
 		OPT__FORCE(&force, N_("replace the tag if exists"), 0),
 		OPT_BOOL(0, "create-reflog", &create_reflog, N_("create a reflog")),
+		OPT_BOOL(0, "no-gpg-sign", &no_gpg_sign, N_("do not GPG-sign tag")),
 
 		OPT_GROUP(N_("Tag listing options")),
 		OPT_COLUMN(0, "column", &colopts, N_("show tag list in columns")),
@@ -445,6 +453,10 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 
 	argc = parse_options(argc, argv, prefix, options, git_tag_usage, 0);
 
+	if (no_gpg_sign) {
+		sign_tag = 0;
+	}
+
 	if (keyid) {
 		opt.sign = 1;
 		set_signing_key(keyid);
@@ -463,7 +475,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	if (cmdmode == 'l')
 		setup_auto_pager("tag", 1);
 
-	if ((create_tag_object || force) && (cmdmode != 0))
+	if ((create_tag_object || force || no_gpg_sign) && (cmdmode != 0))
 		usage_with_options(git_tag_usage, options);
 
 	finalize_colopts(&colopts, -1);
@@ -556,8 +568,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 
 	create_reflog_msg(&object, &reflog_msg);
 
-	if (create_tag_object) {
-		if (force_sign_annotate && !annotate)
+	if (create_tag_object || sign_tag) {
+		if (sign_tag || (force_sign_annotate && !annotate))
 			opt.sign = 1;
 		create_tag(&object, object_ref, tag, &buf, &opt, &prev, &object);
 	}
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 6aeeb279a0..98a07a29d2 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -932,6 +932,27 @@ test_expect_success GPG \
 	test_cmp expect actual
 '
 
+get_tag_header gpgsign-enabled $commit commit $time >expect
+echo "A message" >>expect
+echo '-----BEGIN PGP SIGNATURE-----' >>expect
+test_expect_success GPG \
+	'git tag configured tag.gpgsign enables GPG sign' \
+	'test_config tag.gpgsign true &&
+	git tag -m "A message" gpgsign-enabled &&
+	get_tag_msg gpgsign-enabled>actual &&
+	test_cmp expect actual
+'
+
+get_tag_header no-gpg-sign $commit commit $time >expect
+echo "A message" >>expect
+test_expect_success GPG \
+	'git tag --no-gpg-sign configured tag.gpgsign skip GPG sign' \
+	'test_config tag.gpgsign true &&
+	git tag -a --no-gpg-sign -m "A message" no-gpg-sign &&
+	get_tag_msg no-gpg-sign>actual &&
+	test_cmp expect actual
+'
+
 test_expect_success GPG \
 	'trying to create a signed tag with non-existing -F file should fail' '
 	! test -f nonexistingfile &&
-- 
2.21.0

Re: [PATCH v2] tag: add tag.gpgSign config option to force all tags be GPG-signed

[Johannes Schindelin]

Hi Tigran,

On Tue, 4 Jun 2019, Tigran Mkrtchyan wrote:

> As may CI/CD tools don't allow to control command line options when

s/may/many/, maybe?

> executing `git tag` command, a default value in the configuration file
> will allow to enforce tag signing if required.
>
> The new config-file option tag.gpgSign enforces signed tags. Additional
> command line option --no-gpg-sign is added to disable such behavior if
> needed. E.g.:
>
>     $ git tag -m "commit message"
>
> will generate a GPG signed tag if tag.gpgSign option is true, while
>
>     $ git tag --no-gpg-sign -m "commit message"
>
> will skip the signing step.
>
> Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
> ---
>  Documentation/git-tag.txt |  7 +++++++
>  builtin/tag.c             | 18 +++++++++++++++---

How about adding a section to Documentation/config/tag.txt as well?

Even better: it could be modeled after the description of commit.gpgsign
to which Stefan Beller linked in his review of your earlier patch (see
https://public-inbox.org/git/20131105112840.GZ4589@mars-attacks.org/):

  tag.gpgsign::
	A boolean to specify whether all tags should be GPG signed.
	Use of this option when running in an automated script can
	result in a large number of tags being signed. It is therefore
	convenient to use an agent to avoid typing your gpg passphrase
	several times.

>  t/t7004-tag.sh            | 21 +++++++++++++++++++++
>  3 files changed, 43 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
> index a74e7b926d..d9dbfb4e37 100644
> --- a/Documentation/git-tag.txt
> +++ b/Documentation/git-tag.txt
> @@ -64,6 +64,9 @@ OPTIONS
>  -s::
>  --sign::
>  	Make a GPG-signed tag, using the default e-mail address's key.
> +	The default behabior of tag GPG-signing controlled by `tag.gpgSign`

s/behabior/behavior/ maybe?

And I would also insert an "is" before "controlled".

> +	configuration variable if it exists, or disabled oder otherwise.
> +	See linkgit:git-config[1].
>
>  -u <keyid>::
>  --local-user=<keyid>::
> @@ -193,6 +196,10 @@ This option is only applicable when listing tags without annotation lines.
>  	that of linkgit:git-for-each-ref[1].  When unspecified,
>  	defaults to `%(refname:strip=2)`.
>
> +--no-gpg-sign::

Should this not be `--no-sign`? There is already a `--sign` option, and I
would wager a guess that it automagically handles `--no-sign`...

> +	Countermand `tag.gpgSign` configuration variable that is
> +	set to force each and every tag to be signed.

If you replace "Countermand" by "Override", you could simply merge this
into the section talking about `--sign`...


> diff --git a/builtin/tag.c b/builtin/tag.c
> index ef37dccf86..7f9aef4840 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -33,6 +33,7 @@ static const char * const git_tag_usage[] = {
>
>  static unsigned int colopts;
>  static int force_sign_annotate;
> +static int sign_tag;

Since this holds the value parsed from the config, I would like to see
some indication of that in the name. Maybe something like
`config_sign_tag`?

Also, I would recommend to initialize it with `-1` to be able to discern
between the three states `true`, `false` and `unspecified`.

>  static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting,
>  		     struct ref_format *format)
> @@ -144,6 +145,11 @@ static int git_tag_config(const char *var, const char *value, void *cb)
>  	int status;
>  	struct ref_sorting **sorting_tail = (struct ref_sorting **)cb;
>
> +	if (!strcmp(var, "tag.gpgsign")) {
> +		sign_tag = git_config_bool(var, value);
> +		return 0;
> +	}
> +
>  	if (!strcmp(var, "tag.sort")) {
>  		if (!value)
>  			return config_error_nonbool(var);
> @@ -392,6 +398,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
>  	struct ref_format format = REF_FORMAT_INIT;
>  	int icase = 0;
>  	int edit_flag = 0;
> +	int no_gpg_sign = 0;
>  	struct option options[] = {
>  		OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'),
>  		{ OPTION_INTEGER, 'n', NULL, &filter.lines, N_("n"),
> @@ -413,6 +420,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
>  					N_("use another key to sign the tag")),
>  		OPT__FORCE(&force, N_("replace the tag if exists"), 0),
>  		OPT_BOOL(0, "create-reflog", &create_reflog, N_("create a reflog")),
> +		OPT_BOOL(0, "no-gpg-sign", &no_gpg_sign, N_("do not GPG-sign tag")),
>
>  		OPT_GROUP(N_("Tag listing options")),
>  		OPT_COLUMN(0, "column", &colopts, N_("show tag list in columns")),
> @@ -445,6 +453,10 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
>
>  	argc = parse_options(argc, argv, prefix, options, git_tag_usage, 0);
>
> +	if (no_gpg_sign) {
> +		sign_tag = 0;
> +	}
> +

Hmm. I'd rather like to see this folded into the `--no-sign` option
implied by the `OPT_BOOL('s', "sign", ...)` part. The way this would work
would be to initialize the struct like this:

		struct create_tag_options opt = { .sign = -1 };

then leave

		OPT_BOOL('s', "sign", &opt.sign, N_("annotated and GPG-signed tag")),

as-is, and later do this:

	if (opt.sign < 0)
		opt.sign = config_sign_tag > 0;

> @@ -463,7 +475,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
>  	if (cmdmode == 'l')
>  		setup_auto_pager("tag", 1);
>
> -	if ((create_tag_object || force) && (cmdmode != 0))
> +	if ((create_tag_object || force || no_gpg_sign) && (cmdmode != 0))
>  		usage_with_options(git_tag_usage, options);

Should we really try to be that strict? If so, we would have to test for
`opt.sign > 0` here and make sure that above-mentioned `if (opt.sign < 0)`
block comes *after* this block.

> @@ -556,8 +568,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
>
>  	create_reflog_msg(&object, &reflog_msg);
>
> -	if (create_tag_object) {
> -		if (force_sign_annotate && !annotate)
> +	if (create_tag_object || sign_tag) {
> +		if (sign_tag || (force_sign_annotate && !annotate))
>  			opt.sign = 1;

This would probably be better handled via

-	create_tag_object = (opt.sign || annotate || msg.given || msgfile);
+	create_tag_object = (opt.sign > 0 || config_sign_tag > 0 ||
+		annotate || msg.given || msgfile);

earlier. After all, this assignment of `create_tag_object` suggests that
it is a Boolean that catches *all* cases where a tag should be created.

But then, we would have to be very careful about this, as we don't want to
error out when a user calls `git tag -l` while `tag.gpgSign = true`. So
maybe instead:

-	create_tag_object = (opt.sign || annotate || msg.given || msgfile);
+	create_tag_object = (opt.sign > 0 || annotate || msg.given || msgfile);

and later, after that test whether `(create_tag_object || force) &&
cmd_mode != 0` do:

	if (opt.sign < 0) {
		opt.sign = config_sign_tag > 0;
		create_tag_object ||= opt.sign;

> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
> index 6aeeb279a0..98a07a29d2 100755
> --- a/t/t7004-tag.sh
> +++ b/t/t7004-tag.sh
> @@ -932,6 +932,27 @@ test_expect_success GPG \
>  	test_cmp expect actual
>  '
>
> +get_tag_header gpgsign-enabled $commit commit $time >expect
> +echo "A message" >>expect
> +echo '-----BEGIN PGP SIGNATURE-----' >>expect
> +test_expect_success GPG \
> +	'git tag configured tag.gpgsign enables GPG sign' \
> +	'test_config tag.gpgsign true &&
> +	git tag -m "A message" gpgsign-enabled &&
> +	get_tag_msg gpgsign-enabled>actual &&
> +	test_cmp expect actual
> +'
> +
> +get_tag_header no-gpg-sign $commit commit $time >expect
> +echo "A message" >>expect
> +test_expect_success GPG \
> +	'git tag --no-gpg-sign configured tag.gpgsign skip GPG sign' \
> +	'test_config tag.gpgsign true &&
> +	git tag -a --no-gpg-sign -m "A message" no-gpg-sign &&

With my idea above, this would of course become `--no-sign`.

> +	get_tag_msg no-gpg-sign>actual &&
> +	test_cmp expect actual
> +'
> +
>  test_expect_success GPG \
>  	'trying to create a signed tag with non-existing -F file should fail' '
>  	! test -f nonexistingfile &&

Apart from the things I suggested in the hope to help you improve the
patch, this looks good to me so far.

Thanks,
Johannes

Re: [PATCH v2] tag: add tag.gpgSign config option to force all tags be GPG-signed

[Mkrtchyan, Tigran]

Hi Johannes,

Thanks for the comments. I update the patch ant re-post.

Tigran.

----- Original Message -----
> From: "Johannes Schindelin" <Johannes.Schindelin@gmx.de>
> To: "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de>
> Cc: "git" <git@vger.kernel.org>, "Jonathan Nieder" <jrnieder@gmail.com>
> Sent: Tuesday, June 4, 2019 4:33:08 PM
> Subject: Re: [PATCH v2] tag: add tag.gpgSign config option to force all tags be GPG-signed

> Hi Tigran,
> 
> On Tue, 4 Jun 2019, Tigran Mkrtchyan wrote:
> 
>> As may CI/CD tools don't allow to control command line options when
> 
> s/may/many/, maybe?
> 
>> executing `git tag` command, a default value in the configuration file
>> will allow to enforce tag signing if required.
>>
>> The new config-file option tag.gpgSign enforces signed tags. Additional
>> command line option --no-gpg-sign is added to disable such behavior if
>> needed. E.g.:
>>
>>     $ git tag -m "commit message"
>>
>> will generate a GPG signed tag if tag.gpgSign option is true, while
>>
>>     $ git tag --no-gpg-sign -m "commit message"
>>
>> will skip the signing step.
>>
>> Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
>> ---
>>  Documentation/git-tag.txt |  7 +++++++
>>  builtin/tag.c             | 18 +++++++++++++++---
> 
> How about adding a section to Documentation/config/tag.txt as well?
> 
> Even better: it could be modeled after the description of commit.gpgsign
> to which Stefan Beller linked in his review of your earlier patch (see
> https://public-inbox.org/git/20131105112840.GZ4589@mars-attacks.org/):
> 
>  tag.gpgsign::
>	A boolean to specify whether all tags should be GPG signed.
>	Use of this option when running in an automated script can
>	result in a large number of tags being signed. It is therefore
>	convenient to use an agent to avoid typing your gpg passphrase
>	several times.
> 
>>  t/t7004-tag.sh            | 21 +++++++++++++++++++++
>>  3 files changed, 43 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
>> index a74e7b926d..d9dbfb4e37 100644
>> --- a/Documentation/git-tag.txt
>> +++ b/Documentation/git-tag.txt
>> @@ -64,6 +64,9 @@ OPTIONS
>>  -s::
>>  --sign::
>>  	Make a GPG-signed tag, using the default e-mail address's key.
>> +	The default behabior of tag GPG-signing controlled by `tag.gpgSign`
> 
> s/behabior/behavior/ maybe?
> 
> And I would also insert an "is" before "controlled".
> 
>> +	configuration variable if it exists, or disabled oder otherwise.
>> +	See linkgit:git-config[1].
>>
>>  -u <keyid>::
>>  --local-user=<keyid>::
>> @@ -193,6 +196,10 @@ This option is only applicable when listing tags without
>> annotation lines.
>>  	that of linkgit:git-for-each-ref[1].  When unspecified,
>>  	defaults to `%(refname:strip=2)`.
>>
>> +--no-gpg-sign::
> 
> Should this not be `--no-sign`? There is already a `--sign` option, and I
> would wager a guess that it automagically handles `--no-sign`...
> 
>> +	Countermand `tag.gpgSign` configuration variable that is
>> +	set to force each and every tag to be signed.
> 
> If you replace "Countermand" by "Override", you could simply merge this
> into the section talking about `--sign`...
> 
> 
>> diff --git a/builtin/tag.c b/builtin/tag.c
>> index ef37dccf86..7f9aef4840 100644
>> --- a/builtin/tag.c
>> +++ b/builtin/tag.c
>> @@ -33,6 +33,7 @@ static const char * const git_tag_usage[] = {
>>
>>  static unsigned int colopts;
>>  static int force_sign_annotate;
>> +static int sign_tag;
> 
> Since this holds the value parsed from the config, I would like to see
> some indication of that in the name. Maybe something like
> `config_sign_tag`?
> 
> Also, I would recommend to initialize it with `-1` to be able to discern
> between the three states `true`, `false` and `unspecified`.
> 
>>  static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting,
>>  		     struct ref_format *format)
>> @@ -144,6 +145,11 @@ static int git_tag_config(const char *var, const char
>> *value, void *cb)
>>  	int status;
>>  	struct ref_sorting **sorting_tail = (struct ref_sorting **)cb;
>>
>> +	if (!strcmp(var, "tag.gpgsign")) {
>> +		sign_tag = git_config_bool(var, value);
>> +		return 0;
>> +	}
>> +
>>  	if (!strcmp(var, "tag.sort")) {
>>  		if (!value)
>>  			return config_error_nonbool(var);
>> @@ -392,6 +398,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
>>  	struct ref_format format = REF_FORMAT_INIT;
>>  	int icase = 0;
>>  	int edit_flag = 0;
>> +	int no_gpg_sign = 0;
>>  	struct option options[] = {
>>  		OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'),
>>  		{ OPTION_INTEGER, 'n', NULL, &filter.lines, N_("n"),
>> @@ -413,6 +420,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
>>  					N_("use another key to sign the tag")),
>>  		OPT__FORCE(&force, N_("replace the tag if exists"), 0),
>>  		OPT_BOOL(0, "create-reflog", &create_reflog, N_("create a reflog")),
>> +		OPT_BOOL(0, "no-gpg-sign", &no_gpg_sign, N_("do not GPG-sign tag")),
>>
>>  		OPT_GROUP(N_("Tag listing options")),
>>  		OPT_COLUMN(0, "column", &colopts, N_("show tag list in columns")),
>> @@ -445,6 +453,10 @@ int cmd_tag(int argc, const char **argv, const char
>> *prefix)
>>
>>  	argc = parse_options(argc, argv, prefix, options, git_tag_usage, 0);
>>
>> +	if (no_gpg_sign) {
>> +		sign_tag = 0;
>> +	}
>> +
> 
> Hmm. I'd rather like to see this folded into the `--no-sign` option
> implied by the `OPT_BOOL('s', "sign", ...)` part. The way this would work
> would be to initialize the struct like this:
> 
>		struct create_tag_options opt = { .sign = -1 };
> 
> then leave
> 
>		OPT_BOOL('s', "sign", &opt.sign, N_("annotated and GPG-signed tag")),
> 
> as-is, and later do this:
> 
>	if (opt.sign < 0)
>		opt.sign = config_sign_tag > 0;
> 
>> @@ -463,7 +475,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
>>  	if (cmdmode == 'l')
>>  		setup_auto_pager("tag", 1);
>>
>> -	if ((create_tag_object || force) && (cmdmode != 0))
>> +	if ((create_tag_object || force || no_gpg_sign) && (cmdmode != 0))
>>  		usage_with_options(git_tag_usage, options);
> 
> Should we really try to be that strict? If so, we would have to test for
> `opt.sign > 0` here and make sure that above-mentioned `if (opt.sign < 0)`
> block comes *after* this block.
> 
>> @@ -556,8 +568,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
>>
>>  	create_reflog_msg(&object, &reflog_msg);
>>
>> -	if (create_tag_object) {
>> -		if (force_sign_annotate && !annotate)
>> +	if (create_tag_object || sign_tag) {
>> +		if (sign_tag || (force_sign_annotate && !annotate))
>>  			opt.sign = 1;
> 
> This would probably be better handled via
> 
> -	create_tag_object = (opt.sign || annotate || msg.given || msgfile);
> +	create_tag_object = (opt.sign > 0 || config_sign_tag > 0 ||
> +		annotate || msg.given || msgfile);
> 
> earlier. After all, this assignment of `create_tag_object` suggests that
> it is a Boolean that catches *all* cases where a tag should be created.
> 
> But then, we would have to be very careful about this, as we don't want to
> error out when a user calls `git tag -l` while `tag.gpgSign = true`. So
> maybe instead:
> 
> -	create_tag_object = (opt.sign || annotate || msg.given || msgfile);
> +	create_tag_object = (opt.sign > 0 || annotate || msg.given || msgfile);
> 
> and later, after that test whether `(create_tag_object || force) &&
> cmd_mode != 0` do:
> 
>	if (opt.sign < 0) {
>		opt.sign = config_sign_tag > 0;
>		create_tag_object ||= opt.sign;
> 
>> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
>> index 6aeeb279a0..98a07a29d2 100755
>> --- a/t/t7004-tag.sh
>> +++ b/t/t7004-tag.sh
>> @@ -932,6 +932,27 @@ test_expect_success GPG \
>>  	test_cmp expect actual
>>  '
>>
>> +get_tag_header gpgsign-enabled $commit commit $time >expect
>> +echo "A message" >>expect
>> +echo '-----BEGIN PGP SIGNATURE-----' >>expect
>> +test_expect_success GPG \
>> +	'git tag configured tag.gpgsign enables GPG sign' \
>> +	'test_config tag.gpgsign true &&
>> +	git tag -m "A message" gpgsign-enabled &&
>> +	get_tag_msg gpgsign-enabled>actual &&
>> +	test_cmp expect actual
>> +'
>> +
>> +get_tag_header no-gpg-sign $commit commit $time >expect
>> +echo "A message" >>expect
>> +test_expect_success GPG \
>> +	'git tag --no-gpg-sign configured tag.gpgsign skip GPG sign' \
>> +	'test_config tag.gpgsign true &&
>> +	git tag -a --no-gpg-sign -m "A message" no-gpg-sign &&
> 
> With my idea above, this would of course become `--no-sign`.
> 
>> +	get_tag_msg no-gpg-sign>actual &&
>> +	test_cmp expect actual
>> +'
>> +
>>  test_expect_success GPG \
>>  	'trying to create a signed tag with non-existing -F file should fail' '
>>  	! test -f nonexistingfile &&
> 
> Apart from the things I suggested in the hope to help you improve the
> patch, this looks good to me so far.
> 
> Thanks,
> Johannes

[PATCH v3] tag: add tag.gpgSign config option to force all tags be GPG-signed

[Tigran Mkrtchyan]

As many CI/CD tools don't allow to control command line options when
executing `git tag` command, a default value in the configuration file
will allow to enforce tag signing if required.

The new config-file option tag.gpgSign enforces signed tags. Additional
command line option --no-sign is added to disable such behavior if
needed. E.g.:

    $ git tag -m "commit message"

will generate a GPG signed tag if tag.gpgSign option is true, while

    $ git tag --no-sign -m "commit message"

will skip the signing step.

The combination of -u <key-id> and --no-sign not allowed.

Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
---
 Documentation/config/tag.txt |  7 +++++++
 Documentation/git-tag.txt    |  7 +++++++
 builtin/tag.c                | 23 ++++++++++++++++++-----
 t/t7004-tag.sh               | 21 +++++++++++++++++++++
 4 files changed, 53 insertions(+), 5 deletions(-)

diff --git a/Documentation/config/tag.txt b/Documentation/config/tag.txt
index 663663bdec..675483c3c3 100644
--- a/Documentation/config/tag.txt
+++ b/Documentation/config/tag.txt
@@ -8,6 +8,13 @@ tag.sort::
 	linkgit:git-tag[1]. Without the "--sort=<value>" option provided, the
 	value of this variable will be used as the default.
 
+tag.gpgsign::
+	A boolean to specify whether all tags should be GPG signed.
+	Use of this option when running in an automated script can
+	result in a large number of tags being signed. It is therefore
+	convenient to use an agent to avoid typing your gpg passphrase
+	several times.
+
 tar.umask::
 	This variable can be used to restrict the permission bits of
 	tar archive entries.  The default is 0002, which turns off the
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index a74e7b926d..2e5599a67f 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -64,6 +64,13 @@ OPTIONS
 -s::
 --sign::
 	Make a GPG-signed tag, using the default e-mail address's key.
+	The default behavior of tag GPG-signing is controlled by `tag.gpgSign`
+	configuration variable if it exists, or disabled oder otherwise.
+	See linkgit:git-config[1].
+
+--no-sign::
+	Override `tag.gpgSign` configuration variable that is
+	set to force each and every tag to be signed.
 
 -u <keyid>::
 --local-user=<keyid>::
diff --git a/builtin/tag.c b/builtin/tag.c
index ef37dccf86..ec5fd1dcc0 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -33,6 +33,7 @@ static const char * const git_tag_usage[] = {
 
 static unsigned int colopts;
 static int force_sign_annotate;
+static int config_sign_tag = -1; /* unspecified */
 
 static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting,
 		     struct ref_format *format)
@@ -144,6 +145,11 @@ static int git_tag_config(const char *var, const char *value, void *cb)
 	int status;
 	struct ref_sorting **sorting_tail = (struct ref_sorting **)cb;
 
+	if (!strcmp(var, "tag.gpgsign")) {
+		config_sign_tag = git_config_bool(var, value);
+		return 0;
+	}
+
 	if (!strcmp(var, "tag.sort")) {
 		if (!value)
 			return config_error_nonbool(var);
@@ -442,14 +448,12 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	memset(&opt, 0, sizeof(opt));
 	memset(&filter, 0, sizeof(filter));
 	filter.lines = -1;
+	opt.sign = -1;
 
 	argc = parse_options(argc, argv, prefix, options, git_tag_usage, 0);
 
-	if (keyid) {
-		opt.sign = 1;
-		set_signing_key(keyid);
-	}
-	create_tag_object = (opt.sign || annotate || msg.given || msgfile);
+	if (keyid && !opt.sign)
+		die(_("user key can't be specified with --no-sign option"));
 
 	if (!cmdmode) {
 		if (argc == 0)
@@ -463,6 +467,15 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	if (cmdmode == 'l')
 		setup_auto_pager("tag", 1);
 
+	if (opt.sign == -1)
+		opt.sign = cmdmode ? 0 : config_sign_tag > 0;
+
+	if (keyid) {
+		opt.sign = 1;
+		set_signing_key(keyid);
+	}
+	create_tag_object = (opt.sign || annotate || msg.given || msgfile);
+
 	if ((create_tag_object || force) && (cmdmode != 0))
 		usage_with_options(git_tag_usage, options);
 
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 6aeeb279a0..80eb13d94e 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -932,6 +932,27 @@ test_expect_success GPG \
 	test_cmp expect actual
 '
 
+get_tag_header gpgsign-enabled $commit commit $time >expect
+echo "A message" >>expect
+echo '-----BEGIN PGP SIGNATURE-----' >>expect
+test_expect_success GPG \
+	'git tag configured tag.gpgsign enables GPG sign' \
+	'test_config tag.gpgsign true &&
+	git tag -m "A message" gpgsign-enabled &&
+	get_tag_msg gpgsign-enabled>actual &&
+	test_cmp expect actual
+'
+
+get_tag_header no-sign $commit commit $time >expect
+echo "A message" >>expect
+test_expect_success GPG \
+	'git tag --no-sign configured tag.gpgsign skip GPG sign' \
+	'test_config tag.gpgsign true &&
+	git tag -a --no-sign -m "A message" no-sign &&
+	get_tag_msg no-sign>actual &&
+	test_cmp expect actual
+'
+
 test_expect_success GPG \
 	'trying to create a signed tag with non-existing -F file should fail' '
 	! test -f nonexistingfile &&
-- 
2.22.0.rc3.1.gdcc2db3530.dirty

Re: [PATCH v3] tag: add tag.gpgSign config option to force all tags be GPG-signed

[Todd Zullinger]

Hi,

Tigran Mkrtchyan wrote:
> diff --git a/Documentation/config/tag.txt b/Documentation/config/tag.txt
> index 663663bdec..675483c3c3 100644
> --- a/Documentation/config/tag.txt
> +++ b/Documentation/config/tag.txt
> @@ -8,6 +8,13 @@ tag.sort::
>  	linkgit:git-tag[1]. Without the "--sort=<value>" option provided, the
>  	value of this variable will be used as the default.
>  
> +tag.gpgsign::
> +	A boolean to specify whether all tags should be GPG signed.
> +	Use of this option when running in an automated script can
> +	result in a large number of tags being signed. It is therefore
> +	convenient to use an agent to avoid typing your gpg passphrase
> +	several times.

I think the variable should be camelCased, i.e. tag.gpgSign,
for consistency with other documentation.

-- 
Todd

Re: [PATCH v3] tag: add tag.gpgSign config option to force all tags be GPG-signed

[Junio C Hamano]

Tigran Mkrtchyan <tigran.mkrtchyan@desy.de> writes:

> As many CI/CD tools don't allow to control command line options when
> executing `git tag` command, a default value in the configuration file
> will allow to enforce tag signing if required.

Hmm.  Would these "many" tools still allow arbigrary configuration
set to affect their operation?  It sounds like a bigger issue but it
is a separate one.

> The new config-file option tag.gpgSign enforces signed tags. Additional
> ...
> will skip the signing step.

This paragraph is well written.

> The combination of -u <key-id> and --no-sign not allowed.

This sentence lacks a verb.  Perhaps s/not allowed/is &/.  

But more importantly, I think we should justify why this "not
allowed" makes sense as the design of the feature. A plausible
alternative design would simply follow the "last one wins" paradigm,
where

    git tag -u key # "-u key" implies "-s"

    git tag -u key --no-sign # "--no-sign' trumps the implied "-s"

    git tag --no-sign -u key # "-u key"'s implication of "-s" trumps the
			     # earlier "--no-sign"

and having "[tag] gpgsign" simply adds to the implication early in
the chain to be overridden by later command line options.

Let's explain why "you cannot give -u <key> and --no-sign at the
same time" is better than "the last one wins".

> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
> index a74e7b926d..2e5599a67f 100644
> --- a/Documentation/git-tag.txt
> +++ b/Documentation/git-tag.txt
> @@ -64,6 +64,13 @@ OPTIONS
>  -s::
>  --sign::
>  	Make a GPG-signed tag, using the default e-mail address's key.
> +	The default behavior of tag GPG-signing is controlled by `tag.gpgSign`
> +	configuration variable if it exists, or disabled oder otherwise.
> +	See linkgit:git-config[1].
> +
> +--no-sign::
> +	Override `tag.gpgSign` configuration variable that is
> +	set to force each and every tag to be signed.
>  
>  -u <keyid>::
>  --local-user=<keyid>::

If we justify "-u and --no-sign do not mix", that design needs to be
explained to the end users in the documentation, not just in the
proposed log messsage.

Re: [PATCH v3] tag: add tag.gpgSign config option to force all tags be GPG-signed

[Mkrtchyan, Tigran]

----- Original Message -----
> From: "Junio C Hamano" <gitster@pobox.com>
> To: "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de>
> Cc: "git" <git@vger.kernel.org>, "Jonathan Nieder" <jrnieder@gmail.com>, "Johannes Schindelin"
> <Johannes.Schindelin@gmx.de>
> Sent: Wednesday, June 5, 2019 6:25:46 PM
> Subject: Re: [PATCH v3] tag: add tag.gpgSign config option to force all tags be GPG-signed

> Tigran Mkrtchyan <tigran.mkrtchyan@desy.de> writes:
> 
>> As many CI/CD tools don't allow to control command line options when
>> executing `git tag` command, a default value in the configuration file
>> will allow to enforce tag signing if required.

Must of them blindly execute git commands with some hard-coded combination of
options. It's clear to me, that they are the source of the problem, but
git can be the solution.

> 
> Hmm.  Would these "many" tools still allow arbigrary configuration
> set to affect their operation?  It sounds like a bigger issue but it
> is a separate one.
> 
>> The new config-file option tag.gpgSign enforces signed tags. Additional
>> ...
>> will skip the signing step.
> 
> This paragraph is well written.
> 
>> The combination of -u <key-id> and --no-sign not allowed.
> 
> This sentence lacks a verb.  Perhaps s/not allowed/is &/.
> 

Jup. Sorry.

> But more importantly, I think we should justify why this "not
> allowed" makes sense as the design of the feature. A plausible
> alternative design would simply follow the "last one wins" paradigm,
> where
> 
>    git tag -u key # "-u key" implies "-s"
> 
>    git tag -u key --no-sign # "--no-sign' trumps the implied "-s"
> 
>    git tag --no-sign -u key # "-u key"'s implication of "-s" trumps the
>			     # earlier "--no-sign"
> 
> and having "[tag] gpgsign" simply adds to the implication early in
> the chain to be overridden by later command line options.
> 
> Let's explain why "you cannot give -u <key> and --no-sign at the
> same time" is better than "the last one wins".

This is matter of convention. I never pay attention to the order of options
on the git command line, but I don't put conflicting options either, I hope.
Does git already have position depended options? If yes, then fine with me.
Otherwise, I don't want to introduce ambiguity. Yes, less ambiguity is my
answer to why it's better than "the last one wins".


> 
>> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
>> index a74e7b926d..2e5599a67f 100644
>> --- a/Documentation/git-tag.txt
>> +++ b/Documentation/git-tag.txt
>> @@ -64,6 +64,13 @@ OPTIONS
>>  -s::
>>  --sign::
>>  	Make a GPG-signed tag, using the default e-mail address's key.
>> +	The default behavior of tag GPG-signing is controlled by `tag.gpgSign`
>> +	configuration variable if it exists, or disabled oder otherwise.
>> +	See linkgit:git-config[1].
>> +
>> +--no-sign::
>> +	Override `tag.gpgSign` configuration variable that is
>> +	set to force each and every tag to be signed.
>>  
>>  -u <keyid>::
>>  --local-user=<keyid>::
> 
> If we justify "-u and --no-sign do not mix", that design needs to be
> explained to the end users in the documentation, not just in the
> proposed log messsage.

Make sense.

Thanks,
   Tigran.

Re: [PATCH v3] tag: add tag.gpgSign config option to force all tags be GPG-signed

[Junio C Hamano]

"Mkrtchyan, Tigran" <tigran.mkrtchyan@desy.de> writes:

>> But more importantly, I think we should justify why this "not
>> allowed" makes sense as the design of the feature. A plausible
>> alternative design would simply follow the "last one wins" paradigm,
>> ...
> This is matter of convention.

Oh, if you put it that way, it is quite clear that we should redo
this part, as the rest of Git command line processing all pretty
much follow "last one wins".  I was mostly curious if there was any
strong reason why this combination has to be different.

Re: [PATCH v3] tag: add tag.gpgSign config option to force all tags be GPG-signed

[Mkrtchyan, Tigran]

Thanks for clarification. I will update the patch and re-send it.

Tigran.

----- Original Message -----
> From: "Junio C Hamano" <gitster@pobox.com>
> To: "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de>
> Cc: "git" <git@vger.kernel.org>, "Jonathan Nieder" <jrnieder@gmail.com>, "Johannes Schindelin"
> <Johannes.Schindelin@gmx.de>
> Sent: Wednesday, June 5, 2019 10:46:05 PM
> Subject: Re: [PATCH v3] tag: add tag.gpgSign config option to force all tags be GPG-signed

> "Mkrtchyan, Tigran" <tigran.mkrtchyan@desy.de> writes:
> 
>>> But more importantly, I think we should justify why this "not
>>> allowed" makes sense as the design of the feature. A plausible
>>> alternative design would simply follow the "last one wins" paradigm,
>>> ...
>> This is matter of convention.
> 
> Oh, if you put it that way, it is quite clear that we should redo
> this part, as the rest of Git command line processing all pretty
> much follow "last one wins".  I was mostly curious if there was any
> strong reason why this combination has to be different.

Re: [PATCH v3] tag: add tag.gpgSign config option to force all tags be GPG-signed

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> "Mkrtchyan, Tigran" <tigran.mkrtchyan@desy.de> writes:
>
>>> But more importantly, I think we should justify why this "not
>>> allowed" makes sense as the design of the feature. A plausible
>>> alternative design would simply follow the "last one wins" paradigm,
>>> ...
>> This is matter of convention.
>
> Oh, if you put it that way, it is quite clear that we should redo
> this part, as the rest of Git command line processing all pretty
> much follow "last one wins".  I was mostly curious if there was any
> strong reason why this combination has to be different.

One reason why "last one wins" aka "you have a chance to correct
yourself" is so useful is because people can then use alias to
define leading options, e.g. with

    [alias] myfoo = foo --option1 --option2 --option3

a user can say "git myfoo" to save typing, but in a rare occasion
where only options 2 and 3 (but not option 1) are desired, can use
"git myfoo --no-option1" to countermand it.

Unfortunately, I do not think we can use the same technique for
options that is given to the "git" potty, e.g. with

    [alias] paged-status = --paginate status

you can say "git paged-status" to have your "git status" output sent
to the pager, but you cannot say "git paged-status --no-pager" to
defeat that leading option.  We may want to find a way to correct
it, but it is rather low on the priority scale ;-)

Re: [PATCH v3] tag: add tag.gpgSign config option to force all tags be GPG-signed

[Mkrtchyan, Tigran]

----- Original Message -----
> From: "Junio C Hamano" <gitster@pobox.com>
> To: "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de>
> Cc: "git" <git@vger.kernel.org>, "Jonathan Nieder" <jrnieder@gmail.com>, "Johannes Schindelin"
> <Johannes.Schindelin@gmx.de>
> Sent: Wednesday, June 5, 2019 10:57:34 PM
> Subject: Re: [PATCH v3] tag: add tag.gpgSign config option to force all tags be GPG-signed

> Junio C Hamano <gitster@pobox.com> writes:
> 
>> "Mkrtchyan, Tigran" <tigran.mkrtchyan@desy.de> writes:
>>
>>>> But more importantly, I think we should justify why this "not
>>>> allowed" makes sense as the design of the feature. A plausible
>>>> alternative design would simply follow the "last one wins" paradigm,
>>>> ...
>>> This is matter of convention.
>>
>> Oh, if you put it that way, it is quite clear that we should redo
>> this part, as the rest of Git command line processing all pretty
>> much follow "last one wins".  I was mostly curious if there was any
>> strong reason why this combination has to be different.
> 
> One reason why "last one wins" aka "you have a chance to correct
> yourself" is so useful is because people can then use alias to
> define leading options, e.g. with
> 
>    [alias] myfoo = foo --option1 --option2 --option3
> 
> a user can say "git myfoo" to save typing, but in a rare occasion
> where only options 2 and 3 (but not option 1) are desired, can use
> "git myfoo --no-option1" to countermand it.
> 
> Unfortunately, I do not think we can use the same technique for
> options that is given to the "git" potty, e.g. with
> 
>    [alias] paged-status = --paginate status
> 
> you can say "git paged-status" to have your "git status" output sent
> to the pager, but you cannot say "git paged-status --no-pager" to
> defeat that leading option.  We may want to find a way to correct
> it, but it is rather low on the priority scale ;-)


Makes sense. Thanks for that use case, which I actually use myself :).

BTW, `--no-sign` currently doesn't affects `-u <key>` in any order:

```
    if (keyid) {
        opt.sign = 1;
        set_signing_key(keyid);
    }
```

IOW, `-u <key>` is another way to enable signing, what is, actually,
consistent with man page:

```
       -u <keyid>, --local-user=<keyid>
           Make a GPG-signed tag, using the given key.
```

I will leave it as-is, I will just describe that tag.gpgSign options
doesn't affect '-u' behavior.

Tigran.

[PATCH v4] tag: add tag.gpgSign config option to force all tags be GPG-signed

[Tigran Mkrtchyan]

As many CI/CD tools don't allow to control command line options when
executing `git tag` command, a default value in the configuration file
will allow to enforce tag signing if required.

The new config-file option tag.gpgSign is added to define default behavior
of tag signings. To override default behavior the command line option -s,
--sign and --no-sign can be used:

    $ git tag -m "commit message"

will generate a GPG signed tag if tag.gpgSign option is true, while

    $ git tag --no-sign -m "commit message"

will skip the signing step.

Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
---
 Documentation/config/tag.txt |  8 ++++++++
 Documentation/git-tag.txt    |  7 +++++++
 builtin/tag.c                | 22 ++++++++++++++++------
 t/t7004-tag.sh               | 21 +++++++++++++++++++++
 4 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/Documentation/config/tag.txt b/Documentation/config/tag.txt
index 663663bdec..ef5adb3f42 100644
--- a/Documentation/config/tag.txt
+++ b/Documentation/config/tag.txt
@@ -8,6 +8,14 @@ tag.sort::
 	linkgit:git-tag[1]. Without the "--sort=<value>" option provided, the
 	value of this variable will be used as the default.
 
+tag.gpgSign::
+	A boolean to specify whether all tags should be GPG signed.
+	Use of this option when running in an automated script can
+	result in a large number of tags being signed. It is therefore
+	convenient to use an agent to avoid typing your gpg passphrase
+	several times. Note that this option doesn't affects tag signing
+	behavior enabled by "-u <keyid>" or "--local-user=<keyid>" options.
+
 tar.umask::
 	This variable can be used to restrict the permission bits of
 	tar archive entries.  The default is 0002, which turns off the
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index a74e7b926d..2e5599a67f 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -64,6 +64,13 @@ OPTIONS
 -s::
 --sign::
 	Make a GPG-signed tag, using the default e-mail address's key.
+	The default behavior of tag GPG-signing is controlled by `tag.gpgSign`
+	configuration variable if it exists, or disabled oder otherwise.
+	See linkgit:git-config[1].
+
+--no-sign::
+	Override `tag.gpgSign` configuration variable that is
+	set to force each and every tag to be signed.
 
 -u <keyid>::
 --local-user=<keyid>::
diff --git a/builtin/tag.c b/builtin/tag.c
index ef37dccf86..e0a4c25382 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -33,6 +33,7 @@ static const char * const git_tag_usage[] = {
 
 static unsigned int colopts;
 static int force_sign_annotate;
+static int config_sign_tag = -1; /* unspecified */
 
 static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting,
 		     struct ref_format *format)
@@ -144,6 +145,11 @@ static int git_tag_config(const char *var, const char *value, void *cb)
 	int status;
 	struct ref_sorting **sorting_tail = (struct ref_sorting **)cb;
 
+	if (!strcmp(var, "tag.gpgsign")) {
+		config_sign_tag = git_config_bool(var, value);
+		return 0;
+	}
+
 	if (!strcmp(var, "tag.sort")) {
 		if (!value)
 			return config_error_nonbool(var);
@@ -442,15 +448,10 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	memset(&opt, 0, sizeof(opt));
 	memset(&filter, 0, sizeof(filter));
 	filter.lines = -1;
+	opt.sign = -1;
 
 	argc = parse_options(argc, argv, prefix, options, git_tag_usage, 0);
 
-	if (keyid) {
-		opt.sign = 1;
-		set_signing_key(keyid);
-	}
-	create_tag_object = (opt.sign || annotate || msg.given || msgfile);
-
 	if (!cmdmode) {
 		if (argc == 0)
 			cmdmode = 'l';
@@ -463,6 +464,15 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	if (cmdmode == 'l')
 		setup_auto_pager("tag", 1);
 
+	if (opt.sign == -1)
+		opt.sign = cmdmode ? 0 : config_sign_tag > 0;
+
+	if (keyid) {
+		opt.sign = 1;
+		set_signing_key(keyid);
+	}
+	create_tag_object = (opt.sign || annotate || msg.given || msgfile);
+
 	if ((create_tag_object || force) && (cmdmode != 0))
 		usage_with_options(git_tag_usage, options);
 
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 6aeeb279a0..80eb13d94e 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -932,6 +932,27 @@ test_expect_success GPG \
 	test_cmp expect actual
 '
 
+get_tag_header gpgsign-enabled $commit commit $time >expect
+echo "A message" >>expect
+echo '-----BEGIN PGP SIGNATURE-----' >>expect
+test_expect_success GPG \
+	'git tag configured tag.gpgsign enables GPG sign' \
+	'test_config tag.gpgsign true &&
+	git tag -m "A message" gpgsign-enabled &&
+	get_tag_msg gpgsign-enabled>actual &&
+	test_cmp expect actual
+'
+
+get_tag_header no-sign $commit commit $time >expect
+echo "A message" >>expect
+test_expect_success GPG \
+	'git tag --no-sign configured tag.gpgsign skip GPG sign' \
+	'test_config tag.gpgsign true &&
+	git tag -a --no-sign -m "A message" no-sign &&
+	get_tag_msg no-sign>actual &&
+	test_cmp expect actual
+'
+
 test_expect_success GPG \
 	'trying to create a signed tag with non-existing -F file should fail' '
 	! test -f nonexistingfile &&
-- 
2.22.0.rc3.1.gdbadaa0575