[PATCH v5 2/7] ref-filter: add function to print single ref_array_item

2017-01-15 Thread santiago
From: Lukas Puehringer 

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 
---
 ref-filter.c | 27 +--
 ref-filter.h |  7 +++
 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 1a978405e..5f4b08792 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1361,7 +1361,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;
 
@@ -1374,11 +1374,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++) {
@@ -1389,6 +1385,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.
@@ -1671,6 +1676,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)
+{
+   struct ref_array_item *ref_item;
+   ref_item = new_ref_array_item(name, sha1, 0);
+   ref_item->kind = ref_kind_from_refname(name);
+   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 fc55fa357..7b05592ba 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -109,4 +109,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);
+
 #endif /*  REF_FILTER_H  */
-- 
2.11.0



[PATCH v5 5/7] builtin/tag: add --format argument for tag -v

2017-01-15 Thread santiago
From: Lukas Puehringer 

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 
---
 Documentation/git-tag.txt |  2 +-
 builtin/tag.c | 32 ++--
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 76cfe40d9..586aaa315 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -15,7 +15,7 @@ SYNOPSIS
 'git tag' [-n[]] -l [--contains ] [--points-at ]
[--column[=] | --no-column] [--create-reflog] [--sort=]
[--format=] [--[no-]merged []] [...]
-'git tag' -v ...
+'git tag' -v [--format=] ...
 
 DESCRIPTION
 ---
diff --git a/builtin/tag.c b/builtin/tag.c
index 880677df5..9da11e0c2 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -24,7 +24,7 @@ static const char * const git_tag_usage[] = {
N_("git tag -d ..."),
N_("git tag -l [-n[]] [--contains ] [--points-at ]"
"\n\t\t[--format=] [--[no-]merged []] 
[...]"),
-   N_("git tag -v ..."),
+   N_("git tag -v [--format=] ..."),
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, ref, fmt_pretty, flags);
 }
 
 static int do_sign(struct strbuf *buffer)
@@ -428,9 +437,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.11.0



[PATCH v5 1/7] gpg-interface, tag: add GPG_VERIFY_QUIET flag

2017-01-15 Thread santiago
From: Lukas Puehringer 

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 
---
 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 ea68885ad..85dc9820d 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 d1dcd18cd..291073f6e 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.11.0



[PATCH v5 6/7] t/t7030-verify-tag: Add --format specifier tests

2017-01-15 Thread santiago
From: Santiago Torres 

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 
---
 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 07079a41c..d62ccbb98 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.11.0



[PATCH v5 7/7] t/t7004-tag: Add --format specifier tests

2017-01-15 Thread santiago
From: Santiago Torres 

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 
---
 t/t7004-tag.sh | 16 
 1 file changed, 16 insertions(+)

diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 07869b0c0..b2b81f203 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -874,6 +874,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.11.0



[PATCH v5 0/7] Add --format to tag verification

2017-01-15 Thread santiago
From: Santiago Torres 

This is the fifth iteration of [1][2][3][4], and as a result of the
discussion in [5]. 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 it.

In this re-woll we:

* Squashed Peff's first patch[6] with the second patch of the series. The commit
  message may need work.

* Applied the relevant segment's of Peff's second patch[7] on the rest of the
  series

* Rebased so these patches apply to the tip of the master branch.

Thanks,
-Santiago

[1] http://public-inbox.org/git/20161007210721.20437-1-santi...@nyu.edu/
[2] http://public-inbox.org/git/20160930221806.3398-1-santi...@nyu.edu/
[3] http://public-inbox.org/git/20160922185317.349-1-santi...@nyu.edu/
[4] http://public-inbox.org/git/20160926224233.32702-1-santi...@nyu.edu/
[5] http://public-inbox.org/git/20160607195608.16643-1-santi...@nyu.edu/
[6] 
http://public-inbox.org/git/20161019203546.dfqmi2czcxopg...@sigill.intra.peff.net/
[7] 
http://public-inbox.org/git/20161019203943.epjxnfci7vcqg...@sigill.intra.peff.net/

Lukas Puehringer (4):
  gpg-interface, tag: add GPG_VERIFY_QUIET flag
  ref-filter: add function to print single ref_array_item
  tag: add format specifier to gpg_verify_tag
  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| 32 ++--
 builtin/verify-tag.c | 13 +++--
 gpg-interface.h  |  1 +
 ref-filter.c | 27 +--
 ref-filter.h |  7 +++
 t/t7004-tag.sh   | 16 
 t/t7030-verify-tag.sh| 16 
 tag.c| 22 +++---
 tag.h|  4 ++--
 11 files changed, 113 insertions(+), 29 deletions(-)

-- 
2.11.0



[PATCH v5 3/7] tag: add format specifier to gpg_verify_tag

2017-01-15 Thread santiago
From: Lukas Puehringer 

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 
---
 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 73df72811..880677df5 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 99f8148cf..de10198c4 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 291073f6e..d5a7cfb20 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);
+
return ret;
 }
 
diff --git a/tag.h b/tag.h
index a5721b673..896b9c2dc 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.11.0



[PATCH v5 4/7] builtin/verify-tag: add --format to verify-tag

2017-01-15 Thread santiago
From: Santiago Torres 

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 
---
 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 d590edceb..0b8075dad 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' ...
+'git verify-tag' [--format=] ...
 
 DESCRIPTION
 ---
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index de10198c4..212449f47 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -12,12 +12,14 @@
 #include 
 #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] ..."),
+   N_("git verify-tag [-v | --verbose] [--format=] 
..."),
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.11.0



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

2017-01-17 Thread santiago
From: Santiago Torres 

This is the sixth iteration of [1][2][3][4][5], and as a result of the
discussion in [5]. 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 it.

In this re-woll we:

* Changed the call interface so printing is done outside of verification. 

* Fixed a couple of whitespace issues and whatnot. 

Thanks,
-Santiago

[1] http://public-inbox.org/git/20170115184705.10376-1-santi...@nyu.edu/
[2] http://public-inbox.org/git/20161007210721.20437-1-santi...@nyu.edu/
[3] http://public-inbox.org/git/20160930221806.3398-1-santi...@nyu.edu/
[4] http://public-inbox.org/git/20160922185317.349-1-santi...@nyu.edu/
[5] http://public-inbox.org/git/20160926224233.32702-1-santi...@nyu.edu/
[6] http://public-inbox.org/git/20160607195608.16643-1-santi...@nyu.edu/
[7] 
http://public-inbox.org/git/20161019203546.dfqmi2czcxopg...@sigill.intra.peff.net/
[8] 
http://public-inbox.org/git/20161019203943.epjxnfci7vcqg...@sigill.intra.peff.net/

Lukas Puehringer (3):
  gpg-interface,tag: add GPG_VERIFY_OMIT_STATUS 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| 38 --
 builtin/verify-tag.c | 23 ---
 gpg-interface.h  |  5 +++--
 ref-filter.c | 27 +--
 ref-filter.h |  7 +++
 t/t7004-tag.sh   | 16 
 t/t7030-verify-tag.sh| 16 
 tag.c|  5 -
 10 files changed, 117 insertions(+), 24 deletions(-)

-- 
2.11.0



[PATCH v6 5/6] t/t7030-verify-tag: Add --format specifier tests

2017-01-17 Thread santiago
From: Santiago Torres 

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 
---
 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 07079a41c..d62ccbb98 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.11.0



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

2017-01-17 Thread santiago
From: Lukas Puehringer 

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 
---
 Documentation/git-tag.txt |  2 +-
 builtin/tag.c | 38 --
 2 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 76cfe40d9..586aaa315 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -15,7 +15,7 @@ SYNOPSIS
 'git tag' [-n[]] -l [--contains ] [--points-at ]
[--column[=] | --no-column] [--create-reflog] [--sort=]
[--format=] [--[no-]merged []] [...]
-'git tag' -v ...
+'git tag' -v [--format=] ...
 
 DESCRIPTION
 ---
diff --git a/builtin/tag.c b/builtin/tag.c
index 73df72811..b9da72761 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -24,7 +24,7 @@ static const char * const git_tag_usage[] = {
N_("git tag -d ..."),
N_("git tag -l [-n[]] [--contains ] [--points-at ]"
"\n\t\t[--format=] [--[no-]merged []] 
[...]"),
-   N_("git tag -v ..."),
+   N_("git tag -v [--format=] ..."),
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,22 @@ 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 gpg_verify_tag(sha1, name, GPG_VERIFY_VERBOSE);
+   int flags;
+   char *fmt_pretty = cb_data;
+   flags = GPG_VERIFY_VERBOSE;
+
+   if (fmt_pretty)
+   flags = GPG_VERIFY_OMIT_STATUS;
+
+   if (gpg_verify_tag(sha1, name, flags))
+   return -1;
+
+if (fmt_pretty)
+   pretty_print_ref(name, sha1, fmt_pretty);
+
+   return 0;
 }
 
 static int do_sign(struct strbuf *buffer)
@@ -428,9 +443,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.11.0



[PATCH v6 3/6] builtin/verify-tag: add --format to verify-tag

2017-01-17 Thread santiago
From: Santiago Torres 

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 
---
 Documentation/git-verify-tag.txt |  2 +-
 builtin/verify-tag.c | 23 ---
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-verify-tag.txt b/Documentation/git-verify-tag.txt
index d590edceb..0b8075dad 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' ...
+'git verify-tag' [--format=] ...
 
 DESCRIPTION
 ---
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 99f8148cf..18443bf9f 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -12,12 +12,14 @@
 #include 
 #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] ..."),
+   N_("git verify-tag [-v | --verbose] [--format=] 
..."),
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,13 +50,26 @@ 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_OMIT_STATUS;
+   }
+
while (i < argc) {
unsigned char sha1[20];
const char *name = argv[i++];
-   if (get_sha1(name, sha1))
+   if (get_sha1(name, sha1)) {
had_error = !!error("tag '%s' not found.", name);
-   else if (gpg_verify_tag(sha1, name, flags))
+   continue;
+   }
+
+   if (gpg_verify_tag(sha1, name, flags)) {
had_error = 1;
+   continue;
+   }
+
+   if (fmt_pretty)
+   pretty_print_ref(name, sha1, fmt_pretty);
}
return had_error;
 }
-- 
2.11.0



[PATCH v6 1/6] gpg-interface,tag: add GPG_VERIFY_OMIT_STATUS flag

2017-01-17 Thread santiago
From: Lukas Puehringer 

Functions that print git object information may require that the
gpg-interface functions be silent. Add GPG_VERIFY_OMIT_STATUS flag and
prevent print_signature_buffer from being called if flag is set.

Signed-off-by: Lukas Puehringer 
---
 gpg-interface.h | 5 +++--
 tag.c   | 5 -
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/gpg-interface.h b/gpg-interface.h
index ea68885ad..d2d4fd3a6 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -1,8 +1,9 @@
 #ifndef GPG_INTERFACE_H
 #define GPG_INTERFACE_H
 
-#define GPG_VERIFY_VERBOSE 1
-#define GPG_VERIFY_RAW 2
+#define GPG_VERIFY_VERBOSE 1
+#define GPG_VERIFY_RAW 2
+#define GPG_VERIFY_OMIT_STATUS 4
 
 struct signature_check {
char *payload;
diff --git a/tag.c b/tag.c
index d1dcd18cd..243d1fdbb 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_OMIT_STATUS))
+   print_signature_buffer(&sigc, flags);
 
signature_check_clear(&sigc);
return ret;
-- 
2.11.0



[PATCH v6 2/6] ref-filter: add function to print single ref_array_item

2017-01-17 Thread santiago
From: Lukas Puehringer 

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 
---
 ref-filter.c | 27 +--
 ref-filter.h |  7 +++
 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 1a978405e..5f4b08792 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1361,7 +1361,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;
 
@@ -1374,11 +1374,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++) {
@@ -1389,6 +1385,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.
@@ -1671,6 +1676,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)
+{
+   struct ref_array_item *ref_item;
+   ref_item = new_ref_array_item(name, sha1, 0);
+   ref_item->kind = ref_kind_from_refname(name);
+   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 fc55fa357..7b05592ba 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -109,4 +109,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);
+
 #endif /*  REF_FILTER_H  */
-- 
2.11.0



[PATCH v6 6/6] t/t7004-tag: Add --format specifier tests

2017-01-17 Thread santiago
From: Santiago Torres 

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 
---
 t/t7004-tag.sh | 16 
 1 file changed, 16 insertions(+)

diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 07869b0c0..ba88b556b 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -874,6 +874,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.11.0



[PATCH v4 3/7] tag: add format specifier to gpg_verify_tag

2016-10-07 Thread santiago
From: Lukas Puehringer 

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 
---
 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



[PATCH v4 4/7] builtin/verify-tag: add --format to verify-tag

2016-10-07 Thread santiago
From: Santiago Torres 

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 
---
 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' ...
+'git verify-tag' [--format=] ...
 
 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 
 #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] ..."),
+   N_("git verify-tag [-v | --verbose] [--format=] 
..."),
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



[PATCH v4 1/7] gpg-interface, tag: add GPG_VERIFY_QUIET flag

2016-10-07 Thread santiago
From: Lukas Puehringer 

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 
---
 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



[PATCH v4 2/7] ref-filter: add function to print single ref_array_item

2016-10-07 Thread santiago
From: Lukas Puehringer 

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 
---
 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



[PATCH v4 0/7] Add --format to tag verification

2016-10-07 Thread santiago
From: Santiago Torres 

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-santi...@nyu.edu/
[2] http://public-inbox.org/git/20160922185317.349-1-santi...@nyu.edu/
[3] http://public-inbox.org/git/20160926224233.32702-1-santi...@nyu.edu/
[4] http://public-inbox.org/git/20160607195608.16643-1-santi...@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



[PATCH v4 6/7] t/t7030-verify-tag: Add --format specifier tests

2016-10-07 Thread santiago
From: Santiago Torres 

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 
---
 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



[PATCH v4 5/7] builtin/tag: add --format argument for tag -v

2016-10-07 Thread santiago
From: Lukas Puehringer 

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 
---
 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[]] -l [--contains ] [--points-at ]
[--column[=] | --no-column] [--create-reflog] [--sort=]
[--format=] [--[no-]merged []] [...]
-'git tag' -v ...
+'git tag' -v [--format=] ...
 
 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 ..."),
N_("git tag -l [-n[]] [--contains ] [--points-at ]"
"\n\t\t[--format=] [--[no-]merged []] 
[...]"),
-   N_("git tag -v ..."),
+   N_("git tag -v [--format=] ..."),
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



[PATCH v4 7/7] t/t7004-tag: Add --format specifier tests

2016-10-07 Thread santiago
From: Santiago Torres 

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 
---
 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



[RFC/PATCH] verify-tag: add --check-name flag

2016-06-07 Thread santiago
From: Santiago Torres 

Hello everyone,

In a previous thread [1] we discussed about the possibility of having a
--check-name flag, for the tag-verify command (and possibly git tag -v).
Although many points were in the table, I don't think that it was
conclusive as to whether having it or not. Also, I noticed that the,
refactored interface requires really minmal changes to do this
(see attached patch). 

This is the behavior I had in mind:


santiago at ~/.../.../git ✗ ./git-verify-tag --check-name v2.7.0-rc2
gpg: Signature made Tue 22 Dec 2015 05:46:04 PM EST using RSA ... 
gpg: Good signature from "Junio C Hamano " [...]
...
error: tag name doesn't match tag header!(v2.7.0-rc2)

santiago at ~/.../.../git ✗ ./git-verify-tag --check-name v2.7.0-rc2
gpg: Signature made Tue 22 Dec 2015 05:46:04 PM EST using RSA ...
gpg: Good signature from "Junio C Hamano " [...]
...

The rationale behind this is the following:

1.- Using a tag ref as a check-out mechanism is pretty common by package
managers and other tools. Verifying the tag signature provides
authentication guarantees, but there is no feedback that the
signature being verified belongs to the intended tag.

2.- The tuple tagname + signature can uniquely identify a tag. There
are many tags that can have the same name, but this is mostly due
to the naming policy. Having a tag-ref pointing to the right tag
name with an appropriate signature provides tigther guarantees about
the tag that's being checked-out.

3.- This follows concerns about other people who wish to provide a
tighter binding between the refs and the tag objects. The git-evtag
project is an example of this[2].

What I want to prevent is the following: 

santiago at ~/.../ ✔ pip install -e git+https://.../django/@1.8.3#egg=django
Obtaining django from git+https://.../django/@1.8.3#egg=django
[...] 
    Successfully installed django
santiago at ~/.../ ✔ django-admin.py --version
1.4.11

In this example, the tag retrieved is an annotated tag, signed by the
right developer. In this case signature verification would pass, and
there are no hints that something *might* have be wrong. Needless to
say, Django 1.4.11 is deprecated and vulnerable to really nasty XSS and
SQLi vectors...

I acknowledge that it is possible to provide this by using the SHA1 of
the tag object instead of the tag-ref, but this provides comparable
guarantees while keeping a readable interface. Of course that the sha1
is the "tightest" binding, so this also allows for developers to
remove tags during quick-fixes (as Junio pointed out in [1]) and other
edge cases in which the SHA1 would break.

Of course that using this flag needs to be integrated by package
managers and other tools; I wouldn't mind reaching out and even
proposing patches for the popular ones.

A stub of the intended patch follows. I'll can make a cleaner patch
(e.g., to include this in git tag -v) based on any feedback provided.

[1] http://thread.gmane.org/gmane.comp.version-control.git/284757
[2] http://thread.gmane.org/gmane.comp.version-control.git/288439

---
 builtin/verify-tag.c | 1 +
 tag.c| 8 
 tag.h| 1 +
 3 files changed, 10 insertions(+)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 99f8148..947babe 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -33,6 +33,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_BIT(0, "check-name", &flags, N_("Verify the tag name"), 
TAG_VERIFY_NAME),
OPT_END()
};
 
diff --git a/tag.c b/tag.c
index d1dcd18..591b31e 100644
--- a/tag.c
+++ b/tag.c
@@ -55,6 +55,14 @@ int gpg_verify_tag(const unsigned char *sha1, const char 
*name_to_report,
 
ret = run_gpg_verify(buf, size, flags);
 
+   if (flags & TAG_VERIFY_NAME) {
+   struct tag tag_info;
+   ret += parse_tag_buffer(&tag_info, buf, size);
+   if strncmp(tag_info.tag, name_to_report, size)
+   ret += error("tag name doesn't match tag header!(%s)",
+   tag_info.tag);
+   }
+
free(buf);
return ret;
 }
diff --git a/tag.h b/tag.h
index a5721b6..30c922e 100644
--- a/tag.h
+++ b/tag.h
@@ -2,6 +2,7 @@
 #define TAG_H
 
 #include "object.h"
+#define TAG_VERIFY_NAME 0x10
 
 extern const char *tag_type;
 
-- 
2.8.3

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2016-09-22 Thread santiago
From: Santiago Torres 

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 6/6] builtin/tag: add --format argument for tag -v

2016-09-22 Thread santiago
From: Lukas P 

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 
---
 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 ..."),
N_("git tag -l [-n[]] [--contains ] [--points-at ]"
"\n\t\t[--format=] [--[no-]merged []] 
[...]"),
-   N_("git tag -v ..."),
+   N_("git tag -v [--format=] ..."),
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 3/6] ref-filter: Expose wrappers for ref_item functions

2016-09-22 Thread santiago
From: Lukas P 

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 
---
 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 4/6] tag: add format specifier to gpg_verify_tag

2016-09-22 Thread santiago
From: Lukas P 

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 
---
 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

2016-09-22 Thread santiago
From: Santiago Torres 

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 
---
 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 2/6] gpg-interface: add GPG_VERIFY_QUIET flag

2016-09-22 Thread santiago
From: Lukas P 

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 
---
 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 5/6] builtin/verify-tag: Add --format to verify-tag

2016-09-22 Thread santiago
From: Santiago Torres 

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 
---
 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 
 #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] ..."),
+   N_("git verify-tag [-v | --verbose] [--format=] 
..."),
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



[PATCH v2 3/5] tag: add format specifier to gpg_verify_tag

2016-09-26 Thread santiago
From: Lukas P 

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 P 
---
 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..65e1a96 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)
+   format_ref(name, sha1, fmt_pretty, FILTER_REFS_TAGS);
+
return ret;
 }
 
diff --git a/tag.h b/tag.h
index a5721b6..0b6e458 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



[PATCH v2 1/5] gpg-interface, tag: add GPG_VERIFY_QUIET flag

2016-09-26 Thread santiago
From: Lukas P 

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 P 
---
 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



[PATCH v2 2/5] ref-filter: add function to print single ref_array_item

2016-09-26 Thread santiago
From: Lukas P 

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 format_ref function to create, pretty print and free individual
ref-items.

Signed-off-by: Lukas P 
---
 ref-filter.c | 10 ++
 ref-filter.h |  4 
 2 files changed, 14 insertions(+)

diff --git a/ref-filter.c b/ref-filter.c
index bc551a7..e0aaf5f 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1655,6 +1655,16 @@ void show_ref_array_item(struct ref_array_item *info, 
const char *format, int qu
putchar('\n');
 }
 
+void format_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..1ef7999 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -107,4 +107,8 @@ 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);
 
+/* Pretty-print a single ref */
+void format_ref(const char *name, const unsigned char *sha1, const char 
*format,
+   unsigned kind);
+
 #endif /*  REF_FILTER_H  */
-- 
2.10.0



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

2016-09-26 Thread santiago
From: Lukas P 

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.

Caveat: The change adds a format specifier argument to the
(*each_tag_name_fn) function pointer, i.e. delete_tag now receives this
too, although it does not need it.

Signed-off-by: Lukas P 
---
 builtin/tag.c | 30 --
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 14f3b48..f53227e 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -24,7 +24,7 @@ static const char * const git_tag_usage[] = {
N_("git tag -d ..."),
N_("git tag -l [-n[]] [--contains ] [--points-at ]"
"\n\t\t[--format=] [--[no-]merged []] 
[...]"),
-   N_("git tag -v ..."),
+   N_("git tag -v [--format=] ..."),
NULL
 };
 
@@ -66,9 +66,10 @@ 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, const char 
*fmt_pretty);
 
-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,
+   const char *fmt_pretty)
 {
const char **p;
char ref[PATH_MAX];
@@ -87,14 +88,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, fmt_pretty))
had_error = 1;
}
return had_error;
 }
 
 static int delete_tag(const char *name, const char *ref,
-   const unsigned char *sha1)
+   const unsigned char *sha1, const char 
*fmt_pretty)
 {
if (delete_ref(ref, sha1, 0))
return 1;
@@ -103,9 +104,15 @@ 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, const char 
*fmt_pretty)
 {
-   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)
@@ -424,9 +431,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



[PATCH v2 4/5] builtin/verify-tag: add --format to verify-tag

2016-09-26 Thread santiago
From: Santiago Torres 

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 
---
 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 de10198..a941053 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -12,12 +12,15 @@
 #include 
 #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] ..."),
+   N_("git verify-tag [-v | --verbose] [--format=] 
..."),
NULL
 };
 
+static 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,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



[PATCH v2 0/5] Add --format to tag verification

2016-09-26 Thread santiago
From: Santiago Torres 

This is the second iteration of [1], and as a result of the discussion
in [2].

In this re-roll we:

* Dropped the commit to move the format string parameter to a global
  variable on builtin/tag. We had to change the signature of
  for_each_name_fn to do this.

* Fixed the signed-off-by lines to match the author/committer

[0001]
* Moved the GPG_VERIFY_QUIET flag check into tag.c instead of
  gpg_interface

[0002] 
* Changed the exported function to "format_ref". This way, we can print
  a single formatted ref element. This also made the patch
  cleaner/easier to read.

[0003] 
* We fixed style/formatting errors that we had introduced

This patch applies to 2.10.0 and master.

[1] http://public-inbox.org/git/20160922185317.349-1-santi...@nyu.edu/
[2] http://public-inbox.org/git/20160607195608.16643-1-santi...@nyu.edu/

Lukas P (4):
  gpg-interface, tag: add GPG_VERIFY_QUIET flag
  ref-filter: add function to print single ref_array_item
  tag: add format specifier to gpg_verify_tag
  builtin/tag: add --format argument for tag -v

Santiago Torres (1):
  builtin/verify-tag: add --format to verify-tag

 builtin/tag.c| 30 --
 builtin/verify-tag.c | 16 ++--
 gpg-interface.h  |  1 +
 ref-filter.c | 10 ++
 ref-filter.h |  4 
 tag.c| 22 +++---
 tag.h|  4 ++--
 7 files changed, 66 insertions(+), 21 deletions(-)

-- 
2.10.0



[PATCH v3 1/5] gpg-interface, tag: add GPG_VERIFY_QUIET flag

2016-09-30 Thread santiago
From: Lukas Puehringer 

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 
---
 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



[PATCH v3 0/5] Add --format to tag verification

2016-09-30 Thread santiago
From: Santiago Torres 

This is the third iteration of [1][2], and as a result of the discussion
in [3].

In this re-roll we:

* Fixed all the signed-off-by's

[0002]
* Renamed the function format_ref to pretty_print_ref instead, which
  is a more descriptive name 

[0004] 
* Added the respective line for the new --format parameter in the
  documentation.

[0005] 
* Added mention of the --format flag in the documentation files. 
* Fixed the function signatures, now they take an opaque void *cb_data pointer
  so it can be used in a more general way (by e.g., delete_tag).

This patch applies to 2.10.0 and master.

[1] http://public-inbox.org/git/20160922185317.349-1-santi...@nyu.edu/
[2] http://public-inbox.org/git/20160926224233.32702-1-santi...@nyu.edu/
[3] http://public-inbox.org/git/20160607195608.16643-1-santi...@nyu.edu/

Lukas Puehringer (4):
  gpg-interface, tag: add GPG_VERIFY_QUIET flag
  ref-filter: add function to print single ref_array_item
  tag: add format specifier to gpg_verify_tag
  builtin/tag: add --format argument for tag -v

Santiago Torres (1):
  builtin/verify-tag: add --format to verify-tag

 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 +++
 tag.c| 22 +++---
 tag.h|  4 ++--
 9 files changed, 67 insertions(+), 24 deletions(-)

-- 
2.10.0



[PATCH v3 4/5] builtin/verify-tag: add --format to verify-tag

2016-09-30 Thread santiago
From: Santiago Torres 

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 
---
 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' ...
+'git verify-tag' [--format=] ...
 
 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 
 #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] ..."),
+   N_("git verify-tag [-v | --verbose] [--format=] 
..."),
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;
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



[PATCH v3 3/5] tag: add format specifier to gpg_verify_tag

2016-09-30 Thread santiago
From: Lukas P 

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 
---
 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



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

2016-09-30 Thread santiago
From: Lukas Puehringer 

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 
---
 Documentation/git-tag.txt |  2 +-
 builtin/tag.c | 34 +++---
 2 files changed, 24 insertions(+), 12 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[]] -l [--contains ] [--points-at ]
[--column[=] | --no-column] [--create-reflog] [--sort=]
[--format=] [--[no-]merged []] [...]
-'git tag' -v ...
+'git tag' -v [--format=] ...
 
 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 ..."),
N_("git tag -l [-n[]] [--contains ] [--points-at ]"
"\n\t\t[--format=] [--[no-]merged []] 
[...]"),
-   N_("git tag -v ..."),
+   N_("git tag -v [--format=] ..."),
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



[PATCH v3 2/5] ref-filter: add function to print single ref_array_item

2016-09-30 Thread santiago
From: Lukas Puehringer 

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 format_ref function to create, pretty print and free individual
ref-items.

Signed-off-by: Lukas Puehringer 
---
 ref-filter.c | 10 ++
 ref-filter.h |  3 +++
 2 files changed, 13 insertions(+)

diff --git a/ref-filter.c b/ref-filter.c
index bc551a7..ee3ed67 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1655,6 +1655,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



[PATCH/RFC] builtin/tag: Changes argument format for verify

2016-02-26 Thread santiago
From: Santiago Torres 

The verify tag function converts the commit sha1 to hex and passes it as
a command-line argument to builtin/verify-tag. Given that builtin/verify-tag
already resolves the ref name sha1 equivalent, the sha1 to
hex_sha1 conversion is unnecessary and the ref-name can be used instead.

Signed-off-by: Santiago Torres 
---
 builtin/tag.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 1705c94..5de1161 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -105,8 +105,7 @@ static int verify_tag(const char *name, const char *ref,
const unsigned char *sha1)
 {
const char *argv_verify_tag[] = {"verify-tag",
-   "-v", "SHA1_HEX", NULL};
-   argv_verify_tag[2] = sha1_to_hex(sha1);
+   "-v", name, NULL};
 
if (run_command_v_opt(argv_verify_tag, RUN_GIT_CMD))
return error(_("could not verify the tag '%s'"), name);
-- 
2.7.0.435.g70bd996.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH/RFC] builtin/tag.c: move PGP verification inside builtin.

2016-03-24 Thread santiago
From: Santiago Torres 

The verify tag function is just a thin wrapper around the verify-tag
command. We can avoid one fork call by doing the verification instide
the tag builtin instead.

Signed-off-by: Santiago Torres 
---
 builtin/tag.c | 44 ++--
 1 file changed, 38 insertions(+), 6 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 1705c94..be5d7c7 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -30,6 +30,27 @@ static const char * const git_tag_usage[] = {
 
 static unsigned int colopts;
 
+static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
+{
+   struct signature_check sigc;
+   int len;
+   int ret;
+
+   memset(&sigc, 0, sizeof(sigc));
+
+   len = parse_signature(buf, size);
+
+   if (size == len) {
+   write_in_full(1, buf, len);
+   }
+
+   ret = check_signature(buf, len, buf + len, size - len, &sigc);
+   print_signature_buffer(&sigc, flags);
+
+   signature_check_clear(&sigc);
+   return ret;
+}
+
 static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, 
const char *format)
 {
struct ref_array array;
@@ -104,13 +125,24 @@ 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 char *argv_verify_tag[] = {"verify-tag",
-   "-v", "SHA1_HEX", NULL};
-   argv_verify_tag[2] = sha1_to_hex(sha1);
 
-   if (run_command_v_opt(argv_verify_tag, RUN_GIT_CMD))
-   return error(_("could not verify the tag '%s'"), name);
-   return 0;
+   enum object_type type;
+   unsigned long size;
+   const char* buf;
+   int ret;
+
+   type = sha1_object_info(sha1, NULL);
+   if (type != OBJ_TAG)
+   return error("%s: cannot verify a non-tag object of type %s.",
+   name, typename(type));
+
+   buf = read_sha1_file(sha1, &type, &size);
+   if (!buf)
+   return error("%s: unable to read file.", name);
+
+   ret = run_gpg_verify(buf, size, 0);
+
+   return ret;
 }
 
 static int do_sign(struct strbuf *buffer)
-- 
2.7.3

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] tag.c: move PGP verification code from plumbing

2016-03-24 Thread santiago
From: Santiago Torres 

The verify tag function is just a thin wrapper around the verify-tag
command. We can avoid one fork call by doing the verification inside
the tag builtin instead.

To do this, the run_pgp_verify() and verify_tag() functions are moved to
tag.c. The definition of verify_tag was changed to support extra
arguments that match the builtin/tag and builtin/verify-tag modules. The
SIGPIPE ignore call in tag-verify was also moved to a more sensible
place, now that both modules need it.

The function name was also changed to pgp_verify_tag to avoid conflicts with
mktag.c's.

Signed-off-by: Santiago Torres 
---
 builtin/tag.c| 26 ---
 builtin/verify-tag.c | 60 ++--
 gpg-interface.c  |  6 ++
 tag.c| 48 +
 tag.h|  2 ++
 5 files changed, 72 insertions(+), 70 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 1705c94..af8f3ba 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -65,9 +65,10 @@ 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, unsigned flags);
 
-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,
+   unsigned flags)
 {
const char **p;
char ref[PATH_MAX];
@@ -86,32 +87,22 @@ 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, flags))
had_error = 1;
}
return had_error;
 }
 
 static int delete_tag(const char *name, const char *ref,
-   const unsigned char *sha1)
+   const unsigned char *sha1, unsigned flags)
 {
-   if (delete_ref(ref, sha1, 0))
+   if (delete_ref(ref, sha1, flags))
return 1;
printf(_("Deleted tag '%s' (was %s)\n"), name, find_unique_abbrev(sha1, 
DEFAULT_ABBREV));
return 0;
 }
 
-static int verify_tag(const char *name, const char *ref,
-   const unsigned char *sha1)
-{
-   const char *argv_verify_tag[] = {"verify-tag",
-   "-v", "SHA1_HEX", NULL};
-   argv_verify_tag[2] = sha1_to_hex(sha1);
 
-   if (run_command_v_opt(argv_verify_tag, RUN_GIT_CMD))
-   return error(_("could not verify the tag '%s'"), name);
-   return 0;
-}
 
 static int do_sign(struct strbuf *buffer)
 {
@@ -424,9 +415,10 @@ 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);
+   return for_each_tag_name(argv, delete_tag, 0);
if (cmdmode == 'v')
-   return for_each_tag_name(argv, verify_tag);
+   return for_each_tag_name(argv, pgp_verify_tag,
+   GPG_VERIFY_VERBOSE);
 
if (msg.given || msgfile) {
if (msg.given && msgfile)
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 00663f6..2e6a175 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -18,55 +18,6 @@ static const char * const verify_tag_usage[] = {
NULL
 };
 
-static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
-{
-   struct signature_check sigc;
-   int len;
-   int ret;
-
-   memset(&sigc, 0, sizeof(sigc));
-
-   len = parse_signature(buf, size);
-
-   if (size == len) {
-   if (flags & GPG_VERIFY_VERBOSE)
-   write_in_full(1, buf, len);
-   return error("no signature found");
-   }
-
-   ret = check_signature(buf, len, buf + len, size - len, &sigc);
-   print_signature_buffer(&sigc, flags);
-
-   signature_check_clear(&sigc);
-   return ret;
-}
-
-static int verify_tag(const char *name, unsigned flags)
-{
-   enum object_type type;
-   unsigned char sha1[20];
-   char *buf;
-   unsigned long size;
-   int ret;
-
-   if (get_sha1(name, sha1))
-   return error("tag '%s' not found.", name);
-
-   type = sha1_object_info(sha1, NULL);
-   if (type != OBJ_TAG)
-   return error("%s: cannot verify a non-tag object of type %s.",
-

[PATCH v3 0/4] tag: move PGP verification code to tag.c

2016-04-02 Thread santiago
This is a follow up of [1] and [2]:

v3 (this):
Thanks Eric, Jeff, for the feedback.

 * I separated the patch in multiple sub-patches.
 * I compared the behavior of previous git tag -v and git verify-tag 
   invocations to make sure the behavior is the same
 * I dropped the multi-line comment, as suggested.
 * I fixed the issue with the missing brackets in the while (this is 
   now detected by the test).

v2:

 * I moved the pgp-verification code to tag.c 
 * I added extra arguments so git tag -v and git verify-tag both work
   with the same function
 * Relocated the SIGPIPE handling code in verify-tag to gpg-interface

v1:
 
The verify tag function is just a thin wrapper around the verify-tag
command. We can avoid one fork call by doing the verification inside
the tag builtin instead.


This applies on v2.8.0.

Thanks!
-Santiago

[1]
http://git.661346.n2.nabble.com/PATCH-RFC-builtin-tag-c-move-PGP-verification-inside-builtin-td7651529.html#a7651547
[2] 
http://git.661346.n2.nabble.com/PATCH-tag-c-move-PGP-verification-code-from-plumbing-td7651562.html
 

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 2/4] t/t7030-verify-tag.sh: Adds validation for multiple tags

2016-04-02 Thread santiago
From: Santiago Torres 

The verify-tag command supports mutliple tag names as an argument.
However, no previous tests try to verify multiple tags at once. This
test runs the verify-tag command against three trusted tags (created
previously), and ensures that:

1) Three tags are verified appropriately (grep GOODSIG) and
2) The three tags verified are indeed differently (uniq
-u)

Signed-off-by: Santiago Torres 
---
 t/t7030-verify-tag.sh | 8 
 1 file changed, 8 insertions(+)

diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh
index 4608e71..5918f86 100755
--- a/t/t7030-verify-tag.sh
+++ b/t/t7030-verify-tag.sh
@@ -112,4 +112,12 @@ test_expect_success GPG 'verify signatures with --raw' '
)
 '
 
+test_expect_success GPG 'verify multiple tags' '
+   git verify-tag -v --raw fourth-signed sixth-signed seventh-signed 
2>actual 1> tagnames &&
+   grep -c "GOODSIG" actual > count &&
+   ! grep "BADSIG" actual &&
+   grep -E "tag\ .*" tagnames | uniq -u | wc - l | grep "3"
+'
+
+
 test_done
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 4/4] tag: use pgp_verify_function in tag -v call

2016-04-02 Thread santiago
From: Santiago Torres 

Instead of running the verify-tag plumbing command, we use the
pgp_verify_tag(). This avoids the usage of an extra fork call. To do
this, we extend the number of parameters that tag.c takes, and
verify-tag passes. Redundant calls done in the pgp_verify_tag function
are removed.

Signed-off-by: Santiago Torres 
---
 Notes:
 - In this version I fixed the issues with the brackets (the old patch
   doesn't work in this case due to the new test.

 builtin/tag.c| 28 +---
 builtin/verify-tag.c | 14 --
 tag.c|  7 ++-
 tag.h|  3 ++-
 4 files changed, 25 insertions(+), 27 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 1705c94..3dffdff 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -65,9 +65,10 @@ 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, unsigned flags);
 
-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,
+   unsigned flags)
 {
const char **p;
char ref[PATH_MAX];
@@ -86,33 +87,21 @@ 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, flags))
had_error = 1;
}
return had_error;
 }
 
 static int delete_tag(const char *name, const char *ref,
-   const unsigned char *sha1)
+   const unsigned char *sha1, unsigned flags)
 {
-   if (delete_ref(ref, sha1, 0))
+   if (delete_ref(ref, sha1, flags))
return 1;
printf(_("Deleted tag '%s' (was %s)\n"), name, find_unique_abbrev(sha1, 
DEFAULT_ABBREV));
return 0;
 }
 
-static int verify_tag(const char *name, const char *ref,
-   const unsigned char *sha1)
-{
-   const char *argv_verify_tag[] = {"verify-tag",
-   "-v", "SHA1_HEX", NULL};
-   argv_verify_tag[2] = sha1_to_hex(sha1);
-
-   if (run_command_v_opt(argv_verify_tag, RUN_GIT_CMD))
-   return error(_("could not verify the tag '%s'"), name);
-   return 0;
-}
-
 static int do_sign(struct strbuf *buffer)
 {
return sign_buffer(buffer, buffer, get_signing_key());
@@ -424,9 +413,10 @@ 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);
+   return for_each_tag_name(argv, delete_tag, 0);
if (cmdmode == 'v')
-   return for_each_tag_name(argv, verify_tag);
+   return for_each_tag_name(argv, pgp_verify_tag,
+   GPG_VERIFY_VERBOSE);
 
if (msg.given || msgfile) {
if (msg.given && msgfile)
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index f776778..8abc357 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -30,6 +30,8 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
 {
int i = 1, verbose = 0, had_error = 0;
unsigned flags = 0;
+   unsigned char sha1[20];
+   const char *name;
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),
@@ -46,8 +48,16 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
if (verbose)
flags |= GPG_VERIFY_VERBOSE;
 
-   while (i < argc)
-   if (pgp_verify_tag(argv[i++], flags))
+   while (i < argc) {
+   name = argv[i++];
+   if (get_sha1(name, sha1)) {
+   error("tag '%s' not found.", name);
had_error = 1;
+   }
+
+   if (pgp_verify_tag(name, NULL, sha1, flags))
+   had_error = 1;
+
+   }
return had_error;
 }
diff --git a/tag.c b/tag.c
index 918ae39..2a0b24c 100644
--- a/tag.c
+++ b/tag.c
@@ -29,18 +29,15 @@ static int run_gpg_verify(const char *buf, unsigned long 
size, unsigned flags)
return ret;
 }
 
-int pgp_verify_tag(const char *name, unsigned flags)
+int pgp_verify_tag(const char *name, const char *ref,
+   const unsi

[PATCH v3 3/4] builtin/verify-tag: move verification code to tag.c

2016-04-02 Thread santiago
From: Santiago Torres 

The PGP verification routine for tags could be accessed by other
commands that require it. We do this by moving it to the common tag.c
code. We rename the verify_tag() function to pgp_verify_tag() to avoid
conflicts with the mktag.c function.

Signed-off-by: Santiago Torres 
---
 builtin/verify-tag.c | 51 +--
 tag.c| 50 ++
 tag.h|  1 +
 3 files changed, 52 insertions(+), 50 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 77f070a..f776778 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -18,55 +18,6 @@ static const char * const verify_tag_usage[] = {
NULL
 };
 
-static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
-{
-   struct signature_check sigc;
-   int len;
-   int ret;
-
-   memset(&sigc, 0, sizeof(sigc));
-
-   len = parse_signature(buf, size);
-
-   if (size == len) {
-   if (flags & GPG_VERIFY_VERBOSE)
-   write_in_full(1, buf, len);
-   return error("no signature found");
-   }
-
-   ret = check_signature(buf, len, buf + len, size - len, &sigc);
-   print_signature_buffer(&sigc, flags);
-
-   signature_check_clear(&sigc);
-   return ret;
-}
-
-static int verify_tag(const char *name, unsigned flags)
-{
-   enum object_type type;
-   unsigned char sha1[20];
-   char *buf;
-   unsigned long size;
-   int ret;
-
-   if (get_sha1(name, sha1))
-   return error("tag '%s' not found.", name);
-
-   type = sha1_object_info(sha1, NULL);
-   if (type != OBJ_TAG)
-   return error("%s: cannot verify a non-tag object of type %s.",
-   name, typename(type));
-
-   buf = read_sha1_file(sha1, &type, &size);
-   if (!buf)
-   return error("%s: unable to read file.", name);
-
-   ret = run_gpg_verify(buf, size, flags);
-
-   free(buf);
-   return ret;
-}
-
 static int git_verify_tag_config(const char *var, const char *value, void *cb)
 {
int status = git_gpg_config(var, value, cb);
@@ -96,7 +47,7 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
flags |= GPG_VERIFY_VERBOSE;
 
while (i < argc)
-   if (verify_tag(argv[i++], flags))
+   if (pgp_verify_tag(argv[i++], flags))
had_error = 1;
return had_error;
 }
diff --git a/tag.c b/tag.c
index d72f742..918ae39 100644
--- a/tag.c
+++ b/tag.c
@@ -6,6 +6,56 @@
 
 const char *tag_type = "tag";
 
+static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
+{
+   struct signature_check sigc;
+   int payload_size;
+   int ret;
+
+   memset(&sigc, 0, sizeof(sigc));
+
+   payload_size = parse_signature(buf, size);
+
+   if (size == payload_size) {
+   write_in_full(1, buf, payload_size);
+   return error("No PGP signature found in this tag!");
+   }
+
+   ret = check_signature(buf, payload_size, buf + payload_size,
+   size - payload_size, &sigc);
+   print_signature_buffer(&sigc, flags);
+
+   signature_check_clear(&sigc);
+   return ret;
+}
+
+int pgp_verify_tag(const char *name, unsigned flags)
+{
+
+   enum object_type type;
+   unsigned long size;
+   unsigned char sha1[20];
+   char* buf;
+   int ret;
+
+   if (get_sha1(name, sha1))
+   return error("tag '%s' not found.", name);
+
+   type = sha1_object_info(sha1, NULL);
+   if (type != OBJ_TAG)
+   return error("%s: cannot verify a non-tag object of type %s.",
+   name, typename(type));
+
+   buf = read_sha1_file(sha1, &type, &size);
+   if (!buf)
+   return error("%s: unable to read file.", name);
+
+   ret = run_gpg_verify(buf, size, flags);
+
+   free(buf);
+   return ret;
+}
+
 struct object *deref_tag(struct object *o, const char *warn, int warnlen)
 {
while (o && o->type == OBJ_TAG)
diff --git a/tag.h b/tag.h
index f4580ae..09e71f9 100644
--- a/tag.h
+++ b/tag.h
@@ -17,5 +17,6 @@ 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 pgp_verify_tag(const char *name, unsigned flags);
 
 #endif /* TAG_H */
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 1/4] builtin/verify-tag.c: Ignore SIGPIPE on gpg-interface

2016-04-02 Thread santiago
From: Santiago Torres 

The verify_signed_buffer comand might cause a SIGPIPE signal when the
gpg child process terminates early (due to a bad keyid, for example) and
git tries to write to it afterwards. Previously, ignoring SIGPIPE was
done on the builtin/gpg-verify.c command to avoid this issue. However,
any other caller who wanted to use the verify_signed_buffer command
would have to include this signal call.

Instead, we use sigchain_push(SIGPIPE, SIG_IGN) on the
verify_signed_buffer call (pretty much like in sign_buffer()) so
that any caller is not required to perform this task. This will avoid
possible mistakes by further developers using verify_signed_buffer.

Signed-off-by: Santiago Torres 
---
Notes:
 I dropped the multiline comment altogheter.

 builtin/verify-tag.c | 3 ---
 gpg-interface.c  | 3 +++
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 00663f6..77f070a 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -95,9 +95,6 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
if (verbose)
flags |= GPG_VERIFY_VERBOSE;
 
-   /* sometimes the program was terminated because this signal
-* was received in the process of writing the gpg input: */
-   signal(SIGPIPE, SIG_IGN);
while (i < argc)
if (verify_tag(argv[i++], flags))
had_error = 1;
diff --git a/gpg-interface.c b/gpg-interface.c
index 3dc2fe3..c1f6b2d 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -232,6 +232,8 @@ int verify_signed_buffer(const char *payload, size_t 
payload_size,
if (gpg_output)
gpg.err = -1;
args_gpg[3] = path;
+
+   sigchain_push(SIGPIPE, SIG_IGN);
if (start_command(&gpg)) {
unlink(path);
return error(_("could not run gpg."));
@@ -250,6 +252,7 @@ int verify_signed_buffer(const char *payload, size_t 
payload_size,
close(gpg.out);
 
ret = finish_command(&gpg);
+   sigchain_pop(SIGPIPE);
 
unlink_or_warn(path);
 
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 3/6] builtin/verify-tag: move verification code to tag.c

2016-04-04 Thread santiago
From: Santiago Torres 

The PGP verification routine for tags could be accessed by other
commands that require it. We do this by moving it to the common tag.c
code. We rename the verify_tag() function to gpg_verify_tag() to avoid
conflicts with the mktag.c function.

Signed-off-by: Santiago Torres 
---
Note: In this version, I just blndly moved code around and renamed
the function's name for ease of review. The following patch only renames
variables.

 builtin/verify-tag.c | 51 +--
 tag.c| 49 +
 tag.h|  1 +
 3 files changed, 51 insertions(+), 50 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 77f070a..7e36d53 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -18,55 +18,6 @@ static const char * const verify_tag_usage[] = {
NULL
 };
 
-static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
-{
-   struct signature_check sigc;
-   int len;
-   int ret;
-
-   memset(&sigc, 0, sizeof(sigc));
-
-   len = parse_signature(buf, size);
-
-   if (size == len) {
-   if (flags & GPG_VERIFY_VERBOSE)
-   write_in_full(1, buf, len);
-   return error("no signature found");
-   }
-
-   ret = check_signature(buf, len, buf + len, size - len, &sigc);
-   print_signature_buffer(&sigc, flags);
-
-   signature_check_clear(&sigc);
-   return ret;
-}
-
-static int verify_tag(const char *name, unsigned flags)
-{
-   enum object_type type;
-   unsigned char sha1[20];
-   char *buf;
-   unsigned long size;
-   int ret;
-
-   if (get_sha1(name, sha1))
-   return error("tag '%s' not found.", name);
-
-   type = sha1_object_info(sha1, NULL);
-   if (type != OBJ_TAG)
-   return error("%s: cannot verify a non-tag object of type %s.",
-   name, typename(type));
-
-   buf = read_sha1_file(sha1, &type, &size);
-   if (!buf)
-   return error("%s: unable to read file.", name);
-
-   ret = run_gpg_verify(buf, size, flags);
-
-   free(buf);
-   return ret;
-}
-
 static int git_verify_tag_config(const char *var, const char *value, void *cb)
 {
int status = git_gpg_config(var, value, cb);
@@ -96,7 +47,7 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
flags |= GPG_VERIFY_VERBOSE;
 
while (i < argc)
-   if (verify_tag(argv[i++], flags))
+   if (gpg_verify_tag(argv[i++], flags))
had_error = 1;
return had_error;
 }
diff --git a/tag.c b/tag.c
index d72f742..81e86e6 100644
--- a/tag.c
+++ b/tag.c
@@ -6,6 +6,55 @@
 
 const char *tag_type = "tag";
 
+static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
+{
+   struct signature_check sigc;
+   int len;
+   int ret;
+
+   memset(&sigc, 0, sizeof(sigc));
+
+   len = parse_signature(buf, size);
+
+   if (size == len) {
+   if (flags & GPG_VERIFY_VERBOSE)
+   write_in_full(1, buf, len);
+   return error("no signature found");
+   }
+
+   ret = check_signature(buf, len, buf + len, size - len, &sigc);
+   print_signature_buffer(&sigc, flags);
+
+   signature_check_clear(&sigc);
+   return ret;
+}
+
+int gpg_verify_tag(const char *name, unsigned flags)
+{
+   enum object_type type;
+   unsigned char sha1[20];
+   char *buf;
+   unsigned long size;
+   int ret;
+
+   if (get_sha1(name, sha1))
+   return error("tag '%s' not found.", name);
+
+   type = sha1_object_info(sha1, NULL);
+   if (type != OBJ_TAG)
+   return error("%s: cannot verify a non-tag object of type %s.",
+   name, typename(type));
+
+   buf = read_sha1_file(sha1, &type, &size);
+   if (!buf)
+   return error("%s: unable to read file.", name);
+
+   ret = run_gpg_verify(buf, size, flags);
+
+   free(buf);
+   return ret;
+}
+
 struct object *deref_tag(struct object *o, const char *warn, int warnlen)
 {
while (o && o->type == OBJ_TAG)
diff --git a/tag.h b/tag.h
index f4580ae..877f180 100644
--- a/tag.h
+++ b/tag.h
@@ -17,5 +17,6 @@ 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 char *name, unsigned flags);
 
 #endif /* TAG_H */
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 4/6] tag.c: Replace varialbe name for readability

2016-04-04 Thread santiago
From: Santiago Torres 

The run_gpg_verify function has two variables size, and len. This may
come off as confusing when reading the code. We clarify which one
pertains to the length of the tag headers by renaming len to
payload_length.

Signed-off-by: Santiago Torres 
---
Note: this used to be part of the previous patch on v3. I moved the
renaming part to a single patch to simplify the review. 

 tag.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/tag.c b/tag.c
index 81e86e6..f6443db 100644
--- a/tag.c
+++ b/tag.c
@@ -9,20 +9,21 @@ const char *tag_type = "tag";
 static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
 {
struct signature_check sigc;
-   int len;
+   int payload_length;
int ret;
 
memset(&sigc, 0, sizeof(sigc));
 
-   len = parse_signature(buf, size);
+   payload_length = parse_signature(buf, size);
 
-   if (size == len) {
+   if (size == payload_length) {
if (flags & GPG_VERIFY_VERBOSE)
-   write_in_full(1, buf, len);
+   write_in_full(1, buf, payload_length);
return error("no signature found");
}
 
-   ret = check_signature(buf, len, buf + len, size - len, &sigc);
+   ret = check_signature(buf, payload_length, buf + payload_length,
+   size - payload_length, &sigc);
print_signature_buffer(&sigc, flags);
 
signature_check_clear(&sigc);
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 6/6] tag.c: Change gpg_verify_tag argument to sha1

2016-04-04 Thread santiago
From: Santiago Torres 

The gpg_verify_tag function resolves the ref for any existing object.
However, git tag -v resolves to only tag-refs. We can provide support
for sha1 by moving the refname resolution code out of gpg_verify_tag and
allow for the object's sha1 as an argument.

Signed-off-by: Santiago Torres 
---
 builtin/tag.c|  2 +-
 builtin/verify-tag.c |  9 +++--
 tag.c| 12 
 tag.h|  2 +-
 4 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index f4450f8..398c892 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -104,7 +104,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(name, GPG_VERIFY_VERBOSE);
+   return gpg_verify_tag(sha1, GPG_VERIFY_VERBOSE);
 }
 
 static int do_sign(struct strbuf *buffer)
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 7e36d53..2ff01d8 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -30,6 +30,7 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
 {
int i = 1, verbose = 0, had_error = 0;
unsigned flags = 0;
+   unsigned char sha1[20];
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),
@@ -46,8 +47,12 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
if (verbose)
flags |= GPG_VERIFY_VERBOSE;
 
-   while (i < argc)
-   if (gpg_verify_tag(argv[i++], flags))
+   while (i < argc) {
+   if (get_sha1(argv[i++], sha1))
+   return error("tag '%s' not found.", argv[i]);
+
+   if (gpg_verify_tag(sha1, flags))
had_error = 1;
+   }
return had_error;
 }
diff --git a/tag.c b/tag.c
index f6443db..8ac9de5 100644
--- a/tag.c
+++ b/tag.c
@@ -30,25 +30,21 @@ static int run_gpg_verify(const char *buf, unsigned long 
size, unsigned flags)
return ret;
 }
 
-int gpg_verify_tag(const char *name, unsigned flags)
+int gpg_verify_tag(const unsigned char *sha1, unsigned flags)
 {
enum object_type type;
-   unsigned char sha1[20];
char *buf;
unsigned long size;
int ret;
 
-   if (get_sha1(name, sha1))
-   return error("tag '%s' not found.", name);
-
type = sha1_object_info(sha1, NULL);
if (type != OBJ_TAG)
-   return error("%s: cannot verify a non-tag object of type %s.",
-   name, typename(type));
+   return error("cannot verify a non-tag object of type %s.",
+   typename(type));
 
buf = read_sha1_file(sha1, &type, &size);
if (!buf)
-   return error("%s: unable to read file.", name);
+   return error("unable to read file.");
 
ret = run_gpg_verify(buf, size, flags);
 
diff --git a/tag.h b/tag.h
index 877f180..cb643b9 100644
--- a/tag.h
+++ b/tag.h
@@ -17,6 +17,6 @@ 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 char *name, unsigned flags);
+extern int gpg_verify_tag(const unsigned char *sha1, unsigned flags);
 
 #endif /* TAG_H */
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 0/6] tag: move PGP verification code to tag.c

2016-04-04 Thread santiago
This is a follow up of [1], [2], and [3]:

v4 (this):

Thanks Eric, Peff, and Hannes for the feedback.

 * I relocated the sigchain_push call so it comes after the error on
   gpg-interface (thanks Hannnes for catching this).
 * I updated the unit test to match the discussion on [3]. Now it generates
   the expected output of the tag on the fly for comparison. (This is just
   copy and paste from [3], but I verified that it works by breaking the
   while)
 * I split moving the code and renaming the variables into two patches so
   these are easier to review.
 * I used an adapter on builtin/tag.c instead of redefining all the fn*
   declarations everywhere. This introduces an issue with the way git tag -v
   resolves refnames though. I added a new commit to restore the previous
   behavior of git-tag. I'm not sure if I should've split this into two commits
   though.

v3:
Thanks Eric, Jeff, for the feedback.

 * I separated the patch in multiple sub-patches.
 * I compared the behavior of previous git tag -v and git verify-tag 
   invocations to make sure the behavior is the same
 * I dropped the multi-line comment, as suggested.
 * I fixed the issue with the missing brackets in the while (this is 
   now detected by the test).

v2:

 * I moved the pgp-verification code to tag.c 
 * I added extra arguments so git tag -v and git verify-tag both work
   with the same function
 * Relocated the SIGPIPE handling code in verify-tag to gpg-interface

v1:
 
The verify tag function is just a thin wrapper around the verify-tag
command. We can avoid one fork call by doing the verification inside
the tag builtin instead.


This applies on v2.8.0.

Thanks!
-Santiago

[1]
http://git.661346.n2.nabble.com/PATCH-RFC-builtin-tag-c-move-PGP-verification-inside-builtin-td7651529.html#a7651547
[2] 
http://git.661346.n2.nabble.com/PATCH-tag-c-move-PGP-verification-code-from-plumbing-td7651562.html
 
[3] 
http://git.661346.n2.nabble.com/PATCH-v3-0-4-tag-move-PGP-verification-code-to-tag-c-td7652334.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 5/6] tag: use pgp_verify_function in tag -v call

2016-04-04 Thread santiago
From: Santiago Torres 

Instead of running the verify-tag plumbing command, we use the
pgp_verify_tag(). This avoids the usage of an extra fork call. To do
this, we extend the number of parameters that tag.c takes, and
verify-tag passes. Redundant calls done in the pgp_verify_tag function
are removed.

Signed-off-by: Santiago Torres 
---
Note: This follows Peff's suggestion to use an adapter, instead of 
changing the the fn* pointer structure to match gpg_verify_tag.

 builtin/tag.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 1705c94..f4450f8 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -104,13 +104,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)
 {
-   const char *argv_verify_tag[] = {"verify-tag",
-   "-v", "SHA1_HEX", NULL};
-   argv_verify_tag[2] = sha1_to_hex(sha1);
-
-   if (run_command_v_opt(argv_verify_tag, RUN_GIT_CMD))
-   return error(_("could not verify the tag '%s'"), name);
-   return 0;
+   return gpg_verify_tag(name, GPG_VERIFY_VERBOSE);
 }
 
 static int do_sign(struct strbuf *buffer)
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 2/6] t/t7030-verify-tag.sh: Adds validation for multiple tags

2016-04-04 Thread santiago
From: Santiago Torres 

The verify-tag command supports mutliple tag names as an argument.
However, no previous tests try to verify multiple tags at once. This
test runs the verify-tag command against three tags separately and then
compares the result against the invocation with the same three tags as
an argument. The results shouldn't differ.

Signed-off-by: Santiago Torres 
---
Note: In this version we don't check or uniq for the verify output, but
instead compare against an invocation for each of the three tags indivdually.

 t/t7030-verify-tag.sh | 12 
 1 file changed, 12 insertions(+)

diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh
index 4608e71..f9161332 100755
--- a/t/t7030-verify-tag.sh
+++ b/t/t7030-verify-tag.sh
@@ -112,4 +112,16 @@ test_expect_success GPG 'verify signatures with --raw' '
)
 '
 
+test_expect_success GPG 'verify multiple tags' '
+   tags="fourth-signed sixth-signed seventh-signed" &&
+   for i in $tags; do
+   git verify-tag -v --raw $i || return 1
+   done >expect.stdout 2>expect.stderr.1 &&
+   grep GOODSIG expect.stderr &&
+   git verify-tag -v --raw $tags >actual.stdout 2>actual.stderr.1 &&
+   grep GOODSIG actual.stderr &&
+   test_cmp expect.stdout actual.stdout &&
+   test_cmp expect.stderr actual.stderr
+'
+
 test_done
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 1/6] builtin/verify-tag.c: Ignore SIGPIPE on gpg-interface

2016-04-04 Thread santiago
From: Santiago Torres 

The verify_signed_buffer comand might cause a SIGPIPE signal when the
gpg child process terminates early (due to a bad keyid, for example) and
git tries to write to it afterwards. Previously, ignoring SIGPIPE was
done on the builtin/verify-tag.c command to avoid this issue. However,
any other caller who wanted to use the verify_signed_buffer command
would have to include this signal call.

Instead, we use sigchain_push(SIGPIPE, SIG_IGN) on the
verify_signed_buffer call (pretty much like in sign_buffer()) so
that any caller is not required to perform this task. This will avoid
possible mistakes by further developers using verify_signed_buffer.

Signed-off-by: Santiago Torres 
---
Note: On this version i moved the sigchain_push below the return error
in the same way that sign_buffer does(). Thanks to Johanes Sixt for catching
this. 

 builtin/verify-tag.c | 3 ---
 gpg-interface.c  | 2 ++
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 00663f6..77f070a 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -95,9 +95,6 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
if (verbose)
flags |= GPG_VERIFY_VERBOSE;
 
-   /* sometimes the program was terminated because this signal
-* was received in the process of writing the gpg input: */
-   signal(SIGPIPE, SIG_IGN);
while (i < argc)
if (verify_tag(argv[i++], flags))
had_error = 1;
diff --git a/gpg-interface.c b/gpg-interface.c
index 3dc2fe3..2259938 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -237,6 +237,7 @@ int verify_signed_buffer(const char *payload, size_t 
payload_size,
return error(_("could not run gpg."));
}
 
+   sigchain_push(SIGPIPE, SIG_IGN);
write_in_full(gpg.in, payload, payload_size);
close(gpg.in);
 
@@ -250,6 +251,7 @@ int verify_signed_buffer(const char *payload, size_t 
payload_size,
close(gpg.out);
 
ret = finish_command(&gpg);
+   sigchain_pop(SIGPIPE);
 
unlink_or_warn(path);
 
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 6/6] tag: use gpg_verify_function in tag -v call

2016-04-05 Thread santiago
From: Santiago Torres 

Instead of running the verify-tag plumbing command, we use the
gpg_verify_tag() function within the verify_tag function to avoid doing
an additional fork call.

Signed-off-by: Santiago Torres 
---
 builtin/tag.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 1705c94..398c892 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -104,13 +104,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)
 {
-   const char *argv_verify_tag[] = {"verify-tag",
-   "-v", "SHA1_HEX", NULL};
-   argv_verify_tag[2] = sha1_to_hex(sha1);
-
-   if (run_command_v_opt(argv_verify_tag, RUN_GIT_CMD))
-   return error(_("could not verify the tag '%s'"), name);
-   return 0;
+   return gpg_verify_tag(sha1, GPG_VERIFY_VERBOSE);
 }
 
 static int do_sign(struct strbuf *buffer)
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 3/6] builtin/verify-tag: change variable name for readability

2016-04-05 Thread santiago
From: Santiago Torres 

The run_gpg_verify function has two variables size, and len. This may
come off as confusing when reading the code. We clarify which one
pertains to the length of the tag headers by renaming len to
payload_length.

Signed-off-by: Santiago Torres 
---
 builtin/verify-tag.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 77f070a..1ca9a05 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -21,20 +21,21 @@ static const char * const verify_tag_usage[] = {
 static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
 {
struct signature_check sigc;
-   int len;
+   int payload_size;
int ret;
 
memset(&sigc, 0, sizeof(sigc));
 
-   len = parse_signature(buf, size);
+   payload_size = parse_signature(buf, size);
 
-   if (size == len) {
+   if (size == payload_size) {
if (flags & GPG_VERIFY_VERBOSE)
-   write_in_full(1, buf, len);
+   write_in_full(1, buf, payload_size);
return error("no signature found");
}
 
-   ret = check_signature(buf, len, buf + len, size - len, &sigc);
+   ret = check_signature(buf, payload_size, buf + payload_size,
+   size - payload_size, &sigc);
print_signature_buffer(&sigc, flags);
 
signature_check_clear(&sigc);
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 4/6] builtin/verify-tag: replace name argument with sha1

2016-04-05 Thread santiago
From: Santiago Torres 

This change is meant to prepare verify_tag for libification. Many
existing modules/commands already do the refname to sha1 resolution, so
should avoid resolving the refname twice. To avoid breaking
builtin/verify-tag, we move the refname resolution outside of the
verify_tag() call.

Signed-off-by: Santiago Torres 
---
 builtin/verify-tag.c | 25 -
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 1ca9a05..7a7c376 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -42,25 +42,23 @@ static int run_gpg_verify(const char *buf, unsigned long 
size, unsigned flags)
return ret;
 }
 
-static int verify_tag(const char *name, unsigned flags)
+static int verify_tag(const unsigned char *sha1, unsigned flags)
 {
enum object_type type;
-   unsigned char sha1[20];
char *buf;
+   char *hex_sha1;
unsigned long size;
int ret;
 
-   if (get_sha1(name, sha1))
-   return error("tag '%s' not found.", name);
-
+   hex_sha1 = sha1_to_hex(sha1);
type = sha1_object_info(sha1, NULL);
if (type != OBJ_TAG)
return error("%s: cannot verify a non-tag object of type %s.",
-   name, typename(type));
+   hex_sha1, typename(type));
 
buf = read_sha1_file(sha1, &type, &size);
if (!buf)
-   return error("%s: unable to read file.", name);
+   return error("%s: unable to read file.", hex_sha1);
 
ret = run_gpg_verify(buf, size, flags);
 
@@ -80,6 +78,8 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
 {
int i = 1, verbose = 0, had_error = 0;
unsigned flags = 0;
+   unsigned char sha1[20];
+   const char *name;
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),
@@ -96,8 +96,15 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
if (verbose)
flags |= GPG_VERIFY_VERBOSE;
 
-   while (i < argc)
-   if (verify_tag(argv[i++], flags))
+   while (i < argc) {
+   name = argv[i++];
+   if (get_sha1(name, sha1)) {
+   error("tag '%s' not found.", name);
had_error = 1;
+   continue;
+   }
+   if (verify_tag(sha1, flags))
+   had_error = 1;
+   }
return had_error;
 }
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 1/6] builtin/verify-tag.c: Ignore SIGPIPE on gpg-interface

2016-04-05 Thread santiago
From: Santiago Torres 

The verify_signed_buffer comand might cause a SIGPIPE signal when the
gpg child process terminates early (due to a bad keyid, for example) and
git tries to write to it afterwards. Previously, ignoring SIGPIPE was
done on the builtin/verify-tag.c command to avoid this issue. However,
any other caller who wanted to use the verify_signed_buffer command
would have to include this signal call.

Instead, we use sigchain_push(SIGPIPE, SIG_IGN) on the
verify_signed_buffer call (pretty much like in sign_buffer()) so
that any caller is not required to perform this task. This will avoid
possible mistakes by further developers using verify_signed_buffer.

Signed-off-by: Santiago Torres 
---
 builtin/verify-tag.c | 3 ---
 gpg-interface.c  | 2 ++
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 00663f6..77f070a 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -95,9 +95,6 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
if (verbose)
flags |= GPG_VERIFY_VERBOSE;
 
-   /* sometimes the program was terminated because this signal
-* was received in the process of writing the gpg input: */
-   signal(SIGPIPE, SIG_IGN);
while (i < argc)
if (verify_tag(argv[i++], flags))
had_error = 1;
diff --git a/gpg-interface.c b/gpg-interface.c
index 3dc2fe3..2259938 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -237,6 +237,7 @@ int verify_signed_buffer(const char *payload, size_t 
payload_size,
return error(_("could not run gpg."));
}
 
+   sigchain_push(SIGPIPE, SIG_IGN);
write_in_full(gpg.in, payload, payload_size);
close(gpg.in);
 
@@ -250,6 +251,7 @@ int verify_signed_buffer(const char *payload, size_t 
payload_size,
close(gpg.out);
 
ret = finish_command(&gpg);
+   sigchain_pop(SIGPIPE);
 
unlink_or_warn(path);
 
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 2/6] t7030-verify-tag: Adds validation for multiple tags

2016-04-05 Thread santiago
From: Santiago Torres 

The verify-tag command supports multiple tag names as an argument.
However, existing tests only test for invocation with a single tag, so
we add a test invoking with multiple tags.

Helped-by: Jeff King 
Signed-off-by: Santiago Torres 
---
 t/t7030-verify-tag.sh | 12 
 1 file changed, 12 insertions(+)

diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh
index 4608e71..c01621a 100755
--- a/t/t7030-verify-tag.sh
+++ b/t/t7030-verify-tag.sh
@@ -112,4 +112,16 @@ test_expect_success GPG 'verify signatures with --raw' '
)
 '
 
+test_expect_success GPG 'verify multiple tags' '
+   tags="fourth-signed sixth-signed seventh-signed" &&
+   for i in $tags; do
+   git verify-tag -v --raw $i || return 1
+   done >expect.stdout 2>expect.stderr.1 &&
+   grep "^.GNUPG" expect.stderr &&
+   git verify-tag -v --raw $tags >actual.stdout 2>actual.stderr.1 &&
+   grep "^.GNUPG" actual.stderr &&
+   test_cmp expect.stdout actual.stdout &&
+   test_cmp expect.stderr actual.stderr
+'
+
 test_done
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 5/6] builtin/verify-tag: move verification code to tag.c

2016-04-05 Thread santiago
From: Santiago Torres 

The PGP verification routine for tags could be accessed by other
commands that require it. We do this by moving it to the common tag.c
module. We rename the verify_tag() function to gpg_verify_tag() to avoid
conflicts with the mktag.c function.

Signed-off-by: Santiago Torres 
---
 builtin/verify-tag.c | 50 +-
 tag.c| 48 
 tag.h|  1 +
 3 files changed, 50 insertions(+), 49 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 7a7c376..e9a2005 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -18,54 +18,6 @@ static const char * const verify_tag_usage[] = {
NULL
 };
 
-static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
-{
-   struct signature_check sigc;
-   int payload_size;
-   int ret;
-
-   memset(&sigc, 0, sizeof(sigc));
-
-   payload_size = parse_signature(buf, size);
-
-   if (size == payload_size) {
-   if (flags & GPG_VERIFY_VERBOSE)
-   write_in_full(1, buf, payload_size);
-   return error("no signature found");
-   }
-
-   ret = check_signature(buf, payload_size, buf + payload_size,
-   size - payload_size, &sigc);
-   print_signature_buffer(&sigc, flags);
-
-   signature_check_clear(&sigc);
-   return ret;
-}
-
-static int verify_tag(const unsigned char *sha1, unsigned flags)
-{
-   enum object_type type;
-   char *buf;
-   char *hex_sha1;
-   unsigned long size;
-   int ret;
-
-   hex_sha1 = sha1_to_hex(sha1);
-   type = sha1_object_info(sha1, NULL);
-   if (type != OBJ_TAG)
-   return error("%s: cannot verify a non-tag object of type %s.",
-   hex_sha1, typename(type));
-
-   buf = read_sha1_file(sha1, &type, &size);
-   if (!buf)
-   return error("%s: unable to read file.", hex_sha1);
-
-   ret = run_gpg_verify(buf, size, flags);
-
-   free(buf);
-   return ret;
-}
-
 static int git_verify_tag_config(const char *var, const char *value, void *cb)
 {
int status = git_gpg_config(var, value, cb);
@@ -103,7 +55,7 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
had_error = 1;
continue;
}
-   if (verify_tag(sha1, flags))
+   if (gpg_verify_tag(sha1, flags))
had_error = 1;
}
return had_error;
diff --git a/tag.c b/tag.c
index d72f742..3f7669f 100644
--- a/tag.c
+++ b/tag.c
@@ -6,6 +6,54 @@
 
 const char *tag_type = "tag";
 
+static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
+{
+   struct signature_check sigc;
+   int payload_size;
+   int ret;
+
+   memset(&sigc, 0, sizeof(sigc));
+
+   payload_size = parse_signature(buf, size);
+
+   if (size == payload_size) {
+   if (flags & GPG_VERIFY_VERBOSE)
+   write_in_full(1, buf, payload_size);
+   return error("no signature found");
+   }
+
+   ret = check_signature(buf, payload_size, buf + payload_size,
+   size - payload_size, &sigc);
+   print_signature_buffer(&sigc, flags);
+
+   signature_check_clear(&sigc);
+   return ret;
+}
+
+int gpg_verify_tag(const unsigned char *sha1, unsigned flags)
+{
+   enum object_type type;
+   char *buf;
+   char *hex_sha1;
+   unsigned long size;
+   int ret;
+
+   hex_sha1 = sha1_to_hex(sha1);
+   type = sha1_object_info(sha1, NULL);
+   if (type != OBJ_TAG)
+   return error("%s: cannot verify a non-tag object of type %s.",
+   hex_sha1, typename(type));
+
+   buf = read_sha1_file(sha1, &type, &size);
+   if (!buf)
+   return error("%s: unable to read file.", hex_sha1);
+
+   ret = run_gpg_verify(buf, size, flags);
+
+   free(buf);
+   return ret;
+}
+
 struct object *deref_tag(struct object *o, const char *warn, int warnlen)
 {
while (o && o->type == OBJ_TAG)
diff --git a/tag.h b/tag.h
index f4580ae..cb643b9 100644
--- a/tag.h
+++ b/tag.h
@@ -17,5 +17,6 @@ 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, unsigned flags);
 
 #endif /* TAG_H */
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 0/6] tag: move PGP verification code to tag.c

2016-04-05 Thread santiago
From: Santiago Torres 

v5 (this): 
Added helpful feedback by Eric

 * Reordering of the patches, to avoid temporal inclusion of a regression
 * Fix typos here and there.
 * Review commit messages, as some weren't representative of what the patches
   were doing anymore.
 * Updated t7030 to include Peff's suggestion, and added a helped-by line here
   as it was mostly Peff's code.
 * Updated the error-handling/printing issues that were introduced when.
   libifying the verify_tag function.
   
v4:

Thanks Eric, Jeff, and Hannes for the feedback.

 * I relocated the sigchain_push call so it comes after the error on
   gpg-interface (thanks Hannnes for catching this).
 * I updated the unit test to match the discussion on [3]. Now it generates
   the expected output of the tag on the fly for comparison. (This is just
   copy and paste from [3], but I verified that it works by breaking the
   while)
 * I split moving the code and renaming the variables into two patches so
   these are easier to review.
 * I used an adapter on builtin/tag.c instead of redefining all the fn*
   declarations everywhere. This introduces an issue with the way git tag -v
   resolves refnames though. I added a new commit to restore the previous
   behavior of git-tag. I'm not sure if I should've split this into two commits
   though.

v3:
Thanks Eric, Jeff, for the feedback.

 * I separated the patch in multiple sub-patches.
 * I compared the behavior of previous git tag -v and git verify-tag 
   invocations to make sure the behavior is the same
 * I dropped the multi-line comment, as suggested.
 * I fixed the issue with the missing brackets in the while (this is 
   now detected by the test).

v2:

 * I moved the pgp-verification code to tag.c 
 * I added extra arguments so git tag -v and git verify-tag both work
   with the same function
 * Relocated the SIGPIPE handling code in verify-tag to gpg-interface

v1:
 
The verify tag function is just a thin wrapper around the verify-tag
command. We can avoid one fork call by doing the verification inside
the tag builtin instead.


This applies on v2.8.0.

Thanks!
-Santiago


Santiago Torres (6):
  builtin/verify-tag.c: Ignore SIGPIPE on gpg-interface
  t7030-verify-tag: Adds validation for multiple tags
  builtin/verify-tag: change variable name for readability
  builtin/verify-tag: replace name argument with sha1
  builtin/verify-tag: move verification code to tag.c
  tag: use gpg_verify_function in tag -v call

 builtin/tag.c |  8 +--
 builtin/verify-tag.c  | 65 +--
 gpg-interface.c   |  2 ++
 t/t7030-verify-tag.sh | 12 ++
 tag.c | 48 +
 tag.h |  1 +
 6 files changed, 75 insertions(+), 61 deletions(-)

-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 0/6] Move PGP verification out of verify-tag

2016-04-17 Thread santiago
From: Santiago Torres 

This is a follow up of [1], [2], [3], [4], and [5]. patches 1/6, 2/6 and 3/6
are the same as the corresponding commits in pu.

v6: 
 * As Junio suggested, updated 4/6, to include the name argument and the
   ternary operator to provide more descriptive error messages. I propagated
   these changes to 5/6 and 6/6 as well. I'm unsure about the 80-column
   on 4/6, the ternary operator is rather long.
 * Updated and reviewed the commit messages based on Eric and Junio's
   feedback

v5:
Added helpful feedback by Eric

 * Reordering of the patches, to avoid temporal inclusion of a regression
 * Fix typos here and there.
 * Review commit messages, as some weren't representative of what the patches
   were doing anymore.
 * Updated t7030 to include Peff's suggestion, and added a helped-by line here
   as it was mostly Peff's code.
 * Updated the error-handling/printing issues that were introduced when.
   libifying the verify_tag function.

v4:

Thanks Eric, Jeff, and Hannes for the feedback.

 * I relocated the sigchain_push call so it comes after the error on
   gpg-interface (thanks Hannnes for catching this).
 * I updated the unit test to match the discussion on [3]. Now it generates
   the expected output of the tag on the fly for comparison. (This is just
   copy and paste from [3], but I verified that it works by breaking the
   while)
 * I split moving the code and renaming the variables into two patches so
   these are easier to review.
 * I used an adapter on builtin/tag.c instead of redefining all the fn*
   declarations everywhere. This introduces an issue with the way git tag -v
   resolves refnames though. I added a new commit to restore the previous
   behavior of git-tag. I'm not sure if I should've split this into two commits
   though.

v3:
Thanks Eric, Jeff, for the feedback.

 * I separated the patch in multiple sub-patches.
 * I compared the behavior of previous git tag -v and git verify-tag 
   invocations to make sure the behavior is the same
 * I dropped the multi-line comment, as suggested.
 * I fixed the issue with the missing brackets in the while (this is 
   now detected by the test).

v2:

 * I moved the pgp-verification code to tag.c 
 * I added extra arguments so git tag -v and git verify-tag both work
   with the same function
 * Relocated the SIGPIPE handling code in verify-tag to gpg-interface

v1:
 
The verify tag function is just a thin wrapper around the verify-tag
command. We can avoid one fork call by doing the verification inside
the tag builtin instead.


This applies on v2.8.0. 
Thanks!
-Santiago

[1] http://thread.gmane.org/gmane.comp.version-control.git/287649
[2] http://thread.gmane.org/gmane.comp.version-control.git/289836
[3] http://thread.gmane.org/gmane.comp.version-control.git/290608
[4] http://thread.gmane.org/gmane.comp.version-control.git/290731
[5] http://thread.gmane.org/gmane.comp.version-control.git/290790

Santiago Torres (6):
  builtin/verify-tag.c: ignore SIGPIPE in gpg-interface
  t7030: test verifying multiple tags
  verify-tag: change variable name for readability
  verify-tag: add sha1 argument to verify_tag()
  verify-tag: move verification code to tag.c
  tag -v: verfy directly rather than exec-ing verify-tag

 builtin/tag.c |  8 +--
 builtin/verify-tag.c  | 64 ---
 gpg-interface.c   |  2 ++
 t/t7030-verify-tag.sh | 13 +++
 tag.c | 49 +++
 tag.h |  3 ++-
 6 files changed, 77 insertions(+), 62 deletions(-)

-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 1/6] builtin/verify-tag.c: ignore SIGPIPE in gpg-interface

2016-04-17 Thread santiago
From: Santiago Torres 

The verify_signed_buffer() function may trigger a SIGPIPE when the
GPG child process terminates early (due to a bad keyid, for example)
and Git tries to write to it afterwards.  Previously, ignoring
SIGPIPE was done in builtin/verify-tag.c to avoid this issue.

However, any other caller who wants to call verify_signed_buffer()
would have to do the same.

Use sigchain_push(SIGPIPE, SIG_IGN) in verify_signed_buffer(),
pretty much like in sign_buffer(), so that any caller is not
required to perform this task.

This will avoid possible mistakes by further developers using
verify_signed_buffer().

Signed-off-by: Santiago Torres 
Reviewed-by: Eric Sunshine 
Signed-off-by: Junio C Hamano 
---
 builtin/verify-tag.c | 3 ---
 gpg-interface.c  | 2 ++
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 00663f6..77f070a 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -95,9 +95,6 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
if (verbose)
flags |= GPG_VERIFY_VERBOSE;
 
-   /* sometimes the program was terminated because this signal
-* was received in the process of writing the gpg input: */
-   signal(SIGPIPE, SIG_IGN);
while (i < argc)
if (verify_tag(argv[i++], flags))
had_error = 1;
diff --git a/gpg-interface.c b/gpg-interface.c
index 3dc2fe3..2259938 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -237,6 +237,7 @@ int verify_signed_buffer(const char *payload, size_t 
payload_size,
return error(_("could not run gpg."));
}
 
+   sigchain_push(SIGPIPE, SIG_IGN);
write_in_full(gpg.in, payload, payload_size);
close(gpg.in);
 
@@ -250,6 +251,7 @@ int verify_signed_buffer(const char *payload, size_t 
payload_size,
close(gpg.out);
 
ret = finish_command(&gpg);
+   sigchain_pop(SIGPIPE);
 
unlink_or_warn(path);
 
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 3/6] verify-tag: change variable name for readability

2016-04-17 Thread santiago
From: Santiago Torres 

The run_gpg_verify() function has two variables, size and len.

This may come off as confusing when reading the code.  Clarify which
one pertains to the length of the tag headers by renaming len to
payload_length.

Signed-off-by: Santiago Torres 
Reviewed-by: Eric Sunshine 
Signed-off-by: Junio C Hamano 
---
 builtin/verify-tag.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 77f070a..010353c 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -21,20 +21,21 @@ static const char * const verify_tag_usage[] = {
 static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
 {
struct signature_check sigc;
-   int len;
+   int payload_size;
int ret;
 
memset(&sigc, 0, sizeof(sigc));
 
-   len = parse_signature(buf, size);
+   payload_size = parse_signature(buf, size);
 
-   if (size == len) {
+   if (size == payload_size) {
if (flags & GPG_VERIFY_VERBOSE)
-   write_in_full(1, buf, len);
+   write_in_full(1, buf, payload_size);
return error("no signature found");
}
 
-   ret = check_signature(buf, len, buf + len, size - len, &sigc);
+   ret = check_signature(buf, payload_size, buf + payload_size,
+   size - payload_size, &sigc);
print_signature_buffer(&sigc, flags);
 
signature_check_clear(&sigc);
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 5/6] verify-tag: move verification code to tag.c

2016-04-17 Thread santiago
From: Santiago Torres 

The PGP verification routine for tags could be accessed by other modules
that require to do so.

Publish the verify_tag function in tag.c and rename it to gpg_verify_tag
so it does not conflict with builtin/mktag's static function.

Helped-by: Junio C Hamano 
Signed-off-by: Santiago Torres 
---
 builtin/verify-tag.c | 51 +--
 tag.c| 49 +
 tag.h|  3 ++-
 3 files changed, 52 insertions(+), 51 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 1d1c5c2..4e3b643 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -18,55 +18,6 @@ static const char * const verify_tag_usage[] = {
NULL
 };
 
-static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
-{
-   struct signature_check sigc;
-   int payload_size;
-   int ret;
-
-   memset(&sigc, 0, sizeof(sigc));
-
-   payload_size = parse_signature(buf, size);
-
-   if (size == payload_size) {
-   if (flags & GPG_VERIFY_VERBOSE)
-   write_in_full(1, buf, payload_size);
-   return error("no signature found");
-   }
-
-   ret = check_signature(buf, payload_size, buf + payload_size,
-   size - payload_size, &sigc);
-   print_signature_buffer(&sigc, flags);
-
-   signature_check_clear(&sigc);
-   return ret;
-}
-
-static int verify_tag(const unsigned char *sha1, const char *report_name,
-   unsigned flags)
-{
-   enum object_type type;
-   char *buf;
-   unsigned long size;
-   int ret;
-
-   type = sha1_object_info(sha1, NULL);
-   if (type != OBJ_TAG)
-   return error("%s: cannot verify a non-tag object of type %s.",
-   report_name ? report_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.",
-   report_name ? report_name : 
find_unique_abbrev(sha1, DEFAULT_ABBREV));
-
-   ret = run_gpg_verify(buf, size, flags);
-
-   free(buf);
-   return ret;
-}
-
 static int git_verify_tag_config(const char *var, const char *value, void *cb)
 {
int status = git_gpg_config(var, value, cb);
@@ -103,7 +54,7 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
error("tag '%s' not found.", name);
had_error = 1;
}
-   else if (verify_tag(sha1, name, flags))
+   else if (gpg_verify_tag(sha1, name, flags))
had_error = 1;
}
return had_error;
diff --git a/tag.c b/tag.c
index d72f742..e7f22c6 100644
--- a/tag.c
+++ b/tag.c
@@ -6,6 +6,55 @@
 
 const char *tag_type = "tag";
 
+static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
+{
+   struct signature_check sigc;
+   int payload_size;
+   int ret;
+
+   memset(&sigc, 0, sizeof(sigc));
+
+   payload_size = parse_signature(buf, size);
+
+   if (size == payload_size) {
+   if (flags & GPG_VERIFY_VERBOSE)
+   write_in_full(1, buf, payload_size);
+   return error("no signature found");
+   }
+
+   ret = check_signature(buf, payload_size, buf + payload_size,
+   size - payload_size, &sigc);
+   print_signature_buffer(&sigc, flags);
+
+   signature_check_clear(&sigc);
+   return ret;
+}
+
+int gpg_verify_tag(const unsigned char *sha1, const char *report_name,
+   unsigned flags)
+{
+   enum object_type type;
+   char *buf;
+   unsigned long size;
+   int ret;
+
+   type = sha1_object_info(sha1, NULL);
+   if (type != OBJ_TAG)
+   return error("%s: cannot verify a non-tag object of type %s.",
+   report_name ? report_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.",
+   report_name ? report_name : 
find_unique_abbrev(sha1, DEFAULT_ABBREV));
+
+   ret = run_gpg_verify(buf, size, flags);
+
+   free(buf);
+   return ret;
+}
+
 struct object *deref_tag(struct object *o, const char *warn, int warnlen)
 {
while (o && o->type == OBJ_TAG)
diff --git a/tag.h b/tag.h
index f4580ae..1a8d123 100644
--- a/tag.h
+++ b/tag.h
@@ -17,5 +17,6 @@ extern int parse_tag_buffer(struct tag *item,

[PATCH v6 2/6] t7030: test verifying multiple tags

2016-04-17 Thread santiago
From: Santiago Torres 

The verify-tag command supports multiple tag names to verify, but
existing tests only test for invocation with a single tag.

Add a test invoking it with multiple tags.

Helped-by: Jeff King 
Signed-off-by: Santiago Torres 
Reviewed-by: Eric Sunshine 
Signed-off-by: Junio C Hamano 
---
 t/t7030-verify-tag.sh | 13 +
 1 file changed, 13 insertions(+)

diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh
index 4608e71..07079a4 100755
--- a/t/t7030-verify-tag.sh
+++ b/t/t7030-verify-tag.sh
@@ -112,4 +112,17 @@ test_expect_success GPG 'verify signatures with --raw' '
)
 '
 
+test_expect_success GPG 'verify multiple tags' '
+   tags="fourth-signed sixth-signed seventh-signed" &&
+   for i in $tags
+   do
+   git verify-tag -v --raw $i || return 1
+   done >expect.stdout 2>expect.stderr.1 &&
+   grep "^.GNUPG:." expect.stderr &&
+   git verify-tag -v --raw $tags >actual.stdout 2>actual.stderr.1 &&
+   grep "^.GNUPG:." actual.stderr &&
+   test_cmp expect.stdout actual.stdout &&
+   test_cmp expect.stderr actual.stderr
+'
+
 test_done
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 4/6] verify-tag: add sha1 argument to verify_tag()

2016-04-17 Thread santiago
From: Santiago Torres 

The current interface of verify_tag() resolves reference names to SHA1,
which might be redundant as future callers may resolve the refname to
SHA1 beforehand.

Add a SHA1 parameter to use instead of the name parameter. We also
replace the name argument to report_name and use it for error reporting
only.

Helped-by: Junio C Hamano 
Signed-off-by: Santiago Torres 
---
 builtin/verify-tag.c | 25 -
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 010353c..1d1c5c2 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -42,25 +42,24 @@ static int run_gpg_verify(const char *buf, unsigned long 
size, unsigned flags)
return ret;
 }
 
-static int verify_tag(const char *name, unsigned flags)
+static int verify_tag(const unsigned char *sha1, const char *report_name,
+   unsigned flags)
 {
enum object_type type;
-   unsigned char sha1[20];
char *buf;
unsigned long size;
int ret;
 
-   if (get_sha1(name, sha1))
-   return error("tag '%s' not found.", name);
-
type = sha1_object_info(sha1, NULL);
if (type != OBJ_TAG)
return error("%s: cannot verify a non-tag object of type %s.",
-   name, typename(type));
+   report_name ? report_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);
+   return error("%s: unable to read file.",
+   report_name ? report_name : 
find_unique_abbrev(sha1, DEFAULT_ABBREV));
 
ret = run_gpg_verify(buf, size, flags);
 
@@ -80,6 +79,8 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
 {
int i = 1, verbose = 0, had_error = 0;
unsigned flags = 0;
+   unsigned char sha1[20];
+   const char *name;
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),
@@ -96,8 +97,14 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
if (verbose)
flags |= GPG_VERIFY_VERBOSE;
 
-   while (i < argc)
-   if (verify_tag(argv[i++], flags))
+   while (i < argc) {
+   name = argv[i++];
+   if (get_sha1(name, sha1)) {
+   error("tag '%s' not found.", name);
had_error = 1;
+   }
+   else if (verify_tag(sha1, name, flags))
+   had_error = 1;
+   }
return had_error;
 }
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 6/6] tag -v: verfy directly rather than exec-ing verify-tag

2016-04-17 Thread santiago
From: Santiago Torres 

tag -v forks into verify-tag, which only calls gpg_verify_tag().

Instead of forking to verify-tag, call gpg_verify_tag directly().

Helped-by: Eric Sunshine 
Signed-off-by: Santiago Torres 
---
 builtin/tag.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 1705c94..7b2918e 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -104,13 +104,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)
 {
-   const char *argv_verify_tag[] = {"verify-tag",
-   "-v", "SHA1_HEX", NULL};
-   argv_verify_tag[2] = sha1_to_hex(sha1);
-
-   if (run_command_v_opt(argv_verify_tag, RUN_GIT_CMD))
-   return error(_("could not verify the tag '%s'"), name);
-   return 0;
+   return gpg_verify_tag(sha1, name, GPG_VERIFY_VERBOSE);
 }
 
 static int do_sign(struct strbuf *buffer)
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v7 0/6] Move PGP verification out of verify-tag

2016-04-19 Thread santiago
From: Santiago Torres 

This is a follow up of [1], [2], [3], [4], [5], [6]. patches 1/6, 2/6, are the
same as the corresponding commits in pu.

v7: 
Mostly style/clarity changes mostly. Thanks Peff, Eric and Junio for the
feedback! In summary: 

 * Eric pointed out issues with 3/6's commit message. It doesn't match the one 
   in pu though. I also took the opportunity to update payload_size to a size_t
   as Peff suggested.
 * 4/6 I updated report_name to name_to_report, I updated the commit message 
   and addressed some nits in the code, one of the fixes removed all three nits
   that Eric pointed out. I updated 5/6 to match these changes
 * I gave the commit message on 6/6 another go.

v6: 
 * As Junio suggested, updated 4/6, to include the name argument and the
   ternary operator to provide more descriptive error messages. I propagated
   these changes to 5/6 and 6/6 as well. I'm unsure about the 80-column
   on 4/6, the ternary operator is rather long.
 * Updated and reviewed the commit messages based on Eric and Junio's
   feedback

v5:
Added helpful feedback by Eric

 * Reordering of the patches, to avoid temporal inclusion of a regression
 * Fix typos here and there.
 * Review commit messages, as some weren't representative of what the patches
   were doing anymore.
 * Updated t7030 to include Peff's suggestion, and added a helped-by line here
   as it was mostly Peff's code.
 * Updated the error-handling/printing issues that were introduced when.
   libifying the verify_tag function.

v4:

Thanks Eric, Jeff, and Hannes for the feedback.

 * I relocated the sigchain_push call so it comes after the error on
   gpg-interface (thanks Hannnes for catching this).
 * I updated the unit test to match the discussion on [3]. Now it generates
   the expected output of the tag on the fly for comparison. (This is just
   copy and paste from [3], but I verified that it works by breaking the
   while)
 * I split moving the code and renaming the variables into two patches so
   these are easier to review.
 * I used an adapter on builtin/tag.c instead of redefining all the fn*
   declarations everywhere. This introduces an issue with the way git tag -v
   resolves refnames though. I added a new commit to restore the previous
   behavior of git-tag. I'm not sure if I should've split this into two commits
   though.

v3:
Thanks Eric, Jeff, for the feedback.

 * I separated the patch in multiple sub-patches.
 * I compared the behavior of previous git tag -v and git verify-tag 
   invocations to make sure the behavior is the same
 * I dropped the multi-line comment, as suggested.
 * I fixed the issue with the missing brackets in the while (this is 
   now detected by the test).

v2:

 * I moved the pgp-verification code to tag.c 
 * I added extra arguments so git tag -v and git verify-tag both work
   with the same function
 * Relocated the SIGPIPE handling code in verify-tag to gpg-interface

v1:
 
The verify tag function is just a thin wrapper around the verify-tag
command. We can avoid one fork call by doing the verification inside
the tag builtin instead.


This applies on v2.8.0. 
Thanks!
-Santiago

[1] http://thread.gmane.org/gmane.comp.version-control.git/287649
[2] http://thread.gmane.org/gmane.comp.version-control.git/289836
[3] http://thread.gmane.org/gmane.comp.version-control.git/290608
[4] http://thread.gmane.org/gmane.comp.version-control.git/290731
[5] http://thread.gmane.org/gmane.comp.version-control.git/290790
[6] http://thread.gmane.org/gmane.comp.version-control.git/291780

Santiago Torres (6):
  builtin/verify-tag.c: ignore SIGPIPE in gpg-interface
  t7030: test verifying multiple tags
  verify-tag: Update run_gpg_verfy len variable name and type
  verify-tag: prepare verify_tag for libification
  verify-tag: move tag verification code to tag.c
  tag -v: verfy directly rather than exec-ing verify-tag

 builtin/tag.c |  8 +--
 builtin/verify-tag.c  | 62 +++
 gpg-interface.c   |  2 ++
 t/t7030-verify-tag.sh | 13 +++
 tag.c | 53 +++
 tag.h |  2 ++
 6 files changed, 79 insertions(+), 61 deletions(-)

-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v7 2/6] t7030: test verifying multiple tags

2016-04-19 Thread santiago
From: Santiago Torres 

The verify-tag command supports multiple tag names to verify, but
existing tests only test for invocation with a single tag.

Add a test invoking it with multiple tags.

Helped-by: Jeff King 
Signed-off-by: Santiago Torres 
Reviewed-by: Eric Sunshine 
Signed-off-by: Junio C Hamano 
---
 t/t7030-verify-tag.sh | 13 +
 1 file changed, 13 insertions(+)

diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh
index 4608e71..07079a4 100755
--- a/t/t7030-verify-tag.sh
+++ b/t/t7030-verify-tag.sh
@@ -112,4 +112,17 @@ test_expect_success GPG 'verify signatures with --raw' '
)
 '
 
+test_expect_success GPG 'verify multiple tags' '
+   tags="fourth-signed sixth-signed seventh-signed" &&
+   for i in $tags
+   do
+   git verify-tag -v --raw $i || return 1
+   done >expect.stdout 2>expect.stderr.1 &&
+   grep "^.GNUPG:." expect.stderr &&
+   git verify-tag -v --raw $tags >actual.stdout 2>actual.stderr.1 &&
+   grep "^.GNUPG:." actual.stderr &&
+   test_cmp expect.stdout actual.stdout &&
+   test_cmp expect.stderr actual.stderr
+'
+
 test_done
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v7 1/6] builtin/verify-tag.c: ignore SIGPIPE in gpg-interface

2016-04-19 Thread santiago
From: Santiago Torres 

The verify_signed_buffer() function may trigger a SIGPIPE when the
GPG child process terminates early (due to a bad keyid, for example)
and Git tries to write to it afterwards.  Previously, ignoring
SIGPIPE was done in builtin/verify-tag.c to avoid this issue.

However, any other caller who wants to call verify_signed_buffer()
would have to do the same.

Use sigchain_push(SIGPIPE, SIG_IGN) in verify_signed_buffer(),
pretty much like in sign_buffer(), so that any caller is not
required to perform this task.

This will avoid possible mistakes by further developers using
verify_signed_buffer().

Signed-off-by: Santiago Torres 
Reviewed-by: Eric Sunshine 
Signed-off-by: Junio C Hamano 
---
 builtin/verify-tag.c | 3 ---
 gpg-interface.c  | 2 ++
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 00663f6..77f070a 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -95,9 +95,6 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
if (verbose)
flags |= GPG_VERIFY_VERBOSE;
 
-   /* sometimes the program was terminated because this signal
-* was received in the process of writing the gpg input: */
-   signal(SIGPIPE, SIG_IGN);
while (i < argc)
if (verify_tag(argv[i++], flags))
had_error = 1;
diff --git a/gpg-interface.c b/gpg-interface.c
index 3dc2fe3..2259938 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -237,6 +237,7 @@ int verify_signed_buffer(const char *payload, size_t 
payload_size,
return error(_("could not run gpg."));
}
 
+   sigchain_push(SIGPIPE, SIG_IGN);
write_in_full(gpg.in, payload, payload_size);
close(gpg.in);
 
@@ -250,6 +251,7 @@ int verify_signed_buffer(const char *payload, size_t 
payload_size,
close(gpg.out);
 
ret = finish_command(&gpg);
+   sigchain_pop(SIGPIPE);
 
unlink_or_warn(path);
 
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v7 4/6] verify-tag: prepare verify_tag for libification

2016-04-19 Thread santiago
From: Santiago Torres 

The current interface of verify_tag() resolves reference names to SHA1,
however, the plan is to make this functionality public and the current
interface is cumbersome for callers: they are expected to supply the
textual representation of a sha1/refname. In many cases, this requires
them to turn the sha1 to hex representation, just to be converted back
inside verify_tag.

Add a SHA1 parameter to use instead of the name parameter, and rename
the name parameter to "name_to_report" for reporting purposes only.

Helped-by: Junio C Hamano 
Signed-off-by: Santiago Torres 
---
 builtin/verify-tag.c | 27 ++-
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index fa26e40..01e956f 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -42,25 +42,28 @@ static int run_gpg_verify(const char *buf, unsigned long 
size, unsigned flags)
return ret;
 }
 
-static int verify_tag(const char *name, unsigned flags)
+static int verify_tag(const unsigned char *sha1, const char *name_to_report,
+   unsigned flags)
 {
enum object_type type;
-   unsigned char sha1[20];
char *buf;
unsigned long size;
int ret;
 
-   if (get_sha1(name, sha1))
-   return error("tag '%s' not found.", name);
-
type = sha1_object_info(sha1, NULL);
if (type != OBJ_TAG)
return error("%s: cannot verify a non-tag object of type %s.",
-   name, typename(type));
+   name_to_report ?
+   name_to_report :
+   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);
+   return error("%s: unable to read file.",
+   name_to_report ?
+   name_to_report :
+   find_unique_abbrev(sha1, DEFAULT_ABBREV));
 
ret = run_gpg_verify(buf, size, flags);
 
@@ -80,6 +83,8 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
 {
int i = 1, verbose = 0, had_error = 0;
unsigned flags = 0;
+   unsigned char sha1[20];
+   const char *name;
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),
@@ -96,8 +101,12 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
if (verbose)
flags |= GPG_VERIFY_VERBOSE;
 
-   while (i < argc)
-   if (verify_tag(argv[i++], flags))
+   while (i < argc) {
+   name = argv[i++];
+   if (get_sha1(name, sha1))
+   had_error = !!error("tag '%s' not found.", name);
+   else if (verify_tag(sha1, name, flags))
had_error = 1;
+   }
return had_error;
 }
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v7 5/6] verify-tag: move tag verification code to tag.c

2016-04-19 Thread santiago
From: Santiago Torres 

The PGP verification routine for tags could be accessed by other modules
that require to do so.

Publish the verify_tag function in tag.c and rename it to gpg_verify_tag
so it does not conflict with builtin/mktag's static function.

Helped-by: Junio C Hamano 
Signed-off-by: Santiago Torres 
---
 builtin/verify-tag.c | 55 +---
 tag.c| 53 ++
 tag.h|  2 ++
 3 files changed, 56 insertions(+), 54 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 01e956f..714c899 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -18,59 +18,6 @@ static const char * const verify_tag_usage[] = {
NULL
 };
 
-static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
-{
-   struct signature_check sigc;
-   size_t payload_size;
-   int ret;
-
-   memset(&sigc, 0, sizeof(sigc));
-
-   payload_size = parse_signature(buf, size);
-
-   if (size == payload_size) {
-   if (flags & GPG_VERIFY_VERBOSE)
-   write_in_full(1, buf, payload_size);
-   return error("no signature found");
-   }
-
-   ret = check_signature(buf, payload_size, buf + payload_size,
-   size - payload_size, &sigc);
-   print_signature_buffer(&sigc, flags);
-
-   signature_check_clear(&sigc);
-   return ret;
-}
-
-static int verify_tag(const unsigned char *sha1, const char *name_to_report,
-   unsigned flags)
-{
-   enum object_type type;
-   char *buf;
-   unsigned long size;
-   int ret;
-
-   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 :
-   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 :
-   find_unique_abbrev(sha1, DEFAULT_ABBREV));
-
-   ret = run_gpg_verify(buf, size, flags);
-
-   free(buf);
-   return ret;
-}
-
 static int git_verify_tag_config(const char *var, const char *value, void *cb)
 {
int status = git_gpg_config(var, value, cb);
@@ -105,7 +52,7 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
name = argv[i++];
if (get_sha1(name, sha1))
had_error = !!error("tag '%s' not found.", name);
-   else if (verify_tag(sha1, name, flags))
+   else if (gpg_verify_tag(sha1, name, flags))
had_error = 1;
}
return had_error;
diff --git a/tag.c b/tag.c
index d72f742..ace619a 100644
--- a/tag.c
+++ b/tag.c
@@ -6,6 +6,59 @@
 
 const char *tag_type = "tag";
 
+static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
+{
+   struct signature_check sigc;
+   size_t payload_size;
+   int ret;
+
+   memset(&sigc, 0, sizeof(sigc));
+
+   payload_size = parse_signature(buf, size);
+
+   if (size == payload_size) {
+   if (flags & GPG_VERIFY_VERBOSE)
+   write_in_full(1, buf, payload_size);
+   return error("no signature found");
+   }
+
+   ret = check_signature(buf, payload_size, buf + payload_size,
+   size - payload_size, &sigc);
+   print_signature_buffer(&sigc, flags);
+
+   signature_check_clear(&sigc);
+   return ret;
+}
+
+extern int gpg_verify_tag(const unsigned char *sha1,
+   const char *name_to_report, unsigned flags)
+{
+   enum object_type type;
+   char *buf;
+   unsigned long size;
+   int ret;
+
+   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 :
+   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 :
+   find_unique_abbrev(sha1, DEFAULT_ABBREV));
+
+   ret = run_gpg_verify(buf, size, flags);

[PATCH v7 6/6] tag -v: verfy directly rather than exec-ing verify-tag

2016-04-19 Thread santiago
From: Santiago Torres 

Instead of having tag -v fork to run verify-tag, use the
gpg_verify_tag() function directly.

Helped-by: Eric Sunshine 
Signed-off-by: Santiago Torres 
---
 builtin/tag.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 1705c94..7b2918e 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -104,13 +104,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)
 {
-   const char *argv_verify_tag[] = {"verify-tag",
-   "-v", "SHA1_HEX", NULL};
-   argv_verify_tag[2] = sha1_to_hex(sha1);
-
-   if (run_command_v_opt(argv_verify_tag, RUN_GIT_CMD))
-   return error(_("could not verify the tag '%s'"), name);
-   return 0;
+   return gpg_verify_tag(sha1, name, GPG_VERIFY_VERBOSE);
 }
 
 static int do_sign(struct strbuf *buffer)
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v7 3/6] verify-tag: update variable name and type

2016-04-19 Thread santiago
From: Santiago Torres 

The run_gpg_verify() function has two variables, size and len.

This may come off as confusing when reading the code. Clarify which one
pertains to the length of the tag headers by renaming len to
payload_size. Additionally, change the type of payload_size to size_t to
match the return type of parse_signature.

Signed-off-by: Santiago Torres 
Reviewed-by: Eric Sunshine 
Signed-off-by: Junio C Hamano 
---
 builtin/verify-tag.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 77f070a..fa26e40 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -21,20 +21,21 @@ static const char * const verify_tag_usage[] = {
 static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
 {
struct signature_check sigc;
-   int len;
+   size_t payload_size;
int ret;
 
memset(&sigc, 0, sizeof(sigc));
 
-   len = parse_signature(buf, size);
+   payload_size = parse_signature(buf, size);
 
-   if (size == len) {
+   if (size == payload_size) {
if (flags & GPG_VERIFY_VERBOSE)
-   write_in_full(1, buf, len);
+   write_in_full(1, buf, payload_size);
return error("no signature found");
}
 
-   ret = check_signature(buf, len, buf + len, size - len, &sigc);
+   ret = check_signature(buf, payload_size, buf + payload_size,
+   size - payload_size, &sigc);
print_signature_buffer(&sigc, flags);
 
signature_check_clear(&sigc);
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v8 2/6] t7030: test verifying multiple tags

2016-04-22 Thread santiago
From: Santiago Torres 

The verify-tag command supports multiple tag names to verify, but
existing tests only test for invocation with a single tag.

Add a test invoking it with multiple tags.

Helped-by: Jeff King 
Signed-off-by: Santiago Torres 
Reviewed-by: Eric Sunshine 
Signed-off-by: Junio C Hamano 
---
 t/t7030-verify-tag.sh | 13 +
 1 file changed, 13 insertions(+)

diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh
index 4608e71..07079a4 100755
--- a/t/t7030-verify-tag.sh
+++ b/t/t7030-verify-tag.sh
@@ -112,4 +112,17 @@ test_expect_success GPG 'verify signatures with --raw' '
)
 '
 
+test_expect_success GPG 'verify multiple tags' '
+   tags="fourth-signed sixth-signed seventh-signed" &&
+   for i in $tags
+   do
+   git verify-tag -v --raw $i || return 1
+   done >expect.stdout 2>expect.stderr.1 &&
+   grep "^.GNUPG:." expect.stderr &&
+   git verify-tag -v --raw $tags >actual.stdout 2>actual.stderr.1 &&
+   grep "^.GNUPG:." actual.stderr &&
+   test_cmp expect.stdout actual.stdout &&
+   test_cmp expect.stderr actual.stderr
+'
+
 test_done
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v8 4/6] verify-tag: prepare verify_tag for libification

2016-04-22 Thread santiago
From: Santiago Torres 

The current interface of verify_tag() resolves reference names to SHA1,
however, the plan is to make this functionality public and the current
interface is cumbersome for callers: they are expected to supply the
textual representation of a sha1/refname. In many cases, this requires
them to turn the sha1 to hex representation, just to be converted back
inside verify_tag.

Add a SHA1 parameter to use instead of the name parameter, and rename
the name parameter to "name_to_report" for reporting purposes only.

Helped-by: Junio C Hamano 
Signed-off-by: Santiago Torres 
---
 builtin/verify-tag.c | 26 +-
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index fa26e40..a3d3a43 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -42,25 +42,28 @@ static int run_gpg_verify(const char *buf, unsigned long 
size, unsigned flags)
return ret;
 }
 
-static int verify_tag(const char *name, unsigned flags)
+static int verify_tag(const unsigned char *sha1, const char *name_to_report,
+   unsigned flags)
 {
enum object_type type;
-   unsigned char sha1[20];
char *buf;
unsigned long size;
int ret;
 
-   if (get_sha1(name, sha1))
-   return error("tag '%s' not found.", name);
-
type = sha1_object_info(sha1, NULL);
if (type != OBJ_TAG)
return error("%s: cannot verify a non-tag object of type %s.",
-   name, typename(type));
+   name_to_report ?
+   name_to_report :
+   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);
+   return error("%s: unable to read file.",
+   name_to_report ?
+   name_to_report :
+   find_unique_abbrev(sha1, DEFAULT_ABBREV));
 
ret = run_gpg_verify(buf, size, flags);
 
@@ -96,8 +99,13 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
if (verbose)
flags |= GPG_VERIFY_VERBOSE;
 
-   while (i < argc)
-   if (verify_tag(argv[i++], flags))
+   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_tag(sha1, name, flags))
had_error = 1;
+   }
return had_error;
 }
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v8 3/6] verify-tag: update variable name and type

2016-04-22 Thread santiago
From: Santiago Torres 

The run_gpg_verify() function has two variables, size and len.

This may come off as confusing when reading the code. Clarify which one
pertains to the length of the tag headers by renaming len to
payload_size. Additionally, change the type of payload_size to size_t to
match the return type of parse_signature.

Signed-off-by: Santiago Torres 
Reviewed-by: Eric Sunshine 
Signed-off-by: Junio C Hamano 
---
 builtin/verify-tag.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 77f070a..fa26e40 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -21,20 +21,21 @@ static const char * const verify_tag_usage[] = {
 static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
 {
struct signature_check sigc;
-   int len;
+   size_t payload_size;
int ret;
 
memset(&sigc, 0, sizeof(sigc));
 
-   len = parse_signature(buf, size);
+   payload_size = parse_signature(buf, size);
 
-   if (size == len) {
+   if (size == payload_size) {
if (flags & GPG_VERIFY_VERBOSE)
-   write_in_full(1, buf, len);
+   write_in_full(1, buf, payload_size);
return error("no signature found");
}
 
-   ret = check_signature(buf, len, buf + len, size - len, &sigc);
+   ret = check_signature(buf, payload_size, buf + payload_size,
+   size - payload_size, &sigc);
print_signature_buffer(&sigc, flags);
 
signature_check_clear(&sigc);
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v8 0/6] Move PGP verification out of verify-tag

2016-04-22 Thread santiago
From: Santiago Torres 

This is a follow up of [1], [2], [3], [4], [5], [6], and [7].  patches 1/6,
2/6, and 3/6, are the same as the corresponding commits in pu.

v8:  
Minor nits, I decided to quickly reroll to drop the extern qualifier in tag.c:
  * Eric pointed out that we could block-scope the declaration of name and sha1
in b/verify-tag.c, for 4/6
  * There was a typo in 6/6
  * I dropped the extern qualifier in tag.c for 5/6 as suggested by Ramsay
Jones[8]

v7: 
Mostly style/clarity changes. Thanks Peff, Eric and Junio for the
feedback! In summary: 

 * Eric pointed out issues with 3/6's commit message. It doesn't match the one 
   in pu though. I also took the opportunity to update payload_size to a size_t
   as Peff suggested.
 * 4/6 I updated report_name to name_to_report, I updated the commit message 
   and addressed some nits in the code, one of the fixes removed all three nits
   that Eric pointed out. I updated 5/6 to match these changes
 * I gave the commit message on 6/6 another go.

v6: 
 * As Junio suggested, updated 4/6, to include the name argument and the
   ternary operator to provide more descriptive error messages. I propagated
   these changes to 5/6 and 6/6 as well. I'm unsure about the 80-column
   on 4/6, the ternary operator is rather long.
 * Updated and reviewed the commit messages based on Eric and Junio's
   feedback

v5:
Added helpful feedback by Eric

 * Reordering of the patches, to avoid temporal inclusion of a regression
 * Fix typos here and there.
 * Review commit messages, as some weren't representative of what the patches
   were doing anymore.
 * Updated t7030 to include Peff's suggestion, and added a helped-by line here
   as it was mostly Peff's code.
 * Updated the error-handling/printing issues that were introduced when.
   libifying the verify_tag function.

v4:

Thanks Eric, Jeff, and Hannes for the feedback.

 * I relocated the sigchain_push call so it comes after the error on
   gpg-interface (thanks Hannnes for catching this).
 * I updated the unit test to match the discussion on [3]. Now it generates
   the expected output of the tag on the fly for comparison. (This is just
   copy and paste from [3], but I verified that it works by breaking the
   while)
 * I split moving the code and renaming the variables into two patches so
   these are easier to review.
 * I used an adapter on builtin/tag.c instead of redefining all the fn*
   declarations everywhere. This introduces an issue with the way git tag -v
   resolves refnames though. I added a new commit to restore the previous
   behavior of git-tag. I'm not sure if I should've split this into two commits
   though.

v3:
Thanks Eric, Jeff, for the feedback.

 * I separated the patch in multiple sub-patches.
 * I compared the behavior of previous git tag -v and git verify-tag 
   invocations to make sure the behavior is the same
 * I dropped the multi-line comment, as suggested.
 * I fixed the issue with the missing brackets in the while (this is 
   now detected by the test).

v2:

 * I moved the pgp-verification code to tag.c 
 * I added extra arguments so git tag -v and git verify-tag both work
   with the same function
 * Relocated the SIGPIPE handling code in verify-tag to gpg-interface

v1:
 
The verify tag function is just a thin wrapper around the verify-tag
command. We can avoid one fork call by doing the verification inside
the tag builtin instead.


This applies on v2.8.0. 
Thanks!
-Santiago

[1] http://thread.gmane.org/gmane.comp.version-control.git/287649
[2] http://thread.gmane.org/gmane.comp.version-control.git/289836
[3] http://thread.gmane.org/gmane.comp.version-control.git/290608
[4] http://thread.gmane.org/gmane.comp.version-control.git/290731
[5] http://thread.gmane.org/gmane.comp.version-control.git/290790
[6] http://thread.gmane.org/gmane.comp.version-control.git/291780
[7] http://thread.gmane.org/gmane.comp.version-control.git/291887
[8] http://thread.gmane.org/gmane.comp.version-control.git/292029



Santiago Torres (6):
  builtin/verify-tag.c: ignore SIGPIPE in gpg-interface
  t7030: test verifying multiple tags
  verify-tag: update variable name and type
  verify-tag: prepare verify_tag for libification
  verify-tag: move tag verification code to tag.c
  tag -v: verify directly rather than exec-ing verify-tag

 builtin/tag.c |  8 +--
 builtin/verify-tag.c  | 61 ++-
 gpg-interface.c   |  2 ++
 t/t7030-verify-tag.sh | 13 +++
 tag.c | 53 
 tag.h |  2 ++
 6 files changed, 78 insertions(+), 61 deletions(-)

-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v8 6/6] tag -v: verify directly rather than exec-ing verify-tag

2016-04-22 Thread santiago
From: Santiago Torres 

Instead of having tag -v fork to run verify-tag, use the
gpg_verify_tag() function directly.

Helped-by: Eric Sunshine 
Signed-off-by: Santiago Torres 
---
 builtin/tag.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 1705c94..7b2918e 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -104,13 +104,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)
 {
-   const char *argv_verify_tag[] = {"verify-tag",
-   "-v", "SHA1_HEX", NULL};
-   argv_verify_tag[2] = sha1_to_hex(sha1);
-
-   if (run_command_v_opt(argv_verify_tag, RUN_GIT_CMD))
-   return error(_("could not verify the tag '%s'"), name);
-   return 0;
+   return gpg_verify_tag(sha1, name, GPG_VERIFY_VERBOSE);
 }
 
 static int do_sign(struct strbuf *buffer)
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v8 5/6] verify-tag: move tag verification code to tag.c

2016-04-22 Thread santiago
From: Santiago Torres 

The PGP verification routine for tags could be accessed by other modules
that require to do so.

Publish the verify_tag function in tag.c and rename it to gpg_verify_tag
so it does not conflict with builtin/mktag's static function.

Helped-by: Junio C Hamano 
Signed-off-by: Santiago Torres 
---
 builtin/verify-tag.c | 55 +---
 tag.c| 53 ++
 tag.h|  2 ++
 3 files changed, 56 insertions(+), 54 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index a3d3a43..99f8148 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -18,59 +18,6 @@ static const char * const verify_tag_usage[] = {
NULL
 };
 
-static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
-{
-   struct signature_check sigc;
-   size_t payload_size;
-   int ret;
-
-   memset(&sigc, 0, sizeof(sigc));
-
-   payload_size = parse_signature(buf, size);
-
-   if (size == payload_size) {
-   if (flags & GPG_VERIFY_VERBOSE)
-   write_in_full(1, buf, payload_size);
-   return error("no signature found");
-   }
-
-   ret = check_signature(buf, payload_size, buf + payload_size,
-   size - payload_size, &sigc);
-   print_signature_buffer(&sigc, flags);
-
-   signature_check_clear(&sigc);
-   return ret;
-}
-
-static int verify_tag(const unsigned char *sha1, const char *name_to_report,
-   unsigned flags)
-{
-   enum object_type type;
-   char *buf;
-   unsigned long size;
-   int ret;
-
-   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 :
-   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 :
-   find_unique_abbrev(sha1, DEFAULT_ABBREV));
-
-   ret = run_gpg_verify(buf, size, flags);
-
-   free(buf);
-   return ret;
-}
-
 static int git_verify_tag_config(const char *var, const char *value, void *cb)
 {
int status = git_gpg_config(var, value, cb);
@@ -104,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 (verify_tag(sha1, name, flags))
+   else if (gpg_verify_tag(sha1, name, flags))
had_error = 1;
}
return had_error;
diff --git a/tag.c b/tag.c
index d72f742..8363a0e 100644
--- a/tag.c
+++ b/tag.c
@@ -6,6 +6,59 @@
 
 const char *tag_type = "tag";
 
+static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
+{
+   struct signature_check sigc;
+   size_t payload_size;
+   int ret;
+
+   memset(&sigc, 0, sizeof(sigc));
+
+   payload_size = parse_signature(buf, size);
+
+   if (size == payload_size) {
+   if (flags & GPG_VERIFY_VERBOSE)
+   write_in_full(1, buf, payload_size);
+   return error("no signature found");
+   }
+
+   ret = check_signature(buf, payload_size, buf + payload_size,
+   size - payload_size, &sigc);
+   print_signature_buffer(&sigc, flags);
+
+   signature_check_clear(&sigc);
+   return ret;
+}
+
+int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report, 
+   unsigned flags)
+{
+   enum object_type type;
+   char *buf;
+   unsigned long size;
+   int ret;
+
+   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 :
+   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 :
+   find_unique_abbrev(sha1, DEFAULT_ABBREV));
+
+   ret = run_gpg_verify(buf, size, fl

[PATCH v8 1/6] builtin/verify-tag.c: ignore SIGPIPE in gpg-interface

2016-04-22 Thread santiago
From: Santiago Torres 

The verify_signed_buffer() function may trigger a SIGPIPE when the
GPG child process terminates early (due to a bad keyid, for example)
and Git tries to write to it afterwards.  Previously, ignoring
SIGPIPE was done in builtin/verify-tag.c to avoid this issue.

However, any other caller who wants to call verify_signed_buffer()
would have to do the same.

Use sigchain_push(SIGPIPE, SIG_IGN) in verify_signed_buffer(),
pretty much like in sign_buffer(), so that any caller is not
required to perform this task.

This will avoid possible mistakes by further developers using
verify_signed_buffer().

Signed-off-by: Santiago Torres 
Reviewed-by: Eric Sunshine 
Signed-off-by: Junio C Hamano 
---
 builtin/verify-tag.c | 3 ---
 gpg-interface.c  | 2 ++
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 00663f6..77f070a 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -95,9 +95,6 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
if (verbose)
flags |= GPG_VERIFY_VERBOSE;
 
-   /* sometimes the program was terminated because this signal
-* was received in the process of writing the gpg input: */
-   signal(SIGPIPE, SIG_IGN);
while (i < argc)
if (verify_tag(argv[i++], flags))
had_error = 1;
diff --git a/gpg-interface.c b/gpg-interface.c
index 3dc2fe3..2259938 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -237,6 +237,7 @@ int verify_signed_buffer(const char *payload, size_t 
payload_size,
return error(_("could not run gpg."));
}
 
+   sigchain_push(SIGPIPE, SIG_IGN);
write_in_full(gpg.in, payload, payload_size);
close(gpg.in);
 
@@ -250,6 +251,7 @@ int verify_signed_buffer(const char *payload, size_t 
payload_size,
close(gpg.out);
 
ret = finish_command(&gpg);
+   sigchain_pop(SIGPIPE);
 
unlink_or_warn(path);
 
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC PATCH] builtin:tag:verify_tag: allow gpg output + pretty

2019-04-12 Thread santiago
From: Santiago Torres 

On the git tag -v code, there is a guard to suppress gpg output if a
pretty format is provided. The rationale for this is that the gpg output
*and* the pretty formats together may conflict with each other. However,
both outputs are directed to different output streams and, as such,
they can safely coexist. Drop the guard clause and use
GPG_VERIFY_VERBOSE regardless of the pretty format

Signed-off-by: Santiago Torres 
---
 builtin/tag.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 8c493a569..4b91c769c 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -110,9 +110,6 @@ static int verify_tag(const char *name, const char *ref,
const struct ref_format *format = cb_data;
flags = GPG_VERIFY_VERBOSE;
 
-   if (format->format)
-   flags = GPG_VERIFY_OMIT_STATUS;
-
if (gpg_verify_tag(oid, name, flags))
return -1;
 
-- 
2.21.0



[PATCH 0/2] tag verification: do not mute gpg output

2019-04-27 Thread santiago
From: Santiago Torres 

The default behavior of the tag verification functions used to quiet
down the gpg output if --format was passed. The rationale for this was
to avoid --format to be litterred by the gpg output. However, this may
be unnecessary because the gpg output is already streamed to stderr and
thus can be easily multiplexed. 

Santiago Torres (2):
  builtin/tag: do not omit -v gpg out for --format
  builtin/verify-tag: do not omit gpg on --format

 builtin/tag.c| 6 +++---
 builtin/verify-tag.c | 6 ++
 2 files changed, 5 insertions(+), 7 deletions(-)

-- 
2.21.0



[PATCH 1/2] builtin/tag: do not omit -v gpg out for --format

2019-04-27 Thread santiago
From: Santiago Torres 

The current implementation of git tag -v omits the gpg output when the
--format flag is passed. This may not be useful to users that want to
see the gpg output *and* --format the output of the git tag -v. Instead,
pass the default gpg interface output if --format is specified.

Signed-off-by: Santiago Torres 
---
 builtin/tag.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 02f6bd1279..449d91c13c 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -110,10 +110,10 @@ static int verify_tag(const char *name, const char *ref,
 {
int flags;
const struct ref_format *format = cb_data;
-   flags = GPG_VERIFY_VERBOSE;
+   flags = 0;
 
-   if (format->format)
-   flags = GPG_VERIFY_OMIT_STATUS;
+   if (!format->format)
+   flags = GPG_VERIFY_VERBOSE;
 
if (gpg_verify_tag(oid, name, flags))
return -1;
-- 
2.21.0



[PATCH 2/2] builtin/verify-tag: do not omit gpg on --format

2019-04-27 Thread santiago
From: Santiago Torres 

The current implementation of git-verify-tag omits the gpg output when
the --format flag is passed. This may not be useful to users that want
to see the gpg output *and* --format the output of git verify-tag.
Instead, respect the --raw flag or the default gpg output.

Signed-off-by: Santiago Torres 
---
 builtin/verify-tag.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 6fa04b751a..262e73cb45 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -47,15 +47,13 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
if (argc <= i)
usage_with_options(verify_tag_usage, verify_tag_options);
 
-   if (verbose)
+   if (verbose && !format.format)
flags |= GPG_VERIFY_VERBOSE;
 
-   if (format.format) {
+   if (format.format)
if (verify_ref_format(&format))
usage_with_options(verify_tag_usage,
   verify_tag_options);
-   flags |= GPG_VERIFY_OMIT_STATUS;
-   }
 
while (i < argc) {
struct object_id oid;
-- 
2.21.0



[RFC PATCH] t: lib-gpg: flush agent sockets on startup

2017-07-07 Thread santiago
From: Santiago Torres 

When running gpg-relevant tests, a gpg-daemon is ran for a
trash_directory-specific GNUPGHOME. This daemon creates a unix socket on
the target host, and it will be used on subsequent runs of the same test
script.  Add a call to kill the agent and flush the sockets of the
relevant trash directory.

Signed-off-by: Santiago Torres 
---
 t/lib-gpg.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index ec2aa8f68..22ef2fa87 100755
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -31,6 +31,7 @@ then
chmod 0700 ./gpghome &&
GNUPGHOME="$(pwd)/gpghome" &&
export GNUPGHOME &&
+   gpgconf --kill gpg-agent &&
gpg --homedir "${GNUPGHOME}" 2>/dev/null --import \
"$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&
gpg --homedir "${GNUPGHOME}" 2>/dev/null --import-ownertrust \
-- 
2.13.2



[PATCH] t: lib-gpg: flush gpg agent on startup

2017-07-20 Thread santiago
From: Santiago Torres 

When running gpg-relevant tests, a gpg-daemon is spawned for each
GNUPGHOME used. This daemon may stay running after the test and cache
file descriptors for the trash directories, even after the trash
directory is removed. This leads to ENOENT errors when attempting to
create files if tests are run multiple times.

Add a cleanup script to force flushing the gpg-agent for that GNUPGHOME
(if any) before setting up the GPG relevant-environment.

Helped-by: Junio C Hamano 
Signed-off-by: Santiago Torres 
---
 t/lib-gpg.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index ec2aa8f68..7a6c7ee6f 100755
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -31,6 +31,7 @@ then
chmod 0700 ./gpghome &&
GNUPGHOME="$(pwd)/gpghome" &&
export GNUPGHOME &&
+   gpgconf --kill gpg-agent 2>&1 >/dev/null
gpg --homedir "${GNUPGHOME}" 2>/dev/null --import \
"$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&
gpg --homedir "${GNUPGHOME}" 2>/dev/null --import-ownertrust \
-- 
2.13.3



[PATCH v2] t: lib-gpg: flush gpg agent on startup

2017-07-20 Thread santiago
From: Santiago Torres 

When running gpg-relevant tests, a gpg-daemon is spawned for each
GNUPGHOME used. This daemon may stay running after the test and cache
file descriptors for the trash directories, even after the trash
directory is removed. This leads to ENOENT errors when attempting to
create files if tests are run multiple times.

Add a cleanup script to force flushing the gpg-agent for that GNUPGHOME
(if any) before setting up the GPG relevant-environment.

Helped-by: Junio C Hamano 
Signed-off-by: Santiago Torres 
---
 t/lib-gpg.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index ec2aa8f68..43679a4c6 100755
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -31,6 +31,7 @@ then
chmod 0700 ./gpghome &&
GNUPGHOME="$(pwd)/gpghome" &&
export GNUPGHOME &&
+   (gpgconf --kill gpg-agent 2>&1 >/dev/null || : ) &&
gpg --homedir "${GNUPGHOME}" 2>/dev/null --import \
"$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&
gpg --homedir "${GNUPGHOME}" 2>/dev/null --import-ownertrust \
-- 
2.13.3



Re: Commits are no longer gpg-signed in 2.9.0 when "commit.gpgsign" is enabled

2016-07-11 Thread Santiago Torres
Hi Nils,

I just checked and I have commits made in 2.9 with this option set and I
don't seem to have your issue. Here's what I did:

    santiago at ~/test-signing ✔ git init
Initialized empty Git repository in /home/santiago/test-signing/.git/
    santiago at ~/test-signing ✔ hub create
Updating origin
Enter passphrase for key '/home/santiago/.ssh/id_rsa': 
created repository: SantiagoTorres/test-signing
santiago at ~/test-signing ✔ touch testfile
santiago at ~/test-signing ✔ git add testfile 
santiago at ~/test-signing ✔ git commit
i[master (root-commit) 6de1ad2] TEST: tests git autocommit setting
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 testfile
santiago at ~/test-signing ✔ git log --show-signature
commit 6de1ad20237f042433f8b94a3bf8c7cc41711d90 (HEAD -> refs/heads/master)
gpg: Signature made Mon 11 Jul 2016 10:40:41 AM EDT using RSA key ID 
468F122CE8162295
gpg: Good signature from "Santiago Torres " 
[ultimate]
gpg: aka "Santiago Torres-Arias " 
[ultimate]
Author: Santiago Torres 
Date:   Mon Jul 11 10:40:32 2016 -0400

TEST: tests git autocommit setting
santiago at ~/test-signing ✔ git push origin master
    Enter passphrase for key '/home/santiago/.ssh/id_rsa': 
Counting objects: 3, done.
Writing objects: 100% (3/3), 879 bytes | 0 bytes/s, done.
Total 3 (delta 0), reused 0 (delta 0)
To g...@github.com:SantiagoTorres/test-signing.git
 * [new branch]  master -> master
santiago at ~/test-signing ✔ git --version
git version 2.9.0
santiago at ~/test-signing ✔ 

You can check the github repository 

Do you have a mwe/.gitconfig so I can take a look at it? 

I wonder if this issue is similar to what happened in mutt, where gpg
doesn't show the password prompt using gpg-agent and it silently failed.

Thanks,
-Santiago.

On Mon, Jul 11, 2016 at 01:29:10PM +0200, Nils Fenner wrote:
> Hey Git community,
> 
> since Version 2.9.0, the configuration option "commit.gpgsign" doesn't work
> as users would expect. By committing via 'git gui' (or usual 'git commit'
> without further option), commits are not being auto-signed any longer, when
> "commit.gpgSign" configuration option is set. I also couldn't find a flag to
> "workaround" that situation in the GUI. To my understanding, I now have to
> pass the '-S' option to 'git commit' every time and GUI becomes "impractical
> to use". Surprisingly, nobody seems having noticed this behavioural change
> since the release of 2.9.
> 
> FYI: In the release log, there's a note stating, that this has been altered
> in the context of 'git commit-tree'. Maybe this interferes with "normal"
> behaviour.
> 
> Would be happy hearing from you soon. Thanks!
> 
> Cheers,
> Nils
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Commits are no longer gpg-signed in 2.9.0 when "commit.gpgsign" is enabled

2016-07-11 Thread Santiago Torres
On Mon, Jul 11, 2016 at 06:27:57PM +0200, Nils Fenner wrote:
> Hi Santiago,
> 
> repeated your test here and actually found something interesting. When
> committing via 'git gui', commits are not being gpg-signed, while firing
> a 'git commit' shows the passphrase dialog and signs the commit correctly.

I have the feeling that this is an issue with your gpg configuration not
playing well with the gui. Do you know which pinentry program is being
used (e.g., type /usr/bin/pinentry).

 
> What you mean by wme? Is it the gpg-agent's config or something?
> 

Sorry, I meant a Minimum Working Example, although that'll be hard to
reproduce in this case. Your configuration seems to be proper.

> Hope that points in the right direction.

I think that we can take further troubleshooting off-list. This doesn't
seem to be behavior with git (although we can come back with a bug
report if something is naturally-broken/could be improved)

Cheers!
-Santiago.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[OT] USENIX paper on Git

2016-08-03 Thread Santiago Torres
Hello everyone,

I will be presenting a paper regarding the Git metadata issues that we
discussed at the beginning on the year on USENIX '16. I'm writing To
make everyone in this ML aware that this work exists and to bring
everyone into the loop.

I'm open for feedback and corrections. If anything seems odd imprecise
to the community, I can make an errata in the presentation (at least).
I'll also try to work towards making corrections anywhere if possible;
this is my first publication, so I wasn't sure if it was possible to
share things before they are published. Thankfully, this is OK in
USENIX's book. Here's the link:
http://i2.cdn.turner.com/cnnnext/dam/assets/160730192650-14new-week-in-politics-super-169.jpg

I do mention of work towards fixing these issues in upcoming versions of
Git. This is in reference to the issue with Git tags, although I hope to
continue working on Git in general once I have more time for it. Thanks
again for all the patience reviewing patches and discussing everything.

Thanks!
-Santiago.

P.S. Let me know if anyone is going to USENIX. I'm looking forward to
meeting!

[1] http://thread.gmane.org/gmane.comp.version-control.git/287649 *
I believe it to be this, but gmane seems to be down. 


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [OT] USENIX paper on Git

2016-08-03 Thread Santiago Torres
 > share things before they are published. Thankfully, this is OK in
> > USENIX's book. Here's the link:
> > http://i2.cdn.turner.com/cnnnext/dam/assets/160730192650-14new-week-in-politics-super-169.jpg
> 
> While I had a good laugh, I am wondering whether this is the correct link?

Oh my god, sorry, I meant to p, not to ctrl + v. My head is all over the
place as of late.

Here's the correct link:

http://isis.poly.edu/~jcappos/papers/torres_toto_usenixsec-2016.pdf

Thanks!
-Santiago.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [OT] USENIX paper on Git

2016-08-03 Thread Santiago Torres
On Wed, Aug 03, 2016 at 10:14:21AM -0700, Stefan Beller wrote:
> On Wed, Aug 3, 2016 at 8:25 AM, Santiago Torres  wrote:
> >  > share things before they are published. Thankfully, this is OK in
> >> > USENIX's book. Here's the link:
> >> > http://i2.cdn.turner.com/cnnnext/dam/assets/160730192650-14new-week-in-politics-super-169.jpg
> >>
> >> While I had a good laugh, I am wondering whether this is the correct link?
> >
> > Oh my god, sorry, I meant to p, not to ctrl + v. My head is all over the
> > place as of late.
> >
> > Here's the correct link:
> >
> > http://isis.poly.edu/~jcappos/papers/torres_toto_usenixsec-2016.pdf
> 
> In 4.1 you write:
> > Finally, Git submodules are also vulnerable, as they automatically track
> > a tag (or branch). If a build dependency is included in a project as a part
> > of the submodule, a package might be vulnerable via an underlying library.
> 
> Submodules actually track commits, not tags or branches.
> 
> This is confusing for some users, e.g. the user intended to track
> a library at version 1.1, but it tracks 1234abcd instead (which is what
> 1.1 points at).

I'm assuming that git submodule update does update where the ref points
to, does it not?

let me dig into this and try to take the necessary measures to correct
this

Thanks for the feedback!

-Santiago.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [OT] USENIX paper on Git

2016-08-03 Thread Santiago Torres
e from start though.

> 
> I think this is actually a generalization of the cloning issue
> (where state "A" is simply "I have no existing state yet").
> 
> So it seems like there is room for better tooling around push-certs
> (e.g., to fetch and verify the chaining automatically). 

Yeah, I think that in-band cert distribution and automatic validation
may be a desirable feature, but there may be reasons as to why this is
not appealing. (I already have some in mind).

> I think git in general is quite weak in automatic tooling for
> verifications. There are room for signatures in the data format and
> tools for checking that the bytes haven't been touched, but there's
> almost nothing to tell you that signatures make any sense, tools for
> handling trust, etc.

Yes, from our previous interactions, it seems that git's philosophy
focuses on providing the right information to users/tools and let those
tools make the call of whether something is fishy. I don't think this is
necessarily bad.

> 
> I think your solution also had some mechanisms for adding trusted keys
> as part of the hash chain. I'm not convinced that's something that
> should be part of git's solution in particular, and not an out-of-band
> thing handled as part of the PKI. Because it's really a group key
> management problem, and applies to anything you might sign.

I see. What about, for example, having an official "overlay" on git for
signing and verification of a repository? (e.g., similar to what
monotone does). I see that other VCS's have a plugin mechanism, and they
host official plugins.

Thanks a lot!
-Santiago.

[1] https://isis.poly.edu/~jcappos/papers/kuppusamy_nsdi_16.pdf
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [OT] USENIX paper on Git

2016-08-03 Thread Santiago Torres
On Wed, Aug 03, 2016 at 10:35:54AM -0700, Junio C Hamano wrote:
> Santiago Torres  writes:
> 
> >> Submodules actually track commits, not tags or branches.
> >> 
> >> This is confusing for some users, e.g. the user intended to track a
> >> library at version 1.1, but it tracks 1234abcd instead (which is
> >> what 1.1 points at).
> >
> > I'm assuming that git submodule update does update where the ref
> > points to, does it not?
> 
> I think you may configure the command to do so, instead of the default
> "detach at the commit recorded in the superproject".
> 
> But then your tree immediately will be marked by "git status" as
> "modified" at such a submodule, meaning "what you have in the working
> tree is different from what the commit in the superproject wants you
> to have", I would think.
> 

Ah, I see where is my confusion. Thanks for the correction :)

-Santiago.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


  1   2   3   >