{"thread":{"id":"42109","subject":"[PATCH v8 0/6] Move PGP verification out of verify-tag","startedAt":"2016-04-22T14:51:59Z","lastAt":"2016-04-22T17:22:47Z","messageCount":9,"participants":["santiago@nyu.edu","Eric Sunshine"],"isPatch":true,"patchVersion":8,"patchTotal":6},"messages":[{"id":"284143","messageId":"1461336725-29915-1-git-send-email-santiago@nyu.edu","threadId":"42109","inReplyTo":null,"subject":"[PATCH v8 0/6] Move PGP verification out of verify-tag","fromName":"","fromEmail":"santiago@nyu.edu","sentAt":"2016-04-22T14:51:59Z","receivedAt":"2016-04-22T14:51:59Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"From: Santiago Torres <santiago@nyu.edu>\n\nThis is a follow up of [1], [2], [3], [4], [5], [6], and [7].  patches 1/6,\n2/6, and 3/6, are the same as the corresponding commits in pu.\n\nv8:  \nMinor nits, I decided to quickly reroll to drop the extern qualifier in tag.c:\n  * Eric pointed out that we could block-scope the declaration of name and sha1\n    in b/verify-tag.c, for 4/6\n  * There was a typo in 6/6\n  * I dropped the extern qualifier in tag.c for 5/6 as suggested by Ramsay\n    Jones[8]\n\nv7: \nMostly style/clarity changes. 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[7] http://thread.gmane.org/gmane.comp.version-control.git/291887\n[8] http://thread.gmane.org/gmane.comp.version-control.git/292029\n\n\n\nSantiago Torres (6):\n  builtin/verify-tag.c: ignore SIGPIPE in gpg-interface\n  t7030: test verifying multiple tags\n  verify-tag: update 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: verify directly rather than exec-ing verify-tag\n\n builtin/tag.c         |  8 +------\n builtin/verify-tag.c  | 61 ++++++---------------------------------------------\n gpg-interface.c       |  2 ++\n t/t7030-verify-tag.sh | 13 +++++++++++\n tag.c                 | 53 ++++++++++++++++++++++++++++++++++++++++++++\n tag.h                 |  2 ++\n 6 files changed, 78 insertions(+), 61 deletions(-)\n\n-- \n2.8.0\n"},{"id":"284145","messageId":"1461336725-29915-2-git-send-email-santiago@nyu.edu","threadId":"42109","inReplyTo":"1461336725-29915-1-git-send-email-santiago@nyu.edu","subject":"[PATCH v8 1/6] builtin/verify-tag.c: ignore SIGPIPE in gpg-interface","fromName":"","fromEmail":"santiago@nyu.edu","sentAt":"2016-04-22T14:52:00Z","receivedAt":"2016-04-22T14:52:00Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"From: Santiago Torres <santiago@nyu.edu>\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 <santiago@nyu.edu>\nReviewed-by: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\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":"284146","messageId":"1461336725-29915-3-git-send-email-santiago@nyu.edu","threadId":"42109","inReplyTo":"1461336725-29915-1-git-send-email-santiago@nyu.edu","subject":"[PATCH v8 2/6] t7030: test verifying multiple tags","fromName":"","fromEmail":"santiago@nyu.edu","sentAt":"2016-04-22T14:52:01Z","receivedAt":"2016-04-22T14:52:01Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"From: Santiago Torres <santiago@nyu.edu>\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 <peff@peff.net>\nSigned-off-by: Santiago Torres <santiago@nyu.edu>\nReviewed-by: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\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.1 >expect.stderr &&\n+\tgit verify-tag -v --raw $tags >actual.stdout 2>actual.stderr.1 &&\n+\tgrep \"^.GNUPG:.\" <actual.stderr.1 >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":"284148","messageId":"1461336725-29915-4-git-send-email-santiago@nyu.edu","threadId":"42109","inReplyTo":"1461336725-29915-1-git-send-email-santiago@nyu.edu","subject":"[PATCH v8 3/6] verify-tag: update variable name and type","fromName":"","fromEmail":"santiago@nyu.edu","sentAt":"2016-04-22T14:52:02Z","receivedAt":"2016-04-22T14:52:02Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"From: Santiago Torres <santiago@nyu.edu>\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 <santiago@nyu.edu>\nReviewed-by: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\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":"284144","messageId":"1461336725-29915-5-git-send-email-santiago@nyu.edu","threadId":"42109","inReplyTo":"1461336725-29915-1-git-send-email-santiago@nyu.edu","subject":"[PATCH v8 4/6] verify-tag: prepare verify_tag for libification","fromName":"","fromEmail":"santiago@nyu.edu","sentAt":"2016-04-22T14:52:03Z","receivedAt":"2016-04-22T14:52:03Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"From: Santiago Torres <santiago@nyu.edu>\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 <gitster@pobox.com>\nSigned-off-by: Santiago Torres <santiago@nyu.edu>\n---\n builtin/verify-tag.c | 26 +++++++++++++++++---------\n 1 file changed, 17 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/verify-tag.c b/builtin/verify-tag.c\nindex fa26e40..a3d3a43 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@@ -96,8 +99,13 @@ 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\tunsigned char sha1[20];\n+\t\tconst char *name = 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":"284149","messageId":"1461336725-29915-6-git-send-email-santiago@nyu.edu","threadId":"42109","inReplyTo":"1461336725-29915-1-git-send-email-santiago@nyu.edu","subject":"[PATCH v8 5/6] verify-tag: move tag verification code to tag.c","fromName":"","fromEmail":"santiago@nyu.edu","sentAt":"2016-04-22T14:52:04Z","receivedAt":"2016-04-22T14:52:04Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"From: Santiago Torres <santiago@nyu.edu>\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 <gitster@pobox.com>\nSigned-off-by: Santiago Torres <santiago@nyu.edu>\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 a3d3a43..99f8148 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@@ -104,7 +51,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)\n \t\tconst char *name = 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..8363a0e 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+int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report, \n+\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 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":"284147","messageId":"1461336725-29915-7-git-send-email-santiago@nyu.edu","threadId":"42109","inReplyTo":"1461336725-29915-1-git-send-email-santiago@nyu.edu","subject":"[PATCH v8 6/6] tag -v: verify directly rather than exec-ing verify-tag","fromName":"","fromEmail":"santiago@nyu.edu","sentAt":"2016-04-22T14:52:05Z","receivedAt":"2016-04-22T14:52:05Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"From: Santiago Torres <santiago@nyu.edu>\n\nInstead of having tag -v fork to run verify-tag, use the\ngpg_verify_tag() function directly.\n\nHelped-by: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Santiago Torres <santiago@nyu.edu>\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":"284151","messageId":"CAPig+cSXoVeiHsq1m7Ng_+fP0bY3eR20jJqCmTwUF5a1C-F=LA@mail.gmail.com","threadId":"42109","inReplyTo":"1461336725-29915-6-git-send-email-santiago@nyu.edu","subject":"Re: [PATCH v8 5/6] verify-tag: move tag verification code to tag.c","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-04-22T17:19:21Z","receivedAt":"2016-04-22T17:19:21Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Apr 22, 2016 at 10:52 AM,  <santiago@nyu.edu> wrote:\n> The PGP verification routine for tags could be accessed by other modules\n> that require to do so.\n>\n> Publish the verify_tag function in tag.c and rename it to gpg_verify_tag\n> so it does not conflict with builtin/mktag's static function.\n>\n> Helped-by: Junio C Hamano <gitster@pobox.com>\n> Signed-off-by: Santiago Torres <santiago@nyu.edu>\n> ---\n> diff --git a/tag.c b/tag.c\n> @@ -6,6 +6,59 @@\n> +int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report,\n\nNit: This line has trailing whitespace. Probably not worth a re-roll.\n\n> +               unsigned flags)\n> +{\n> +       enum object_type type;\n> +       char *buf;\n> +       unsigned long size;\n> +       int ret;\n> +\n> +       type = sha1_object_info(sha1, NULL);\n> +       if (type != OBJ_TAG)\n> +               return error(\"%s: cannot verify a non-tag object of type %s.\",\n> +                               name_to_report ?\n> +                               name_to_report :\n> +                               find_unique_abbrev(sha1, DEFAULT_ABBREV),\n> +                               typename(type));\n> +\n> +       buf = read_sha1_file(sha1, &type, &size);\n> +       if (!buf)\n> +               return error(\"%s: unable to read file.\",\n> +                               name_to_report ?\n> +                               name_to_report :\n> +                               find_unique_abbrev(sha1, DEFAULT_ABBREV));\n> +\n> +       ret = run_gpg_verify(buf, size, flags);\n> +\n> +       free(buf);\n> +       return ret;\n> +}\n"},{"id":"284152","messageId":"CAPig+cR9i1a7pxOxV4QU2TnoJWKn4mHHVT2tG3+uRysw=sc6qQ@mail.gmail.com","threadId":"42109","inReplyTo":"1461336725-29915-1-git-send-email-santiago@nyu.edu","subject":"Re: [PATCH v8 0/6] Move PGP verification out of verify-tag","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-04-22T17:22:47Z","receivedAt":"2016-04-22T17:22:47Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Apr 22, 2016 at 10:51 AM,  <santiago@nyu.edu> wrote:\n> This is a follow up of [1], [2], [3], [4], [5], [6], and [7].  patches 1/6,\n> 2/6, and 3/6, are the same as the corresponding commits in pu.\n>\n> v8:\n> Minor nits, I decided to quickly reroll to drop the extern qualifier in tag.c:\n>   * Eric pointed out that we could block-scope the declaration of name and sha1\n>     in b/verify-tag.c, for 4/6\n>   * There was a typo in 6/6\n>   * I dropped the extern qualifier in tag.c for 5/6 as suggested by Ramsay\n>     Jones[8]\n\nThanks, this version seems to address the few minor review comments\nfrom v7. Regardless of the whitespace nit in patch 5/6, this series is\nstill:\n\n    Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>\n"}]}