Re: [PATCH v3 4/4] tag: use pgp_verify_function in tag -v call

[Santiago Torres <santiago@nyu.edu>]

> > diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
> > index f776778..8abc357 100644
> > --- a/builtin/verify-tag.c
> > +++ b/builtin/verify-tag.c
> > @@ -30,6 +30,8 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)
> >  {
> >  	int i = 1, verbose = 0, had_error = 0;
> >  	unsigned flags = 0;
> > +	unsigned char sha1[20];
> > +	const char *name;
> >  	const struct option verify_tag_options[] = {
> >  		OPT__VERBOSE(&verbose, N_("print tag contents")),
> >  		OPT_BIT(0, "raw", &flags, N_("print raw gpg status output"), GPG_VERIFY_RAW),
> > @@ -46,8 +48,16 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)
> >  	if (verbose)
> >  		flags |= GPG_VERIFY_VERBOSE;
> >  
> > -	while (i < argc)
> > -		if (pgp_verify_tag(argv[i++], flags))
> > +	while (i < argc) {
> > +		name = argv[i++];
> > +		if (get_sha1(name, sha1)) {
> > +			error("tag '%s' not found.", name);
> >  			had_error = 1;
> > +		}
> > +
> > +		if (pgp_verify_tag(name, NULL, sha1, flags))
> > +			had_error = 1;
> > +
> > +	}
> 
> So this is a good example of the rippling I mentioned earlier.
> 
> As a side note, it might actually be an improvement for pgp_verify_tag
> to take a sha1 (so that git-tag is sure that it is verifying the same
> object that it is printing), but that refactoring should probably come
> separately, I think.
> 
> -Peff

Just to be sure, this refactoring is something we should still include
in this set of patches, right? I think that otherwise we'd lose the
desambigutaion that git tag -v does in this patch.

I also think that most of the rippling is gone if we use and adaptor as
you suggested. Should I add a patch on top of this to support a sha1 as
part for gpg_verify_tag()?

Thanks!
-Santiago.