Re: [PATCH] Documentation: clarify signature verification
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 10, 2016, 18:46 UTC
- Message-ID
- <xmqqa8l1ti8d.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20160409200756.GA22694@hashbang.sh>
The Fox in the Shell <KellerFuchs@hashbang.sh> writes:
Show 14 quoted lines
> Hi, > > I encountered some issues with the git documentation while modifying > my deployment scripts to enforce that the tree being fetched was > signed by a trusted key. > > It was unclear which commits needed to be signed (in the case of `git > merge`) and what were the criteria for the signature to be considered > valid. > > Here is a patch proposal. > > Signed-off-by: The Fox in the Shell <KellerFuchs@hashbang.sh> > ---
I'll leave commenting on and suggesting updates for the above to others.
Show 13 quoted lines
> diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt > index f08e9b8..edd50bf 100644 > --- 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.
I'd suggest doing the addition of the last two lines as a standalone patch, and make the remainder a separate patch on top.
Show 12 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?
Show 12 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?