{"thread":{"id":"42074","subject":"[PATCH v7 4/6] verify-tag: prepare verify_tag for libification","startedAt":"2016-04-19T17:47:15Z","lastAt":"2016-04-19T19:36:28Z","messageCount":11,"participants":["santiago@nyu.edu","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":7,"patchTotal":6},"messages":[{"id":"283810","messageId":"1461088041-19264-1-git-send-email-santiago@nyu.edu","threadId":"42074","inReplyTo":null,"subject":"[PATCH v7 0/6] Move PGP verification out of verify-tag","fromName":"","fromEmail":"santiago@nyu.edu","sentAt":"2016-04-19T17:47:15Z","receivedAt":"2016-04-19T17:47:15Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"From: Santiago Torres \n\nThis is a follow up of [1], [2], [3], [4], [5], [6]. patches 1/6, 2/6, are the\nsame as the corresponding commits in pu.\n\nv7: \nMostly style/clarity changes mostly. Thanks Peff, Eric and Junio for the\nfeedback! In summary: \n\n * Eric pointed out issues with 3/6's commit message. It doesn't match the one \n in pu though. I also took the opportunity to update payload_size to a size_t\n as Peff suggested.\n * 4/6 I updated report_name to name_to_report, I updated the commit message \n and addressed some nits in the code, one of the fixes removed all three nits\n that Eric pointed out. I updated 5/6 to match these changes\n * I gave the commit message on 6/6 another go.\n\nv6: \n * As Junio suggested, updated 4/6, to include the name argument and the\n ternary operator to provide more descriptive error messages. I propagated\n these changes to 5/6 and 6/6 as well. I'm unsure about the 80-column\n on 4/6, the ternary operator is rather long.\n * Updated and reviewed the commit messages based on Eric and Junio's\n feedback\n\nv5:\nAdded helpful feedback by Eric\n\n * Reordering of the patches, to avoid temporal inclusion of a regression\n * Fix typos here and there.\n * Review commit messages, as some weren't representative of what the patches\n were doing anymore.\n * Updated t7030 to include Peff's suggestion, and added a helped-by line here\n as it was mostly Peff's code.\n * Updated the error-handling/printing issues that were introduced when.\n libifying the verify_tag function.\n\nv4:\n\nThanks Eric, Jeff, and Hannes for the feedback.\n\n * I relocated the sigchain_push call so it comes after the error on\n gpg-interface (thanks Hannnes for catching this).\n * I updated the unit test to match the discussion on [3]. Now it generates\n the expected output of the tag on the fly for comparison. (This is just\n copy and paste from [3], but I verified that it works by breaking the\n while)\n * I split moving the code and renaming the variables into two patches so\n these are easier to review.\n * I used an adapter on builtin/tag.c instead of redefining all the fn*\n declarations everywhere. This introduces an issue with the way git tag -v\n resolves refnames though. I added a new commit to restore the previous\n behavior of git-tag. I'm not sure if I should've split this into two commits\n though.\n\nv3:\nThanks Eric, Jeff, for the feedback.\n\n * I separated the patch in multiple sub-patches.\n * I compared the behavior of previous git tag -v and git verify-tag \n invocations to make sure the behavior is the same\n * I dropped the multi-line comment, as suggested.\n * I fixed the issue with the missing brackets in the while (this is \n now detected by the test).\n\nv2:\n\n * I moved the pgp-verification code to tag.c \n * I added extra arguments so git tag -v and git verify-tag both work\n with the same function\n * Relocated the SIGPIPE handling code in verify-tag to gpg-interface\n\nv1:\n \nThe verify tag function is just a thin wrapper around the verify-tag\ncommand. We can avoid one fork call by doing the verification inside\nthe tag builtin instead.\n\n\nThis applies on v2.8.0. \nThanks!\n-Santiago\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/287649\n[2] http://thread.gmane.org/gmane.comp.version-control.git/289836\n[3] http://thread.gmane.org/gmane.comp.version-control.git/290608\n[4] http://thread.gmane.org/gmane.comp.version-control.git/290731\n[5] http://thread.gmane.org/gmane.comp.version-control.git/290790\n[6] http://thread.gmane.org/gmane.comp.version-control.git/291780\n\nSantiago Torres (6):\n builtin/verify-tag.c: ignore SIGPIPE in gpg-interface\n t7030: test verifying multiple tags\n verify-tag: Update run_gpg_verfy len variable name and type\n verify-tag: prepare verify_tag for libification\n verify-tag: move tag verification code to tag.c\n tag -v: verfy directly rather than exec-ing verify-tag\n\n builtin/tag.c | 8 +------\n builtin/verify-tag.c | 62 +++++++--------------------------------------------\n gpg-interface.c | 2 ++\n t/t7030-verify-tag.sh | 13 +++++++++++\n tag.c | 53 +++++++++++++++++++++++++++++++++++++++++++\n tag.h | 2 ++\n 6 files changed, 79 insertions(+), 61 deletions(-)\n\n-- \n2.8.0\n"},{"id":"283808","messageId":"1461088041-19264-2-git-send-email-santiago@nyu.edu","threadId":"42074","inReplyTo":"1461088041-19264-1-git-send-email-santiago@nyu.edu","subject":"[PATCH v7 1/6] builtin/verify-tag.c: ignore SIGPIPE in gpg-interface","fromName":"","fromEmail":"santiago@nyu.edu","sentAt":"2016-04-19T17:47:16Z","receivedAt":"2016-04-19T17:47:16Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"From: Santiago Torres \n\nThe verify_signed_buffer() function may trigger a SIGPIPE when the\nGPG child process terminates early (due to a bad keyid, for example)\nand Git tries to write to it afterwards. Previously, ignoring\nSIGPIPE was done in builtin/verify-tag.c to avoid this issue.\n\nHowever, any other caller who wants to call verify_signed_buffer()\nwould have to do the same.\n\nUse sigchain_push(SIGPIPE, SIG_IGN) in verify_signed_buffer(),\npretty much like in sign_buffer(), so that any caller is not\nrequired to perform this task.\n\nThis will avoid possible mistakes by further developers using\nverify_signed_buffer().\n\nSigned-off-by: Santiago Torres \nReviewed-by: Eric Sunshine \nSigned-off-by: Junio C Hamano \n---\n builtin/verify-tag.c | 3 ---\n gpg-interface.c | 2 ++\n 2 files changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/verify-tag.c b/builtin/verify-tag.c\nindex 00663f6..77f070a 100644\n--- a/builtin/verify-tag.c\n+++ b/builtin/verify-tag.c\n@@ -95,9 +95,6 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)\n \tif (verbose)\n \t\tflags |= GPG_VERIFY_VERBOSE;\n \n-\t/* sometimes the program was terminated because this signal\n-\t * was received in the process of writing the gpg input: */\n-\tsignal(SIGPIPE, SIG_IGN);\n \twhile (i < argc)\n \t\tif (verify_tag(argv[i++], flags))\n \t\t\thad_error = 1;\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 3dc2fe3..2259938 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -237,6 +237,7 @@ int verify_signed_buffer(const char *payload, size_t payload_size,\n \t\treturn error(_(\"could not run gpg.\"));\n \t}\n \n+\tsigchain_push(SIGPIPE, SIG_IGN);\n \twrite_in_full(gpg.in, payload, payload_size);\n \tclose(gpg.in);\n \n@@ -250,6 +251,7 @@ int verify_signed_buffer(const char *payload, size_t payload_size,\n \tclose(gpg.out);\n \n \tret = finish_command(&gpg);\n+\tsigchain_pop(SIGPIPE);\n \n \tunlink_or_warn(path);\n \n-- \n2.8.0\n"},{"id":"283811","messageId":"1461088041-19264-3-git-send-email-santiago@nyu.edu","threadId":"42074","inReplyTo":"1461088041-19264-1-git-send-email-santiago@nyu.edu","subject":"[PATCH v7 2/6] t7030: test verifying multiple tags","fromName":"","fromEmail":"santiago@nyu.edu","sentAt":"2016-04-19T17:47:17Z","receivedAt":"2016-04-19T17:47:17Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"From: Santiago Torres \n\nThe verify-tag command supports multiple tag names to verify, but\nexisting tests only test for invocation with a single tag.\n\nAdd a test invoking it with multiple tags.\n\nHelped-by: Jeff King \nSigned-off-by: Santiago Torres \nReviewed-by: Eric Sunshine \nSigned-off-by: Junio C Hamano \n---\n t/t7030-verify-tag.sh | 13 +++++++++++++\n 1 file changed, 13 insertions(+)\n\ndiff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh\nindex 4608e71..07079a4 100755\n--- a/t/t7030-verify-tag.sh\n+++ b/t/t7030-verify-tag.sh\n@@ -112,4 +112,17 @@ test_expect_success GPG 'verify signatures with --raw' '\n \t)\n '\n \n+test_expect_success GPG 'verify multiple tags' '\n+\ttags=\"fourth-signed sixth-signed seventh-signed\" &&\n+\tfor i in $tags\n+\tdo\n+\t\tgit verify-tag -v --raw $i || return 1\n+\tdone >expect.stdout 2>expect.stderr.1 &&\n+\tgrep \"^.GNUPG:.\" expect.stderr &&\n+\tgit verify-tag -v --raw $tags >actual.stdout 2>actual.stderr.1 &&\n+\tgrep \"^.GNUPG:.\" actual.stderr &&\n+\ttest_cmp expect.stdout actual.stdout &&\n+\ttest_cmp expect.stderr actual.stderr\n+'\n+\n test_done\n-- \n2.8.0\n"},{"id":"283812","messageId":"1461088041-19264-4-git-send-email-santiago@nyu.edu","threadId":"42074","inReplyTo":"1461088041-19264-1-git-send-email-santiago@nyu.edu","subject":"[PATCH v7 3/6] verify-tag: update variable name and type","fromName":"","fromEmail":"santiago@nyu.edu","sentAt":"2016-04-19T17:47:18Z","receivedAt":"2016-04-19T17:47:18Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"From: Santiago Torres \n\nThe run_gpg_verify() function has two variables, size and len.\n\nThis may come off as confusing when reading the code. Clarify which one\npertains to the length of the tag headers by renaming len to\npayload_size. Additionally, change the type of payload_size to size_t to\nmatch the return type of parse_signature.\n\nSigned-off-by: Santiago Torres \nReviewed-by: Eric Sunshine \nSigned-off-by: Junio C Hamano \n---\n builtin/verify-tag.c | 11 ++++++-----\n 1 file changed, 6 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/verify-tag.c b/builtin/verify-tag.c\nindex 77f070a..fa26e40 100644\n--- a/builtin/verify-tag.c\n+++ b/builtin/verify-tag.c\n@@ -21,20 +21,21 @@ static const char * const verify_tag_usage[] = {\n static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)\n {\n \tstruct signature_check sigc;\n-\tint len;\n+\tsize_t payload_size;\n \tint ret;\n \n \tmemset(&sigc, 0, sizeof(sigc));\n \n-\tlen = parse_signature(buf, size);\n+\tpayload_size = parse_signature(buf, size);\n \n-\tif (size == len) {\n+\tif (size == payload_size) {\n \t\tif (flags & GPG_VERIFY_VERBOSE)\n-\t\t\twrite_in_full(1, buf, len);\n+\t\t\twrite_in_full(1, buf, payload_size);\n \t\treturn error(\"no signature found\");\n \t}\n \n-\tret = check_signature(buf, len, buf + len, size - len, &sigc);\n+\tret = check_signature(buf, payload_size, buf + payload_size,\n+\t\t\t\tsize - payload_size, &sigc);\n \tprint_signature_buffer(&sigc, flags);\n \n \tsignature_check_clear(&sigc);\n-- \n2.8.0\n"},{"id":"283807","messageId":"1461088041-19264-5-git-send-email-santiago@nyu.edu","threadId":"42074","inReplyTo":"1461088041-19264-1-git-send-email-santiago@nyu.edu","subject":"[PATCH v7 4/6] verify-tag: prepare verify_tag for libification","fromName":"","fromEmail":"santiago@nyu.edu","sentAt":"2016-04-19T17:47:19Z","receivedAt":"2016-04-19T17:47:19Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"From: Santiago Torres \n\nThe current interface of verify_tag() resolves reference names to SHA1,\nhowever, the plan is to make this functionality public and the current\ninterface is cumbersome for callers: they are expected to supply the\ntextual representation of a sha1/refname. In many cases, this requires\nthem to turn the sha1 to hex representation, just to be converted back\ninside verify_tag.\n\nAdd a SHA1 parameter to use instead of the name parameter, and rename\nthe name parameter to \"name_to_report\" for reporting purposes only.\n\nHelped-by: Junio C Hamano \nSigned-off-by: Santiago Torres \n---\n builtin/verify-tag.c | 27 ++++++++++++++++++---------\n 1 file changed, 18 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/verify-tag.c b/builtin/verify-tag.c\nindex fa26e40..01e956f 100644\n--- a/builtin/verify-tag.c\n+++ b/builtin/verify-tag.c\n@@ -42,25 +42,28 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)\n \treturn ret;\n }\n \n-static int verify_tag(const char *name, unsigned flags)\n+static int verify_tag(const unsigned char *sha1, const char *name_to_report,\n+\t\t\tunsigned flags)\n {\n \tenum object_type type;\n-\tunsigned char sha1[20];\n \tchar *buf;\n \tunsigned long size;\n \tint ret;\n \n-\tif (get_sha1(name, sha1))\n-\t\treturn error(\"tag '%s' not found.\", name);\n-\n \ttype = sha1_object_info(sha1, NULL);\n \tif (type != OBJ_TAG)\n \t\treturn error(\"%s: cannot verify a non-tag object of type %s.\",\n-\t\t\t\tname, typename(type));\n+\t\t\t\tname_to_report ?\n+\t\t\t\tname_to_report :\n+\t\t\t\tfind_unique_abbrev(sha1, DEFAULT_ABBREV),\n+\t\t\t\ttypename(type));\n \n \tbuf = read_sha1_file(sha1, &type, &size);\n \tif (!buf)\n-\t\treturn error(\"%s: unable to read file.\", name);\n+\t\treturn error(\"%s: unable to read file.\",\n+\t\t\t\tname_to_report ?\n+\t\t\t\tname_to_report :\n+\t\t\t\tfind_unique_abbrev(sha1, DEFAULT_ABBREV));\n \n \tret = run_gpg_verify(buf, size, flags);\n \n@@ -80,6 +83,8 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)\n {\n \tint i = 1, verbose = 0, had_error = 0;\n \tunsigned flags = 0;\n+\tunsigned char sha1[20];\n+\tconst char *name;\n \tconst struct option verify_tag_options[] = {\n \t\tOPT__VERBOSE(&verbose, N_(\"print tag contents\")),\n \t\tOPT_BIT(0, \"raw\", &flags, N_(\"print raw gpg status output\"), GPG_VERIFY_RAW),\n@@ -96,8 +101,12 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)\n \tif (verbose)\n \t\tflags |= GPG_VERIFY_VERBOSE;\n \n-\twhile (i < argc)\n-\t\tif (verify_tag(argv[i++], flags))\n+\twhile (i < argc) {\n+\t\tname = argv[i++];\n+\t\tif (get_sha1(name, sha1))\n+\t\t\thad_error = !!error(\"tag '%s' not found.\", name);\n+\t\telse if (verify_tag(sha1, name, flags))\n \t\t\thad_error = 1;\n+\t}\n \treturn had_error;\n }\n-- \n2.8.0\n"},{"id":"283813","messageId":"1461088041-19264-6-git-send-email-santiago@nyu.edu","threadId":"42074","inReplyTo":"1461088041-19264-1-git-send-email-santiago@nyu.edu","subject":"[PATCH v7 5/6] verify-tag: move tag verification code to tag.c","fromName":"","fromEmail":"santiago@nyu.edu","sentAt":"2016-04-19T17:47:20Z","receivedAt":"2016-04-19T17:47:20Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"From: Santiago Torres \n\nThe PGP verification routine for tags could be accessed by other modules\nthat require to do so.\n\nPublish the verify_tag function in tag.c and rename it to gpg_verify_tag\nso it does not conflict with builtin/mktag's static function.\n\nHelped-by: Junio C Hamano \nSigned-off-by: Santiago Torres \n---\n builtin/verify-tag.c | 55 +---------------------------------------------------\n tag.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++\n tag.h | 2 ++\n 3 files changed, 56 insertions(+), 54 deletions(-)\n\ndiff --git a/builtin/verify-tag.c b/builtin/verify-tag.c\nindex 01e956f..714c899 100644\n--- a/builtin/verify-tag.c\n+++ b/builtin/verify-tag.c\n@@ -18,59 +18,6 @@ static const char * const verify_tag_usage[] = {\n \t\tNULL\n };\n \n-static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)\n-{\n-\tstruct signature_check sigc;\n-\tsize_t payload_size;\n-\tint ret;\n-\n-\tmemset(&sigc, 0, sizeof(sigc));\n-\n-\tpayload_size = parse_signature(buf, size);\n-\n-\tif (size == payload_size) {\n-\t\tif (flags & GPG_VERIFY_VERBOSE)\n-\t\t\twrite_in_full(1, buf, payload_size);\n-\t\treturn error(\"no signature found\");\n-\t}\n-\n-\tret = check_signature(buf, payload_size, buf + payload_size,\n-\t\t\t\tsize - payload_size, &sigc);\n-\tprint_signature_buffer(&sigc, flags);\n-\n-\tsignature_check_clear(&sigc);\n-\treturn ret;\n-}\n-\n-static int verify_tag(const unsigned char *sha1, const char *name_to_report,\n-\t\t\tunsigned flags)\n-{\n-\tenum object_type type;\n-\tchar *buf;\n-\tunsigned long size;\n-\tint ret;\n-\n-\ttype = sha1_object_info(sha1, NULL);\n-\tif (type != OBJ_TAG)\n-\t\treturn error(\"%s: cannot verify a non-tag object of type %s.\",\n-\t\t\t\tname_to_report ?\n-\t\t\t\tname_to_report :\n-\t\t\t\tfind_unique_abbrev(sha1, DEFAULT_ABBREV),\n-\t\t\t\ttypename(type));\n-\n-\tbuf = read_sha1_file(sha1, &type, &size);\n-\tif (!buf)\n-\t\treturn error(\"%s: unable to read file.\",\n-\t\t\t\tname_to_report ?\n-\t\t\t\tname_to_report :\n-\t\t\t\tfind_unique_abbrev(sha1, DEFAULT_ABBREV));\n-\n-\tret = run_gpg_verify(buf, size, flags);\n-\n-\tfree(buf);\n-\treturn ret;\n-}\n-\n static int git_verify_tag_config(const char *var, const char *value, void *cb)\n {\n \tint status = git_gpg_config(var, value, cb);\n@@ -105,7 +52,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)\n \t\tname = argv[i++];\n \t\tif (get_sha1(name, sha1))\n \t\t\thad_error = !!error(\"tag '%s' not found.\", name);\n-\t\telse if (verify_tag(sha1, name, flags))\n+\t\telse if (gpg_verify_tag(sha1, name, flags))\n \t\t\thad_error = 1;\n \t}\n \treturn had_error;\ndiff --git a/tag.c b/tag.c\nindex d72f742..ace619a 100644\n--- a/tag.c\n+++ b/tag.c\n@@ -6,6 +6,59 @@\n \n const char *tag_type = \"tag\";\n \n+static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)\n+{\n+\tstruct signature_check sigc;\n+\tsize_t payload_size;\n+\tint ret;\n+\n+\tmemset(&sigc, 0, sizeof(sigc));\n+\n+\tpayload_size = parse_signature(buf, size);\n+\n+\tif (size == payload_size) {\n+\t\tif (flags & GPG_VERIFY_VERBOSE)\n+\t\t\twrite_in_full(1, buf, payload_size);\n+\t\treturn error(\"no signature found\");\n+\t}\n+\n+\tret = check_signature(buf, payload_size, buf + payload_size,\n+\t\t\t\tsize - payload_size, &sigc);\n+\tprint_signature_buffer(&sigc, flags);\n+\n+\tsignature_check_clear(&sigc);\n+\treturn ret;\n+}\n+\n+extern int gpg_verify_tag(const unsigned char *sha1,\n+\t\tconst char *name_to_report, unsigned flags)\n+{\n+\tenum object_type type;\n+\tchar *buf;\n+\tunsigned long size;\n+\tint ret;\n+\n+\ttype = sha1_object_info(sha1, NULL);\n+\tif (type != OBJ_TAG)\n+\t\treturn error(\"%s: cannot verify a non-tag object of type %s.\",\n+\t\t\t\tname_to_report ?\n+\t\t\t\tname_to_report :\n+\t\t\t\tfind_unique_abbrev(sha1, DEFAULT_ABBREV),\n+\t\t\t\ttypename(type));\n+\n+\tbuf = read_sha1_file(sha1, &type, &size);\n+\tif (!buf)\n+\t\treturn error(\"%s: unable to read file.\",\n+\t\t\t\tname_to_report ?\n+\t\t\t\tname_to_report :\n+\t\t\t\tfind_unique_abbrev(sha1, DEFAULT_ABBREV));\n+\n+\tret = run_gpg_verify(buf, size, flags);\n+\n+\tfree(buf);\n+\treturn ret;\n+}\n+\n struct object *deref_tag(struct object *o, const char *warn, int warnlen)\n {\n \twhile (o && o->type == OBJ_TAG)\ndiff --git a/tag.h b/tag.h\nindex f4580ae..a5721b6 100644\n--- a/tag.h\n+++ b/tag.h\n@@ -17,5 +17,7 @@ extern int parse_tag_buffer(struct tag *item, const void *data, unsigned long si\n extern int parse_tag(struct tag *item);\n extern struct object *deref_tag(struct object *, const char *, int);\n extern struct object *deref_tag_noverify(struct object *);\n+extern int gpg_verify_tag(const unsigned char *sha1,\n+\t\tconst char *name_to_report, unsigned flags);\n \n #endif /* TAG_H */\n-- \n2.8.0\n"},{"id":"283809","messageId":"1461088041-19264-7-git-send-email-santiago@nyu.edu","threadId":"42074","inReplyTo":"1461088041-19264-1-git-send-email-santiago@nyu.edu","subject":"[PATCH v7 6/6] tag -v: verfy directly rather than exec-ing verify-tag","fromName":"","fromEmail":"santiago@nyu.edu","sentAt":"2016-04-19T17:47:21Z","receivedAt":"2016-04-19T17:47:21Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"From: Santiago Torres \n\nInstead of having tag -v fork to run verify-tag, use the\ngpg_verify_tag() function directly.\n\nHelped-by: Eric Sunshine \nSigned-off-by: Santiago Torres \n---\n builtin/tag.c | 8 +-------\n 1 file changed, 1 insertion(+), 7 deletions(-)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 1705c94..7b2918e 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -104,13 +104,7 @@ static int delete_tag(const char *name, const char *ref,\n static int verify_tag(const char *name, const char *ref,\n \t\t\t\tconst unsigned char *sha1)\n {\n-\tconst char *argv_verify_tag[] = {\"verify-tag\",\n-\t\t\t\t\t\"-v\", \"SHA1_HEX\", NULL};\n-\targv_verify_tag[2] = sha1_to_hex(sha1);\n-\n-\tif (run_command_v_opt(argv_verify_tag, RUN_GIT_CMD))\n-\t\treturn error(_(\"could not verify the tag '%s'\"), name);\n-\treturn 0;\n+\treturn gpg_verify_tag(sha1, name, GPG_VERIFY_VERBOSE);\n }\n \n static int do_sign(struct strbuf *buffer)\n-- \n2.8.0\n"},{"id":"283829","messageId":"CAPig+cTO_vC1_N79G9yMkiyjYtEeBdsivz-A4jOMw0r-vRWWaw@mail.gmail.com","threadId":"42074","inReplyTo":"1461088041-19264-5-git-send-email-santiago@nyu.edu","subject":"Re: [PATCH v7 4/6] verify-tag: prepare verify_tag for libification","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-04-19T19:05:01Z","receivedAt":"2016-04-19T19:05:01Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Apr 19, 2016 at 1:47 PM, wrote:\n> The current interface of verify_tag() resolves reference names to SHA1,\n> however, the plan is to make this functionality public and the current\n> interface is cumbersome for callers: they are expected to supply the\n> textual representation of a sha1/refname. In many cases, this requires\n> them to turn the sha1 to hex representation, just to be converted back\n> inside verify_tag.\n>\n> Add a SHA1 parameter to use instead of the name parameter, and rename\n> the name parameter to \"name_to_report\" for reporting purposes only.\n\nI'd have probably called this \"display_name\", but then I suppose it\nsuffers the same issue Junio mentioned previously about it sounding\nlike a boolean. Anyhow, as long as Junio is happy with it, that's what\nmatters.\n\nThis version of the patch is nicely improved. One nit below.\n\n> Signed-off-by: Santiago Torres \n> ---\n> diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c\n> @@ -80,6 +83,8 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)\n> {\n> int i = 1, verbose = 0, had_error = 0;\n> unsigned flags = 0;\n> + unsigned char sha1[20];\n> + const char *name;\n\nMentioned previously[1]: These two declarations could be moved inside\nthe while-loop scope (below).\n\n[1]: http://article.gmane.org/gmane.comp.version-control.git/291813\n\n> const struct option verify_tag_options[] = {\n> OPT__VERBOSE(&verbose, N_(\"print tag contents\")),\n> OPT_BIT(0, \"raw\", &flags, N_(\"print raw gpg status output\"), GPG_VERIFY_RAW),\n> @@ -96,8 +101,12 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)\n> if (verbose)\n> flags |= GPG_VERIFY_VERBOSE;\n>\n> - while (i < argc)\n> - if (verify_tag(argv[i++], flags))\n> + while (i < argc) {\n> + name = argv[i++];\n> + if (get_sha1(name, sha1))\n> + had_error = !!error(\"tag '%s' not found.\", name);\n> + else if (verify_tag(sha1, name, flags))\n> had_error = 1;\n> + }\n> return had_error;\n> }\n> --\n> 2.8.0\n"},{"id":"283830","messageId":"CAPig+cQrnrO2+hzg5nc_uBbENOn1_JH1PfHYCg-ivZVTtYRvXg@mail.gmail.com","threadId":"42074","inReplyTo":"1461088041-19264-7-git-send-email-santiago@nyu.edu","subject":"Re: [PATCH v7 6/6] tag -v: verfy directly rather than exec-ing verify-tag","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-04-19T19:06:47Z","receivedAt":"2016-04-19T19:06:47Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Apr 19, 2016 at 1:47 PM, wrote:\n> tag -v: verfy directly rather than exec-ing verify-tag\n\ns/verfy/verify:\n\n> Instead of having tag -v fork to run verify-tag, use the\n> gpg_verify_tag() function directly.\n\nThis description is easy enough to understand. Thanks.\n\n> Signed-off-by: Santiago Torres \n> ---\n> diff --git a/builtin/tag.c b/builtin/tag.c\n> index 1705c94..7b2918e 100644\n> --- a/builtin/tag.c\n> +++ b/builtin/tag.c\n> @@ -104,13 +104,7 @@ static int delete_tag(const char *name, const char *ref,\n> static int verify_tag(const char *name, const char *ref,\n> const unsigned char *sha1)\n> {\n> - const char *argv_verify_tag[] = {\"verify-tag\",\n> - \"-v\", \"SHA1_HEX\", NULL};\n> - argv_verify_tag[2] = sha1_to_hex(sha1);\n> -\n> - if (run_command_v_opt(argv_verify_tag, RUN_GIT_CMD))\n> - return error(_(\"could not verify the tag '%s'\"), name);\n> - return 0;\n> + return gpg_verify_tag(sha1, name, GPG_VERIFY_VERBOSE);\n> }\n"},{"id":"283832","messageId":"CAPig+cRz4stVQWFD-NroVHft2xFvyZJi1ePX9T4zZ3k3=X6ZrA@mail.gmail.com","threadId":"42074","inReplyTo":"1461088041-19264-1-git-send-email-santiago@nyu.edu","subject":"Re: [PATCH v7 0/6] Move PGP verification out of verify-tag","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-04-19T19:10:00Z","receivedAt":"2016-04-19T19:10:00Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Apr 19, 2016 at 1:47 PM, wrote:\n> This is a follow up of [1], [2], [3], [4], [5], [6]. patches 1/6, 2/6, are the\n> same as the corresponding commits in pu.\n>\n> v7:\n> Mostly style/clarity changes mostly. Thanks Peff, Eric and Junio for the\n> feedback! In summary:\n>\n> * Eric pointed out issues with 3/6's commit message. It doesn't match the one\n> in pu though. I also took the opportunity to update payload_size to a size_t\n> as Peff suggested.\n> * 4/6 I updated report_name to name_to_report, I updated the commit message\n> and addressed some nits in the code, one of the fixes removed all three nits\n> that Eric pointed out. I updated 5/6 to match these changes\n> * I gave the commit message on 6/6 another go.\n\nThanks, this re-roll looks good. My reviews mention a couple nits, but\nI hope they are not worth a re-roll (perhaps Junio can address them\nwhen/if he picks up the series).\n\nAs before, the entire series is:\n\n Reviewed-by: Eric Sunshine \n"},{"id":"283836","messageId":"xmqq7fft1jcz.fsf@gitster.mtv.corp.google.com","threadId":"42074","inReplyTo":"CAPig+cTO_vC1_N79G9yMkiyjYtEeBdsivz-A4jOMw0r-vRWWaw@mail.gmail.com","subject":"Re: [PATCH v7 4/6] verify-tag: prepare verify_tag for libification","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-19T19:36:28Z","receivedAt":"2016-04-19T19:36:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> I'd have probably called this \"display_name\", but then I suppose it\n> suffers the same issue Junio mentioned previously about it sounding\n> like a boolean. Anyhow, as long as Junio is happy with it, that's what\n> matters.\n\nNo ;-) I am just trying to help people come up with patches in a\nbetter shape. Somehow \"display name\" does not bother me as much as\n\"report name\" did. name_to_report may be a mouthful, but it conveys\nwhat it is clearly, and I think it is good enough.\n>\n> This version of the patch is nicely improved. One nit below.\n>\n>> Signed-off-by: Santiago Torres \n>> ---\n>> diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c\n>> @@ -80,6 +83,8 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)\n>> {\n>> int i = 1, verbose = 0, had_error = 0;\n>> unsigned flags = 0;\n>> + unsigned char sha1[20];\n>> + const char *name;\n>\n> Mentioned previously[1]: These two declarations could be moved inside\n> the while-loop scope (below).\n>\n> [1]: http://article.gmane.org/gmane.comp.version-control.git/291813\n\nYup.\n\n>\n>> const struct option verify_tag_options[] = {\n>> OPT__VERBOSE(&verbose, N_(\"print tag contents\")),\n>> OPT_BIT(0, \"raw\", &flags, N_(\"print raw gpg status output\"), GPG_VERIFY_RAW),\n>> @@ -96,8 +101,12 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)\n>> if (verbose)\n>> flags |= GPG_VERIFY_VERBOSE;\n>>\n>> - while (i < argc)\n>> - if (verify_tag(argv[i++], flags))\n>> + while (i < argc) {\n>> + name = argv[i++];\n>> + if (get_sha1(name, sha1))\n>> + had_error = !!error(\"tag '%s' not found.\", name);\n>> + else if (verify_tag(sha1, name, flags))\n>> had_error = 1;\n>> + }\n>> return had_error;\n>> }\n>> --\n>> 2.8.0\n"}]}