* [PATCH v4 1/7] gpg-interface, tag: add GPG_VERIFY_QUIET flag
2016-10-07 21:07 [PATCH v4 0/7] Add --format to tag verification santiago
@ 2016-10-07 21:07 ` santiago
2016-10-19 8:51 ` Jeff King
2016-10-07 21:07 ` [PATCH v4 2/7] ref-filter: add function to print single ref_array_item santiago
` (6 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: santiago @ 2016-10-07 21:07 UTC (permalink / raw)
To: git; +Cc: gitster, peff, sunshine, walters, Lukas Puehringer
From: Lukas Puehringer <luk.puehringer@gmail.com>
Functions that print git object information may require that the
gpg-interface functions be silent. Add GPG_VERIFY_QUIET flag and prevent
print_signature_buffer from being called if flag is set.
Signed-off-by: Lukas Puehringer <luk.puehringer@gmail.com>
---
gpg-interface.h | 1 +
tag.c | 5 ++++-
2 files changed, 5 insertions(+), 1 deletion(-)
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;
diff --git a/tag.c b/tag.c
index d1dcd18..291073f 100644
--- a/tag.c
+++ b/tag.c
@@ -3,6 +3,7 @@
#include "commit.h"
#include "tree.h"
#include "blob.h"
+#include "gpg-interface.h"
const char *tag_type = "tag";
@@ -24,7 +25,9 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
ret = check_signature(buf, payload_size, buf + payload_size,
size - payload_size, &sigc);
- print_signature_buffer(&sigc, flags);
+
+ if (!(flags & GPG_VERIFY_QUIET))
+ print_signature_buffer(&sigc, flags);
signature_check_clear(&sigc);
return ret;
--
2.10.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v4 1/7] gpg-interface, tag: add GPG_VERIFY_QUIET flag
2016-10-07 21:07 ` [PATCH v4 1/7] gpg-interface, tag: add GPG_VERIFY_QUIET flag santiago
@ 2016-10-19 8:51 ` Jeff King
0 siblings, 0 replies; 23+ messages in thread
From: Jeff King @ 2016-10-19 8:51 UTC (permalink / raw)
To: santiago; +Cc: git, gitster, sunshine, walters, Lukas Puehringer
On Fri, Oct 07, 2016 at 05:07:15PM -0400, santiago@nyu.edu wrote:
> From: Lukas Puehringer <luk.puehringer@gmail.com>
>
> Functions that print git object information may require that the
> gpg-interface functions be silent. Add GPG_VERIFY_QUIET flag and prevent
> print_signature_buffer from being called if flag is set.
The layering here is a little funny. The gpg-interface code allocates a
new flag, but none of its functions do anything with it. Instead, it's
acted on only by the run_gpg_verify() command local to tag.c.
I guess this "flags" variable comes to us from other callsites via
gpg_verify_tag. That's still in tag.c, but it's arguably part of the gpg
interface.
So I think it's OK.
-Peff
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v4 2/7] ref-filter: add function to print single ref_array_item
2016-10-07 21:07 [PATCH v4 0/7] Add --format to tag verification santiago
2016-10-07 21:07 ` [PATCH v4 1/7] gpg-interface, tag: add GPG_VERIFY_QUIET flag santiago
@ 2016-10-07 21:07 ` santiago
2016-10-19 8:55 ` Jeff King
2016-10-07 21:07 ` [PATCH v4 3/7] tag: add format specifier to gpg_verify_tag santiago
` (5 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: santiago @ 2016-10-07 21:07 UTC (permalink / raw)
To: git; +Cc: gitster, peff, sunshine, walters, Lukas Puehringer
From: Lukas Puehringer <luk.puehringer@gmail.com>
ref-filter functions are useful for printing git object information
using a format specifier. However, some other modules may not want to use
this functionality on a ref-array but only print a single item.
Expose a pretty_print_ref function to create, pretty print and free
individual ref-items.
Signed-off-by: Lukas Puehringer <luk.puehringer@gmail.com>
---
ref-filter.c | 10 ++++++++++
ref-filter.h | 3 +++
2 files changed, 13 insertions(+)
diff --git a/ref-filter.c b/ref-filter.c
index 9adbb8a..cfbcd73 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1637,6 +1637,16 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu
putchar('\n');
}
+void pretty_print_ref(const char *name, const unsigned char *sha1,
+ const char *format, unsigned kind)
+{
+ struct ref_array_item *ref_item;
+ ref_item = new_ref_array_item(name, sha1, 0);
+ ref_item->kind = kind;
+ show_ref_array_item(ref_item, format, 0);
+ free_array_item(ref_item);
+}
+
/* 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..3d23090 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -107,4 +107,7 @@ 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);
+void pretty_print_ref(const char *name, const unsigned char *sha1,
+ const char *format, unsigned kind);
+
#endif /* REF_FILTER_H */
--
2.10.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v4 2/7] ref-filter: add function to print single ref_array_item
2016-10-07 21:07 ` [PATCH v4 2/7] ref-filter: add function to print single ref_array_item santiago
@ 2016-10-19 8:55 ` Jeff King
2016-10-19 9:16 ` Jeff King
0 siblings, 1 reply; 23+ messages in thread
From: Jeff King @ 2016-10-19 8:55 UTC (permalink / raw)
To: santiago; +Cc: git, gitster, sunshine, walters, Lukas Puehringer
On Fri, Oct 07, 2016 at 05:07:16PM -0400, santiago@nyu.edu wrote:
> From: Lukas Puehringer <luk.puehringer@gmail.com>
>
> ref-filter functions are useful for printing git object information
> using a format specifier. However, some other modules may not want to use
> this functionality on a ref-array but only print a single item.
>
> Expose a pretty_print_ref function to create, pretty print and free
> individual ref-items.
Makes sense.
> diff --git a/ref-filter.h b/ref-filter.h
> index 14d435e..3d23090 100644
> --- a/ref-filter.h
> +++ b/ref-filter.h
> @@ -107,4 +107,7 @@ 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);
>
> +void pretty_print_ref(const char *name, const unsigned char *sha1,
> + const char *format, unsigned kind);
> +
What are the possible values for "kind"? I guess these should come from
FILTER_REFS_TAGS, BRANCHES, etc. It's probably worth documenting that.
Alternatively, is it possible to just determine this from the name? It
looks like filter_ref_kind() is how it happens for a normal ref-filter.
-Peff
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 2/7] ref-filter: add function to print single ref_array_item
2016-10-19 8:55 ` Jeff King
@ 2016-10-19 9:16 ` Jeff King
2016-10-19 17:07 ` Santiago Torres
0 siblings, 1 reply; 23+ messages in thread
From: Jeff King @ 2016-10-19 9:16 UTC (permalink / raw)
To: santiago; +Cc: git, gitster, sunshine, walters, Lukas Puehringer
On Wed, Oct 19, 2016 at 04:55:43AM -0400, Jeff King wrote:
> > diff --git a/ref-filter.h b/ref-filter.h
> > index 14d435e..3d23090 100644
> > --- a/ref-filter.h
> > +++ b/ref-filter.h
> > @@ -107,4 +107,7 @@ 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);
> >
> > +void pretty_print_ref(const char *name, const unsigned char *sha1,
> > + const char *format, unsigned kind);
> > +
>
> What are the possible values for "kind"? I guess these should come from
> FILTER_REFS_TAGS, BRANCHES, etc. It's probably worth documenting that.
> Alternatively, is it possible to just determine this from the name? It
> looks like filter_ref_kind() is how it happens for a normal ref-filter.
I guess that may complicate things for the caller you add in this
series, which may not have a fully-qualified refname (which is obviously
how filter_ref_kind() figures it out). I'd argue that is a bug, though,
as things like "%(refname)" are generally expected to print out the
fully refname ("git tag --format=%(refname)" does so, and you can use
"%(refname:short)" if you want the shorter part).
-Peff
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 2/7] ref-filter: add function to print single ref_array_item
2016-10-19 9:16 ` Jeff King
@ 2016-10-19 17:07 ` Santiago Torres
2016-10-19 20:35 ` Jeff King
0 siblings, 1 reply; 23+ messages in thread
From: Santiago Torres @ 2016-10-19 17:07 UTC (permalink / raw)
To: Jeff King; +Cc: git, gitster, sunshine, walters, Lukas Puehringer
[-- Attachment #1: Type: text/plain, Size: 1623 bytes --]
On Wed, Oct 19, 2016 at 05:16:42AM -0400, Jeff King wrote:
> On Wed, Oct 19, 2016 at 04:55:43AM -0400, Jeff King wrote:
>
> > > diff --git a/ref-filter.h b/ref-filter.h
> > > index 14d435e..3d23090 100644
> > > --- a/ref-filter.h
> > > +++ b/ref-filter.h
> > > @@ -107,4 +107,7 @@ 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);
> > >
> > > +void pretty_print_ref(const char *name, const unsigned char *sha1,
> > > + const char *format, unsigned kind);
> > > +
> >
> > What are the possible values for "kind"? I guess these should come from
> > FILTER_REFS_TAGS, BRANCHES, etc. It's probably worth documenting that.
> > Alternatively, is it possible to just determine this from the name? It
> > looks like filter_ref_kind() is how it happens for a normal ref-filter.
>
> I guess that may complicate things for the caller you add in this
> series, which may not have a fully-qualified refname (which is obviously
> how filter_ref_kind() figures it out). I'd argue that is a bug, though,
> as things like "%(refname)" are generally expected to print out the
> fully refname ("git tag --format=%(refname)" does so, and you can use
> "%(refname:short)" if you want the shorter part).
Hmm, I hadn't actually noticed that. Do you have any suggestions in how to
address this?
In general this feels like a consequence of disambiguating .git/tags/*
within builtin/tag.c rather than letting ref-filter figure it out.
Thanks,
-Santiago.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 2/7] ref-filter: add function to print single ref_array_item
2016-10-19 17:07 ` Santiago Torres
@ 2016-10-19 20:35 ` Jeff King
2016-10-19 20:35 ` [PATCH 1/2] ref-filter: split ref_kind_from_filter Jeff King
2016-10-19 20:39 ` [PATCH 2/2] tag: send fully qualified refnames to verify_tag_and_format Jeff King
0 siblings, 2 replies; 23+ messages in thread
From: Jeff King @ 2016-10-19 20:35 UTC (permalink / raw)
To: Santiago Torres; +Cc: git, gitster, sunshine, walters, Lukas Puehringer
On Wed, Oct 19, 2016 at 01:07:34PM -0400, Santiago Torres wrote:
> > I guess that may complicate things for the caller you add in this
> > series, which may not have a fully-qualified refname (which is obviously
> > how filter_ref_kind() figures it out). I'd argue that is a bug, though,
> > as things like "%(refname)" are generally expected to print out the
> > fully refname ("git tag --format=%(refname)" does so, and you can use
> > "%(refname:short)" if you want the shorter part).
>
> Hmm, I hadn't actually noticed that. Do you have any suggestions in how to
> address this?
>
> In general this feels like a consequence of disambiguating .git/tags/*
> within builtin/tag.c rather than letting ref-filter figure it out.
The partial solution would look like something below. It's not too bad
because git-tag always knows that it's working a ref in the refs/tags
namespace (and we don't even have to qualify it ourselves,
for_each_tag_name already does it for us).
But verify-tag feeds arbitrary sha1 expressions. See the notes in the
second patch.
[1/2]: ref-filter: split ref_kind_from_filter
[2/2]: tag: send fully qualified refnames to verify_tag_and_format
builtin/tag.c | 2 +-
ref-filter.c | 21 +++++++++++++--------
ref-filter.h | 6 +++++-
tag.c | 2 +-
4 files changed, 20 insertions(+), 11 deletions(-)
-Peff
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 1/2] ref-filter: split ref_kind_from_filter
2016-10-19 20:35 ` Jeff King
@ 2016-10-19 20:35 ` Jeff King
2016-10-19 21:23 ` Junio C Hamano
2016-10-19 20:39 ` [PATCH 2/2] tag: send fully qualified refnames to verify_tag_and_format Jeff King
1 sibling, 1 reply; 23+ messages in thread
From: Jeff King @ 2016-10-19 20:35 UTC (permalink / raw)
To: Santiago Torres; +Cc: git, gitster, sunshine, walters, Lukas Puehringer
This function does two things: if we know we are filtering
only a certain kind of ref, then we can immediately know
that we have that kind. If not, then we compute the kind
from the fully-qualified refname. The latter half is useful
for other callers; let's split it out.
Signed-off-by: Jeff King <peff@peff.net>
---
ref-filter.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index cfbcd73..77ec9de 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1329,7 +1329,7 @@ static struct ref_array_item *new_ref_array_item(const char *refname,
return ref;
}
-static int filter_ref_kind(struct ref_filter *filter, const char *refname)
+static int ref_kind_from_refname(const char *refname)
{
unsigned int i;
@@ -1342,11 +1342,7 @@ static int filter_ref_kind(struct ref_filter *filter, const char *refname)
{ "refs/tags/", FILTER_REFS_TAGS}
};
- if (filter->kind == FILTER_REFS_BRANCHES ||
- filter->kind == FILTER_REFS_REMOTES ||
- filter->kind == FILTER_REFS_TAGS)
- return filter->kind;
- else if (!strcmp(refname, "HEAD"))
+ if (!strcmp(refname, "HEAD"))
return FILTER_REFS_DETACHED_HEAD;
for (i = 0; i < ARRAY_SIZE(ref_kind); i++) {
@@ -1357,6 +1353,15 @@ static int filter_ref_kind(struct ref_filter *filter, const char *refname)
return FILTER_REFS_OTHERS;
}
+static int filter_ref_kind(struct ref_filter *filter, const char *refname)
+{
+ if (filter->kind == FILTER_REFS_BRANCHES ||
+ filter->kind == FILTER_REFS_REMOTES ||
+ filter->kind == FILTER_REFS_TAGS)
+ return filter->kind;
+ return ref_kind_from_refname(refname);
+}
+
/*
* A call-back given to for_each_ref(). Filter refs and keep them for
* later object processing.
--
2.10.1.619.g16351a7
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 1/2] ref-filter: split ref_kind_from_filter
2016-10-19 20:35 ` [PATCH 1/2] ref-filter: split ref_kind_from_filter Jeff King
@ 2016-10-19 21:23 ` Junio C Hamano
2016-10-19 21:33 ` Jeff King
0 siblings, 1 reply; 23+ messages in thread
From: Junio C Hamano @ 2016-10-19 21:23 UTC (permalink / raw)
To: Jeff King; +Cc: Santiago Torres, git, sunshine, walters, Lukas Puehringer
Jeff King <peff@peff.net> writes:
> Subject: Re: [PATCH 1/2] ref-filter: split ref_kind_from_filter
I think you meant ref_kind_from_refname() ;-)
Looks like a good idea.
> This function does two things: if we know we are filtering
> only a certain kind of ref, then we can immediately know
> that we have that kind. If not, then we compute the kind
> from the fully-qualified refname. The latter half is useful
> for other callers; let's split it out.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> ref-filter.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/ref-filter.c b/ref-filter.c
> index cfbcd73..77ec9de 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1329,7 +1329,7 @@ static struct ref_array_item *new_ref_array_item(const char *refname,
> return ref;
> }
>
> -static int filter_ref_kind(struct ref_filter *filter, const char *refname)
> +static int ref_kind_from_refname(const char *refname)
> {
> unsigned int i;
>
> @@ -1342,11 +1342,7 @@ static int filter_ref_kind(struct ref_filter *filter, const char *refname)
> { "refs/tags/", FILTER_REFS_TAGS}
> };
>
> - if (filter->kind == FILTER_REFS_BRANCHES ||
> - filter->kind == FILTER_REFS_REMOTES ||
> - filter->kind == FILTER_REFS_TAGS)
> - return filter->kind;
> - else if (!strcmp(refname, "HEAD"))
> + if (!strcmp(refname, "HEAD"))
> return FILTER_REFS_DETACHED_HEAD;
>
> for (i = 0; i < ARRAY_SIZE(ref_kind); i++) {
> @@ -1357,6 +1353,15 @@ static int filter_ref_kind(struct ref_filter *filter, const char *refname)
> return FILTER_REFS_OTHERS;
> }
>
> +static int filter_ref_kind(struct ref_filter *filter, const char *refname)
> +{
> + if (filter->kind == FILTER_REFS_BRANCHES ||
> + filter->kind == FILTER_REFS_REMOTES ||
> + filter->kind == FILTER_REFS_TAGS)
> + return filter->kind;
> + return ref_kind_from_refname(refname);
> +}
> +
> /*
> * A call-back given to for_each_ref(). Filter refs and keep them for
> * later object processing.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/2] ref-filter: split ref_kind_from_filter
2016-10-19 21:23 ` Junio C Hamano
@ 2016-10-19 21:33 ` Jeff King
0 siblings, 0 replies; 23+ messages in thread
From: Jeff King @ 2016-10-19 21:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Santiago Torres, git, sunshine, walters, Lukas Puehringer
On Wed, Oct 19, 2016 at 02:23:41PM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > Subject: Re: [PATCH 1/2] ref-filter: split ref_kind_from_filter
>
> I think you meant ref_kind_from_refname() ;-)
>
> Looks like a good idea.
Heh, I actually meant filter_ref_kind(), which is the original function.
But any name that is actually a real function would do. :)
-Peff
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 2/2] tag: send fully qualified refnames to verify_tag_and_format
2016-10-19 20:35 ` Jeff King
2016-10-19 20:35 ` [PATCH 1/2] ref-filter: split ref_kind_from_filter Jeff King
@ 2016-10-19 20:39 ` Jeff King
2016-10-20 16:57 ` Santiago Torres
1 sibling, 1 reply; 23+ messages in thread
From: Jeff King @ 2016-10-19 20:39 UTC (permalink / raw)
To: Santiago Torres; +Cc: git, gitster, sunshine, walters, Lukas Puehringer
The ref-filter code generally expects to see fully qualified
refs, so that things like "%(refname)" and "%(refname:short)"
work as expected. We can do so easily from git-tag, which
always works with refnames in the refs/tags namespace. As a
bonus, we can drop the "kind" parameter from
pretty_print_ref() and just deduce it automatically.
Unfortunately, things are not so simple for verify-tag,
which takes an arbitrary sha1 expression. It has no clue if
a refname as used or not, and whether it was in the
refs/tags namespace.
In an ideal world, get_sha1_with_context() would optionally
tell us about any refs we resolved while it was working, and
we could just feed that refname (and then in cases where we
didn't use a ref at all, like a bare sha1, we could fallback
to just showing the sha1 name the user gave us).
Signed-off-by: Jeff King <peff@peff.net>
---
I think you'd really just squash the various bits of this into your
series at the right spots, though I don't mind it on top, either.
The big question is to what degree we should care about the verify-tag
case. I don't think it's any worse off with this change than it is with
your series (its "kind" becomes "OTHER", but I don't think that is
actually used for display at all; the name remains the same). I'd be OK
with leaving it like this, as a known bug, until get_sha1_with_context()
learns to tell us about the ref. It's an unhandled corner case in a
brand-new feature, not a regression in an existing one.
builtin/tag.c | 2 +-
ref-filter.c | 4 ++--
ref-filter.h | 6 +++++-
tag.c | 2 +-
4 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index 49aeb50..18eab7e 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -114,7 +114,7 @@ static int verify_tag(const char *name, const char *ref,
if (fmt_pretty)
flags = GPG_VERIFY_QUIET;
- return verify_and_format_tag(sha1, name, fmt_pretty, flags);
+ return verify_and_format_tag(sha1, ref, fmt_pretty, flags);
}
static int do_sign(struct strbuf *buffer)
diff --git a/ref-filter.c b/ref-filter.c
index 77ec9de..74da17a 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1643,11 +1643,11 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu
}
void pretty_print_ref(const char *name, const unsigned char *sha1,
- const char *format, unsigned kind)
+ const char *format)
{
struct ref_array_item *ref_item;
ref_item = new_ref_array_item(name, sha1, 0);
- ref_item->kind = kind;
+ ref_item->kind = ref_kind_from_refname(name);
show_ref_array_item(ref_item, format, 0);
free_array_item(ref_item);
}
diff --git a/ref-filter.h b/ref-filter.h
index 3d23090..fed2f5e 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -107,7 +107,11 @@ 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);
+/*
+ * Print a single ref, outside of any ref-filter. Note that the
+ * name must be a fully qualified refname.
+ */
void pretty_print_ref(const char *name, const unsigned char *sha1,
- const char *format, unsigned kind);
+ const char *format);
#endif /* REF_FILTER_H */
diff --git a/tag.c b/tag.c
index d3512c0..d5a7cfb 100644
--- a/tag.c
+++ b/tag.c
@@ -62,7 +62,7 @@ int verify_and_format_tag(const unsigned char *sha1, const char *name,
free(buf);
if (fmt_pretty)
- pretty_print_ref(name, sha1, fmt_pretty, FILTER_REFS_TAGS);
+ pretty_print_ref(name, sha1, fmt_pretty);
return ret;
}
--
2.10.1.619.g16351a7
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] tag: send fully qualified refnames to verify_tag_and_format
2016-10-19 20:39 ` [PATCH 2/2] tag: send fully qualified refnames to verify_tag_and_format Jeff King
@ 2016-10-20 16:57 ` Santiago Torres
2016-10-20 21:39 ` Jeff King
0 siblings, 1 reply; 23+ messages in thread
From: Santiago Torres @ 2016-10-20 16:57 UTC (permalink / raw)
To: Jeff King; +Cc: git, gitster, sunshine, walters, Lukas Puehringer
[-- Attachment #1: Type: text/plain, Size: 1762 bytes --]
On Wed, Oct 19, 2016 at 04:39:44PM -0400, Jeff King wrote:
> The ref-filter code generally expects to see fully qualified
> refs, so that things like "%(refname)" and "%(refname:short)"
> work as expected. We can do so easily from git-tag, which
> always works with refnames in the refs/tags namespace. As a
> bonus, we can drop the "kind" parameter from
> pretty_print_ref() and just deduce it automatically.
>
> Unfortunately, things are not so simple for verify-tag,
> which takes an arbitrary sha1 expression. It has no clue if
> a refname as used or not, and whether it was in the
> refs/tags namespace.
>
> In an ideal world, get_sha1_with_context() would optionally
> tell us about any refs we resolved while it was working, and
> we could just feed that refname (and then in cases where we
> didn't use a ref at all, like a bare sha1, we could fallback
> to just showing the sha1 name the user gave us).
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> I think you'd really just squash the various bits of this into your
> series at the right spots, though I don't mind it on top, either.
>
> The big question is to what degree we should care about the verify-tag
> case. I don't think it's any worse off with this change than it is with
> your series (its "kind" becomes "OTHER", but I don't think that is
> actually used for display at all; the name remains the same). I'd be OK
> with leaving it like this, as a known bug, until get_sha1_with_context()
> learns to tell us about the ref. It's an unhandled corner case in a
> brand-new feature, not a regression in an existing one.
I see now, I think I can sprinkle some of these changes on 2/7 then. The
rest should be doing 4/7 and 5/7. Does this sound ok?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] tag: send fully qualified refnames to verify_tag_and_format
2016-10-20 16:57 ` Santiago Torres
@ 2016-10-20 21:39 ` Jeff King
0 siblings, 0 replies; 23+ messages in thread
From: Jeff King @ 2016-10-20 21:39 UTC (permalink / raw)
To: Santiago Torres; +Cc: git, gitster, sunshine, walters, Lukas Puehringer
On Thu, Oct 20, 2016 at 12:57:01PM -0400, Santiago Torres wrote:
> > I think you'd really just squash the various bits of this into your
> > series at the right spots, though I don't mind it on top, either.
> >
> > The big question is to what degree we should care about the verify-tag
> > case. I don't think it's any worse off with this change than it is with
> > your series (its "kind" becomes "OTHER", but I don't think that is
> > actually used for display at all; the name remains the same). I'd be OK
> > with leaving it like this, as a known bug, until get_sha1_with_context()
> > learns to tell us about the ref. It's an unhandled corner case in a
> > brand-new feature, not a regression in an existing one.
>
> I see now, I think I can sprinkle some of these changes on 2/7 then. The
> rest should be doing 4/7 and 5/7. Does this sound ok?
Yep, that sounds about right.
-Peff
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v4 3/7] tag: add format specifier to gpg_verify_tag
2016-10-07 21:07 [PATCH v4 0/7] Add --format to tag verification santiago
2016-10-07 21:07 ` [PATCH v4 1/7] gpg-interface, tag: add GPG_VERIFY_QUIET flag santiago
2016-10-07 21:07 ` [PATCH v4 2/7] ref-filter: add function to print single ref_array_item santiago
@ 2016-10-07 21:07 ` santiago
2016-10-07 21:07 ` [PATCH v4 4/7] builtin/verify-tag: add --format to verify-tag santiago
` (4 subsequent siblings)
7 siblings, 0 replies; 23+ messages in thread
From: santiago @ 2016-10-07 21:07 UTC (permalink / raw)
To: git; +Cc: gitster, peff, sunshine, walters, Lukas Puehringer
From: Lukas Puehringer <luk.puehringer@gmail.com>
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 <luk.puehringer@gmail.com>
---
builtin/tag.c | 2 +-
builtin/verify-tag.c | 2 +-
tag.c | 17 +++++++++++------
tag.h | 4 ++--
4 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index 50e4ae5..14f3b48 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -105,7 +105,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..de10198 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -51,7 +51,7 @@ 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))
+ 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 291073f..d3512c0 100644
--- a/tag.c
+++ b/tag.c
@@ -4,6 +4,7 @@
#include "tree.h"
#include "blob.h"
#include "gpg-interface.h"
+#include "ref-filter.h"
const char *tag_type = "tag";
@@ -33,8 +34,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,
+ const char *fmt_pretty, unsigned flags)
{
enum object_type type;
char *buf;
@@ -44,21 +45,25 @@ int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report,
type = sha1_object_info(sha1, NULL);
if (type != OBJ_TAG)
return error("%s: cannot verify a non-tag object of type %s.",
- name_to_report ?
- name_to_report :
+ name ?
+ name :
find_unique_abbrev(sha1, DEFAULT_ABBREV),
typename(type));
buf = read_sha1_file(sha1, &type, &size);
if (!buf)
return error("%s: unable to read file.",
- name_to_report ?
- name_to_report :
+ name ?
+ name :
find_unique_abbrev(sha1, DEFAULT_ABBREV));
ret = run_gpg_verify(buf, size, flags);
free(buf);
+
+ if (fmt_pretty)
+ pretty_print_ref(name, sha1, fmt_pretty, FILTER_REFS_TAGS);
+
return ret;
}
diff --git a/tag.h b/tag.h
index a5721b6..896b9c2 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,
+ const char *fmt_pretty, unsigned flags);
#endif /* TAG_H */
--
2.10.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v4 4/7] builtin/verify-tag: add --format to verify-tag
2016-10-07 21:07 [PATCH v4 0/7] Add --format to tag verification santiago
` (2 preceding siblings ...)
2016-10-07 21:07 ` [PATCH v4 3/7] tag: add format specifier to gpg_verify_tag santiago
@ 2016-10-07 21:07 ` santiago
2016-10-19 9:04 ` Jeff King
2016-10-07 21:07 ` [PATCH v4 5/7] builtin/tag: add --format argument for tag -v santiago
` (3 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: santiago @ 2016-10-07 21:07 UTC (permalink / raw)
To: git; +Cc: gitster, peff, sunshine, walters, Santiago Torres
From: Santiago Torres <santiago@nyu.edu>
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 <santiago@nyu.edu>
---
Documentation/git-verify-tag.txt | 2 +-
builtin/verify-tag.c | 13 +++++++++++--
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-verify-tag.txt b/Documentation/git-verify-tag.txt
index d590edc..0b8075d 100644
--- a/Documentation/git-verify-tag.txt
+++ b/Documentation/git-verify-tag.txt
@@ -8,7 +8,7 @@ git-verify-tag - Check the GPG signature of tags
SYNOPSIS
--------
[verse]
-'git verify-tag' <tag>...
+'git verify-tag' [--format=<format>] <tag>...
DESCRIPTION
-----------
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index de10198..745b6a6 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -12,12 +12,14 @@
#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
};
+
static int git_verify_tag_config(const char *var, const char *value, void *cb)
{
int status = git_gpg_config(var, value, cb);
@@ -30,9 +32,11 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)
{
int i = 1, verbose = 0, had_error = 0;
unsigned flags = 0;
+ char *fmt_pretty = NULL;
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,12 +50,17 @@ 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))
+ else if (verify_and_format_tag(sha1, name, fmt_pretty, flags))
had_error = 1;
}
return had_error;
--
2.10.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v4 4/7] builtin/verify-tag: add --format to verify-tag
2016-10-07 21:07 ` [PATCH v4 4/7] builtin/verify-tag: add --format to verify-tag santiago
@ 2016-10-19 9:04 ` Jeff King
0 siblings, 0 replies; 23+ messages in thread
From: Jeff King @ 2016-10-19 9:04 UTC (permalink / raw)
To: santiago; +Cc: git, gitster, sunshine, walters
On Fri, Oct 07, 2016 at 05:07:18PM -0400, santiago@nyu.edu wrote:
> @@ -46,12 +50,17 @@ 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;
> + }
I see why you would want to disable the normal output when there is a
custom format. But it seems a shame that the GPG information cannot be
retrieved as part of that format.
I think in the long run we'd want something like pretty.c's "%G"
placeholders for the ref-filter pretty-printer. But I think we are OK to
do this patch without it. It allows at least:
tag=v2.0.0
got=$(git verify-tag --format='%(tag)' "$tag") &&
test "$tag" = "$got" ||
echo >&2 "verification failed"
which is better than what we have now, and can be extended in the
future.
-Peff
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v4 5/7] builtin/tag: add --format argument for tag -v
2016-10-07 21:07 [PATCH v4 0/7] Add --format to tag verification santiago
` (3 preceding siblings ...)
2016-10-07 21:07 ` [PATCH v4 4/7] builtin/verify-tag: add --format to verify-tag santiago
@ 2016-10-07 21:07 ` santiago
2016-10-19 9:10 ` Jeff King
2016-10-07 21:07 ` [PATCH v4 6/7] t/t7030-verify-tag: Add --format specifier tests santiago
` (2 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: santiago @ 2016-10-07 21:07 UTC (permalink / raw)
To: git; +Cc: gitster, peff, sunshine, walters, Lukas Puehringer
From: Lukas Puehringer <luk.puehringer@gmail.com>
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.
The callback function for for_each_tag_name() didn't allow callers to
pass custom data to their callback functions. Add a new opaque pointer
to each_tag_name_fn's parameter to allow this.
Signed-off-by: Lukas Puehringer <luk.puehringer@gmail.com>
---
Documentation/git-tag.txt | 2 +-
builtin/tag.c | 34 +++++++++++++++++++++++-----------
builtin/verify-tag.c | 2 +-
3 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 7ecca8e..3bb5e3c 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -15,7 +15,7 @@ SYNOPSIS
'git tag' [-n[<num>]] -l [--contains <commit>] [--points-at <object>]
[--column[=<options>] | --no-column] [--create-reflog] [--sort=<key>]
[--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]
-'git tag' -v <tagname>...
+'git tag' -v [--format=<format>] <tagname>...
DESCRIPTION
-----------
diff --git a/builtin/tag.c b/builtin/tag.c
index 14f3b48..7730fd0 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
};
@@ -66,15 +66,17 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con
}
typedef int (*each_tag_name_fn)(const char *name, const char *ref,
- const unsigned char *sha1);
+ const unsigned char *sha1, void *cb_data);
-static int for_each_tag_name(const char **argv, each_tag_name_fn fn)
+static int for_each_tag_name(const char **argv, each_tag_name_fn fn,
+ void *cb_data)
{
const char **p;
char ref[PATH_MAX];
int had_error = 0;
unsigned char sha1[20];
+
for (p = argv; *p; p++) {
if (snprintf(ref, sizeof(ref), "refs/tags/%s", *p)
>= sizeof(ref)) {
@@ -87,14 +89,14 @@ static int for_each_tag_name(const char **argv, each_tag_name_fn fn)
had_error = 1;
continue;
}
- if (fn(*p, ref, sha1))
+ if (fn(*p, ref, sha1, cb_data))
had_error = 1;
}
return had_error;
}
static int delete_tag(const char *name, const char *ref,
- const unsigned char *sha1)
+ const unsigned char *sha1, void *cb_data)
{
if (delete_ref(ref, sha1, 0))
return 1;
@@ -103,9 +105,16 @@ static int delete_tag(const char *name, const char *ref,
}
static int verify_tag(const char *name, const char *ref,
- const unsigned char *sha1)
+ const unsigned char *sha1, void *cb_data)
{
- return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE);
+ int flags;
+ char *fmt_pretty = cb_data;
+ 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)
@@ -334,7 +343,7 @@ 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;
+ char *format = NULL;
struct option options[] = {
OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'),
{ OPTION_INTEGER, 'n', NULL, &filter.lines, N_("n"),
@@ -424,9 +433,12 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
if (filter.merge_commit)
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')
- return for_each_tag_name(argv, verify_tag);
+ return for_each_tag_name(argv, delete_tag, NULL);
+ if (cmdmode == 'v') {
+ if (format)
+ verify_ref_format(format);
+ return for_each_tag_name(argv, verify_tag, format);
+ }
if (msg.given || msgfile) {
if (msg.given && msgfile)
--
2.10.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v4 5/7] builtin/tag: add --format argument for tag -v
2016-10-07 21:07 ` [PATCH v4 5/7] builtin/tag: add --format argument for tag -v santiago
@ 2016-10-19 9:10 ` Jeff King
0 siblings, 0 replies; 23+ messages in thread
From: Jeff King @ 2016-10-19 9:10 UTC (permalink / raw)
To: santiago; +Cc: git, gitster, sunshine, walters, Lukas Puehringer
On Fri, Oct 07, 2016 at 05:07:19PM -0400, santiago@nyu.edu wrote:
> Adding --format to git tag -v mutes the default output of the GPG
> verification and instead prints the formatted tag object.
The same comments apply to "mutes" here, as to the previous patch (which
is to say I think we may want something more, but this is probably OK
for now).
> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
> index 7ecca8e..3bb5e3c 100644
> --- a/Documentation/git-tag.txt
> +++ b/Documentation/git-tag.txt
> @@ -15,7 +15,7 @@ SYNOPSIS
> 'git tag' [-n[<num>]] -l [--contains <commit>] [--points-at <object>]
> [--column[=<options>] | --no-column] [--create-reflog] [--sort=<key>]
> [--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]
> -'git tag' -v <tagname>...
> +'git tag' -v [--format=<format>] <tagname>...
Just thinking out loud, but if we had ref-filter placeholders that
triggered GPG verification, you could do this all with the listing mode,
like:
git tag --format='%(gpgstatus) %(tag) %(refname:short)'
and verify multiple tags (or give a single tag to limit it to just one).
I don't think that's any kind of blocker for this series. We already
have "-v", and adding --format to it is reasonable, even if we
eventually move to a world where people can use the listing mode. Like I
said, just thinking out loud.
> +static int for_each_tag_name(const char **argv, each_tag_name_fn fn,
> + void *cb_data)
> {
> const char **p;
> char ref[PATH_MAX];
> int had_error = 0;
> unsigned char sha1[20];
>
> +
> for (p = argv; *p; p++) {
Extra space?
> static int verify_tag(const char *name, const char *ref,
> - const unsigned char *sha1)
> + const unsigned char *sha1, void *cb_data)
> {
> - return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE);
> + int flags;
Probably doesn't matter much, but these flags are defined as "unsigned"
elsewhere.
-Peff
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v4 6/7] t/t7030-verify-tag: Add --format specifier tests
2016-10-07 21:07 [PATCH v4 0/7] Add --format to tag verification santiago
` (4 preceding siblings ...)
2016-10-07 21:07 ` [PATCH v4 5/7] builtin/tag: add --format argument for tag -v santiago
@ 2016-10-07 21:07 ` santiago
2016-10-19 9:13 ` Jeff King
2016-10-07 21:07 ` [PATCH v4 7/7] t/t7004-tag: " santiago
2016-10-11 17:43 ` [PATCH v4 0/7] Add --format to tag verification Santiago Torres
7 siblings, 1 reply; 23+ messages in thread
From: santiago @ 2016-10-07 21:07 UTC (permalink / raw)
To: git; +Cc: gitster, peff, sunshine, walters, Santiago Torres
From: Santiago Torres <santiago@nyu.edu>
Verify-tag now provides --format specifiers to inspect and ensure the
contents of the tag are proper. We add two tests to ensure this
functionality works as expected: the return value should indicate if
verification passed, and the format specifiers must be respected.
Signed-off-by: Santiago Torres <santiago@nyu.edu>
---
t/t7030-verify-tag.sh | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh
index 07079a4..ce37fd9 100755
--- a/t/t7030-verify-tag.sh
+++ b/t/t7030-verify-tag.sh
@@ -125,4 +125,20 @@ test_expect_success GPG 'verify multiple tags' '
test_cmp expect.stderr actual.stderr
'
+test_expect_success 'verifying tag with --format' '
+ cat >expect <<-\EOF &&
+ tagname : fourth-signed
+ EOF
+ git verify-tag --format="tagname : %(tag)" "fourth-signed" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'verifying a forged tag with --format fail and format accordingly' '
+ cat >expect <<-\EOF &&
+ tagname : 7th forged-signed
+ EOF
+ test_must_fail git verify-tag --format="tagname : %(tag)" $(cat forged1.tag) >actual-forged &&
+ test_cmp expect actual-forged
+'
+
test_done
--
2.10.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v4 6/7] t/t7030-verify-tag: Add --format specifier tests
2016-10-07 21:07 ` [PATCH v4 6/7] t/t7030-verify-tag: Add --format specifier tests santiago
@ 2016-10-19 9:13 ` Jeff King
0 siblings, 0 replies; 23+ messages in thread
From: Jeff King @ 2016-10-19 9:13 UTC (permalink / raw)
To: santiago; +Cc: git, gitster, sunshine, walters
On Fri, Oct 07, 2016 at 05:07:20PM -0400, santiago@nyu.edu wrote:
> Verify-tag now provides --format specifiers to inspect and ensure the
> contents of the tag are proper. We add two tests to ensure this
> functionality works as expected: the return value should indicate if
> verification passed, and the format specifiers must be respected.
Sounds good.
> +test_expect_success 'verifying tag with --format' '
> + cat >expect <<-\EOF &&
> + tagname : fourth-signed
> + EOF
> + git verify-tag --format="tagname : %(tag)" "fourth-signed" >actual &&
> + test_cmp expect actual
> +'
I suppose it's a matter of style, but for a single-line expectation I
would just do:
echo "tagname : fourth-signed" >expect &&
which is shorter and saves a process invocation.
Ditto on the next patch (which IMHO could just be squashed into a single
patch).
-Peff
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v4 7/7] t/t7004-tag: Add --format specifier tests
2016-10-07 21:07 [PATCH v4 0/7] Add --format to tag verification santiago
` (5 preceding siblings ...)
2016-10-07 21:07 ` [PATCH v4 6/7] t/t7030-verify-tag: Add --format specifier tests santiago
@ 2016-10-07 21:07 ` santiago
2016-10-11 17:43 ` [PATCH v4 0/7] Add --format to tag verification Santiago Torres
7 siblings, 0 replies; 23+ messages in thread
From: santiago @ 2016-10-07 21:07 UTC (permalink / raw)
To: git; +Cc: gitster, peff, sunshine, walters, Santiago Torres
From: Santiago Torres <santiago@nyu.edu>
tag -v now supports --format specifiers to inspect the contents of a tag
upon verification. Add two tests to ensure this behavior is respected in
future changes.
Signed-off-by: Santiago Torres <santiago@nyu.edu>
---
t/t7004-tag.sh | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 8b0f71a..633b089 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -847,6 +847,22 @@ test_expect_success GPG 'verifying a forged tag should fail' '
test_must_fail git tag -v forged-tag
'
+test_expect_success 'verifying a proper tag with --format pass and format accordingly' '
+ cat >expect <<-\EOF &&
+ tagname : signed-tag
+ EOF
+ git tag -v --format="tagname : %(tag)" "signed-tag" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'verifying a forged tag with --format fail and format accordingly' '
+ cat >expect <<-\EOF &&
+ tagname : forged-tag
+ EOF
+ test_must_fail git tag -v --format="tagname : %(tag)" "forged-tag" >actual &&
+ test_cmp expect actual
+'
+
# blank and empty messages for signed tags:
get_tag_header empty-signed-tag $commit commit $time >expect
--
2.10.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v4 0/7] Add --format to tag verification
2016-10-07 21:07 [PATCH v4 0/7] Add --format to tag verification santiago
` (6 preceding siblings ...)
2016-10-07 21:07 ` [PATCH v4 7/7] t/t7004-tag: " santiago
@ 2016-10-11 17:43 ` Santiago Torres
7 siblings, 0 replies; 23+ messages in thread
From: Santiago Torres @ 2016-10-11 17:43 UTC (permalink / raw)
To: git; +Cc: gitster, peff, sunshine, walters
[-- Attachment #1: Type: text/plain, Size: 2504 bytes --]
Hi,
I noticed there were no replies for this thread. I was curious if it got
buried because I sent it on the Friday evening before a long weekend.
I don't mean to pressure or anything.
Thanks!
-Santiago.
On Fri, Oct 07, 2016 at 05:07:14PM -0400, santiago@nyu.edu wrote:
> From: Santiago Torres <santiago@nyu.edu>
>
> This is the fourth iteration of the series in [1][2][3], which comes as a
> result of the discussion in [4]. The main goal of this patch series is to bring
> --format to git tag verification so that upper-layer tools can inspect the
> content of a tag and make decisions based on those contents.
>
> In this re-woll we:
>
> * Fixed the author fields and signed off by's throughout the patch
> series
>
> * Added two more patches with unit tests to ensure the format specifier
> behaves as expected
>
> * Fixed a missing initialization for the format specifier in verify-tag which
> caused a crash.
>
> * Fixed an outdated git commit message that had the previous name of a
> function definition.
>
> Thanks,
> -Santiago
>
> [1] http://public-inbox.org/git/20160930221806.3398-1-santiago@nyu.edu/
> [2] http://public-inbox.org/git/20160922185317.349-1-santiago@nyu.edu/
> [3] http://public-inbox.org/git/20160926224233.32702-1-santiago@nyu.edu/
> [4] http://public-inbox.org/git/20160607195608.16643-1-santiago@nyu.edu/
>
>
> Lukas Puehringer (4):
> tag: add format specifier to gpg_verify_tag
> gpg-interface, tag: add GPG_VERIFY_QUIET flag
> ref-filter: add function to print single ref_array_item
> builtin/tag: add --format argument for tag -v
>
> Santiago Torres (3):
> builtin/verify-tag: add --format to verify-tag
> t/t7030-verify-tag: Add --format specifier tests
> t/t7004-tag: Add --format specifier tests
>
> Documentation/git-tag.txt | 2 +-
> Documentation/git-verify-tag.txt | 2 +-
> builtin/tag.c | 34 +++++++++++++++++++++++-----------
> builtin/verify-tag.c | 13 +++++++++++--
> gpg-interface.h | 1 +
> ref-filter.c | 10 ++++++++++
> ref-filter.h | 3 +++
> t/t7004-tag.sh | 16 ++++++++++++++++
> t/t7030-verify-tag.sh | 16 ++++++++++++++++
> tag.c | 22 +++++++++++++++-------
> tag.h | 4 ++--
> 11 files changed, 99 insertions(+), 24 deletions(-)
>
> --
> 2.10.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread