patchhooks/update: Add a hooks.denyunsignedtags option

Introduce an option to deny unsigned tags from entering a repository. This is useful in teams where members forget to sign their release tags.

It does not actually check whether the signature is actually complete or valid, it just checks for the beginning of a signature, as further checks would be too involved.

This effectively also denies un-annotated tags, as those are unsigned by definition.

Signed-off-by: Julian Andres Klode <jak@debian.org>
---

Note: Submitted for review on Sep 12, re-asked on Sep 22, but
no feedback, so I assume it's good to go, see
http://thread.gmane.org/gmane.comp.version-control.git/277722
for details

 templates/hooks--update.sample | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

Julian Andres Klode <jak@debian.org> writes:

You assumed wrong. No response merely means that the patch did not find anybody who is interested enough to even comment on it.

There aren't much details there, but thanks for the pointer. It gave me a good guess as to why nobody commented on it. You said "locally fixed" without showing the fixed result and then asked "any comments?" People didn't have anything to comment on.

Anyway, let's see what we have here.

See "git log --oneline --no-merges" and notice the local convention. s/Add/add/;

Somehow this combination of "allow-unannotated" and "deny-unsigned" bothers me. Would it make the resulting code and logic more consistent and easier to follow if this new setting is renamed to "$allowunsigned", I wonder...

Can we do something with the overly long line, by the way?

When $denyunsignedtag is in effect, isn't the user saying that _all_ tags _must_ be signed? In other words, isn't it a configuration error if allowunannotated and denyunsignedtag are both set to true?

And if you reject that configuration error early, then perhaps you can even only check allowunannotated here without touching anything in this hunk, perhaps doing something like this upfront:

    allowunannotated=$(git config --bool hooks.allowunannotated || echo false)
    allowunsigned=$(git config --bool hooks.allowunsigned || echo true)

    case "$allowunannotated,$allowunsigned" in
    true,false)
            echo >&2 "*** inconsistent setting"
            exit 1
    esac

Then at this point of the code, if your new condition holds true, i.e. $allowunsigned is set to "false", $allowunannotated must be something other than "true", so you do not have to touch the "if".

Again, can we do something with the overly long line?

Use of "cat-file -p" is a bad manner in scripts, as we reserve the right to change what "-p" output looks like purely on human usability. "cat-file tag", perhaps?

Also,

	$ git grep ' PGP '

in our source tells me that we use a bit tighter pattern even when we are casually trying to see if the thing looks like a PGP signed payload.

	if test "$allowunsigned" = "true" ||
           git cat-file "$newrev" |
           grep -q '^-----BEGIN PGP SIGNATURE-----$'
	then
		...

or something?

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

I think an intelligent reader would have understood what I meant, but the 'cat-file' in the above needs to say what type of thing it is asking to dump, i.e.

 	if test "$allowunsigned" = "true" ||
            git cat-file tag "$newrev" |
            grep -q '^-----BEGIN PGP SIGNATURE-----$'
 	then

Sorry for the noise.

On Monday, December 21, 2015, Julian Andres Klode <jak@debian.org> wrote:

Hmm. Is this diagnostic sufficient to help the person resolve the issue? Isn't it actively misleading to advise using '-a'? Perhaps a distinct message is warranted?

(Alternately, if you follow Junio's advice and disallow this combination of options, then this issue becomes moot.)

The diagnostic for $allowunannotated gives helpful advice about how to resolve the problem. Should this one follow suit?

Also consistency might suggest patterning this message after the one for $allowunannotated. Perhaps something like this:

    The unsigned tag $short_refname is not allowed in this repository.
    Use 'git tag -s' for tags you want to propagate.

or something.