Re: [RFC PATCH] builtin:tag:verify_tag: allow gpg output + pretty

From: Jeff King <peff@peff.net>
Date: Apr 22, 2019, 15:27 UTC
Message-ID: <20190422152726.GB1633@sigill.intra.peff.net>
In-Reply-To: <20190412201432.11328-1-santiago@nyu.edu>

On Fri, Apr 12, 2019 at 04:14:32PM -0400, santiago@nyu.edu wrote:

Show 8 quoted lines

> From: Santiago Torres <santiago@nyu.edu>
> 
> On the git tag -v code, there is a guard to suppress gpg output if a
> pretty format is provided. The rationale for this is that the gpg output
> *and* the pretty formats together may conflict with each other. However,
> both outputs are directed to different output streams and, as such,
> they can safely coexist. Drop the guard clause and use
> GPG_VERIFY_VERBOSE regardless of the pretty format

I think this makes sense. My first worry was whether this would be surprising to any callers, but as you note, they go to different streams.

However, I don't think this patch is quite right, as it causes us to dump the whole tag contents to stdout, as well. E.g.:

  [before]
  $ git tag -v --format='foo %(tag)' v2.21.0
  foo v2.21.0

  [after]
  $ git tag -v --format='foo %(tag)' v2.21.0
  object 8104ec994ea3849a968b4667d072fedd1e688642
  type commit
  tag v2.21.0
  tagger Junio C Hamano <gitster@pobox.com> 1551023739 -0800
  
  Git 2.21
  gpg: Signature made Sun Feb 24 10:55:39 2019 EST
  gpg:                using RSA key E1F036B1FEE7221FC778ECEFB0B5E88696AFE6CB
  gpg: Good signature from "Junio C Hamano <gitster@pobox.com>" [full]
  gpg:                 aka "Junio C Hamano <jch@google.com>" [full]
  gpg:                 aka "Junio C Hamano <junio@pobox.com>" [full]
  foo v2.21.0

I think "git verify-tag" would need similar treatment, too:

  $ git verify-tag v2.21.0
  gpg: Signature made Sun Feb 24 10:55:39 2019 EST
  gpg:                using RSA key E1F036B1FEE7221FC778ECEFB0B5E88696AFE6CB
  gpg: Good signature from "Junio C Hamano <gitster@pobox.com>" [full]
  gpg:                 aka "Junio C Hamano <jch@google.com>" [full]
  gpg:                 aka "Junio C Hamano <junio@pobox.com>" [full]

  $ git verify-tag --format='foo %(tag)' v2.21.0
  foo v2.21.0

In some ways I'm less concerned about verify-tag, though, because the point is that it should be scriptable. And scraping gpg's stderr is not ideal there. We should be parsing --status-fd ourselves and making the result available via format specifier, similar to the way "log --format=%G?" works.

So I think ultimately that's the direction we want to go, but I think in the meantime restoring the gpg output to stderr especially for the porcelain "git tag -v" makes sense for human eyes.

-Peff