[RFC/PATCH 0/6] Add --format to tag verification

STALE3604d

Revision rfc of 6 in this series.

9 messages, 3 authors, 2016-09-24 · open the first message on its own page

[RFC/PATCH 0/6] Add --format to tag verification

From: <hidden>
Date: 2016-09-22 18:53:47

From: Santiago Torres <redacted>

Hello everyone,

This is a followup on [1]. There we discussed what would be the best way
to provide automated scripts with mechanisms to inspect the contents of
a tag upon verification.

We struggled a little bit with how to make this fit the current git
codebase in the best way. Specifically, we are not sure if adding the
GPG_QUIET flags and/or exposing the primitives to allocate individual
git_ref_item's would be the best way forward.

This applies on the current HEAD as well as v 2.9.10.

Thanks!
-Santiago.

P.S. Gmane seems to be broken for git after it was rebooted. Should we ping
them about it?

[1] http://lists-archives.com/git/869122-verify-tag-add-check-name-flag.html

Lukas P (4):
  gpg-interface: add GPG_VERIFY_QUIET flag
  ref-filter: Expose wrappers for ref_item functions
  tag: add format specifier to gpg_verify_tag
  builtin/tag: add --format argument for tag -v

Santiago Torres (2):
  builtin/tag: move format specifier to global var
  builtin/verify-tag: Add --format to verify-tag

 builtin/tag.c        | 33 +++++++++++++++++++++------------
 builtin/verify-tag.c | 17 ++++++++++++++---
 gpg-interface.c      |  3 +++
 gpg-interface.h      |  1 +
 ref-filter.c         | 20 ++++++++++++++++++++
 ref-filter.h         | 10 ++++++++++
 tag.c                | 14 ++++++++++++--
 tag.h                |  4 ++--
 8 files changed, 83 insertions(+), 19 deletions(-)

-- 
2.10.0

[PATCH 3/6] ref-filter: Expose wrappers for ref_item functions

From: <hidden>
Date: 2016-09-22 18:53:40

From: Lukas P <redacted>

Ref-filter functions are useful for printing git object information
without a format specifier. However, some functions may not want to use
a complete ref-array, and just a single item instead. Expose
create/show/free functions for ref_array_items through wrappers around
the original functions.

Signed-off-by: Lukas Puehringer <redacted>
---
 ref-filter.c | 20 ++++++++++++++++++++
 ref-filter.h | 10 ++++++++++
 2 files changed, 30 insertions(+)

diff --git a/ref-filter.c b/ref-filter.c
index 9adbb8a..b013799 100644
--- a/ref-filter.c
+++ b/ref-filter.c

@@ -1329,6 +1329,14 @@ static struct ref_array_item *new_ref_array_item(const char *refname,
 	return ref;
 }
 
+/* Wrapper: Create ref_array_item w/o referencing container in function name */
+struct ref_array_item *new_ref_item(const char *refname,
+						 const unsigned char *objectname,
+						 int flag)
+{
+	return new_ref_array_item(refname, objectname, flag);
+}
+
 static int filter_ref_kind(struct ref_filter *filter, const char *refname)
 {
 	unsigned int i;

@@ -1426,6 +1434,12 @@ static void free_array_item(struct ref_array_item *item)
 	free(item);
 }
 
+/* Wrapper: Free ref_array_item w/o referencing container in function name */
+void free_ref_item(struct ref_array_item *ref_item)
+{
+	free_array_item(ref_item);
+}
+
 /* Free all memory allocated for ref_array */
 void ref_array_clear(struct ref_array *array)
 {

@@ -1637,6 +1651,12 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu
 	putchar('\n');
 }
 
+/* Wrapper: Show ref_array_item w/o referencing container in function name */
+void show_ref_item(struct ref_array_item *ref_item, const char *format, int quote_style)
+{
+	show_ref_array_item(ref_item, format, quote_style);
+}
+
 /*  If no sorting option is given, use refname to sort as default */
 struct ref_sorting *ref_default_sorting(void)
 {

diff --git a/ref-filter.h b/ref-filter.h
index 14d435e..0f0ffe9 100644
--- a/ref-filter.h
+++ b/ref-filter.h

@@ -107,4 +107,14 @@ struct ref_sorting *ref_default_sorting(void);
 /*  Function to parse --merged and --no-merged options */
 int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset);
 
+/*
+ * Wrappers exposing the ref_array_item data structure independently
+ * of the container ref_array, e.g. to format-print individual refs.
+ */
+struct ref_array_item *new_ref_item(const char *refname,
+		const unsigned char *objectname, int flag);
+void show_ref_item(struct ref_array_item *ref_item, const char *format,
+		int quote_style);
+void free_ref_item(struct ref_array_item *ref_item);
+
 #endif /*  REF_FILTER_H  */

-- 
2.10.0

[PATCH 6/6] builtin/tag: add --format argument for tag -v

From: <hidden>
Date: 2016-09-22 18:53:52

From: Lukas P <redacted>

Adding --format to git tag -v mutes the default output of the GPG
verification and instead prints the formatted tag object.

This allows callers to cross-check the tagname from refs/tags with
the tagname from the tag object header upon GPG verification.

Signed-off-by: Lukas Puehringer <redacted>
---
 builtin/tag.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 94ed8a2..3dd1e65 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c

@@ -24,7 +24,7 @@ static const char * const git_tag_usage[] = {
 	N_("git tag -d <tagname>..."),
 	N_("git tag -l [-n[<num>]] [--contains <commit>] [--points-at <object>]"
 		"\n\t\t[--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]"),
-	N_("git tag -v <tagname>..."),
+	N_("git tag -v [--format=<format>] <tagname>..."),
 	NULL
 };

@@ -106,7 +106,13 @@ static int delete_tag(const char *name, const char *ref,
 static int verify_tag(const char *name, const char *ref,
 				const unsigned char *sha1)
 {
-	return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE);
+	int flags;
+	flags = GPG_VERIFY_VERBOSE;
+
+	if (fmt_pretty)
+		flags = GPG_VERIFY_QUIET;
+
+	return verify_and_format_tag(sha1, name, fmt_pretty, flags);
 }
 
 static int do_sign(struct strbuf *buffer)

@@ -425,8 +431,11 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 		die(_("--merged and --no-merged option are only allowed with -l"));
 	if (cmdmode == 'd')
 		return for_each_tag_name(argv, delete_tag);
-	if (cmdmode == 'v')
+	if (cmdmode == 'v') {
+		if (fmt_pretty)
+			verify_ref_format(fmt_pretty);
 		return for_each_tag_name(argv, verify_tag);
+	}
 
 	if (msg.given || msgfile) {
 		if (msg.given && msgfile)

-- 
2.10.0

[PATCH 2/6] gpg-interface: add GPG_VERIFY_QUIET flag

From: <hidden>
Date: 2016-09-22 18:54:03

From: Lukas P <redacted>

Functions that print git object information may require that the
gpg-interface functions be silent. Add a GPG_VERIFY_QUIET to prevent
functions such as `print_signature_buffer` from printing any output and
only return whether signature verification passed or not.

Signed-off-by: Lukas Puehringer <redacted>
---
 gpg-interface.c | 3 +++
 gpg-interface.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/gpg-interface.c b/gpg-interface.c
index 8672eda..b82bc50 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c

@@ -88,6 +88,9 @@ int check_signature(const char *payload, size_t plen, const char *signature,
 
 void print_signature_buffer(const struct signature_check *sigc, unsigned flags)
 {
+	if (flags & GPG_VERIFY_QUIET)
+		return;
+
 	const char *output = flags & GPG_VERIFY_RAW ?
 		sigc->gpg_status : sigc->gpg_output;

diff --git a/gpg-interface.h b/gpg-interface.h
index ea68885..85dc982 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h

@@ -3,6 +3,7 @@
 
 #define GPG_VERIFY_VERBOSE	1
 #define GPG_VERIFY_RAW		2
+#define GPG_VERIFY_QUIET	4
 
 struct signature_check {
 	char *payload;

-- 
2.10.0

[PATCH 4/6] tag: add format specifier to gpg_verify_tag

From: <hidden>
Date: 2016-09-22 18:54:06

From: Lukas P <redacted>

Calling functions for gpg_verify_tag() may desire to print relevant
information about the header for further verification. Add an optional
format argument to print any desired information after GPG verification.

Signed-off-by: Lukas Puehringer <redacted>
---
 builtin/tag.c        |  2 +-
 builtin/verify-tag.c |  6 ++++--
 tag.c                | 14 ++++++++++++--
 tag.h                |  4 ++--
 4 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index dbf271f..94ed8a2 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c

@@ -106,7 +106,7 @@ static int delete_tag(const char *name, const char *ref,
 static int verify_tag(const char *name, const char *ref,
 				const unsigned char *sha1)
 {
-	return gpg_verify_tag(sha1, name, GPG_VERIFY_VERBOSE);
+	return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE);
 }
 
 static int do_sign(struct strbuf *buffer)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 99f8148..7a1121b 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c

@@ -51,8 +51,10 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)
 		const char *name = argv[i++];
 		if (get_sha1(name, sha1))
 			had_error = !!error("tag '%s' not found.", name);
-		else if (gpg_verify_tag(sha1, name, flags))
-			had_error = 1;
+		else {
+			if (verify_and_format_tag(sha1, name, NULL, flags))
+				had_error = 1;
+		}
 	}
 	return had_error;
 }

diff --git a/tag.c b/tag.c
index d1dcd18..e08da51 100644
--- a/tag.c
+++ b/tag.c

@@ -3,6 +3,7 @@
 #include "commit.h"
 #include "tree.h"
 #include "blob.h"
+#include "ref-filter.h"
 
 const char *tag_type = "tag";

@@ -30,8 +31,8 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
 	return ret;
 }
 
-int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report,
-		unsigned flags)
+int verify_and_format_tag(const unsigned char *sha1, const char *name_to_report,
+		const char *fmt_pretty, unsigned flags)
 {
 	enum object_type type;
 	char *buf;

@@ -56,6 +57,15 @@ int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report,
 	ret = run_gpg_verify(buf, size, flags);
 
 	free(buf);
+
+	if (fmt_pretty) {
+		struct ref_array_item *ref_item;
+		ref_item = new_ref_item(name_to_report, sha1, 0);
+		ref_item->kind = FILTER_REFS_TAGS;
+		show_ref_item(ref_item, fmt_pretty, 0);
+		free_ref_item(ref_item);
+	}
+
 	return ret;
 }

diff --git a/tag.h b/tag.h
index a5721b6..460b6f9 100644
--- a/tag.h
+++ b/tag.h

@@ -17,7 +17,7 @@ extern int parse_tag_buffer(struct tag *item, const void *data, unsigned long si
 extern int parse_tag(struct tag *item);
 extern struct object *deref_tag(struct object *, const char *, int);
 extern struct object *deref_tag_noverify(struct object *);
-extern int gpg_verify_tag(const unsigned char *sha1,
-		const char *name_to_report, unsigned flags);
+extern int verify_and_format_tag(const unsigned char *sha1,
+		const char *name_to_report, const char *fmt_pretty, unsigned flags);
 
 #endif /* TAG_H */

-- 
2.10.0

[PATCH 1/6] builtin/tag: move format specifier to global var

From: <hidden>
Date: 2016-09-22 18:54:07

From: Santiago Torres <redacted>

The format specifier will be likely used in other functions throughout
git tag. One likely candidate to require format strings in the future is
the gpg_verify_tag function. However, changing the signature of
functions such as for_each_ref or verify_tag would be quite burdensome.

Instead, we move the format string specifier to a static global variable
that modules can access in the same way git-log and other modules handle
this case.

Signed-off-by: Santiago Torres <redacted>
---
 builtin/tag.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 50e4ae5..dbf271f 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c

@@ -30,8 +30,9 @@ static const char * const git_tag_usage[] = {
 
 static unsigned int colopts;
 static int force_sign_annotate;
+static const char *fmt_pretty;
 
-static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, const char *format)
+static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting)
 {
 	struct ref_array array;
 	char *to_free = NULL;

@@ -42,23 +43,23 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con
 	if (filter->lines == -1)
 		filter->lines = 0;
 
-	if (!format) {
+	if (!fmt_pretty) {
 		if (filter->lines) {
 			to_free = xstrfmt("%s %%(contents:lines=%d)",
 					  "%(align:15)%(refname:strip=2)%(end)",
 					  filter->lines);
-			format = to_free;
+			fmt_pretty = to_free;
 		} else
-			format = "%(refname:strip=2)";
+			fmt_pretty = "%(refname:strip=2)";
 	}
 
-	verify_ref_format(format);
+	verify_ref_format(fmt_pretty);
 	filter->with_commit_tag_algo = 1;
 	filter_refs(&array, filter, FILTER_REFS_TAGS);
 	ref_array_sort(sorting, &array);
 
 	for (i = 0; i < array.nr; i++)
-		show_ref_array_item(array.items[i], format, 0);
+		show_ref_array_item(array.items[i], fmt_pretty, 0);
 	ref_array_clear(&array);
 	free(to_free);

@@ -334,7 +335,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	struct strbuf err = STRBUF_INIT;
 	struct ref_filter filter;
 	static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
-	const char *format = NULL;
 	struct option options[] = {
 		OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'),
 		{ OPTION_INTEGER, 'n', NULL, &filter.lines, N_("n"),

@@ -369,7 +369,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 			OPTION_CALLBACK, 0, "points-at", &filter.points_at, N_("object"),
 			N_("print only tags of the object"), 0, parse_opt_object_name
 		},
-		OPT_STRING(  0 , "format", &format, N_("format"), N_("format to use for the output")),
+		OPT_STRING(  0 , "format", &fmt_pretty, N_("format"), N_("format to use for the output")),
 		OPT_END()
 	};

@@ -410,7 +410,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 			run_column_filter(colopts, &copts);
 		}
 		filter.name_patterns = argv;
-		ret = list_tags(&filter, sorting, format);
+		ret = list_tags(&filter, sorting);
 		if (column_active(colopts))
 			stop_column_filter();
 		return ret;

-- 
2.10.0

[PATCH 5/6] builtin/verify-tag: Add --format to verify-tag

From: <hidden>
Date: 2016-09-22 18:54:33

From: Santiago Torres <redacted>

Callers of verify-tag may want to cross-check the tagname from refs/tags
with the tagname from the tag object header upon GPG verification. This
is to avoid tag refs that point to an incorrect object.

Add a --format parameter to git verify-tag to print the formatted tag
object header in addition to or instead of the --verbose or --raw GPG
verification output.

Signed-off-by: Santiago Torres <redacted>
---
 builtin/verify-tag.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 7a1121b..319d469 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c

@@ -12,12 +12,15 @@
 #include <signal.h>
 #include "parse-options.h"
 #include "gpg-interface.h"
+#include "ref-filter.h"
 
 static const char * const verify_tag_usage[] = {
-		N_("git verify-tag [-v | --verbose] <tag>..."),
+		N_("git verify-tag [-v | --verbose] [--format=<format>] <tag>..."),
 		NULL
 };
 
+char *fmt_pretty;
+
 static int git_verify_tag_config(const char *var, const char *value, void *cb)
 {
 	int status = git_gpg_config(var, value, cb);

@@ -33,6 +36,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)
 	const struct option verify_tag_options[] = {
 		OPT__VERBOSE(&verbose, N_("print tag contents")),
 		OPT_BIT(0, "raw", &flags, N_("print raw gpg status output"), GPG_VERIFY_RAW),
+		OPT_STRING(  0 , "format", &fmt_pretty, N_("format"), N_("format to use for the output")),
 		OPT_END()
 	};

@@ -46,13 +50,18 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)
 	if (verbose)
 		flags |= GPG_VERIFY_VERBOSE;
 
+	if (fmt_pretty) {
+		verify_ref_format(fmt_pretty);
+		flags |= GPG_VERIFY_QUIET;
+	}
+
 	while (i < argc) {
 		unsigned char sha1[20];
 		const char *name = argv[i++];
 		if (get_sha1(name, sha1))
 			had_error = !!error("tag '%s' not found.", name);
 		else {
-			if (verify_and_format_tag(sha1, name, NULL, flags))
+			if (verify_and_format_tag(sha1, name, fmt_pretty, flags))
 				had_error = 1;
 		}
 	}

-- 
2.10.0

Re: [RFC/PATCH 0/6] Add --format to tag verification

From: Stefan Beller <hidden>
Date: 2016-09-22 19:02:00

On Thu, Sep 22, 2016 at 11:53 AM,  [off-list ref] wrote:

P.S. Gmane seems to be broken for git after it was rebooted. Should we ping
them about it?

I think most of the git developers have moved on and reference emails by
message id. An archive of all messages of the mailing list is found at

    public-inbox.org/git/

(You can git-clone it to have a distributed copy of the whole archive)

    public-inbox.org/git/<message-id>/
    public-inbox.org/git/<message-id>/raw

is a good point to link to.

However, feel free to ping gmane. :)

Thanks,
Stefan

Re: [RFC/PATCH 0/6] Add --format to tag verification

From: Jakub Narębski <hidden>
Date: 2016-09-24 19:10:21

W dniu 22.09.2016 o 21:01, Stefan Beller pisze:

On Thu, Sep 22, 2016 at 11:53 AM,  [off-list ref] wrote:

quoted

P.S. Gmane seems to be broken for git after it was rebooted. Should we ping
them about it?

I think most of the git developers have moved on and reference emails by
message id. An archive of all messages of the mailing list is found at

    public-inbox.org/git/

(You can git-clone it to have a distributed copy of the whole archive)

    public-inbox.org/git/<message-id>/
    public-inbox.org/git/<message-id>/raw

is a good point to link to.

However, feel free to ping gmane. :)

Two relevant articles are the following:
 [1]: https://lwn.net/Articles/695695/
 [2]: https://lwn.net/Articles/699704/

In short: Gmane creator and maintainer wanted to retire from being
maintainer[1] because of DDoS attack against its web interface, so he
stopped supporting wen interface (NNTP continues working). Thus
public-inbox.org was created (or just advertised more). Later
Gmane got new maintainer[2], but without code for web interface
(I don't know why it is).

So there is hope that Gmane web interface would be up some time in
the future; in meantime one can use public-inbox URLs.

HTH,
-- 
Jakub Narębski

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help