On Tue, Mar 22, 2016 at 12:48:50PM -0700, Junio C Hamano wrote:
> Laurent Arnoud <laurent@xxxxxxxxxx> 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.
>
> > @@ -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);
>
> > 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 ...
>
> > 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.
>
> > 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.
>
> > 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.
Yes you're right, I will fix that...
--
Laurent
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html