Re: [PATCH] Documentation: clarify signature verification
On Sun, Apr 10, 2016 at 11:46:10AM -0700, Junio C Hamano wrote:
Show 15 quoted lines
> > --- 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.
Show 18 quoted lines
> > 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.
Show 15 quoted lines
> > 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
KellerFuchs <kellerfuchs@hashbang.sh>