All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: The Fox in the Shell <KellerFuchs@hashbang.sh>
Cc: git@vger.kernel.org,
	"Michael J. Gruber" <git@drmicha.warpmail.net>,
	"Brian M. Carlson" <sandals@crustytoothpaste.ath.cx>
Subject: Re: [PATCH] Documentation: clarify signature verification
Date: Sun, 10 Apr 2016 11:46:10 -0700	[thread overview]
Message-ID: <xmqqa8l1ti8d.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20160409200756.GA22694@hashbang.sh> (The Fox in the Shell's message of "Sat, 9 Apr 2016 20:08:39 +0000")

The Fox in the Shell <KellerFuchs@hashbang.sh> writes:

> 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.

> 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.

> 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?

> 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?

  reply	other threads:[~2016-04-10 18:47 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-09 20:08 [PATCH] Documentation: clarify signature verification The Fox in the Shell
2016-04-10 18:46 ` Junio C Hamano [this message]
2016-04-11  0:32   ` KellerFuchs
2016-04-11 16:41     ` Junio C Hamano
2016-04-12  1:00       ` KellerFuchs
2016-04-12 15:48         ` Junio C Hamano
2016-05-13  9:51 ` Fox in the shell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqa8l1ti8d.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=KellerFuchs@hashbang.sh \
    --cc=git@drmicha.warpmail.net \
    --cc=git@vger.kernel.org \
    --cc=sandals@crustytoothpaste.ath.cx \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.