{"thread":{"id":"44138","subject":"[PATCH 3/6] ref-filter: Expose wrappers for ref_item functions","startedAt":"2016-09-22T18:53:40Z","lastAt":"2016-09-24T19:10:21Z","messageCount":19,"participants":["santiago@nyu.edu","Stefan Beller","Junio C Hamano","Santiago Torres","Jakub Narębski"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"302389","messageId":"20160922185317.349-4-santiago@nyu.edu","threadId":"44138","inReplyTo":"20160922185317.349-1-santiago@nyu.edu","subject":"[PATCH 3/6] ref-filter: Expose wrappers for ref_item functions","fromName":"","fromEmail":"santiago@nyu.edu","sentAt":"2016-09-22T18:53:14Z","receivedAt":"2016-09-22T18:53:40Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"From: Lukas P <luk.puehringer@gmail.com>\n\nRef-filter functions are useful for printing git object information\nwithout a format specifier. However, some functions may not want to use\na complete ref-array, and just a single item instead. Expose\ncreate/show/free functions for ref_array_items through wrappers around\nthe original functions.\n\nSigned-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>\n---\n ref-filter.c | 20 ++++++++++++++++++++\n ref-filter.h | 10 ++++++++++\n 2 files changed, 30 insertions(+)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 9adbb8a..b013799 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1329,6 +1329,14 @@ static struct ref_array_item *new_ref_array_item(const char *refname,\n \treturn ref;\n }\n \n+/* Wrapper: Create ref_array_item w/o referencing container in function name */\n+struct ref_array_item *new_ref_item(const char *refname,\n+\t\t\t\t\t\t const unsigned char *objectname,\n+\t\t\t\t\t\t int flag)\n+{\n+\treturn new_ref_array_item(refname, objectname, flag);\n+}\n+\n static int filter_ref_kind(struct ref_filter *filter, const char *refname)\n {\n \tunsigned int i;\n@@ -1426,6 +1434,12 @@ static void free_array_item(struct ref_array_item *item)\n \tfree(item);\n }\n \n+/* Wrapper: Free ref_array_item w/o referencing container in function name */\n+void free_ref_item(struct ref_array_item *ref_item)\n+{\n+\tfree_array_item(ref_item);\n+}\n+\n /* Free all memory allocated for ref_array */\n void ref_array_clear(struct ref_array *array)\n {\n@@ -1637,6 +1651,12 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu\n \tputchar('\\n');\n }\n \n+/* Wrapper: Show ref_array_item w/o referencing container in function name */\n+void show_ref_item(struct ref_array_item *ref_item, const char *format, int quote_style)\n+{\n+\tshow_ref_array_item(ref_item, format, quote_style);\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..0f0ffe9 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -107,4 +107,14 @@ 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+/*\n+ * Wrappers exposing the ref_array_item data structure independently\n+ * of the container ref_array, e.g. to format-print individual refs.\n+ */\n+struct ref_array_item *new_ref_item(const char *refname,\n+\t\tconst unsigned char *objectname, int flag);\n+void show_ref_item(struct ref_array_item *ref_item, const char *format,\n+\t\tint quote_style);\n+void free_ref_item(struct ref_array_item *ref_item);\n+\n #endif /*  REF_FILTER_H  */\n-- \n2.10.0\n\n"},{"id":"302390","messageId":"20160922185317.349-1-santiago@nyu.edu","threadId":"44138","inReplyTo":null,"subject":"[RFC/PATCH 0/6] Add --format to tag verification","fromName":"","fromEmail":"santiago@nyu.edu","sentAt":"2016-09-22T18:53:11Z","receivedAt":"2016-09-22T18:53:47Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"From: Santiago Torres <santiago@nyu.edu>\n\nHello everyone,\n\nThis is a followup on [1]. There we discussed what would be the best way\nto provide automated scripts with mechanisms to inspect the contents of\na tag upon verification.\n\nWe struggled a little bit with how to make this fit the current git\ncodebase in the best way. Specifically, we are not sure if adding the\nGPG_QUIET flags and/or exposing the primitives to allocate individual\ngit_ref_item's would be the best way forward.\n\nThis applies on the current HEAD as well as v 2.9.10.\n\nThanks!\n-Santiago.\n\nP.S. Gmane seems to be broken for git after it was rebooted. Should we ping\nthem about it?\n\n[1] http://lists-archives.com/git/869122-verify-tag-add-check-name-flag.html\n\nLukas P (4):\n  gpg-interface: add GPG_VERIFY_QUIET flag\n  ref-filter: Expose wrappers for ref_item functions\n  tag: add format specifier to gpg_verify_tag\n  builtin/tag: add --format argument for tag -v\n\nSantiago Torres (2):\n  builtin/tag: move format specifier to global var\n  builtin/verify-tag: Add --format to verify-tag\n\n builtin/tag.c        | 33 +++++++++++++++++++++------------\n builtin/verify-tag.c | 17 ++++++++++++++---\n gpg-interface.c      |  3 +++\n gpg-interface.h      |  1 +\n ref-filter.c         | 20 ++++++++++++++++++++\n ref-filter.h         | 10 ++++++++++\n tag.c                | 14 ++++++++++++--\n tag.h                |  4 ++--\n 8 files changed, 83 insertions(+), 19 deletions(-)\n\n-- \n2.10.0\n\n"},{"id":"302391","messageId":"20160922185317.349-7-santiago@nyu.edu","threadId":"44138","inReplyTo":"20160922185317.349-1-santiago@nyu.edu","subject":"[PATCH 6/6] builtin/tag: add --format argument for tag -v","fromName":"","fromEmail":"santiago@nyu.edu","sentAt":"2016-09-22T18:53:17Z","receivedAt":"2016-09-22T18:53:52Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"From: Lukas P <luk.puehringer@gmail.com>\n\nAdding --format to git tag -v mutes the default output of the GPG\nverification and instead prints the formatted tag object.\n\nThis allows callers to cross-check the tagname from refs/tags with\nthe tagname from the tag object header upon GPG verification.\n\nSigned-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>\n---\n builtin/tag.c | 15 ++++++++++++---\n 1 file changed, 12 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 94ed8a2..3dd1e65 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 <tagname>...\"),\n \tN_(\"git tag -l [-n[<num>]] [--contains <commit>] [--points-at <object>]\"\n \t\t\"\\n\\t\\t[--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]\"),\n-\tN_(\"git tag -v <tagname>...\"),\n+\tN_(\"git tag -v [--format=<format>] <tagname>...\"),\n \tNULL\n };\n \n@@ -106,7 +106,13 @@ 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 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@@ -425,8 +431,11 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\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+\tif (cmdmode == 'v') {\n+\t\tif (fmt_pretty)\n+\t\t\tverify_ref_format(fmt_pretty);\n \t\treturn for_each_tag_name(argv, verify_tag);\n+\t}\n \n \tif (msg.given || msgfile) {\n \t\tif (msg.given && msgfile)\n-- \n2.10.0\n\n"},{"id":"302392","messageId":"20160922185317.349-3-santiago@nyu.edu","threadId":"44138","inReplyTo":"20160922185317.349-1-santiago@nyu.edu","subject":"[PATCH 2/6] gpg-interface: add GPG_VERIFY_QUIET flag","fromName":"","fromEmail":"santiago@nyu.edu","sentAt":"2016-09-22T18:53:13Z","receivedAt":"2016-09-22T18:54:03Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"From: Lukas P <luk.puehringer@gmail.com>\n\nFunctions that print git object information may require that the\ngpg-interface functions be silent. Add a GPG_VERIFY_QUIET to prevent\nfunctions such as `print_signature_buffer` from printing any output and\nonly return whether signature verification passed or not.\n\nSigned-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>\n---\n gpg-interface.c | 3 +++\n gpg-interface.h | 1 +\n 2 files changed, 4 insertions(+)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 8672eda..b82bc50 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -88,6 +88,9 @@ int check_signature(const char *payload, size_t plen, const char *signature,\n \n void print_signature_buffer(const struct signature_check *sigc, unsigned flags)\n {\n+\tif (flags & GPG_VERIFY_QUIET)\n+\t\treturn;\n+\n \tconst char *output = flags & GPG_VERIFY_RAW ?\n \t\tsigc->gpg_status : sigc->gpg_output;\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;\n-- \n2.10.0\n\n"},{"id":"302393","messageId":"20160922185317.349-5-santiago@nyu.edu","threadId":"44138","inReplyTo":"20160922185317.349-1-santiago@nyu.edu","subject":"[PATCH 4/6] tag: add format specifier to gpg_verify_tag","fromName":"","fromEmail":"santiago@nyu.edu","sentAt":"2016-09-22T18:53:15Z","receivedAt":"2016-09-22T18:54:06Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"From: Lukas P <luk.puehringer@gmail.com>\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 Puehringer <lukas.puehringer@nyu.edu>\n---\n builtin/tag.c        |  2 +-\n builtin/verify-tag.c |  6 ++++--\n tag.c                | 14 ++++++++++++--\n tag.h                |  4 ++--\n 4 files changed, 19 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex dbf271f..94ed8a2 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -106,7 +106,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..7a1121b 100644\n--- a/builtin/verify-tag.c\n+++ b/builtin/verify-tag.c\n@@ -51,8 +51,10 @@ 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\t\thad_error = 1;\n+\t\telse {\n+\t\t\tif (verify_and_format_tag(sha1, name, NULL, flags))\n+\t\t\t\thad_error = 1;\n+\t\t}\n \t}\n \treturn had_error;\n }\ndiff --git a/tag.c b/tag.c\nindex d1dcd18..e08da51 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 \"ref-filter.h\"\n \n const char *tag_type = \"tag\";\n \n@@ -30,8 +31,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_to_report,\n+\t\tconst char *fmt_pretty, unsigned flags)\n {\n \tenum object_type type;\n \tchar *buf;\n@@ -56,6 +57,15 @@ int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report,\n \tret = run_gpg_verify(buf, size, flags);\n \n \tfree(buf);\n+\n+\tif (fmt_pretty) {\n+\t\tstruct ref_array_item *ref_item;\n+\t\tref_item = new_ref_item(name_to_report, sha1, 0);\n+\t\tref_item->kind = FILTER_REFS_TAGS;\n+\t\tshow_ref_item(ref_item, fmt_pretty, 0);\n+\t\tfree_ref_item(ref_item);\n+\t}\n+\n \treturn ret;\n }\n \ndiff --git a/tag.h b/tag.h\nindex a5721b6..460b6f9 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,\n+\t\tconst char *name_to_report, const char *fmt_pretty, unsigned flags);\n \n #endif /* TAG_H */\n-- \n2.10.0\n\n"},{"id":"302394","messageId":"20160922185317.349-2-santiago@nyu.edu","threadId":"44138","inReplyTo":"20160922185317.349-1-santiago@nyu.edu","subject":"[PATCH 1/6] builtin/tag: move format specifier to global var","fromName":"","fromEmail":"santiago@nyu.edu","sentAt":"2016-09-22T18:53:12Z","receivedAt":"2016-09-22T18:54:07Z","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 format specifier will be likely used in other functions throughout\ngit tag. One likely candidate to require format strings in the future is\nthe gpg_verify_tag function. However, changing the signature of\nfunctions such as for_each_ref or verify_tag would be quite burdensome.\n\nInstead, we move the format string specifier to a static global variable\nthat modules can access in the same way git-log and other modules handle\nthis case.\n\nSigned-off-by: Santiago Torres <santiago@nyu.edu>\n---\n builtin/tag.c | 18 +++++++++---------\n 1 file changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 50e4ae5..dbf271f 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -30,8 +30,9 @@ static const char * const git_tag_usage[] = {\n \n static unsigned int colopts;\n static int force_sign_annotate;\n+static const char *fmt_pretty;\n \n-static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, const char *format)\n+static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting)\n {\n \tstruct ref_array array;\n \tchar *to_free = NULL;\n@@ -42,23 +43,23 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con\n \tif (filter->lines == -1)\n \t\tfilter->lines = 0;\n \n-\tif (!format) {\n+\tif (!fmt_pretty) {\n \t\tif (filter->lines) {\n \t\t\tto_free = xstrfmt(\"%s %%(contents:lines=%d)\",\n \t\t\t\t\t  \"%(align:15)%(refname:strip=2)%(end)\",\n \t\t\t\t\t  filter->lines);\n-\t\t\tformat = to_free;\n+\t\t\tfmt_pretty = to_free;\n \t\t} else\n-\t\t\tformat = \"%(refname:strip=2)\";\n+\t\t\tfmt_pretty = \"%(refname:strip=2)\";\n \t}\n \n-\tverify_ref_format(format);\n+\tverify_ref_format(fmt_pretty);\n \tfilter->with_commit_tag_algo = 1;\n \tfilter_refs(&array, filter, FILTER_REFS_TAGS);\n \tref_array_sort(sorting, &array);\n \n \tfor (i = 0; i < array.nr; i++)\n-\t\tshow_ref_array_item(array.items[i], format, 0);\n+\t\tshow_ref_array_item(array.items[i], fmt_pretty, 0);\n \tref_array_clear(&array);\n \tfree(to_free);\n \n@@ -334,7 +335,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tstruct strbuf err = STRBUF_INIT;\n \tstruct ref_filter filter;\n \tstatic struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;\n-\tconst char *format = NULL;\n \tstruct option options[] = {\n \t\tOPT_CMDMODE('l', \"list\", &cmdmode, N_(\"list tag names\"), 'l'),\n \t\t{ OPTION_INTEGER, 'n', NULL, &filter.lines, N_(\"n\"),\n@@ -369,7 +369,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\t\tOPTION_CALLBACK, 0, \"points-at\", &filter.points_at, N_(\"object\"),\n \t\t\tN_(\"print only tags of the object\"), 0, parse_opt_object_name\n \t\t},\n-\t\tOPT_STRING(  0 , \"format\", &format, N_(\"format\"), N_(\"format to use for the output\")),\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@@ -410,7 +410,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\t\trun_column_filter(colopts, &copts);\n \t\t}\n \t\tfilter.name_patterns = argv;\n-\t\tret = list_tags(&filter, sorting, format);\n+\t\tret = list_tags(&filter, sorting);\n \t\tif (column_active(colopts))\n \t\t\tstop_column_filter();\n \t\treturn ret;\n-- \n2.10.0\n\n"},{"id":"302395","messageId":"20160922185317.349-6-santiago@nyu.edu","threadId":"44138","inReplyTo":"20160922185317.349-1-santiago@nyu.edu","subject":"[PATCH 5/6] builtin/verify-tag: Add --format to verify-tag","fromName":"","fromEmail":"santiago@nyu.edu","sentAt":"2016-09-22T18:53:16Z","receivedAt":"2016-09-22T18:54:33Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"From: Santiago Torres <santiago@nyu.edu>\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 <santiago@nyu.edu>\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 7a1121b..319d469 100644\n--- a/builtin/verify-tag.c\n+++ b/builtin/verify-tag.c\n@@ -12,12 +12,15 @@\n #include <signal.h>\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] <tag>...\"),\n+\t\tN_(\"git verify-tag [-v | --verbose] [--format=<format>] <tag>...\"),\n \t\tNULL\n };\n \n+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,13 +50,18 @@ 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 {\n-\t\t\tif (verify_and_format_tag(sha1, name, NULL, flags))\n+\t\t\tif (verify_and_format_tag(sha1, name, fmt_pretty, flags))\n \t\t\t\thad_error = 1;\n \t\t}\n \t}\n-- \n2.10.0\n\n"},{"id":"302396","messageId":"CAGZ79kZ+eETHm2xuorRqP9OPKdETZSOuuY+SWPR_=J6MwJedRg@mail.gmail.com","threadId":"44138","inReplyTo":"20160922185317.349-1-santiago@nyu.edu","subject":"Re: [RFC/PATCH 0/6] Add --format to tag verification","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-09-22T19:01:46Z","receivedAt":"2016-09-22T19:02:00Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Sep 22, 2016 at 11:53 AM,  <santiago@nyu.edu> wrote:\n\n>\n> P.S. Gmane seems to be broken for git after it was rebooted. Should we ping\n> them about it?\n\nI think most of the git developers have moved on and reference emails by\nmessage id. An archive of all messages of the mailing list is found at\n\n    public-inbox.org/git/\n\n(You can git-clone it to have a distributed copy of the whole archive)\n\n    public-inbox.org/git/<message-id>/\n    public-inbox.org/git/<message-id>/raw\n\nis a good point to link to.\n\nHowever, feel free to ping gmane. :)\n\nThanks,\nStefan\n"},{"id":"302403","messageId":"xmqqbmzf4raz.fsf@gitster.mtv.corp.google.com","threadId":"44138","inReplyTo":"20160922185317.349-2-santiago@nyu.edu","subject":"Re: [PATCH 1/6] builtin/tag: move format specifier to global var","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-22T20:23:32Z","receivedAt":"2016-09-22T20:23:41Z","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 <santiago@nyu.edu>\n>\n> The format specifier will be likely used in other functions throughout\n> git tag. One likely candidate to require format strings in the future is\n> the gpg_verify_tag function. However, changing the signature of\n> functions such as for_each_ref or verify_tag would be quite burdensome.\n\nI do not understand the above excuse.  for-each-ref takes a\ncallback data pointer exactly because it wants you to be able to\nextend what data the callback function gets without changing its\nsignature.  builtin/tag.c::verify_tag() is a helper static to the\nfile--why should it be \"burdensome\" to change it to fit your needs?\n\nAdding technical debt by going backwards is never a good idea\nespecially done to add a new feature that is not desperately needed.\n\n"},{"id":"302404","messageId":"xmqq7fa34r86.fsf@gitster.mtv.corp.google.com","threadId":"44138","inReplyTo":"20160922185317.349-3-santiago@nyu.edu","subject":"Re: [PATCH 2/6] gpg-interface: add GPG_VERIFY_QUIET flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-22T20:25:13Z","receivedAt":"2016-09-22T20:25:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"santiago@nyu.edu writes:\n\n> diff --git a/gpg-interface.c b/gpg-interface.c\n> index 8672eda..b82bc50 100644\n> --- a/gpg-interface.c\n> +++ b/gpg-interface.c\n> @@ -88,6 +88,9 @@ int check_signature(const char *payload, size_t plen, const char *signature,\n>  \n>  void print_signature_buffer(const struct signature_check *sigc, unsigned flags)\n>  {\n> +\tif (flags & GPG_VERIFY_QUIET)\n> +\t\treturn;\n> +\n>  \tconst char *output = flags & GPG_VERIFY_RAW ?\n>  \t\tsigc->gpg_status : sigc->gpg_output;\n\nThis will not compile with -Werror=declaration-after-statement.\n"},{"id":"302406","messageId":"xmqqzimz3brm.fsf@gitster.mtv.corp.google.com","threadId":"44138","inReplyTo":"20160922185317.349-3-santiago@nyu.edu","subject":"Re: [PATCH 2/6] gpg-interface: add GPG_VERIFY_QUIET flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-22T20:44:29Z","receivedAt":"2016-09-22T20:44:38Z","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 <luk.puehringer@gmail.com>\n\nPlease match this with S-o-b: below.\n\n>\n> Functions that print git object information may require that the\n> gpg-interface functions be silent. Add a GPG_VERIFY_QUIET to prevent\n> functions such as `print_signature_buffer` from printing any output and\n> only return whether signature verification passed or not.\n>\n> Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>\n> ---\n>  gpg-interface.c | 3 +++\n>  gpg-interface.h | 1 +\n>  2 files changed, 4 insertions(+)\n>\n> diff --git a/gpg-interface.c b/gpg-interface.c\n> index 8672eda..b82bc50 100644\n> --- a/gpg-interface.c\n> +++ b/gpg-interface.c\n> @@ -88,6 +88,9 @@ int check_signature(const char *payload, size_t plen, const char *signature,\n>  \n>  void print_signature_buffer(const struct signature_check *sigc, unsigned flags)\n>  {\n> +\tif (flags & GPG_VERIFY_QUIET)\n> +\t\treturn;\n> +\n>  \tconst char *output = flags & GPG_VERIFY_RAW ?\n>  \t\tsigc->gpg_status : sigc->gpg_output;\n\nThis has only two callsites, which both know what flags they are\npassing.  Doesn't it make more sense to drop this patch (and\npossibly addition of GPG_VERIFY_QUIET flag as well) and teach them\nnot to call this function in the first place?\n\n> diff --git a/gpg-interface.h b/gpg-interface.h\n> index 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;\n"},{"id":"302407","messageId":"xmqqvaxn3bgx.fsf@gitster.mtv.corp.google.com","threadId":"44138","inReplyTo":"20160922185317.349-4-santiago@nyu.edu","subject":"Re: [PATCH 3/6] ref-filter: Expose wrappers for ref_item functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-22T20:50:54Z","receivedAt":"2016-09-22T20:53:07Z","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 <luk.puehringer@gmail.com>\n>\n> Ref-filter functions are useful for printing git object information\n> without a format specifier. However, some functions may not want to use\n> a complete ref-array, and just a single item instead. Expose\n> create/show/free functions for ref_array_items through wrappers around\n> the original functions.\n>\n> Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>\n> ---\n>  ref-filter.c | 20 ++++++++++++++++++++\n>  ref-filter.h | 10 ++++++++++\n>  2 files changed, 30 insertions(+)\n>\n> diff --git a/ref-filter.c b/ref-filter.c\n> index 9adbb8a..b013799 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -1329,6 +1329,14 @@ static struct ref_array_item *new_ref_array_item(const char *refname,\n>  \treturn ref;\n>  }\n>  \n> +/* Wrapper: Create ref_array_item w/o referencing container in function name */\n> +struct ref_array_item *new_ref_item(const char *refname,\n> +\t\t\t\t\t\t const unsigned char *objectname,\n> +\t\t\t\t\t\t int flag)\n> +{\n> +\treturn new_ref_array_item(refname, objectname, flag);\n> +}\n\nWhy?  As a public function name, new_ref_item() is a horrible one,\nas there are other structures about \"ref\" elsewhere in the system.\nIf a new caller needs to be able to get a new ref_array_item, you\nare better off just exposing it, not an ill-named wrapper.\n\n>  static int filter_ref_kind(struct ref_filter *filter, const char *refname)\n>  {\n>  \tunsigned int i;\n> @@ -1426,6 +1434,12 @@ static void free_array_item(struct ref_array_item *item)\n>  \tfree(item);\n>  }\n>  \n> +/* Wrapper: Free ref_array_item w/o referencing container in function name */\n> +void free_ref_item(struct ref_array_item *ref_item)\n> +{\n> +\tfree_array_item(ref_item);\n> +}\n\nAgain, why?  free_array_item() is a horrible name for a public\nfunction, and it is OK to rename it to free_ref_array_item() while\ngiving external callers an access to it, so that their names are\ndescriptive enough to convey that they are about ref_array_item\nstructure used in ref-filter API while at the same time making it\nclear to readers that the two functions with related names are\nindeed related.\n\n> @@ -1637,6 +1651,12 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu\n>  \tputchar('\\n');\n>  }\n>  \n> +/* Wrapper: Show ref_array_item w/o referencing container in function name */\n> +void show_ref_item(struct ref_array_item *ref_item, const char *format, int quote_style)\n> +{\n> +\tshow_ref_array_item(ref_item, format, quote_style);\n> +}\n\nDitto.\n"},{"id":"302409","messageId":"xmqqponv3b4x.fsf@gitster.mtv.corp.google.com","threadId":"44138","inReplyTo":"20160922185317.349-5-santiago@nyu.edu","subject":"Re: [PATCH 4/6] tag: add format specifier to gpg_verify_tag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-22T20:58:06Z","receivedAt":"2016-09-22T20:58:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"santiago@nyu.edu writes:\n\n> Calling functions for gpg_verify_tag() may desire to print relevant\n> information about the header for further verification. Add an optional\n> format argument to print any desired information after GPG verification.\n\n> diff --git a/builtin/tag.c b/builtin/tag.c\n> index dbf271f..94ed8a2 100644\n> --- a/builtin/tag.c\n> +++ b/builtin/tag.c\n> @@ -106,7 +106,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)\n> diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c\n> index 99f8148..7a1121b 100644\n> --- a/builtin/verify-tag.c\n> +++ b/builtin/verify-tag.c\n> @@ -51,8 +51,10 @@ 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\t\thad_error = 1;\n> +\t\telse {\n> +\t\t\tif (verify_and_format_tag(sha1, name, NULL, flags))\n> +\t\t\t\thad_error = 1;\n> +\t\t}\n\nRevert the unnecessary reformatting here.\n\n> @@ -56,6 +57,15 @@ int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report,\n>  \tret = run_gpg_verify(buf, size, flags);\n>  \n>  \tfree(buf);\n> +\n> +\tif (fmt_pretty) {\n> +\t\tstruct ref_array_item *ref_item;\n> +\t\tref_item = new_ref_item(name_to_report, sha1, 0);\n> +\t\tref_item->kind = FILTER_REFS_TAGS;\n> +\t\tshow_ref_item(ref_item, fmt_pretty, 0);\n> +\t\tfree_ref_item(ref_item);\n> +\t}\n\nI haven't seen 5/6 and 6/6, but if this is the only user of the 3/6,\nit would be much better to have a single function to format a ref\nexported from ref-filter.[ch] so that this one can say\n\n\tif (fmt_pretty)\n        \tformat_ref(name_to_report, sha1, FILTER_REFS_TAGS);\n\nor something like that, instead of doing three that will always be\nused together in quick succession in the above pattern.\n"},{"id":"302414","messageId":"xmqqintn3aai.fsf@gitster.mtv.corp.google.com","threadId":"44138","inReplyTo":"20160922185317.349-6-santiago@nyu.edu","subject":"Re: [PATCH 5/6] builtin/verify-tag: Add --format to verify-tag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-22T21:16:21Z","receivedAt":"2016-09-22T21:16: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 <santiago@nyu.edu>\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 <santiago@nyu.edu>\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 7a1121b..319d469 100644\n> --- a/builtin/verify-tag.c\n> +++ b/builtin/verify-tag.c\n> @@ -12,12 +12,15 @@\n>  #include <signal.h>\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] <tag>...\"),\n> +\t\tN_(\"git verify-tag [-v | --verbose] [--format=<format>] <tag>...\"),\n>  \t\tNULL\n>  };\n>  \n> +char *fmt_pretty;\n\nDoes this have to be extern?  I do not think so; prepend \"static \"\nin front of it.\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 {\n> -\t\t\tif (verify_and_format_tag(sha1, name, NULL, flags))\n> +\t\t\tif (verify_and_format_tag(sha1, name, fmt_pretty, flags))\n\nOK.  The callchain from here is\n\n    verify_and_format_tag()\n    -> run_gpg_verify()\n      -> print_signature_buffer()\n\nso not cramming QUIET into the flags parameter that is already\npassed is cumbersome.  As I said in my earlier review, it would make\nmore sense to have the conditional NOT in print_signature_buffer()\nbut in its caller, but it still is OK to add GPG_VERIFY_QUIET bit\nto the flag, which you would check in run_gpg_verify() to decide not\nto call print_signature_buffer().\n\n"},{"id":"302415","messageId":"xmqqeg4b39ye.fsf@gitster.mtv.corp.google.com","threadId":"44138","inReplyTo":"20160922185317.349-7-santiago@nyu.edu","subject":"Re: [PATCH 6/6] builtin/tag: add --format argument for tag -v","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-22T21:23:37Z","receivedAt":"2016-09-22T21:23:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"santiago@nyu.edu writes:\n\n> @@ -425,8 +431,11 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\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> +\tif (cmdmode == 'v') {\n> +\t\tif (fmt_pretty)\n> +\t\t\tverify_ref_format(fmt_pretty);\n>  \t\treturn for_each_tag_name(argv, verify_tag);\n> +\t}\n\nOK, you said something about for_each_ref() in an earlier commit,\nbut what you meant was this one, which takes each_tag_name_fn.\n\nThe function for_each_tag_name(), the type each_tag_name_fn, and the\nfunction of that type verify_tag(), are ALL file-scope static in\nthis single file, builtin/tag.c.  It seems to me that it is not\nnecessary to make the format string global at all.\n\n> @@ -425,8 +431,11 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\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> +\tif (cmdmode == 'v') {\n> +\t\tif (fmt_pretty)\n> +\t\t\tverify_ref_format(fmt_pretty);\n>  \t\treturn for_each_tag_name(argv, verify_tag);\n> +\t}\n\nThere are minor implementation and design issues I spotted, but\noverall I think the feature the series attempts to add may be a good\nthing to have.\n\nThanks.\n"},{"id":"302442","messageId":"20160923143358.cl3he5bg27mfxwoy@LykOS.localdomain","threadId":"44138","inReplyTo":"xmqqeg4b39ye.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 6/6] builtin/tag: add --format argument for tag -v","fromName":"Santiago Torres","fromEmail":"santiago@nyu.edu","sentAt":"2016-09-23T14:34:00Z","receivedAt":"2016-09-23T14:34:09Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"> OK, you said something about for_each_ref() in an earlier commit,\n> but what you meant was this one, which takes each_tag_name_fn.\n\nOh yeah, sorry for the confusion.\n\n> \n> The function for_each_tag_name(), the type each_tag_name_fn, and the\n> function of that type verify_tag(), are ALL file-scope static in\n> this single file, builtin/tag.c.  It seems to me that it is not\n> necessary to make the format string global at all.\n\nOh, ok. I was thinking that this was preferred over changing the\nsignature of those functions. (I drew my conclusion from log.c). I'll\ntake this other road then.\n> \n> ... \n> \n> There are minor implementation and design issues I spotted, but\n> overall I think the feature the series attempts to add may be a good\n> thing to have.\n> \n\nThanks for the review! I'll re-roll shortly.\n-Santiago. \n\n\n> Thanks.\n"},{"id":"302443","messageId":"20160923143517.6afqausmuf7ugc43@LykOS.localdomain","threadId":"44138","inReplyTo":"xmqqintn3aai.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 5/6] builtin/verify-tag: Add --format to verify-tag","fromName":"Santiago Torres","fromEmail":"santiago@nyu.edu","sentAt":"2016-09-23T14:35:18Z","receivedAt":"2016-09-23T14:35:35Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"On Thu, Sep 22, 2016 at 02:16:21PM -0700, Junio C Hamano wrote:\n> santiago@nyu.edu writes:\n> \n> > From: Santiago Torres <santiago@nyu.edu>\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 <santiago@nyu.edu>\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 7a1121b..319d469 100644\n> > --- a/builtin/verify-tag.c\n> > +++ b/builtin/verify-tag.c\n> > @@ -12,12 +12,15 @@\n> >  #include <signal.h>\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] <tag>...\"),\n> > +\t\tN_(\"git verify-tag [-v | --verbose] [--format=<format>] <tag>...\"),\n> >  \t\tNULL\n> >  };\n> >  \n> > +char *fmt_pretty;\n> \n> Does this have to be extern?  I do not think so; prepend \"static \"\n> in front of it.\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 {\n> > -\t\t\tif (verify_and_format_tag(sha1, name, NULL, flags))\n> > +\t\t\tif (verify_and_format_tag(sha1, name, fmt_pretty, flags))\n> \n> OK.  The callchain from here is\n> \n>     verify_and_format_tag()\n>     -> run_gpg_verify()\n>       -> print_signature_buffer()\n> \n> so not cramming QUIET into the flags parameter that is already\n> passed is cumbersome.  As I said in my earlier review, it would make\n> more sense to have the conditional NOT in print_signature_buffer()\n> but in its caller, but it still is OK to add GPG_VERIFY_QUIET bit\n> to the flag, which you would check in run_gpg_verify() to decide not\n> to call print_signature_buffer().\n> \n\nYeah, in retrospect, this sounds like a more reasonable approach than\ndoing it on gpg-nterface. I'll keep the QUIET bit then.\n"},{"id":"302444","messageId":"20160923143625.ynnzgkikj625f6ij@LykOS.localdomain","threadId":"44138","inReplyTo":"xmqqponv3b4x.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 4/6] tag: add format specifier to gpg_verify_tag","fromName":"Santiago Torres","fromEmail":"santiago@nyu.edu","sentAt":"2016-09-23T14:36:25Z","receivedAt":"2016-09-23T14:36:33Z","isPatch":true,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"On Thu, Sep 22, 2016 at 01:58:06PM -0700, Junio C Hamano wrote:\n> santiago@nyu.edu writes:\n> \n> > Calling functions for gpg_verify_tag() may desire to print relevant\n> > information about the header for further verification. Add an optional\n> > format argument to print any desired information after GPG verification.\n> \n> > diff --git a/builtin/tag.c b/builtin/tag.c\n> > index dbf271f..94ed8a2 100644\n> > --- a/builtin/tag.c\n> > +++ b/builtin/tag.c\n> > @@ -106,7 +106,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)\n> > diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c\n> > index 99f8148..7a1121b 100644\n> > --- a/builtin/verify-tag.c\n> > +++ b/builtin/verify-tag.c\n> > @@ -51,8 +51,10 @@ 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\t\thad_error = 1;\n> > +\t\telse {\n> > +\t\t\tif (verify_and_format_tag(sha1, name, NULL, flags))\n> > +\t\t\t\thad_error = 1;\n> > +\t\t}\n> \n> Revert the unnecessary reformatting here.\n> \n> > @@ -56,6 +57,15 @@ int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report,\n> >  \tret = run_gpg_verify(buf, size, flags);\n> >  \n> >  \tfree(buf);\n> > +\n> > +\tif (fmt_pretty) {\n> > +\t\tstruct ref_array_item *ref_item;\n> > +\t\tref_item = new_ref_item(name_to_report, sha1, 0);\n> > +\t\tref_item->kind = FILTER_REFS_TAGS;\n> > +\t\tshow_ref_item(ref_item, fmt_pretty, 0);\n> > +\t\tfree_ref_item(ref_item);\n> > +\t}\n> \n> I haven't seen 5/6 and 6/6, but if this is the only user of the 3/6,\n> it would be much better to have a single function to format a ref\n> exported from ref-filter.[ch] so that this one can say\n> \n> \tif (fmt_pretty)\n>         \tformat_ref(name_to_report, sha1, FILTER_REFS_TAGS);\n> \n> or something like that, instead of doing three that will always be\n> used together in quick succession in the above pattern.\n\nOh, this sounds like a better alternative. This would be instead of 0003\nright? \n\nThanks,\n-Santiago.\n"},{"id":"302486","messageId":"9ec55b69-ddd7-ebee-96e3-120422578592@gmail.com","threadId":"44138","inReplyTo":"CAGZ79kZ+eETHm2xuorRqP9OPKdETZSOuuY+SWPR_=J6MwJedRg@mail.gmail.com","subject":"Re: [RFC/PATCH 0/6] Add --format to tag verification","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2016-09-24T19:09:52Z","receivedAt":"2016-09-24T19:10:21Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"W dniu 22.09.2016 o 21:01, Stefan Beller pisze:\n> On Thu, Sep 22, 2016 at 11:53 AM,  <santiago@nyu.edu> wrote:\n> \n>>\n>> P.S. Gmane seems to be broken for git after it was rebooted. Should we ping\n>> them about it?\n> \n> I think most of the git developers have moved on and reference emails by\n> message id. An archive of all messages of the mailing list is found at\n> \n>     public-inbox.org/git/\n> \n> (You can git-clone it to have a distributed copy of the whole archive)\n> \n>     public-inbox.org/git/<message-id>/\n>     public-inbox.org/git/<message-id>/raw\n> \n> is a good point to link to.\n> \n> However, feel free to ping gmane. :)\n\nTwo relevant articles are the following:\n [1]: https://lwn.net/Articles/695695/\n [2]: https://lwn.net/Articles/699704/\n\nIn short: Gmane creator and maintainer wanted to retire from being\nmaintainer[1] because of DDoS attack against its web interface, so he\nstopped supporting wen interface (NNTP continues working). Thus\npublic-inbox.org was created (or just advertised more). Later\nGmane got new maintainer[2], but without code for web interface\n(I don't know why it is).\n\nSo there is hope that Gmane web interface would be up some time in\nthe future; in meantime one can use public-inbox URLs.\n\nHTH,\n-- \nJakub Narębski \n\n\n\n"}]}