Re: [PATCH 0/4] Expose gpgsig in pretty-print

["Michał Górny" <mgorny@gentoo.org>]

[-- Attachment #1: Type: text/plain, Size: 3537 bytes --]

On Fri, 2018-12-14 at 11:07 -0500, John Passaro wrote:
> On Thu, Dec 13, 2018 at 11:12 PM Michał Górny <mgorny@gentoo.org> wrote:
> > 
> > On Thu, 2018-12-13 at 16:22 -0500, John Passaro wrote:
> > > Currently, users who do not have GPG installed have no way to discern
> > > signed from unsigned commits without examining raw commit data. I
> > > propose two new pretty-print placeholders to expose this information:
> > > 
> > > %GR: full ("R"aw) contents of gpgsig header
> > > %G+: Y/N if the commit has nonempty gpgsig header or not
> > > 
> > > The second is of course much more likely to be used, but having exposed
> > > the one, exposing the other too adds almost no complexity.
> > > 
> > > I'm open to suggestion on the names of these placeholders.
> > > 
> > > This commit is based on master but e5a329a279 ("run-command: report exec
> > > failure" 2018-12-11) is required for the tests to pass.
> > > 
> > > One note is that this change touches areas of the pretty-format
> > > documentation that are radically revamped in aw/pretty-trailers: see
> > > 42617752d4 ("doc: group pretty-format.txt placeholders descriptions"
> > > 2018-12-08). I have another version of this branch based on that branch
> > > as well, so you can use that in case conflicts with aw/pretty-trailers
> > > arise.
> > > 
> > > See:
> > > - https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig
> > > - https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig--based-on-aw-pretty-trailers
> > > 
> > > John Passaro (4):
> > >   pretty: expose raw commit signature
> > >   t/t7510-signed-commit.sh: test new placeholders
> > >   doc, tests: pretty behavior when gpg missing
> > >   docs/pretty-formats: add explanation + copy edits
> > > 
> > >  Documentation/pretty-formats.txt |  21 ++++--
> > >  pretty.c                         |  36 ++++++++-
> > >  t/t7510-signed-commit.sh         | 125 +++++++++++++++++++++++++++++--
> > >  3 files changed, 167 insertions(+), 15 deletions(-)
> > > 
> > > 
> > > base-commit: 5d826e972970a784bd7a7bdf587512510097b8c7
> > > prerequisite-patch-id: aedfe228fd293714d9cd0392ac22ff1cba7365db
> > 
> > Just a suggestion: since the raw signature is not very useful without
> > the commit data to check it against, and the commit data is non-trivial
> > to construct (requires mangling raw data anyway), maybe you could either
> > add another placeholder to get the data for signature verification, or
> > (alternatively or simultaneously) add a placeholder that prints both
> > data and signature in the OpenPGP message format (i.e. something you can
> > pass straight to 'gpg --verify').
> > 
> 
> That's a great idea!
> 
> Then I might rename the other new placeholders too:
> 
> %Gs: signed commit signature (blank when unsigned)
> %Gp: signed commit payload (i.e. in practice minus the gpgsig header;
> also blank when unsigned as well)
> %Gq: query/question whether is signed commit ("Y"/"N")
> 
> Thus establishing %G<lowercase> as the gpg-related placeholders that
> do not actually require gpg.
> 
> And add a test that %Gp%n%Gs or the like passes gpg --verify.
> 
> I'll put in a v2 later today or tomorrow. Thank you for the feedback!
> 

Technically speaking, '%Gp%n%Gs' sounds a bit odd, given that
the payload needs to be preceded by the PGP message header but instead
of footer it has the signature's header.  Also note that some lines in
the payload may need to be escaped.

-- 
Best regards,
Michał Górny

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 963 bytes --]