Re: [PATCH 5/6] builtin/verify-tag: Add --format to verify-tag
On Thu, Sep 22, 2016 at 02:16:21PM -0700, Junio C Hamano wrote: > santi...@nyu.edu writes: > > > From: Santiago Torres> > > > Callers of verify-tag may want to cross-check the tagname from refs/tags > > with the tagname from the tag object header upon GPG verification. This > > is to avoid tag refs that point to an incorrect object. > > > > Add a --format parameter to git verify-tag to print the formatted tag > > object header in addition to or instead of the --verbose or --raw GPG > > verification output. > > > > Signed-off-by: Santiago Torres > > --- > > builtin/verify-tag.c | 13 +++-- > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c > > index 7a1121b..319d469 100644 > > --- a/builtin/verify-tag.c > > +++ b/builtin/verify-tag.c > > @@ -12,12 +12,15 @@ > > #include > > #include "parse-options.h" > > #include "gpg-interface.h" > > +#include "ref-filter.h" > > > > static const char * const verify_tag_usage[] = { > > - N_("git verify-tag [-v | --verbose] ..."), > > + N_("git verify-tag [-v | --verbose] [--format=] > > ..."), > > NULL > > }; > > > > +char *fmt_pretty; > > Does this have to be extern? I do not think so; prepend "static " > in front of it. > > > while (i < argc) { > > unsigned char sha1[20]; > > const char *name = argv[i++]; > > if (get_sha1(name, sha1)) > > had_error = !!error("tag '%s' not found.", name); > > else { > > - if (verify_and_format_tag(sha1, name, NULL, flags)) > > + if (verify_and_format_tag(sha1, name, fmt_pretty, > > flags)) > > OK. The callchain from here is > > verify_and_format_tag() > -> run_gpg_verify() > -> print_signature_buffer() > > so not cramming QUIET into the flags parameter that is already > passed is cumbersome. As I said in my earlier review, it would make > more sense to have the conditional NOT in print_signature_buffer() > but in its caller, but it still is OK to add GPG_VERIFY_QUIET bit > to the flag, which you would check in run_gpg_verify() to decide not > to call print_signature_buffer(). > Yeah, in retrospect, this sounds like a more reasonable approach than doing it on gpg-nterface. I'll keep the QUIET bit then. signature.asc Description: PGP signature
Re: [PATCH 5/6] builtin/verify-tag: Add --format to verify-tag
santi...@nyu.edu writes: > From: Santiago Torres> > Callers of verify-tag may want to cross-check the tagname from refs/tags > with the tagname from the tag object header upon GPG verification. This > is to avoid tag refs that point to an incorrect object. > > Add a --format parameter to git verify-tag to print the formatted tag > object header in addition to or instead of the --verbose or --raw GPG > verification output. > > Signed-off-by: Santiago Torres > --- > builtin/verify-tag.c | 13 +++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c > index 7a1121b..319d469 100644 > --- a/builtin/verify-tag.c > +++ b/builtin/verify-tag.c > @@ -12,12 +12,15 @@ > #include > #include "parse-options.h" > #include "gpg-interface.h" > +#include "ref-filter.h" > > static const char * const verify_tag_usage[] = { > - N_("git verify-tag [-v | --verbose] ..."), > + N_("git verify-tag [-v | --verbose] [--format=] > ..."), > NULL > }; > > +char *fmt_pretty; Does this have to be extern? I do not think so; prepend "static " in front of it. > while (i < argc) { > unsigned char sha1[20]; > const char *name = argv[i++]; > if (get_sha1(name, sha1)) > had_error = !!error("tag '%s' not found.", name); > else { > - if (verify_and_format_tag(sha1, name, NULL, flags)) > + if (verify_and_format_tag(sha1, name, fmt_pretty, > flags)) OK. The callchain from here is verify_and_format_tag() -> run_gpg_verify() -> print_signature_buffer() so not cramming QUIET into the flags parameter that is already passed is cumbersome. As I said in my earlier review, it would make more sense to have the conditional NOT in print_signature_buffer() but in its caller, but it still is OK to add GPG_VERIFY_QUIET bit to the flag, which you would check in run_gpg_verify() to decide not to call print_signature_buffer().
[PATCH 5/6] builtin/verify-tag: Add --format to verify-tag
From: Santiago TorresCallers of verify-tag may want to cross-check the tagname from refs/tags with the tagname from the tag object header upon GPG verification. This is to avoid tag refs that point to an incorrect object. Add a --format parameter to git verify-tag to print the formatted tag object header in addition to or instead of the --verbose or --raw GPG verification output. Signed-off-by: Santiago Torres --- builtin/verify-tag.c | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index 7a1121b..319d469 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -12,12 +12,15 @@ #include #include "parse-options.h" #include "gpg-interface.h" +#include "ref-filter.h" static const char * const verify_tag_usage[] = { - N_("git verify-tag [-v | --verbose] ..."), + N_("git verify-tag [-v | --verbose] [--format=] ..."), NULL }; +char *fmt_pretty; + static int git_verify_tag_config(const char *var, const char *value, void *cb) { int status = git_gpg_config(var, value, cb); @@ -33,6 +36,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) const struct option verify_tag_options[] = { OPT__VERBOSE(, N_("print tag contents")), OPT_BIT(0, "raw", , N_("print raw gpg status output"), GPG_VERIFY_RAW), + OPT_STRING( 0 , "format", _pretty, N_("format"), N_("format to use for the output")), OPT_END() }; @@ -46,13 +50,18 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) if (verbose) flags |= GPG_VERIFY_VERBOSE; + if (fmt_pretty) { + verify_ref_format(fmt_pretty); + flags |= GPG_VERIFY_QUIET; + } + while (i < argc) { unsigned char sha1[20]; const char *name = argv[i++]; if (get_sha1(name, sha1)) had_error = !!error("tag '%s' not found.", name); else { - if (verify_and_format_tag(sha1, name, NULL, flags)) + if (verify_and_format_tag(sha1, name, fmt_pretty, flags)) had_error = 1; } } -- 2.10.0