{"thread":{"id":"44166","subject":"[PATCH v2 0/5] Add --format to tag verification","startedAt":"2016-09-26T22:42:54Z","lastAt":"2016-09-27T18:37:53Z","messageCount":17,"participants":["santiago@nyu.edu","Philip Oakley","Santiago Torres","Junio C Hamano","Lukas Pühringer","Stefan Beller"],"isPatch":true,"patchVersion":2,"patchTotal":5},"messages":[{"id":"302649","messageId":"20160926224233.32702-1-santiago@nyu.edu","threadId":"44166","inReplyTo":null,"subject":"[PATCH v2 0/5] Add --format to tag verification","fromName":"","fromEmail":"santiago@nyu.edu","sentAt":"2016-09-26T22:42:28Z","receivedAt":"2016-09-26T22:42:54Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"From: Santiago Torres \n\nThis is the second iteration of [1], and as a result of the discussion\nin [2].\n\nIn this re-roll we:\n\n* Dropped the commit to move the format string parameter to a global\n variable on builtin/tag. We had to change the signature of\n for_each_name_fn to do this.\n\n* Fixed the signed-off-by lines to match the author/committer\n\n[0001]\n* Moved the GPG_VERIFY_QUIET flag check into tag.c instead of\n gpg_interface\n\n[0002] \n* Changed the exported function to \"format_ref\". This way, we can print\n a single formatted ref element. This also made the patch\n cleaner/easier to read.\n\n[0003] \n* We fixed style/formatting errors that we had introduced\n\nThis patch applies to 2.10.0 and master.\n\n[1] http://public-inbox.org/git/20160922185317.349-1-santiago@nyu.edu/\n[2] http://public-inbox.org/git/20160607195608.16643-1-santiago@nyu.edu/\n\nLukas P (4):\n gpg-interface, tag: add GPG_VERIFY_QUIET flag\n ref-filter: add function to print single ref_array_item\n tag: add format specifier to gpg_verify_tag\n builtin/tag: add --format argument for tag -v\n\nSantiago Torres (1):\n builtin/verify-tag: add --format to verify-tag\n\n builtin/tag.c | 30 ++++++++++++++++++++----------\n builtin/verify-tag.c | 16 ++++++++++++++--\n gpg-interface.h | 1 +\n ref-filter.c | 10 ++++++++++\n ref-filter.h | 4 ++++\n tag.c | 22 +++++++++++++++-------\n tag.h | 4 ++--\n 7 files changed, 66 insertions(+), 21 deletions(-)\n\n-- \n2.10.0\n\n"},{"id":"302650","messageId":"20160926224233.32702-2-santiago@nyu.edu","threadId":"44166","inReplyTo":"20160926224233.32702-1-santiago@nyu.edu","subject":"[PATCH v2 1/5] gpg-interface, tag: add GPG_VERIFY_QUIET flag","fromName":"","fromEmail":"santiago@nyu.edu","sentAt":"2016-09-26T22:42:29Z","receivedAt":"2016-09-26T22:42:57Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"From: Lukas P \n\nFunctions that print git object information may require that the\ngpg-interface functions be silent. Add GPG_VERIFY_QUIET flag and prevent\nprint_signature_buffer from being called if flag is set.\n\nSigned-off-by: Lukas P \n---\n gpg-interface.h | 1 +\n tag.c | 5 ++++-\n 2 files changed, 5 insertions(+), 1 deletion(-)\n\ndiff --git a/gpg-interface.h b/gpg-interface.h\nindex ea68885..85dc982 100644\n--- a/gpg-interface.h\n+++ b/gpg-interface.h\n@@ -3,6 +3,7 @@\n \n #define GPG_VERIFY_VERBOSE\t1\n #define GPG_VERIFY_RAW\t\t2\n+#define GPG_VERIFY_QUIET\t4\n \n struct signature_check {\n \tchar *payload;\ndiff --git a/tag.c b/tag.c\nindex d1dcd18..291073f 100644\n--- a/tag.c\n+++ b/tag.c\n@@ -3,6 +3,7 @@\n #include \"commit.h\"\n #include \"tree.h\"\n #include \"blob.h\"\n+#include \"gpg-interface.h\"\n \n const char *tag_type = \"tag\";\n \n@@ -24,7 +25,9 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)\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+\tif (!(flags & GPG_VERIFY_QUIET))\n+\t\tprint_signature_buffer(&sigc, flags);\n \n \tsignature_check_clear(&sigc);\n \treturn ret;\n-- \n2.10.0\n\n"},{"id":"302651","messageId":"20160926224233.32702-3-santiago@nyu.edu","threadId":"44166","inReplyTo":"20160926224233.32702-1-santiago@nyu.edu","subject":"[PATCH v2 2/5] ref-filter: add function to print single ref_array_item","fromName":"","fromEmail":"santiago@nyu.edu","sentAt":"2016-09-26T22:42:30Z","receivedAt":"2016-09-26T22:43:04Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"From: Lukas P \n\nref-filter functions are useful for printing git object information\nusing a format specifier. However, some other modules may not want to use\nthis functionality on a ref-array but only print a single item.\n\nExpose a format_ref function to create, pretty print and free individual\nref-items.\n\nSigned-off-by: Lukas P \n---\n ref-filter.c | 10 ++++++++++\n ref-filter.h | 4 ++++\n 2 files changed, 14 insertions(+)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex bc551a7..e0aaf5f 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1655,6 +1655,16 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu\n \tputchar('\\n');\n }\n \n+void format_ref(const char *name, const unsigned char *sha1, const char *format,\n+\t\tunsigned kind)\n+{\n+\tstruct ref_array_item *ref_item;\n+\tref_item = new_ref_array_item(name, sha1, 0);\n+\tref_item->kind = kind;\n+\tshow_ref_array_item(ref_item, format, 0);\n+\tfree_array_item(ref_item);\n+}\n+\n /* If no sorting option is given, use refname to sort as default */\n struct ref_sorting *ref_default_sorting(void)\n {\ndiff --git a/ref-filter.h b/ref-filter.h\nindex 14d435e..1ef7999 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -107,4 +107,8 @@ struct ref_sorting *ref_default_sorting(void);\n /* Function to parse --merged and --no-merged options */\n int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset);\n \n+/* Pretty-print a single ref */\n+void format_ref(const char *name, const unsigned char *sha1, const char *format,\n+\t\tunsigned kind);\n+\n #endif /* REF_FILTER_H */\n-- \n2.10.0\n\n"},{"id":"302652","messageId":"20160926224233.32702-4-santiago@nyu.edu","threadId":"44166","inReplyTo":"20160926224233.32702-1-santiago@nyu.edu","subject":"[PATCH v2 3/5] tag: add format specifier to gpg_verify_tag","fromName":"","fromEmail":"santiago@nyu.edu","sentAt":"2016-09-26T22:42:31Z","receivedAt":"2016-09-26T22:43:07Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"From: Lukas P \n\nCalling functions for gpg_verify_tag() may desire to print relevant\ninformation about the header for further verification. Add an optional\nformat argument to print any desired information after GPG verification.\n\nSigned-off-by: Lukas P \n---\n builtin/tag.c | 2 +-\n builtin/verify-tag.c | 2 +-\n tag.c | 17 +++++++++++------\n tag.h | 4 ++--\n 4 files changed, 15 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 50e4ae5..14f3b48 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -105,7 +105,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-\treturn gpg_verify_tag(sha1, name, GPG_VERIFY_VERBOSE);\n+\treturn verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE);\n }\n \n static int do_sign(struct strbuf *buffer)\ndiff --git a/builtin/verify-tag.c b/builtin/verify-tag.c\nindex 99f8148..de10198 100644\n--- a/builtin/verify-tag.c\n+++ b/builtin/verify-tag.c\n@@ -51,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 (gpg_verify_tag(sha1, name, flags))\n+\t\telse if (verify_and_format_tag(sha1, name, NULL, flags))\n \t\t\thad_error = 1;\n \t}\n \treturn had_error;\ndiff --git a/tag.c b/tag.c\nindex 291073f..65e1a96 100644\n--- a/tag.c\n+++ b/tag.c\n@@ -4,6 +4,7 @@\n #include \"tree.h\"\n #include \"blob.h\"\n #include \"gpg-interface.h\"\n+#include \"ref-filter.h\"\n \n const char *tag_type = \"tag\";\n \n@@ -33,8 +34,8 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)\n \treturn ret;\n }\n \n-int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report,\n-\t\tunsigned flags)\n+int verify_and_format_tag(const unsigned char *sha1, const char *name,\n+\t\tconst char *fmt_pretty, unsigned flags)\n {\n \tenum object_type type;\n \tchar *buf;\n@@ -44,21 +45,25 @@ int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report,\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\tname ?\n+\t\t\t\tname :\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\tname ?\n+\t\t\t\tname :\n \t\t\t\tfind_unique_abbrev(sha1, DEFAULT_ABBREV));\n \n \tret = run_gpg_verify(buf, size, flags);\n \n \tfree(buf);\n+\n+\tif (fmt_pretty)\n+\t\tformat_ref(name, sha1, fmt_pretty, FILTER_REFS_TAGS);\n+\n \treturn ret;\n }\n \ndiff --git a/tag.h b/tag.h\nindex a5721b6..0b6e458 100644\n--- a/tag.h\n+++ b/tag.h\n@@ -17,7 +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+extern int verify_and_format_tag(const unsigned char *sha1, const char *name,\n+\t\tconst char *fmt_pretty, unsigned flags);\n \n #endif /* TAG_H */\n-- \n2.10.0\n\n"},{"id":"302653","messageId":"20160926224233.32702-6-santiago@nyu.edu","threadId":"44166","inReplyTo":"20160926224233.32702-1-santiago@nyu.edu","subject":"[PATCH v2 5/5] builtin/tag: add --format argument for tag -v","fromName":"","fromEmail":"santiago@nyu.edu","sentAt":"2016-09-26T22:42:33Z","receivedAt":"2016-09-26T22:43:14Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"From: Lukas P \n\nAdding --format to git tag -v mutes the default output of the GPG\nverification and instead prints the formatted tag object.\nThis allows callers to cross-check the tagname from refs/tags with\nthe tagname from the tag object header upon GPG verification.\n\nCaveat: The change adds a format specifier argument to the\n(*each_tag_name_fn) function pointer, i.e. delete_tag now receives this\ntoo, although it does not need it.\n\nSigned-off-by: Lukas P \n---\n builtin/tag.c | 30 ++++++++++++++++++++----------\n 1 file changed, 20 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 14f3b48..f53227e 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -24,7 +24,7 @@ static const char * const git_tag_usage[] = {\n \tN_(\"git tag -d ...\"),\n \tN_(\"git tag -l [-n[]] [--contains ] [--points-at ]\"\n \t\t\"\\n\\t\\t[--format=] [--[no-]merged []] [...]\"),\n-\tN_(\"git tag -v ...\"),\n+\tN_(\"git tag -v [--format=] ...\"),\n \tNULL\n };\n \n@@ -66,9 +66,10 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con\n }\n \n typedef int (*each_tag_name_fn)(const char *name, const char *ref,\n-\t\t\t\tconst unsigned char *sha1);\n+\t\t\t\tconst unsigned char *sha1, const char *fmt_pretty);\n \n-static int for_each_tag_name(const char **argv, each_tag_name_fn fn)\n+static int for_each_tag_name(const char **argv, each_tag_name_fn fn,\n+\t\tconst char *fmt_pretty)\n {\n \tconst char **p;\n \tchar ref[PATH_MAX];\n@@ -87,14 +88,14 @@ static int for_each_tag_name(const char **argv, each_tag_name_fn fn)\n \t\t\thad_error = 1;\n \t\t\tcontinue;\n \t\t}\n-\t\tif (fn(*p, ref, sha1))\n+\t\tif (fn(*p, ref, sha1, fmt_pretty))\n \t\t\thad_error = 1;\n \t}\n \treturn had_error;\n }\n \n static int delete_tag(const char *name, const char *ref,\n-\t\t\t\tconst unsigned char *sha1)\n+\t\t\t\tconst unsigned char *sha1, const char *fmt_pretty)\n {\n \tif (delete_ref(ref, sha1, 0))\n \t\treturn 1;\n@@ -103,9 +104,15 @@ static int delete_tag(const char *name, const char *ref,\n }\n \n static int verify_tag(const char *name, const char *ref,\n-\t\t\t\tconst unsigned char *sha1)\n+\t\t\t\tconst unsigned char *sha1, const char *fmt_pretty)\n {\n-\treturn verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE);\n+\tint flags;\n+\tflags = GPG_VERIFY_VERBOSE;\n+\n+\tif (fmt_pretty)\n+\t\tflags = GPG_VERIFY_QUIET;\n+\n+\treturn verify_and_format_tag(sha1, name, fmt_pretty, flags);\n }\n \n static int do_sign(struct strbuf *buffer)\n@@ -424,9 +431,12 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tif (filter.merge_commit)\n \t\tdie(_(\"--merged and --no-merged option are only allowed with -l\"));\n \tif (cmdmode == 'd')\n-\t\treturn for_each_tag_name(argv, delete_tag);\n-\tif (cmdmode == 'v')\n-\t\treturn for_each_tag_name(argv, verify_tag);\n+\t\treturn for_each_tag_name(argv, delete_tag, NULL);\n+\tif (cmdmode == 'v') {\n+\t\tif (format)\n+\t\t\tverify_ref_format(format);\n+\t\treturn for_each_tag_name(argv, verify_tag, format);\n+\t}\n \n \tif (msg.given || msgfile) {\n \t\tif (msg.given && msgfile)\n-- \n2.10.0\n\n"},{"id":"302654","messageId":"20160926224233.32702-5-santiago@nyu.edu","threadId":"44166","inReplyTo":"20160926224233.32702-1-santiago@nyu.edu","subject":"[PATCH v2 4/5] builtin/verify-tag: add --format to verify-tag","fromName":"","fromEmail":"santiago@nyu.edu","sentAt":"2016-09-26T22:42:32Z","receivedAt":"2016-09-26T22:43:17Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"From: Santiago Torres \n\nCallers of verify-tag may want to cross-check the tagname from refs/tags\nwith the tagname from the tag object header upon GPG verification. This\nis to avoid tag refs that point to an incorrect object.\n\nAdd a --format parameter to git verify-tag to print the formatted tag\nobject header in addition to or instead of the --verbose or --raw GPG\nverification output.\n\nSigned-off-by: Santiago Torres \n---\n builtin/verify-tag.c | 13 +++++++++++--\n 1 file changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/verify-tag.c b/builtin/verify-tag.c\nindex de10198..a941053 100644\n--- a/builtin/verify-tag.c\n+++ b/builtin/verify-tag.c\n@@ -12,12 +12,15 @@\n #include \n #include \"parse-options.h\"\n #include \"gpg-interface.h\"\n+#include \"ref-filter.h\"\n \n static const char * const verify_tag_usage[] = {\n-\t\tN_(\"git verify-tag [-v | --verbose] ...\"),\n+\t\tN_(\"git verify-tag [-v | --verbose] [--format=] ...\"),\n \t\tNULL\n };\n \n+static char *fmt_pretty;\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@@ -33,6 +36,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)\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+\t\tOPT_STRING( 0 , \"format\", &fmt_pretty, N_(\"format\"), N_(\"format to use for the output\")),\n \t\tOPT_END()\n \t};\n \n@@ -46,12 +50,17 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)\n \tif (verbose)\n \t\tflags |= GPG_VERIFY_VERBOSE;\n \n+\tif (fmt_pretty) {\n+\t\tverify_ref_format(fmt_pretty);\n+\t\tflags |= GPG_VERIFY_QUIET;\n+\t}\n+\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_and_format_tag(sha1, name, NULL, flags))\n+\t\telse if (verify_and_format_tag(sha1, name, fmt_pretty, flags))\n \t\t\thad_error = 1;\n \t}\n \treturn had_error;\n-- \n2.10.0\n\n"},{"id":"302685","messageId":"D5DC2D99E1F84C308DDB99096B0E3BCD@PhilipOakley","threadId":"44166","inReplyTo":"20160926224233.32702-5-santiago@nyu.edu","subject":"Re: [PATCH v2 4/5] builtin/verify-tag: add --format to verify-tag","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2016-09-27T07:44:57Z","receivedAt":"2016-09-27T07:45:54Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \n> From: Santiago Torres \n>\n> Callers of verify-tag may want to cross-check the tagname from refs/tags\n> with the tagname from the tag object header upon GPG verification. This\n> is to avoid tag refs that point to an incorrect object.\n>\n> Add a --format parameter to git verify-tag to print the formatted tag\n> object header in addition to or instead of the --verbose or --raw GPG\n> verification output.\n>\n> Signed-off-by: Santiago Torres \n> ---\n> builtin/verify-tag.c | 13 +++++++++++--\n> 1 file changed, 11 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c\n> index de10198..a941053 100644\n> --- a/builtin/verify-tag.c\n> +++ b/builtin/verify-tag.c\n> @@ -12,12 +12,15 @@\n> #include \n> #include \"parse-options.h\"\n> #include \"gpg-interface.h\"\n> +#include \"ref-filter.h\"\n>\n> static const char * const verify_tag_usage[] = {\n> - N_(\"git verify-tag [-v | --verbose] ...\"),\n> + N_(\"git verify-tag [-v | --verbose] [--format=] ...\"),\n\nDoes this require a corresponding documentation change? (also 5/5)\n\n> NULL\n> };\n>\n> +static char *fmt_pretty;\n> +\n> static int git_verify_tag_config(const char *var, const char *value, void \n> *cb)\n> {\n> int status = git_gpg_config(var, value, cb);\n> @@ -33,6 +36,7 @@ int cmd_verify_tag(int argc, const char **argv, const \n> char *prefix)\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\"), \n> GPG_VERIFY_RAW),\n> + OPT_STRING( 0 , \"format\", &fmt_pretty, N_(\"format\"), N_(\"format to use \n> for the output\")),\n> OPT_END()\n> };\n>\n> @@ -46,12 +50,17 @@ int cmd_verify_tag(int argc, const char **argv, const \n> char *prefix)\n> if (verbose)\n> flags |= GPG_VERIFY_VERBOSE;\n>\n> + if (fmt_pretty) {\n> + verify_ref_format(fmt_pretty);\n> + flags |= GPG_VERIFY_QUIET;\n> + }\n> +\n> while (i < argc) {\n> unsigned char sha1[20];\n> const char *name = argv[i++];\n> if (get_sha1(name, sha1))\n> had_error = !!error(\"tag '%s' not found.\", name);\n> - else if (verify_and_format_tag(sha1, name, NULL, flags))\n> + else if (verify_and_format_tag(sha1, name, fmt_pretty, flags))\n> had_error = 1;\n> }\n> return had_error;\n> -- \n> 2.10.0\n>\n--\nPhilip \n\n"},{"id":"302703","messageId":"20160927143814.z54h3wi55t5xnqlu@LykOS.localdomain","threadId":"44166","inReplyTo":"D5DC2D99E1F84C308DDB99096B0E3BCD@PhilipOakley","subject":"Re: [PATCH v2 4/5] builtin/verify-tag: add --format to verify-tag","fromName":"Santiago Torres","fromEmail":"santiago@nyu.edu","sentAt":"2016-09-27T14:38:15Z","receivedAt":"2016-09-27T14:38:34Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"> > static const char * const verify_tag_usage[] = {\n> > - N_(\"git verify-tag [-v | --verbose] ...\"),\n> > + N_(\"git verify-tag [-v | --verbose] [--format=] ...\"),\n> \n> Does this require a corresponding documentation change? (also 5/5)\n> \n\nYes, I'll work on this while I wait for more reviews.\n\nThanks!\n-Santiago.\n"},{"id":"302724","messageId":"xmqqfuolp7nk.fsf@gitster.mtv.corp.google.com","threadId":"44166","inReplyTo":"20160926224233.32702-3-santiago@nyu.edu","subject":"Re: [PATCH v2 2/5] ref-filter: add function to print single ref_array_item","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-27T17:35:59Z","receivedAt":"2016-09-27T17:36:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"santiago@nyu.edu writes:\n\n> From: Lukas P \n>\n> ref-filter functions are useful for printing git object information\n> using a format specifier. However, some other modules may not want to use\n> this functionality on a ref-array but only print a single item.\n>\n> Expose a format_ref function to create, pretty print and free individual\n> ref-items.\n>\n> Signed-off-by: Lukas P \n> ---\n> +void format_ref(const char *name, const unsigned char *sha1, const char *format,\n> +\t\tunsigned kind)\n> +{\n> +\tstruct ref_array_item *ref_item;\n> +\tref_item = new_ref_array_item(name, sha1, 0);\n> +\tref_item->kind = kind;\n> +\tshow_ref_array_item(ref_item, format, 0);\n> +\tfree_array_item(ref_item);\n> +}\n> +\n> /* If no sorting option is given, use refname to sort as default */\n> struct ref_sorting *ref_default_sorting(void)\n> {\n> diff --git a/ref-filter.h b/ref-filter.h\n> index 14d435e..1ef7999 100644\n> --- a/ref-filter.h\n> +++ b/ref-filter.h\n> @@ -107,4 +107,8 @@ struct ref_sorting *ref_default_sorting(void);\n> /* Function to parse --merged and --no-merged options */\n> int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset);\n> \n> +/* Pretty-print a single ref */\n> +void format_ref(const char *name, const unsigned char *sha1, const char *format,\n> +\t\tunsigned kind);\n\nThe fact that you felt a need for comment before its name is a\nstrong sign that the name is not sufficiently descriptive and\nunderstandable for readers to tell what the function is for.\n\nWould pretty_print_ref() or show_ref_pretty() better names, perhaps?\n\n"},{"id":"302725","messageId":"xmqqbmz9p7m0.fsf@gitster.mtv.corp.google.com","threadId":"44166","inReplyTo":"20160926224233.32702-2-santiago@nyu.edu","subject":"Re: [PATCH v2 1/5] gpg-interface, tag: add GPG_VERIFY_QUIET flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-27T17:36:55Z","receivedAt":"2016-09-27T17:37:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"santiago@nyu.edu writes:\n\n> From: Lukas P \n>\n> Functions that print git object information may require that the\n> gpg-interface functions be silent. Add GPG_VERIFY_QUIET flag and prevent\n> print_signature_buffer from being called if flag is set.\n>\n> Signed-off-by: Lukas P \n\nAre you and Lukas sure that \"Lukas P\" is how luk.puehringer wants to\nbe known by the world? Just checking.\n\n"},{"id":"302726","messageId":"xmqq7f9xp7ev.fsf@gitster.mtv.corp.google.com","threadId":"44166","inReplyTo":"20160926224233.32702-5-santiago@nyu.edu","subject":"Re: [PATCH v2 4/5] builtin/verify-tag: add --format to verify-tag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-27T17:41:12Z","receivedAt":"2016-09-27T17:41:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"santiago@nyu.edu writes:\n\n> From: Santiago Torres \n>\n> Callers of verify-tag may want to cross-check the tagname from refs/tags\n> with the tagname from the tag object header upon GPG verification. This\n> is to avoid tag refs that point to an incorrect object.\n>\n> Add a --format parameter to git verify-tag to print the formatted tag\n> object header in addition to or instead of the --verbose or --raw GPG\n> verification output.\n>\n> Signed-off-by: Santiago Torres \n> ---\n> builtin/verify-tag.c | 13 +++++++++++--\n> 1 file changed, 11 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c\n> index de10198..a941053 100644\n> --- a/builtin/verify-tag.c\n> +++ b/builtin/verify-tag.c\n> @@ -12,12 +12,15 @@\n> #include \n> #include \"parse-options.h\"\n> #include \"gpg-interface.h\"\n> +#include \"ref-filter.h\"\n> \n> static const char * const verify_tag_usage[] = {\n> -\t\tN_(\"git verify-tag [-v | --verbose] ...\"),\n> +\t\tN_(\"git verify-tag [-v | --verbose] [--format=] ...\"),\n> \t\tNULL\n> };\n> \n> +static char *fmt_pretty;\n> +\n\nI'd suggest to remove this, and then ...\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> @@ -33,6 +36,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)\n\n... instead add\n\n\tchar *fmt_pretty = NULL;\n\nhere, imitating the way \"&flags\" is handled. You do not need it to\nbe visible outside the function.\n\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> +\t\tOPT_STRING( 0 , \"format\", &fmt_pretty, N_(\"format\"), N_(\"format to use for the output\")),\n> \t\tOPT_END()\n> \t};\n> \n> @@ -46,12 +50,17 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)\n> \tif (verbose)\n> \t\tflags |= GPG_VERIFY_VERBOSE;\n> \n> +\tif (fmt_pretty) {\n> +\t\tverify_ref_format(fmt_pretty);\n> +\t\tflags |= GPG_VERIFY_QUIET;\n> +\t}\n> +\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_and_format_tag(sha1, name, NULL, flags))\n> +\t\telse if (verify_and_format_tag(sha1, name, fmt_pretty, flags))\n> \t\t\thad_error = 1;\n> \t}\n> \treturn had_error;\n"},{"id":"302728","messageId":"xmqq37klp6yp.fsf@gitster.mtv.corp.google.com","threadId":"44166","inReplyTo":"20160926224233.32702-6-santiago@nyu.edu","subject":"Re: [PATCH v2 5/5] builtin/tag: add --format argument for tag -v","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-27T17:50:54Z","receivedAt":"2016-09-27T17:51:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"santiago@nyu.edu writes:\n\n> From: Lukas P \n>\n> Adding --format to git tag -v mutes the default output of the GPG\n> verification and instead prints the formatted tag object.\n> This allows callers to cross-check the tagname from refs/tags with\n> the tagname from the tag object header upon GPG verification.\n>\n> Caveat: The change adds a format specifier argument to the\n> (*each_tag_name_fn) function pointer, i.e. delete_tag now receives this\n> too, although it does not need it.\n\nThat's an interesting \"caveat\".\n\nGenerally it is a good idea to give an additional opaque pointer to\ncallback functions of iteration API so that code that uses the\niteration can pass custom data to its callback.\n\nLooking at the way you enhanced each_tag_name_fn, however, you added\na specific argument instead; that is the only reason why you need a\n\"caveat\". If it were \"void *\", it would have been in line with the\nusual practice, not worth mentioning as a \"caveat\", but could even\nbe advertised as a feature, replacing the last \"Caveat\" paragraph\nwith something like this:\n\n\tThe callback function for for_each_tag_name() didn't allow\n\tcallers to pass custom data to their callback functions.\n\tAdd a new opaque pointer to each_tag_name_fn's parameter to\n\tallow this.\n\n> Signed-off-by: Lukas P \n> ---\n> builtin/tag.c | 30 ++++++++++++++++++++----------\n> 1 file changed, 20 insertions(+), 10 deletions(-)\n>\n> diff --git a/builtin/tag.c b/builtin/tag.c\n> index 14f3b48..f53227e 100644\n> --- a/builtin/tag.c\n> +++ b/builtin/tag.c\n> @@ -24,7 +24,7 @@ static const char * const git_tag_usage[] = {\n> \tN_(\"git tag -d ...\"),\n> \tN_(\"git tag -l [-n[]] [--contains ] [--points-at ]\"\n> \t\t\"\\n\\t\\t[--format=] [--[no-]merged []] [...]\"),\n> -\tN_(\"git tag -v ...\"),\n> +\tN_(\"git tag -v [--format=] ...\"),\n> \tNULL\n> };\n> \n> @@ -66,9 +66,10 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con\n> }\n> \n> typedef int (*each_tag_name_fn)(const char *name, const char *ref,\n> -\t\t\t\tconst unsigned char *sha1);\n> +\t\t\t\tconst unsigned char *sha1, const char *fmt_pretty);\n\nYou'd replace \"const char *fmt_pretty\" with \"void *cb_data\" here, and...\n> \n> -static int for_each_tag_name(const char **argv, each_tag_name_fn fn)\n> +static int for_each_tag_name(const char **argv, each_tag_name_fn fn,\n> +\t\tconst char *fmt_pretty)\n\n... also here. Then introduce fmt_pretty as an auto variable in the\nfunction ...\n\n> {\n> \tconst char **p;\n\n... by adding this line here:\n\n\tconst char *fmt_pretty = cb_data;\n\n> \tchar ref[PATH_MAX];\n> @@ -87,14 +88,14 @@ static int for_each_tag_name(const char **argv, each_tag_name_fn fn)\n> \t\t\thad_error = 1;\n> \t\t\tcontinue;\n> \t\t}\n> -\t\tif (fn(*p, ref, sha1))\n> +\t\tif (fn(*p, ref, sha1, fmt_pretty))\n> \t\t\thad_error = 1;\n> \t}\n> \treturn had_error;\n> }\n> \n> static int delete_tag(const char *name, const char *ref,\n> -\t\t\t\tconst unsigned char *sha1)\n> +\t\t\t\tconst unsigned char *sha1, const char *fmt_pretty)\n\nAnd this \"const char *fmt_pretty\" also becomes \"void *cb_data\"...\n\n> {\n> \tif (delete_ref(ref, sha1, 0))\n> \t\treturn 1;\n> @@ -103,9 +104,15 @@ static int delete_tag(const char *name, const char *ref,\n> }\n> \n> static int verify_tag(const char *name, const char *ref,\n> -\t\t\t\tconst unsigned char *sha1)\n> +\t\t\t\tconst unsigned char *sha1, const char *fmt_pretty)\n\n... and here. Reintroduce fmt_pretty as a name local to the\nfunction by doing the same thing as for_each_tag_name() above.\n\n> {\n> -\treturn verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE);\n> +\tint flags;\n> +\tflags = GPG_VERIFY_VERBOSE;\n> +\n> +\tif (fmt_pretty)\n> +\t\tflags = GPG_VERIFY_QUIET;\n> +\n> +\treturn verify_and_format_tag(sha1, name, fmt_pretty, flags);\n> }\n> \n> static int do_sign(struct strbuf *buffer)\n> @@ -424,9 +431,12 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n> \tif (filter.merge_commit)\n> \t\tdie(_(\"--merged and --no-merged option are only allowed with -l\"));\n> \tif (cmdmode == 'd')\n> -\t\treturn for_each_tag_name(argv, delete_tag);\n> -\tif (cmdmode == 'v')\n> -\t\treturn for_each_tag_name(argv, verify_tag);\n> +\t\treturn for_each_tag_name(argv, delete_tag, NULL);\n> +\tif (cmdmode == 'v') {\n> +\t\tif (format)\n> +\t\t\tverify_ref_format(format);\n> +\t\treturn for_each_tag_name(argv, verify_tag, format);\n> +\t}\n\nThanks.\n"},{"id":"302730","messageId":"15BEFDC3-81B2-47FC-A213-F37B4E6020D2@gmail.com","threadId":"44166","inReplyTo":"xmqqbmz9p7m0.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 1/5] gpg-interface, tag: add GPG_VERIFY_QUIET flag","fromName":"Lukas Pühringer","fromEmail":"luk.puehringer@gmail.com","sentAt":"2016-09-27T18:17:35Z","receivedAt":"2016-09-27T18:18:01Z","isPatch":true,"sender":{"key":"luk.puehringer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/589324?v=4"},"body":"Thanks for checking. I am fine with Lukas P, unless git prefers full last names. In that case I am fine with changing too.\n\nBest,\nLukas P\n\n> On Sep 27, 2016, at 1:36 PM, Junio C Hamano wrote:\n> \n> santiago@nyu.edu writes:\n> \n>> From: Lukas P \n>> \n>> Functions that print git object information may require that the\n>> gpg-interface functions be silent. Add GPG_VERIFY_QUIET flag and prevent\n>> print_signature_buffer from being called if flag is set.\n>> \n>> Signed-off-by: Lukas P \n> \n> Are you and Lukas sure that \"Lukas P\" is how luk.puehringer wants to\n> be known by the world? Just checking.\n> \n\n"},{"id":"302732","messageId":"xmqqponpnqyf.fsf@gitster.mtv.corp.google.com","threadId":"44166","inReplyTo":"15BEFDC3-81B2-47FC-A213-F37B4E6020D2@gmail.com","subject":"Re: [PATCH v2 1/5] gpg-interface, tag: add GPG_VERIFY_QUIET flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-27T18:22:00Z","receivedAt":"2016-09-27T18:22:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lukas Pühringer writes:\n\n> Thanks for checking. I am fine with Lukas P, unless git prefers\n> full last names. In that case I am fine with changing too.\n\nWe do prefer full names, so that it would be consistent with court\ndocument when you are involved in copyright inflingement case ;-)\n"},{"id":"302733","messageId":"3D863101-CA73-4718-A1E7-A1664A5BDB58@gmail.com","threadId":"44166","inReplyTo":"xmqqponpnqyf.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 1/5] gpg-interface, tag: add GPG_VERIFY_QUIET flag","fromName":"Lukas Pühringer","fromEmail":"luk.puehringer@gmail.com","sentAt":"2016-09-27T18:25:06Z","receivedAt":"2016-09-27T18:25:13Z","isPatch":true,"sender":{"key":"luk.puehringer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/589324?v=4"},"body":"Good, I will change it to 'Lukas Puehringer' then, when we send you the updated batch of patches, that address your latest comments.\n\nThanks,\nLukas\n\n> On Sep 27, 2016, at 2:22 PM, Junio C Hamano wrote:\n> \n> Lukas Pühringer writes:\n> \n>> Thanks for checking. I am fine with Lukas P, unless git prefers\n>> full last names. In that case I am fine with changing too.\n> \n> We do prefer full names, so that it would be consistent with court\n> document when you are involved in copyright inflingement case ;-)\n\n"},{"id":"302735","messageId":"CAGZ79kYitOd5-Cxr0VzhRH70B=z2gh1q2aB5xiwe+HwHuU7dYg@mail.gmail.com","threadId":"44166","inReplyTo":"3D863101-CA73-4718-A1E7-A1664A5BDB58@gmail.com","subject":"Re: [PATCH v2 1/5] gpg-interface, tag: add GPG_VERIFY_QUIET flag","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-09-27T18:31:35Z","receivedAt":"2016-09-27T18:31:41Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Sep 27, 2016 at 11:25 AM, Lukas Pühringer\n wrote:\n> Good, I will change it to 'Lukas Puehringer' then, when we send you the updated batch of patches, that address your latest comments.\n\nNo need to stay full ASCII. German umlauts are fine.\n(See `git shortlog -s` for all the contributor names, there are also\nother alphabets in use)\n\nStefan\n"},{"id":"302736","messageId":"618EBDCB-0BD7-493D-AE08-67133B06CEA9@gmail.com","threadId":"44166","inReplyTo":"CAGZ79kYitOd5-Cxr0VzhRH70B=z2gh1q2aB5xiwe+HwHuU7dYg@mail.gmail.com","subject":"Re: [PATCH v2 1/5] gpg-interface, tag: add GPG_VERIFY_QUIET flag","fromName":"Lukas Pühringer","fromEmail":"luk.puehringer@gmail.com","sentAt":"2016-09-27T18:37:46Z","receivedAt":"2016-09-27T18:37:53Z","isPatch":true,"sender":{"key":"luk.puehringer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/589324?v=4"},"body":"I made it a habit to use ‘ue’ instead of ‘ü' outside of German speaking countries and in coding. It makes my life easier.\nBut thanks for the hint.\n\nLukas\n> On Sep 27, 2016, at 2:31 PM, Stefan Beller wrote:\n> \n> On Tue, Sep 27, 2016 at 11:25 AM, Lukas Pühringer\n> wrote:\n>> Good, I will change it to 'Lukas Puehringer' then, when we send you the updated batch of patches, that address your latest comments.\n> \n> No need to stay full ASCII. German umlauts are fine.\n> (See `git shortlog -s` for all the contributor names, there are also\n> other alphabets in use)\n> \n> Stefan\n\n"}]}