Re: [PATCH v5 0/6] tag: move PGP verification code to tag.c

[Santiago Torres <santiago@nyu.edu>]

Sorry, forgot to add, this is a follow on to [1].

Thanks,
-Santiago.

[1]:
http://git.661346.n2.nabble.com/PATCH-v4-0-6-tag-move-PGP-verification-code-to-tag-c-td7652451.html
On Tue, Apr 05, 2016 at 12:07:23PM -0400, santiago@nyu.edu wrote:
> From: Santiago Torres <santiago@nyu.edu>
> 
> v5 (this): 
> Added helpful feedback by Eric
> 
>  * Reordering of the patches, to avoid temporal inclusion of a regression
>  * Fix typos here and there.
>  * Review commit messages, as some weren't representative of what the patches
>    were doing anymore.
>  * Updated t7030 to include Peff's suggestion, and added a helped-by line here
>    as it was mostly Peff's code.
>  * Updated the error-handling/printing issues that were introduced when.
>    libifying the verify_tag function.
>    
> v4:
> 
> Thanks Eric, Jeff, and Hannes for the feedback.
> 
>  * I relocated the sigchain_push call so it comes after the error on
>    gpg-interface (thanks Hannnes for catching this).
>  * I updated the unit test to match the discussion on [3]. Now it generates
>    the expected output of the tag on the fly for comparison. (This is just
>    copy and paste from [3], but I verified that it works by breaking the
>    while)
>  * I split moving the code and renaming the variables into two patches so
>    these are easier to review.
>  * I used an adapter on builtin/tag.c instead of redefining all the fn*
>    declarations everywhere. This introduces an issue with the way git tag -v
>    resolves refnames though. I added a new commit to restore the previous
>    behavior of git-tag. I'm not sure if I should've split this into two commits
>    though.
> 
> v3:
> Thanks Eric, Jeff, for the feedback.
> 
>  * I separated the patch in multiple sub-patches.
>  * I compared the behavior of previous git tag -v and git verify-tag 
>    invocations to make sure the behavior is the same
>  * I dropped the multi-line comment, as suggested.
>  * I fixed the issue with the missing brackets in the while (this is 
>    now detected by the test).
> 
> v2:
> 
>  * I moved the pgp-verification code to tag.c 
>  * I added extra arguments so git tag -v and git verify-tag both work
>    with the same function
>  * Relocated the SIGPIPE handling code in verify-tag to gpg-interface
> 
> v1:
>  
> The verify tag function is just a thin wrapper around the verify-tag
> command. We can avoid one fork call by doing the verification inside
> the tag builtin instead.
> 
> 
> This applies on v2.8.0.
> 
> Thanks!
> -Santiago
> 
> 
> Santiago Torres (6):
>   builtin/verify-tag.c: Ignore SIGPIPE on gpg-interface
>   t7030-verify-tag: Adds validation for multiple tags
>   builtin/verify-tag: change variable name for readability
>   builtin/verify-tag: replace name argument with sha1
>   builtin/verify-tag: move verification code to tag.c
>   tag: use gpg_verify_function in tag -v call
> 
>  builtin/tag.c         |  8 +------
>  builtin/verify-tag.c  | 65 +++++++++------------------------------------------
>  gpg-interface.c       |  2 ++
>  t/t7030-verify-tag.sh | 12 ++++++++++
>  tag.c                 | 48 +++++++++++++++++++++++++++++++++++++
>  tag.h                 |  1 +
>  6 files changed, 75 insertions(+), 61 deletions(-)
> 
> -- 
> 2.8.0
>