threads / patch / 45042

patchtag: add a config option for setting --annotate by default

Subject: [PATCH] tag: add a config option for setting --annotate by default

## tl;dr

3 messages between Feb 4, 2017 and Feb 5, 2017. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

David Aguilar· Feb 4, 2017, 02:14 UTC · lore

Make it easier for users to remember to annotate their tags. Allow setting the default value for "--annotate" via the "tag.annotate" configuration variable.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 Documentation/config.txt |  5 +++++
 builtin/tag.c            | 15 ++++++++++++---
 t/t7004-tag.sh           | 23 +++++++++++++++++++++++
 3 files changed, 40 insertions(+), 3 deletions(-)
Show changes to 3 files +40 −3

Documentation/config.txt, builtin/tag.c, t/t7004-tag.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index af2ae4cc02..0d562b97e9 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2945,6 +2945,11 @@ submodule.alternateErrorStrategy
 	as computed via `submodule.alternateLocation`. Possible values are
 	`ignore`, `info`, `die`. Default is `die`.
 
+tag.annotate::
+	A boolean to specify whether annotated tags should be created by
+	default.  If `--no-annotate` is specified on the command line,
+	it takes precedence over this option.
+
 tag.forceSignAnnotated::
 	A boolean to specify whether annotated tags created should be GPG signed.
 	If `--annotate` is specified on the command line, it takes
diff --git a/builtin/tag.c b/builtin/tag.c
index 73df728114..1cf9bb73ad 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -29,6 +29,7 @@ static const char * const git_tag_usage[] = {
 };
 
 static unsigned int colopts;
+static int force_annotate;
 static int force_sign_annotate;
 
 static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, const char *format)
@@ -161,6 +162,10 @@ static int git_tag_config(const char *var, const char *value, void *cb)
 	status = git_gpg_config(var, value, cb);
 	if (status)
 		return status;
+	if (!strcmp(var, "tag.annotate")) {
+		force_annotate = git_config_bool(var, value);
+		return 0;
+	}
 	if (!strcmp(var, "tag.forcesignannotated")) {
 		force_sign_annotate = git_config_bool(var, value);
 		return 0;
@@ -326,7 +331,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	struct create_tag_options opt;
 	char *cleanup_arg = NULL;
 	int create_reflog = 0;
-	int annotate = 0, force = 0;
+	int annotate = -1, force = 0;
 	int cmdmode = 0, create_tag_object = 0;
 	const char *msgfile = NULL, *keyid = NULL;
 	struct msg_arg msg = { 0, STRBUF_INIT };
@@ -387,11 +392,15 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 		opt.sign = 1;
 		set_signing_key(keyid);
 	}
-	create_tag_object = (opt.sign || annotate || msg.given || msgfile);
 
 	if (argc == 0 && !cmdmode)
 		cmdmode = 'l';
 
+	if (force_annotate && !cmdmode && annotate == -1)
+		annotate = 1;
+
+	create_tag_object = (opt.sign || annotate > 0 || msg.given || msgfile);
+
 	if ((create_tag_object || force) && (cmdmode != 0))
 		usage_with_options(git_tag_usage, options);
 
@@ -478,7 +487,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 		die(_("Invalid cleanup mode %s"), cleanup_arg);
 
 	if (create_tag_object) {
-		if (force_sign_annotate && !annotate)
+		if (force_sign_annotate && annotate == -1)
 			opt.sign = 1;
 		create_tag(object, tag, &buf, &opt, prev, object);
 	}
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 1cfa8a21d2..5ba52b57dd 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -754,6 +754,29 @@ echo from a fake editor.
 EOF
 chmod +x fakeeditor
 
+get_tag_header config-annotate $commit commit $time >expect
+./fakeeditor >>expect
+test_expect_success 'tag.annotate creates annotated tags' '
+	test_config tag.annotate true &&
+	GIT_EDITOR=./fakeeditor git tag config-annotate &&
+	get_tag_msg config-annotate >actual &&
+	test_cmp expect actual
+'
+test_expect_success 'tag --no-annotate overrides tag.annotate=true config' '
+	test_config tag.annotate true &&
+	GIT_EDITOR=false git tag --no-annotate cli-override-tag-annotate &&
+	tag_exists cli-override-tag-annotate
+'
+
+get_tag_header config-no-annotate $commit commit $time >expect
+./fakeeditor >>expect
+test_expect_success 'tag --annotate overrides tag.annotate=false config' '
+	test_config tag.annotate false &&
+	GIT_EDITOR=./fakeeditor git tag --annotate config-no-annotate &&
+	get_tag_msg config-no-annotate >actual &&
+	test_cmp expect actual
+'
+
 get_tag_header implied-sign $commit commit $time >expect
 ./fakeeditor >>expect
 echo '-----BEGIN PGP SIGNATURE-----' >>expect

-- 
2.11.0.486.gcc949b6e67.dirty
Junio C Hamano· Feb 4, 2017, 05:02 UTC · re: David Aguilar · lore

Re: [PATCH] tag: add a config option for setting --annotate by default

David Aguilar <davvid@gmail.com> writes:
Show 6 quoted lines
> Make it easier for users to remember to annotate their tags.
> Allow setting the default value for "--annotate" via the "tag.annotate"
> configuration variable.
>
> Signed-off-by: David Aguilar <davvid@gmail.com>
> ---

I do not care too strongly about this, but I need to point out that this will have fallout to tools and scripts. E.g. if you have this configured and try to create a new tag in gitk, wouldn't this part

	if {$msg != {}} {
	    exec git tag -a -m $msg $tag $id
	} else {
	    exec git tag $tag $id
	}

try to open an editor somehow to get the message even when $msg is an empty string? I think the same problem already exists for the tag.forceSignAnnotated variable we already have added, though.

Show 16 quoted lines
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index af2ae4cc02..0d562b97e9 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2945,6 +2945,11 @@ submodule.alternateErrorStrategy
>  	as computed via `submodule.alternateLocation`. Possible values are
>  	`ignore`, `info`, `die`. Default is `die`.
>  
> +tag.annotate::
> +	A boolean to specify whether annotated tags should be created by
> +	default.  If `--no-annotate` is specified on the command line,
> +	it takes precedence over this option.
> +
>  tag.forceSignAnnotated::
>  	A boolean to specify whether annotated tags created should be GPG signed.
>  	If `--annotate` is specified on the command line, it takes
David Aguilar· Feb 5, 2017, 04:10 UTC · re: Junio C Hamano · lore

Re: [PATCH] tag: add a config option for setting --annotate by default

On Fri, Feb 03, 2017 at 09:02:47PM -0800, Junio C Hamano wrote:
Show 22 quoted lines
> David Aguilar <davvid@gmail.com> writes:
> 
> > Make it easier for users to remember to annotate their tags.
> > Allow setting the default value for "--annotate" via the "tag.annotate"
> > configuration variable.
> >
> > Signed-off-by: David Aguilar <davvid@gmail.com>
> > ---
> 
> I do not care too strongly about this, but I need to point out that
> this will have fallout to tools and scripts.  E.g. if you have this
> configured and try to create a new tag in gitk, wouldn't this part
> 
> 	if {$msg != {}} {
> 	    exec git tag -a -m $msg $tag $id
> 	} else {
> 	    exec git tag $tag $id
> 	}
> 
> try to open an editor somehow to get the message even when $msg is
> an empty string?  I think the same problem already exists for the
> tag.forceSignAnnotated variable we already have added, though.

That's true. I should have put "RFC" in the subject line. Let's drop this patch unless there's others that find it useful.

How do you feel about a patch to add "git merge --signoff", for consistency with "git commit"?

The rationale is that there might be situations (evil merges, or even regular merges depending on the project) where someone might want to signoff on their merges.

-- 
David

← back to recent threads