Re: [PATCH v5] Add the option to force sign annotated tags
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 22, 2016, 19:48 UTC
- Message-ID
- <xmqqshziguot.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20160322193617.GG20083@spk-laptop>
Laurent Arnoud <laurent@spkdev.net> writes:
> The `tag.forcesignannotated` config option allows to sign > annotated tags automatically.
It looks like it does a lot more than that to me, though.
Show 17 quoted lines
> @@ -327,7 +333,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
> char *cleanup_arg = NULL;
> int create_reflog = 0;
> int annotate = 0, force = 0;
> - int cmdmode = 0;
> + int cmdmode = 0, create_tag_object = 0;
> const char *msgfile = NULL, *keyid = NULL;
> struct msg_arg msg = { 0, STRBUF_INIT };
> struct ref_transaction *transaction;
> @@ -385,12 +391,13 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
> opt.sign = 1;
> set_signing_key(keyid);
> }
> - if (opt.sign)
> - annotate = 1;
> + if (opt.sign || annotate || force_sign_annotate)
> + create_tag_object = 1;This means that create_tag_object is always on if the configuration is set and there is no way to override that from the command line, doesn't it? I cannot see how a user would create a lightweight tag if this configuration variable is set with this change.
I think it makes sense to have this here instead of these two lines:
create_tag_object = (opt.sign || annotate || msg.given || msgfile);
Show 5 quoted lines
> if (argc == 0 && !cmdmode) > cmdmode = 'l'; > > - if ((annotate || msg.given || msgfile || force) && (cmdmode != 0)) > + if ((create_tag_object || msg.given || msgfile || force) && (cmdmode != 0))
and then simplify this to
if ((create_tag_object || force) && (cmdmode != 0))
perhaps? Then ...
Show 9 quoted lines
> usage_with_options(git_tag_usage, options);
>
> finalize_colopts(&colopts, -1);
> @@ -431,7 +438,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
> if (msg.given || msgfile) {
> if (msg.given && msgfile)
> die(_("only one -F or -m option is allowed."));
> - annotate = 1;
> + create_tag_object = 1;... this line can just go, as we are taking the presense of various ways to say "I'll give this message to the resulting tag" as the sign that the user wants to create a tag object much earlier.
Show 13 quoted lines
> if (msg.given)
> strbuf_addbuf(&buf, &(msg.buf));
> else {
> @@ -474,8 +481,11 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
> else
> die(_("Invalid cleanup mode %s"), cleanup_arg);
>
> - if (annotate)
> + if (create_tag_object) {
> + if (force_sign_annotate && !annotate)
> + opt.sign = 1;
> create_tag(object, tag, &buf, &opt, prev, object);
> + }And this hunk is OK.
Show 15 quoted lines
> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh > index cf3469b..be95318 100755 > --- a/t/t7004-tag.sh > +++ b/t/t7004-tag.sh > @@ -775,6 +775,39 @@ test_expect_success GPG '-s implies annotated tag' ' > test_cmp expect actual > ' > > +get_tag_header config-implied-annotate $commit commit $time >expect > +./fakeeditor >>expect > +echo '-----BEGIN PGP SIGNATURE-----' >>expect > +test_expect_success GPG \ > + 'git tag -s implied if configured with tag.forcesignannotated' \ > + 'test_config tag.forcesignannotated true && > + GIT_EDITOR=./fakeeditor git tag config-implied-annotate &&
This contradicts with what you said earlier in your <20160321192904.GC20083@spk-laptop> aka http://thread.gmane.org/gmane.comp.version-control.git/289322/focus=289441
> If you are forcing users to always leave a message and then further
> forcing users to always sign with the single new configuration, i.e.
>
> $ git tag v1.0
> ... opens the editor to ask for a message ...
> ... then makes the user sign with GPG ... I'm not forcing this type of user to enable global
configuration, that will be annoying for them of course.But this test expects that this invocation of "git tag $tagname" forces the user to give a message with editor and sign it, instead of creating a lightweight tag.