[PATCH] Documentation: clarify signature verification v2

[PATCH] Documentation: clarify signature verification v2

[Fox in the shell]

Hi,

Here is a second attempt at this patch.
Sorry for the delay, life somewhat got in the way.

--
Clarify which commits need to be signed.

Uniformise the vocabulary used wrt. key/signature validity with OpenPGP:
- a signature is valid if made by a key with a valid uid;
- in the default trust-model, a uid is valid if signed by a trusted key;
- a key is trusted if the (local) user set a trust level for it.

Thanks to Junio C Hamano <gitster@pobox.com> for reviewing
  the first attempt at this patch.
---
 Documentation/merge-options.txt  | 7 +++++--
 Documentation/pretty-formats.txt | 4 ++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index f08e9b8..30808a0 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -89,8 +89,11 @@ option can be used to override --squash.
 
 --verify-signatures::
 --no-verify-signatures::
-	Verify that the commits being merged have good and trusted GPG signatures
-	and abort the merge in case they do not.
+	Verify that the tip commit of the side branch being merged is
+	signed with a valid key, i.e. a key that has a valid uid: in the
+	default trust model, this means the signing key has been signed by
+	a trusted key.  If the tip commit of the side branch is not signed
+	with a valid key, the merge is aborted.
 
 --summary::
 --no-summary::
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
 - '%GS': show the name of the signer for a signed commit
 - '%GK': show the key used to sign a signed commit
 - '%gD': reflog selector, e.g., `refs/stash@{1}`
-- 
2.1.4

Re: [PATCH] Documentation: clarify signature verification v2

[Pranit Bauva]

On Thu, May 12, 2016 at 12:20 PM, Fox in the shell
<KellerFuchs@hashbang.sh> wrote:
>
> Hi,
>
> Here is a second attempt at this patch.
> Sorry for the delay, life somewhat got in the way.
>

Its okay! We understand that things might take a little more time than expected!

> --
> Clarify which commits need to be signed.
>
> Uniformise the vocabulary used wrt. key/signature validity with OpenPGP:
> - a signature is valid if made by a key with a valid uid;
> - in the default trust-model, a uid is valid if signed by a trusted key;
> - a key is trusted if the (local) user set a trust level for it.
>
> Thanks to Junio C Hamano <gitster@pobox.com> for reviewing
>   the first attempt at this patch.
> ---

Its good to provide links to the previous version[1] of the patch.
Please go through the Documentation/SubmittingPatches once.
Seems like Junio was waiting for someone to point this out[2].

A couple of notes of how to submit the patches:
 * You have cc'ied the reviewers. Good!

 * Include the version no (v2) inside the [PATCH] like [PATCH v2]

 * The next version should be as a reply to the previous one.
    Hint: use --in-reply-to with git-send-email

 * git-am is used to pick up these patches and it gets the subject
   of the email and strips of [PATCH ...] and then uses the other stuff
   in the commit message headline.

 * The rest of the commit message are the words before ---.
    So currently git-am will pick up your paragraph as commit message:

          "Hi,

            Here is a second attempt at this patch.
            Sorry for the delay, life somewhat got in the way."

    which is quite undesirable as a commit message.

 * Comments are put after ---. So your paragraph
      "Clarify which commits need to be signed.

        Uniformise the vocabulary used wrt. key/signature validity with OpenPGP
         - a signature is valid if made by a key with a valid uid;
         - in the default trust-model, a uid is valid if signed by a
trusted key;
         - a key is trusted if the (local) user set a trust level for it.

           Thanks to Junio C Hamano <gitster@pobox.com> for reviewing
           the first attempt at this patch."

    is actually treated as a comment.

 * Also your signoff is missing.

 * If you want to credit someone then its better to use syntax like:
      "Helped-by: Junio C Hamano <gitster@pobox.com>"

 * It also seems like you probably wanted to add the
   "Reviewed-by:" tag. Please note only the reviewers can
   add that tag.

>  Documentation/merge-options.txt  | 7 +++++--
>  Documentation/pretty-formats.txt | 4 ++--
>  2 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
> index f08e9b8..30808a0 100644
> --- a/Documentation/merge-options.txt
> +++ b/Documentation/merge-options.txt
> @@ -89,8 +89,11 @@ option can be used to override --squash.
>
>  --verify-signatures::
>  --no-verify-signatures::
> -       Verify that the commits being merged have good and trusted GPG signatures
> -       and abort the merge in case they do not.
> +       Verify that the tip commit of the side branch being merged is
> +       signed with a valid key, i.e. a key that has a valid uid: in the
> +       default trust model, this means the signing key has been signed by
> +       a trusted key.  If the tip commit of the side branch is not signed
> +       with a valid key, the merge is aborted.
>
>  --summary::
>  --no-summary::
> 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
>  - '%GS': show the name of the signer for a signed commit
>  - '%GK': show the key used to sign a signed commit
>  - '%gD': reflog selector, e.g., `refs/stash@{1}`
> --
> 2.1.4

[1]: http://thread.gmane.org/gmane.comp.version-control.git/291123
[2]: http://article.gmane.org/gmane.comp.version-control.git/291185

Re: [PATCH] Documentation: clarify signature verification v2

[Pranit Bauva]

On Thu, May 12, 2016 at 1:34 PM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
> On Thu, May 12, 2016 at 12:20 PM, Fox in the shell
> <KellerFuchs@hashbang.sh> wrote:
>>
>> Hi,
>>
>> Here is a second attempt at this patch.
>> Sorry for the delay, life somewhat got in the way.
>>
>
> Its okay! We understand that things might take a little more time than expected!
>
>> --
>> Clarify which commits need to be signed.
>>
>> Uniformise the vocabulary used wrt. key/signature validity with OpenPGP:
>> - a signature is valid if made by a key with a valid uid;
>> - in the default trust-model, a uid is valid if signed by a trusted key;
>> - a key is trusted if the (local) user set a trust level for it.
>>
>> Thanks to Junio C Hamano <gitster@pobox.com> for reviewing
>>   the first attempt at this patch.
>> ---
>
> Its good to provide links to the previous version[1] of the patch.
> Please go through the Documentation/SubmittingPatches once.
> Seems like Junio was waiting for someone to point this out[2].
>
> A couple of notes of how to submit the patches:
>  * You have cc'ied the reviewers. Good!
>
>  * Include the version no (v2) inside the [PATCH] like [PATCH v2]
>
>  * The next version should be as a reply to the previous one.
>     Hint: use --in-reply-to with git-send-email
>
>  * git-am is used to pick up these patches and it gets the subject
>    of the email and strips of [PATCH ...] and then uses the other stuff
>    in the commit message headline.
>
>  * The rest of the commit message are the words before ---.
>     So currently git-am will pick up your paragraph as commit message:
>
>           "Hi,
>
>             Here is a second attempt at this patch.
>             Sorry for the delay, life somewhat got in the way."
>
>     which is quite undesirable as a commit message.
>
>  * Comments are put after ---. So your paragraph
>       "Clarify which commits need to be signed.
>
>         Uniformise the vocabulary used wrt. key/signature validity with OpenPGP
>          - a signature is valid if made by a key with a valid uid;
>          - in the default trust-model, a uid is valid if signed by a
> trusted key;
>          - a key is trusted if the (local) user set a trust level for it.
>
>            Thanks to Junio C Hamano <gitster@pobox.com> for reviewing
>            the first attempt at this patch."
>
>     is actually treated as a comment.
>
>  * Also your signoff is missing.
>
>  * If you want to credit someone then its better to use syntax like:
>       "Helped-by: Junio C Hamano <gitster@pobox.com>"
>
>  * It also seems like you probably wanted to add the
>    "Reviewed-by:" tag. Please note only the reviewers can
>    add that tag.
>
>>  Documentation/merge-options.txt  | 7 +++++--
>>  Documentation/pretty-formats.txt | 4 ++--
>>  2 files changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
>> index f08e9b8..30808a0 100644
>> --- a/Documentation/merge-options.txt
>> +++ b/Documentation/merge-options.txt
>> @@ -89,8 +89,11 @@ option can be used to override --squash.
>>
>>  --verify-signatures::
>>  --no-verify-signatures::
>> -       Verify that the commits being merged have good and trusted GPG signatures
>> -       and abort the merge in case they do not.
>> +       Verify that the tip commit of the side branch being merged is
>> +       signed with a valid key, i.e. a key that has a valid uid: in the
>> +       default trust model, this means the signing key has been signed by
>> +       a trusted key.  If the tip commit of the side branch is not signed
>> +       with a valid key, the merge is aborted.
>>
>>  --summary::
>>  --no-summary::
>> 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
>>  - '%GS': show the name of the signer for a signed commit
>>  - '%GK': show the key used to sign a signed commit
>>  - '%gD': reflog selector, e.g., `refs/stash@{1}`
>> --
>> 2.1.4
>
> [1]: http://thread.gmane.org/gmane.comp.version-control.git/291123
> [2]: http://article.gmane.org/gmane.comp.version-control.git/291185

Forgot to mention. It would be preferable to use your real name in the signoff.

Re: [PATCH] Documentation: clarify signature verification v2

[Junio C Hamano]

Pranit Bauva <pranit.bauva@gmail.com> writes:

> Seems like Junio was waiting for someone to point this out[2].

Thanks. I think you covered most of them correctly; I only have one
thing to add.

>  * Comments are put after ---. So your paragraph
>       "Clarify which commits need to be signed.
>
>         Uniformise the vocabulary used wrt. key/signature validity with OpenPGP
>          - a signature is valid if made by a key with a valid uid;
>          - in the default trust-model, a uid is valid if signed by a
> trusted key;
>          - a key is trusted if the (local) user set a trust level for it.
>
>            Thanks to Junio C Hamano <gitster@pobox.com> for reviewing
>            the first attempt at this patch."
>
>     is actually treated as a comment.

This is half-true, I think. The message you are responding to had
only two dashes, not three.

The usual way to do what the original wanted to do is like this:

	... e-mail headers like From:, Subject:, ...

	Hi,

        Here is a second attempt.

        -- >8 --
        Subject: Documentation: clarify --verify signature

	Clarify that only the signature of the commit at the tip of
	the branch being merged is checked.  Also align the
	vocabulary to describe key & signature validity with those
	used by OpenPGP, namely:

         - a signature is valid if ...
         ...
         - a key is trusted if ...

	Signed-off-by: A U Thor <au@thor.xz>
        ---
         Documentation/merge-options.txt | ... diffstat comes here
        
Notice the "-- >8 --" (cut here) line.  "am" will notice it, discard
what it has read so far and restart from there.

Re: [PATCH] Documentation: clarify signature verification v2

[Pranit Bauva]

On Thu, May 12, 2016 at 10:08 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Pranit Bauva <pranit.bauva@gmail.com> writes:
>
>> Seems like Junio was waiting for someone to point this out[2].
>
> Thanks. I think you covered most of them correctly; I only have one
> thing to add.
>
>>  * Comments are put after ---. So your paragraph
>>       "Clarify which commits need to be signed.
>>
>>         Uniformise the vocabulary used wrt. key/signature validity with OpenPGP
>>          - a signature is valid if made by a key with a valid uid;
>>          - in the default trust-model, a uid is valid if signed by a
>> trusted key;
>>          - a key is trusted if the (local) user set a trust level for it.
>>
>>            Thanks to Junio C Hamano <gitster@pobox.com> for reviewing
>>            the first attempt at this patch."
>>
>>     is actually treated as a comment.
>
> This is half-true, I think. The message you are responding to had
> only two dashes, not three.

My bad. Didn't carefully notice it.

>
> The usual way to do what the original wanted to do is like this:
>
>         ... e-mail headers like From:, Subject:, ...
>
>         Hi,
>
>         Here is a second attempt.
>
>         -- >8 --
>         Subject: Documentation: clarify --verify signature
>
>         Clarify that only the signature of the commit at the tip of
>         the branch being merged is checked.  Also align the
>         vocabulary to describe key & signature validity with those
>         used by OpenPGP, namely:
>
>          - a signature is valid if ...
>          ...
>          - a key is trusted if ...
>
>         Signed-off-by: A U Thor <au@thor.xz>
>         ---
>          Documentation/merge-options.txt | ... diffstat comes here
>
> Notice the "-- >8 --" (cut here) line.  "am" will notice it, discard
> what it has read so far and restart from there.

Not having used this personally, I forgot to mention this. Thanks for
mentioning it!
Looks like you have written the commit message for him. :)

Re: [PATCH] Documentation: clarify signature verification v2

[KellerFuchs]

Thanks a lot for the feedback.

I read Documentation/SubmittingPatches before sending the original patch,
  but it seems not everything had sunk in.
  (And I definitely should have read it again before sending v2...)


I will resubmit the patch, then.

On Thu, May 12, 2016 at 09:38:59AM -0700, Junio C Hamano wrote:
> Pranit Bauva <pranit.bauva@gmail.com> writes:
> 
> > Seems like Junio was waiting for someone to point this out[2].
> 
> Thanks. I think you covered most of them correctly; I only have one
> thing to add.
> 
> >  * Comments are put after ---. So your paragraph
> >       "Clarify which commits need to be signed.
> >
> >         Uniformise the vocabulary used wrt. key/signature validity with OpenPGP
> >          - a signature is valid if made by a key with a valid uid;
> >          - in the default trust-model, a uid is valid if signed by a
> > trusted key;
> >          - a key is trusted if the (local) user set a trust level for it.
> >
> >            Thanks to Junio C Hamano <gitster@pobox.com> for reviewing
> >            the first attempt at this patch."
> >
> >     is actually treated as a comment.
> 
> This is half-true, I think. The message you are responding to had
> only two dashes, not three.
> 
> The usual way to do what the original wanted to do is like this:
> 
> 	... e-mail headers like From:, Subject:, ...
> 
> 	Hi,
> 
>         Here is a second attempt.
> 
>         -- >8 --
>         Subject: Documentation: clarify --verify signature
> 
> 	Clarify that only the signature of the commit at the tip of
> 	the branch being merged is checked.  Also align the
> 	vocabulary to describe key & signature validity with those
> 	used by OpenPGP, namely:
> 
>          - a signature is valid if ...
>          ...
>          - a key is trusted if ...
> 
> 	Signed-off-by: A U Thor <au@thor.xz>
>         ---
>          Documentation/merge-options.txt | ... diffstat comes here
>         
> Notice the "-- >8 --" (cut here) line.  "am" will notice it, discard
> what it has read so far and restart from there.
>