Re: [PATCH] Documentation: clarify signature verification

[KellerFuchs <KellerFuchs@hashbang.sh>]

On Sun, Apr 10, 2016 at 11:46:10AM -0700, Junio C Hamano wrote:
> > --- a/Documentation/merge-options.txt
> > +++ b/Documentation/merge-options.txt
> > @@ -89,8 +89,10 @@ option can be used to override --squash.
> >  
> >  --verify-signatures::
> >  --no-verify-signatures::
> > -	Verify that the commits being merged have good and trusted GPG signatures
> > +	Verify that the commits being merged have good and valid GPG signatures
> >  	and abort the merge in case they do not.
> > +	For instance, when running `git merge --verify-signature remote/branch`,
> > +	only the head commit on `remote/branch` needs to be signed.
> 
> The first part of this change and all other changes are of dubious
> value, but the last two lines is truly an improvement--it adds
> missing information people who use the feature may care about.

The reason for the first edit is that “trusted” and “valid” are OpenPGP
  concepts: a key is trusted if the user set a trust level for it,
  and a uid is valid if it has been signed by a trusted key [0].

Most of my confusion came from this, since it sounded like the signature
  would only be accepted if it came from a key with a non-zero ownertrust.

[0] That actually only holds for the default trust model,
    so I'm oversimplifying a bit here.

> I'd suggest doing the addition of the last two lines as a standalone
> patch, and make the remainder a separate patch on top.

Sure, will do when submitting for inclusion.

> > diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
> > index 671cebd..29b19b9 100644
> > --- a/Documentation/pretty-formats.txt
> > +++ b/Documentation/pretty-formats.txt
> > @@ -143,8 +143,8 @@ ifndef::git-rev-list[]
> >  - '%N': commit notes
> >  endif::git-rev-list[]
> >  - '%GG': raw verification message from GPG for a signed commit
> > -- '%G?': show "G" for a Good signature, "B" for a Bad signature, "U" for a good,
> > -  untrusted signature and "N" for no signature
> > +- '%G?': show "G" for a good (valid) signature, "B" for a bad signature,
> > +  "U" for a good signature with unknown validity and "N" for no signature
> 
> The reason I said the other changes are of dubious value is shown
> very well in this hunk.  I am not sure if it is an improvement to
> rephrase "Good" to "good (valid)" and "untrusted" to "good signature
> with unknown validity".  They are saying pretty much the same thing,
> no?

As said above, it was about consistency with the OpenPGP terminology.

If git wants to have it's own vocabulary for that (which I would argue
  against), then it would need to be defined somewhere.

> > diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
> > index 54b88b6..62cbae2 100644
> > --- a/Documentation/pretty-options.txt
> > +++ b/Documentation/pretty-options.txt
> > @@ -78,5 +78,5 @@ being displayed. Examples: "--notes=foo" will show only notes from
> >  endif::git-rev-list[]
> >  
> >  --show-signature::
> > -	Check the validity of a signed commit object by passing the signature
> > -	to `gpg --verify` and show the output.
> > +	Check the validity of a signed commit object, by passing the signature
> > +	to `gpg --verify`, and show the output.
> 
> The update one may be gramattically correct, but I personally find
> the original easier to read.  Is there a reason for this change?

That one is arguably more dubious, and I would happily drop it.
For some reason, I kept parsing it as “Check the validity [...] by
  (passing the signature to `gpg --verify` and showing the output)”.


Best regards,

  kf