More formatting with 'git tag -l'

More formatting with 'git tag -l'

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 519 bytes --]

Hello,

Would it be feasible to add more formatting options to 'git tag -l'.
Right now, it's just '-n' but if more formatting options were added,
I think I could use annotated tags as a nice source for autogenerated
'NEWS' file.

What I am most concerned about is the date of last commit contained
in the tag. Having an option to format each printed tag and use that
information along with tag details in a format string (like the one
used by 'git log') would be great.

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 316 bytes --]

Re: More formatting with 'git tag -l'

[Jeff King]

On Mon, Aug 29, 2011 at 09:10:18PM +0200, Michał Górny wrote:

> Would it be feasible to add more formatting options to 'git tag -l'.
> Right now, it's just '-n' but if more formatting options were added,
> I think I could use annotated tags as a nice source for autogenerated
> 'NEWS' file.
> 
> What I am most concerned about is the date of last commit contained
> in the tag. Having an option to format each printed tag and use that
> information along with tag details in a format string (like the one
> used by 'git log') would be great.

You can do something similar with 'git for-each-ref', which is probably
what you should be using if you are scripting, anyway (as it is
"plumbing" and guaranteed not to change in future releases). Something
like:

  FORMAT='%(refname:short)
  %(taggerdate)
  %(subject)'
  git for-each-ref --format="$FORMAT" refs/tags/

If you want to do more complex things, try the "--shell" option (or
--perl, --python, etc), which makes it easy to write little scriptlets
in your format, like:

  FORMAT='
    REF=%(refname:short)
    TDATE=%(taggerdate)
    CDATE=%(committerdate)
    if test -z "$TDATE"; then
      echo "$REF: unannotated tag, commit date is $CDATE"
    else
      echo "$REF: annotated tag, tag date is $TDATE"
    fi
  '
  git for-each-ref --shell --format="$FORMAT" refs/tags/ | sh

See "git help for-each-ref" for more discussion and examples.

-Peff

Re: More formatting with 'git tag -l'

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 1118 bytes --]

On Mon, 29 Aug 2011 15:36:58 -0400
Jeff King <peff@peff.net> wrote:

> On Mon, Aug 29, 2011 at 09:10:18PM +0200, Michał Górny wrote:
> 
> > Would it be feasible to add more formatting options to 'git tag -l'.
> > Right now, it's just '-n' but if more formatting options were added,
> > I think I could use annotated tags as a nice source for
> > autogenerated 'NEWS' file.
> > 
> > What I am most concerned about is the date of last commit contained
> > in the tag. Having an option to format each printed tag and use that
> > information along with tag details in a format string (like the one
> > used by 'git log') would be great.
> 
> You can do something similar with 'git for-each-ref', which is
> probably what you should be using if you are scripting, anyway (as it
> is "plumbing" and guaranteed not to change in future releases).
> Something like:

Thanks, that looks great. The only thing I am missing right now is
a simple %(body) variant which would strip the GPG signature. I'd
really like to avoid establishing a shell oneliner to do that.

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 316 bytes --]

Re: More formatting with 'git tag -l'

[Jeff King]

On Mon, Aug 29, 2011 at 11:20:11PM +0200, Michał Górny wrote:

> > You can do something similar with 'git for-each-ref', which is
> > probably what you should be using if you are scripting, anyway (as it
> > is "plumbing" and guaranteed not to change in future releases).
> > Something like:
> 
> Thanks, that looks great. The only thing I am missing right now is
> a simple %(body) variant which would strip the GPG signature. I'd
> really like to avoid establishing a shell oneliner to do that.

Yeah, I don't think that's possible with the current code. I don't think
it would be unreasonable to add "%(tagcontents)" and "%(tagsignature)"
or something similar, though.

Want to try a patch?

-Peff

Re: More formatting with 'git tag -l'

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 987 bytes --]

On Mon, 29 Aug 2011 17:37:57 -0400
Jeff King <peff@peff.net> wrote:

> On Mon, Aug 29, 2011 at 11:20:11PM +0200, Michał Górny wrote:
> 
> > > You can do something similar with 'git for-each-ref', which is
> > > probably what you should be using if you are scripting, anyway
> > > (as it is "plumbing" and guaranteed not to change in future
> > > releases). Something like:
> > 
> > Thanks, that looks great. The only thing I am missing right now is
> > a simple %(body) variant which would strip the GPG signature. I'd
> > really like to avoid establishing a shell oneliner to do that.
> 
> Yeah, I don't think that's possible with the current code. I don't
> think it would be unreasonable to add "%(tagcontents)" and
> "%(tagsignature)" or something similar, though.
> 
> Want to try a patch?

I'll take a look at the code tomorrow and see if I can assemble
something useful. Someone will have to design nice names though.

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 316 bytes --]

[PATCH] git-for-each-ref: move GPG sigs off %(body) to %(signature).

[Michał Górny]

When grabbing a %(body) or %(contents) off a tag, one doesn't really
expect to get the GPG signature as well (as it's basically useless
without the complete signed text). Thus, strip it off those two tags,
and make available via %(signature) if anyone needs it.

Signed-off-by: Michał Górny <mgorny@gentoo.org>
---
 Documentation/git-for-each-ref.txt |    3 ++-
 builtin/for-each-ref.c             |   35 +++++++++++++++++++++++++++++------
 2 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 152e695..7145c46 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -103,7 +103,8 @@ and `date` to extract the named component.
 
 The first line of the message in a commit and tag object is
 `subject`, the remaining lines are `body`.  The whole message
-is `contents`.
+is `contents`.  If the message is GPG-signed, the signature is
+`signature`.
 
 For sorting purposes, fields with numeric values sort in numeric
 order (`objectsize`, `authordate`, `committerdate`, `taggerdate`).
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 89e75c6..fa5c292 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -10,6 +10,9 @@
 #include "parse-options.h"
 #include "remote.h"
 
+/* Used to strip signature off body */
+#define PGP_SIGNATURE "-----BEGIN PGP SIGNATURE-----\n"
+
 /* Quoting styles */
 #define QUOTE_NONE 0
 #define QUOTE_SHELL 1
@@ -69,6 +72,7 @@ static struct {
 	{ "subject" },
 	{ "body" },
 	{ "contents" },
+	{ "signature" },
 	{ "upstream" },
 	{ "symref" },
 	{ "flag" },
@@ -458,7 +462,7 @@ static void grab_person(const char *who, struct atom_value *val, int deref, stru
 	}
 }
 
-static void find_subpos(const char *buf, unsigned long sz, const char **sub, const char **body)
+static void find_subpos(const char *buf, unsigned long sz, const char **sub, const char **body, const char **sig)
 {
 	while (*buf) {
 		const char *eol = strchr(buf, '\n');
@@ -478,18 +482,34 @@ static void find_subpos(const char *buf, unsigned long sz, const char **sub, con
 	buf = strchr(buf, '\n');
 	if (!buf) {
 		*body = "";
+		*sig = *body;
 		return; /* no body */
 	}
 	while (*buf == '\n')
 		buf++; /* skip blank between subject and body */
 	*body = buf;
+	*sig = *body;
+
+	/* look for GPG signature */
+	while (*buf) {
+		const char *eol = strchr(buf, '\n');
+		if (!eol) {
+			*sig = buf + strlen(buf);
+			return;
+		}
+		if (!strncmp(eol + 1, PGP_SIGNATURE, ARRAY_SIZE(PGP_SIGNATURE)-1)) {
+			*sig = eol + 1;
+			break; /* found end of body */
+		}
+		buf = eol + 1;
+	}
 }
 
 /* See grab_values */
 static void grab_sub_body_contents(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
 {
 	int i;
-	const char *subpos = NULL, *bodypos = NULL;
+	const char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;
 
 	for (i = 0; i < used_atom_cnt; i++) {
 		const char *name = used_atom[i];
@@ -500,19 +520,22 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
 			name++;
 		if (strcmp(name, "subject") &&
 		    strcmp(name, "body") &&
-		    strcmp(name, "contents"))
+		    strcmp(name, "contents") &&
+		    strcmp(name, "signature"))
 			continue;
 		if (!subpos)
-			find_subpos(buf, sz, &subpos, &bodypos);
+			find_subpos(buf, sz, &subpos, &bodypos, &sigpos);
 		if (!subpos)
 			return;
 
 		if (!strcmp(name, "subject"))
 			v->s = copy_line(subpos);
 		else if (!strcmp(name, "body"))
-			v->s = xstrdup(bodypos);
+			v->s = xstrndup(bodypos, sigpos - bodypos);
 		else if (!strcmp(name, "contents"))
-			v->s = xstrdup(subpos);
+			v->s = xstrndup(subpos, sigpos - subpos);
+		else if (!strcmp(name, "signature"))
+			v->s = xstrdup(sigpos);
 	}
 }
 
-- 
1.7.6.1

Re: [PATCH] git-for-each-ref: move GPG sigs off %(body) to %(signature).

[Michael J Gruber]

Michał Górny venit, vidit, dixit 30.08.2011 10:57:

Welcome to yet another MG (and a fresh spelling of Michael) on the list ;)

> When grabbing a %(body) or %(contents) off a tag, one doesn't really
> expect to get the GPG signature as well (as it's basically useless
> without the complete signed text). Thus, strip it off those two tags,
> and make available via %(signature) if anyone needs it.

No, please do not change %(contents). It is the complete content which
(together with the header) enters into the sha1 calculation.

You will probably also face opposition as regards to %(body), changing
existing behaviour.

In fact, I wish we didn't have %(body) but %(contents:body) just like
other modifiers such as :short.

I think I'd go for

%(contents:signature)

and implement

%(contents:subject) the same as %(subject)
%(contents:body) as contents minus subject minus signature

and slowly deprecate %(subject) and %(body) (simply un-document for now).

Jeff will have more to say about this. Please see below for the sig
detection:

> Signed-off-by: Michał Górny <mgorny@gentoo.org>
> ---
>  Documentation/git-for-each-ref.txt |    3 ++-
>  builtin/for-each-ref.c             |   35 +++++++++++++++++++++++++++++------
>  2 files changed, 31 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
> index 152e695..7145c46 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -103,7 +103,8 @@ and `date` to extract the named component.
>  
>  The first line of the message in a commit and tag object is
>  `subject`, the remaining lines are `body`.  The whole message
> -is `contents`.
> +is `contents`.  If the message is GPG-signed, the signature is
> +`signature`.
>  
>  For sorting purposes, fields with numeric values sort in numeric
>  order (`objectsize`, `authordate`, `committerdate`, `taggerdate`).
> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
> index 89e75c6..fa5c292 100644
> --- a/builtin/for-each-ref.c
> +++ b/builtin/for-each-ref.c
> @@ -10,6 +10,9 @@
>  #include "parse-options.h"
>  #include "remote.h"
>  
> +/* Used to strip signature off body */
> +#define PGP_SIGNATURE "-----BEGIN PGP SIGNATURE-----\n"
> +
>  /* Quoting styles */
>  #define QUOTE_NONE 0
>  #define QUOTE_SHELL 1
> @@ -69,6 +72,7 @@ static struct {
>  	{ "subject" },
>  	{ "body" },
>  	{ "contents" },
> +	{ "signature" },
>  	{ "upstream" },
>  	{ "symref" },
>  	{ "flag" },
> @@ -458,7 +462,7 @@ static void grab_person(const char *who, struct atom_value *val, int deref, stru
>  	}
>  }
>  
> -static void find_subpos(const char *buf, unsigned long sz, const char **sub, const char **body)
> +static void find_subpos(const char *buf, unsigned long sz, const char **sub, const char **body, const char **sig)
>  {
>  	while (*buf) {
>  		const char *eol = strchr(buf, '\n');
> @@ -478,18 +482,34 @@ static void find_subpos(const char *buf, unsigned long sz, const char **sub, con
>  	buf = strchr(buf, '\n');
>  	if (!buf) {
>  		*body = "";
> +		*sig = *body;
>  		return; /* no body */
>  	}
>  	while (*buf == '\n')
>  		buf++; /* skip blank between subject and body */
>  	*body = buf;
> +	*sig = *body;
> +
> +	/* look for GPG signature */

Again I have to say no. Please look at

3d5854e (tag: recognize rfc1991 signatures, 2010-11-10)

which uses the factored out signature detection as introduced in the
previous commits. Thanks!

> +	while (*buf) {
> +		const char *eol = strchr(buf, '\n');
> +		if (!eol) {
> +			*sig = buf + strlen(buf);
> +			return;
> +		}
> +		if (!strncmp(eol + 1, PGP_SIGNATURE, ARRAY_SIZE(PGP_SIGNATURE)-1)) {
> +			*sig = eol + 1;
> +			break; /* found end of body */
> +		}
> +		buf = eol + 1;
> +	}
>  }
>  
>  /* See grab_values */
>  static void grab_sub_body_contents(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
>  {
>  	int i;
> -	const char *subpos = NULL, *bodypos = NULL;
> +	const char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;
>  
>  	for (i = 0; i < used_atom_cnt; i++) {
>  		const char *name = used_atom[i];
> @@ -500,19 +520,22 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
>  			name++;
>  		if (strcmp(name, "subject") &&
>  		    strcmp(name, "body") &&
> -		    strcmp(name, "contents"))
> +		    strcmp(name, "contents") &&
> +		    strcmp(name, "signature"))
>  			continue;
>  		if (!subpos)
> -			find_subpos(buf, sz, &subpos, &bodypos);
> +			find_subpos(buf, sz, &subpos, &bodypos, &sigpos);
>  		if (!subpos)
>  			return;
>  
>  		if (!strcmp(name, "subject"))
>  			v->s = copy_line(subpos);
>  		else if (!strcmp(name, "body"))
> -			v->s = xstrdup(bodypos);
> +			v->s = xstrndup(bodypos, sigpos - bodypos);
>  		else if (!strcmp(name, "contents"))
> -			v->s = xstrdup(subpos);
> +			v->s = xstrndup(subpos, sigpos - subpos);
> +		else if (!strcmp(name, "signature"))
> +			v->s = xstrdup(sigpos);
>  	}
>  }
>  

Re: [PATCH] git-for-each-ref: move GPG sigs off %(body) to %(signature).

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 1095 bytes --]

On Tue, 30 Aug 2011 11:43:44 +0200
Michael J Gruber <git@drmicha.warpmail.net> wrote:

> > When grabbing a %(body) or %(contents) off a tag, one doesn't really
> > expect to get the GPG signature as well (as it's basically useless
> > without the complete signed text). Thus, strip it off those two
> > tags, and make available via %(signature) if anyone needs it.
> 
> No, please do not change %(contents). It is the complete content which
> (together with the header) enters into the sha1 calculation.
> 
> You will probably also face opposition as regards to %(body), changing
> existing behaviour.
> 
> In fact, I wish we didn't have %(body) but %(contents:body) just like
> other modifiers such as :short.

Yeah, I was wondering why it is like that.

> I think I'd go for
> 
> %(contents:signature)
> 
> and implement
> 
> %(contents:subject) the same as %(subject)
> %(contents:body) as contents minus subject minus signature

How should I implement the ':' magic? Through adding a new type or just
putting ':' in place?

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 316 bytes --]

Re: [PATCH] git-for-each-ref: move GPG sigs off %(body) to %(signature).

[Jeff King]

On Tue, Aug 30, 2011 at 11:43:44AM +0200, Michael J Gruber wrote:

> > When grabbing a %(body) or %(contents) off a tag, one doesn't really
> > expect to get the GPG signature as well (as it's basically useless
> > without the complete signed text). Thus, strip it off those two tags,
> > and make available via %(signature) if anyone needs it.
> 
> No, please do not change %(contents). It is the complete content which
> (together with the header) enters into the sha1 calculation.
> 
> You will probably also face opposition as regards to %(body), changing
> existing behaviour.

Yeah. If it were 2005, I think it might make sense to have
non-overlapping individual keys to get each part of the tag. But it's
not worth breaking backwards compatibility of "%(body)" today. So it has
to remain as-is, and we have to introduce a new key for "the body
without the signature".

> In fact, I wish we didn't have %(body) but %(contents:body) just like
> other modifiers such as :short.
> 
> I think I'd go for
> 
> %(contents:signature)

That makes some sense to me, though it is a little weird that the
":signature" modifier works only for tags, and not commits. In other
cases, whole keys either work or don't work (e.g., "taggerdate"). But I
guess it is not that big a deal.

> %(contents:subject) the same as %(subject)
> %(contents:body) as contents minus subject minus signature
> 
> and slowly deprecate %(subject) and %(body) (simply un-document for now).

That leaves no way to get what "%(body)" provides now, right? I wonder
if anyone cares. You can always ask for:

  %(contents:body)%(contents:signature)

I guess.

> > +	/* look for GPG signature */
> 
> Again I have to say no. Please look at
> 
> 3d5854e (tag: recognize rfc1991 signatures, 2010-11-10)
> 
> which uses the factored out signature detection as introduced in the
> previous commits. Thanks!

Yeah. More correct, and it's less code, too. :)

-Peff

[PATCH] for-each-ref: add split message parts to %(contents:*).

[Michał Górny]

Now %(contents:subject) contains the message subject, %(contents:body)
main body part and %(contents:signature) GPG signature.
---
 Documentation/git-for-each-ref.txt |    7 ++++---
 builtin/for-each-ref.c             |   22 +++++++++++++++++-----
 2 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 152e695..c872b88 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -101,9 +101,10 @@ Fields that have name-email-date tuple as its value (`author`,
 `committer`, and `tagger`) can be suffixed with `name`, `email`,
 and `date` to extract the named component.
 
-The first line of the message in a commit and tag object is
-`subject`, the remaining lines are `body`.  The whole message
-is `contents`.
+The complete message in a commit and tag object is `contents`.
+Its first line is `contents:subject`, the remaining lines
+are `contents:body` and the optional GPG signature
+is `contents:signature`.
 
 For sorting purposes, fields with numeric values sort in numeric
 order (`objectsize`, `authordate`, `committerdate`, `taggerdate`).
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 89e75c6..4854ab4 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -69,6 +69,9 @@ static struct {
 	{ "subject" },
 	{ "body" },
 	{ "contents" },
+	{ "contents:subject" },
+	{ "contents:body" },
+	{ "contents:signature" },
 	{ "upstream" },
 	{ "symref" },
 	{ "flag" },
@@ -458,7 +461,7 @@ static void grab_person(const char *who, struct atom_value *val, int deref, stru
 	}
 }
 
-static void find_subpos(const char *buf, unsigned long sz, const char **sub, const char **body)
+static void find_subpos(const char *buf, unsigned long sz, const char **sub, const char **body, const char **signature)
 {
 	while (*buf) {
 		const char *eol = strchr(buf, '\n');
@@ -478,18 +481,20 @@ static void find_subpos(const char *buf, unsigned long sz, const char **sub, con
 	buf = strchr(buf, '\n');
 	if (!buf) {
 		*body = "";
+		*signature = *body;
 		return; /* no body */
 	}
 	while (*buf == '\n')
 		buf++; /* skip blank between subject and body */
 	*body = buf;
+	*signature = buf + parse_signature(buf, strlen(buf));
 }
 
 /* See grab_values */
 static void grab_sub_body_contents(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
 {
 	int i;
-	const char *subpos = NULL, *bodypos = NULL;
+	const char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;
 
 	for (i = 0; i < used_atom_cnt; i++) {
 		const char *name = used_atom[i];
@@ -500,19 +505,26 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
 			name++;
 		if (strcmp(name, "subject") &&
 		    strcmp(name, "body") &&
-		    strcmp(name, "contents"))
+		    strcmp(name, "contents") &&
+		    strcmp(name, "contents:subject") &&
+		    strcmp(name, "contents:body") &&
+		    strcmp(name, "contents:signature"))
 			continue;
 		if (!subpos)
-			find_subpos(buf, sz, &subpos, &bodypos);
+			find_subpos(buf, sz, &subpos, &bodypos, &sigpos);
 		if (!subpos)
 			return;
 
-		if (!strcmp(name, "subject"))
+		if (!strcmp(name, "subject") || !strcmp(name, "contents:subject"))
 			v->s = copy_line(subpos);
 		else if (!strcmp(name, "body"))
 			v->s = xstrdup(bodypos);
 		else if (!strcmp(name, "contents"))
 			v->s = xstrdup(subpos);
+		else if (!strcmp(name, "contents:body"))
+			v->s = xstrndup(bodypos, sigpos - bodypos);
+		else if (!strcmp(name, "contents:signature"))
+			v->s = xstrdup(sigpos);
 	}
 }
 
-- 
1.7.6.1

Re: [PATCH] for-each-ref: add split message parts to %(contents:*).

[Jeff King]

On Wed, Aug 31, 2011 at 11:11:49AM +0200, Michał Górny wrote:

> @@ -478,18 +481,20 @@ static void find_subpos(const char *buf, unsigned long sz, const char **sub, con
>  	buf = strchr(buf, '\n');
>  	if (!buf) {
>  		*body = "";
> +		*signature = *body;
>  		return; /* no body */
>  	}
>  	while (*buf == '\n')
>  		buf++; /* skip blank between subject and body */
>  	*body = buf;
> +	*signature = buf + parse_signature(buf, strlen(buf));

Hmm. I had doubts at first that "buf" is guaranteed to be
NUL-terminated, since we are passing around the "sz" parameter (though
note that we also use strchr already). But I think it is OK, as the
buffer comes from read_sha1_file, which defensively NUL-terminates all
objects we get.

Other than that, the patch looks fine. It could use some tests, so I'll
follow up with some:

  [1/2]: t7004: factor out gpg setup
  [2/2]: t6300: test new content:* for-each-ref placeholders

-Peff

[PATCH 1/2] t7004: factor out gpg setup

[Jeff King]

Other test scripts may want to look at or verify signed
tags, and the setup is non-trivial. Let's factor this out
into lib-gpg.sh for other tests to use.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/lib-gpg.sh                     |   26 ++++++++++++++++++++++++++
 t/{t7004 => lib-gpg}/pubring.gpg |  Bin 1164 -> 1164 bytes
 t/{t7004 => lib-gpg}/random_seed |  Bin 600 -> 600 bytes
 t/{t7004 => lib-gpg}/secring.gpg |  Bin 1237 -> 1237 bytes
 t/{t7004 => lib-gpg}/trustdb.gpg |  Bin 1280 -> 1280 bytes
 t/t7004-tag.sh                   |   29 +----------------------------
 6 files changed, 27 insertions(+), 28 deletions(-)
 create mode 100755 t/lib-gpg.sh
 rename t/{t7004 => lib-gpg}/pubring.gpg (100%)
 rename t/{t7004 => lib-gpg}/random_seed (100%)
 rename t/{t7004 => lib-gpg}/secring.gpg (100%)
 rename t/{t7004 => lib-gpg}/trustdb.gpg (100%)

diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
new file mode 100755
index 0000000..28463fb
--- /dev/null
+++ b/t/lib-gpg.sh
@@ -0,0 +1,26 @@
+#!/bin/sh
+
+gpg_version=`gpg --version 2>&1`
+if test $? = 127; then
+	say "You do not seem to have gpg installed"
+else
+	# As said here: http://www.gnupg.org/documentation/faqs.html#q6.19
+	# the gpg version 1.0.6 didn't parse trust packets correctly, so for
+	# that version, creation of signed tags using the generated key fails.
+	case "$gpg_version" in
+	'gpg (GnuPG) 1.0.6'*)
+		say "Your version of gpg (1.0.6) is too buggy for testing"
+		;;
+	*)
+		# key generation info: gpg --homedir t/lib-gpg --gen-key
+		# Type DSA and Elgamal, size 2048 bits, no expiration date.
+		# Name and email: C O Mitter <committer@example.com>
+		# No password given, to enable non-interactive operation.
+		cp -R "$TEST_DIRECTORY"/lib-gpg ./gpghome
+		chmod 0700 gpghome
+		GNUPGHOME="$(pwd)/gpghome"
+		export GNUPGHOME
+		test_set_prereq GPG
+		;;
+	esac
+fi
diff --git a/t/t7004/pubring.gpg b/t/lib-gpg/pubring.gpg
similarity index 100%
rename from t/t7004/pubring.gpg
rename to t/lib-gpg/pubring.gpg
diff --git a/t/t7004/random_seed b/t/lib-gpg/random_seed
similarity index 100%
rename from t/t7004/random_seed
rename to t/lib-gpg/random_seed
diff --git a/t/t7004/secring.gpg b/t/lib-gpg/secring.gpg
similarity index 100%
rename from t/t7004/secring.gpg
rename to t/lib-gpg/secring.gpg
diff --git a/t/t7004/trustdb.gpg b/t/lib-gpg/trustdb.gpg
similarity index 100%
rename from t/t7004/trustdb.gpg
rename to t/lib-gpg/trustdb.gpg
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 097ce2b..5922f43 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -8,6 +8,7 @@ test_description='git tag
 Tests for operations with tags.'
 
 . ./test-lib.sh
+. ../lib-gpg.sh
 
 # creating and listing lightweight tags:
 
@@ -585,24 +586,6 @@ test_expect_success \
 	test_cmp expect actual
 '
 
-# subsequent tests require gpg; check if it is available
-gpg --version >/dev/null 2>/dev/null
-if [ $? -eq 127 ]; then
-	say "# gpg not found - skipping tag signing and verification tests"
-else
-	# As said here: http://www.gnupg.org/documentation/faqs.html#q6.19
-	# the gpg version 1.0.6 didn't parse trust packets correctly, so for
-	# that version, creation of signed tags using the generated key fails.
-	case "$(gpg --version)" in
-	'gpg (GnuPG) 1.0.6'*)
-		say "Skipping signed tag tests, because a bug in 1.0.6 version"
-		;;
-	*)
-		test_set_prereq GPG
-		;;
-	esac
-fi
-
 # trying to verify annotated non-signed tags:
 
 test_expect_success GPG \
@@ -625,16 +608,6 @@ test_expect_success GPG \
 
 # creating and verifying signed tags:
 
-# key generation info: gpg --homedir t/t7004 --gen-key
-# Type DSA and Elgamal, size 2048 bits, no expiration date.
-# Name and email: C O Mitter <committer@example.com>
-# No password given, to enable non-interactive operation.
-
-cp -R "$TEST_DIRECTORY"/t7004 ./gpghome
-chmod 0700 gpghome
-GNUPGHOME="$(pwd)/gpghome"
-export GNUPGHOME
-
 get_tag_header signed-tag $commit commit $time >expect
 echo 'A signed tag message' >>expect
 echo '-----BEGIN PGP SIGNATURE-----' >>expect
-- 
1.7.6.10.g62f04

[PATCH 2/2] t6300: test new content:* for-each-ref placeholders

[Jeff King]

For commits and unsigned tags, these are the same as %(body)
and %(subject). This patch also introduces a signed tag and
checks for-each-ref's output.

These tests would fit better near the top of the script;
however, creating the extra tag throws off the output of
later tests. Those tests can't just be adjusted to the new
output, either, as the presence of the signed tag depends on
the GPG prerequisite.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/lib-gpg.sh            |    8 ++++++++
 t/t6300-for-each-ref.sh |   44 ++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index 28463fb..05824fa 100755
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -24,3 +24,11 @@ else
 		;;
 	esac
 fi
+
+sanitize_pgp() {
+	perl -ne '
+		/^-----END PGP/ and $in_pgp = 0;
+		print unless $in_pgp;
+		/^-----BEGIN PGP/ and $in_pgp = 1;
+	'
+}
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 7dc8a51..24f39de 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -6,6 +6,7 @@
 test_description='for-each-ref test'
 
 . ./test-lib.sh
+. ../lib-gpg.sh
 
 # Mon Jul 3 15:18:43 2006 +0000
 datestamp=1151939923
@@ -35,13 +36,15 @@ test_expect_success 'Create upstream config' '
 
 test_atom() {
 	case "$1" in
-		head) ref=refs/heads/master ;;
-		 tag) ref=refs/tags/testtag ;;
+		head) ref=refs/heads/master; prereq= ;;
+		 tag) ref=refs/tags/testtag; prereq= ;;
+	      signed) ref=refs/tags/signed-tag; prereq=GPG ;;
 	esac
 	printf '%s\n' "$3" >expected
-	test_expect_${4:-success} "basic atom: $1 $2" "
+	test_expect_${4:-success} $prereq "basic atom: $1 $2" "
 		git for-each-ref --format='%($2)' $ref >actual &&
-		test_cmp expected actual
+		sanitize_pgp <actual >actual.clean &&
+		test_cmp expected actual.clean
 	"
 }
 
@@ -71,7 +74,10 @@ test_atom head taggerdate ''
 test_atom head creator 'C O Mitter <committer@example.com> 1151939923 +0200'
 test_atom head creatordate 'Mon Jul 3 17:18:43 2006 +0200'
 test_atom head subject 'Initial'
+test_atom head contents:subject 'Initial'
 test_atom head body ''
+test_atom head contents:body ''
+test_atom head contents:signature ''
 test_atom head contents 'Initial
 '
 
@@ -101,7 +107,10 @@ test_atom tag taggerdate 'Mon Jul 3 17:18:45 2006 +0200'
 test_atom tag creator 'C O Mitter <committer@example.com> 1151939925 +0200'
 test_atom tag creatordate 'Mon Jul 3 17:18:45 2006 +0200'
 test_atom tag subject 'Tagging at 1151939927'
+test_atom tag contents:subject 'Tagging at 1151939927'
 test_atom tag body ''
+test_atom tag contents:body ''
+test_atom tag contents:signature ''
 test_atom tag contents 'Tagging at 1151939927
 '
 
@@ -359,4 +368,31 @@ test_expect_success 'an unusual tag with an incomplete line' '
 
 '
 
+test_expect_success GPG 'create signed tag' '
+	cat <<-\EOF |
+	subject line
+
+	body contents
+	EOF
+	git tag -s -F - signed-tag
+'
+
+test_atom signed contents 'subject line
+
+body contents
+-----BEGIN PGP SIGNATURE-----
+-----END PGP SIGNATURE-----
+'
+test_atom signed body 'body contents
+-----BEGIN PGP SIGNATURE-----
+-----END PGP SIGNATURE-----
+'
+test_atom signed contents:body 'body contents
+'
+test_atom signed contents:subject 'subject line'
+test_atom signed subject 'subject line'
+test_atom signed contents:signature '-----BEGIN PGP SIGNATURE-----
+-----END PGP SIGNATURE-----
+'
+
 test_done
-- 
1.7.6.10.g62f04

Re: [PATCH] for-each-ref: add split message parts to %(contents:*).

[Junio C Hamano]

Michał Górny <mgorny@gentoo.org> writes:

> Now %(contents:subject) contains the message subject, %(contents:body)
> main body part and %(contents:signature) GPG signature.
> ---

Please sign-off when submitting the final round of this patch.

> +The complete message in a commit and tag object is `contents`.
> +Its first line is `contents:subject`, the remaining lines
> +are `contents:body` and the optional GPG signature
> +is `contents:signature`.

To match the parsing of commit objects, I would prefer to see "subject" to
mean "the first paragraph" (usually the first line alone but that is
purely from convention), but that probably is a separate topic.

To paraphrase the last part of your sentence, if a tag is merely annotated
and not signed, contents:signature would be empty (I am just making sure
that I am reading the description correctly).

Is it possible to get %(contents) with a combination of these three new
variants, or does the calling script need to see if each part is empty and
decide where to place newlines?  IOW, a naïve attempt:

	--format='%(contents:subject)\n%(contents:body)\n%(contents:signature)'

is not equivalent to

	--format='%(contents)'

right?

> @@ -478,18 +481,20 @@ static void find_subpos(const char *buf, unsigned long sz, const char **sub, con
>  	buf = strchr(buf, '\n');
>  	if (!buf) {
>  		*body = "";
> +		*signature = *body;
>  		return; /* no body */
>  	}
>  	while (*buf == '\n')
>  		buf++; /* skip blank between subject and body */
>  	*body = buf;
> +	*signature = buf + parse_signature(buf, strlen(buf));

If there is no signature, parse_signature() would return (size_t) 0, no?
I suspect it may be easier for the caller if *signature pointed at the
terminating NUL at the end of the whole thing in such a case. Otherwise,
the caller that finds the buf and the signature points at the same address
cannot tell if the object was a signed tag with no message

	$ git tag -s -m "" empty-tag

or if it was an annotated but not signed tag.

Or better yet, you may even want to stuff NULL there if there is no signature
so that it is absolutely clear for the caller which case it is.

>  static void grab_sub_body_contents(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
>  {
>  	int i;
> -	const char *subpos = NULL, *bodypos = NULL;
> +	const char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;
>  
>  	for (i = 0; i < used_atom_cnt; i++) {
>  		const char *name = used_atom[i];
> @@ -500,19 +505,26 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
>  			name++;
>  		if (strcmp(name, "subject") &&
>  		    strcmp(name, "body") &&
> -		    strcmp(name, "contents"))
> +		    strcmp(name, "contents") &&
> +		    strcmp(name, "contents:subject") &&
> +		    strcmp(name, "contents:body") &&
> +		    strcmp(name, "contents:signature"))
>  			continue;
>  		if (!subpos)
> -			find_subpos(buf, sz, &subpos, &bodypos);
> +			find_subpos(buf, sz, &subpos, &bodypos, &sigpos);
>  		if (!subpos)
>  			return;
>  
> -		if (!strcmp(name, "subject"))
> +		if (!strcmp(name, "subject") || !strcmp(name, "contents:subject"))
>  			v->s = copy_line(subpos);
>  		else if (!strcmp(name, "body"))
>  			v->s = xstrdup(bodypos);
>  		else if (!strcmp(name, "contents"))
>  			v->s = xstrdup(subpos);
> +		else if (!strcmp(name, "contents:body"))
> +			v->s = xstrndup(bodypos, sigpos - bodypos);
> +		else if (!strcmp(name, "contents:signature"))
> +			v->s = xstrdup(sigpos);

Again, how does this work for an annotated but not signed tag?

Re: [PATCH] for-each-ref: add split message parts to %(contents:*).

[Jeff King]

On Wed, Aug 31, 2011 at 03:54:35PM -0700, Junio C Hamano wrote:

> > +The complete message in a commit and tag object is `contents`.
> > +Its first line is `contents:subject`, the remaining lines
> > +are `contents:body` and the optional GPG signature
> > +is `contents:signature`.
> 
> To match the parsing of commit objects, I would prefer to see "subject" to
> mean "the first paragraph" (usually the first line alone but that is
> purely from convention), but that probably is a separate topic.

Good idea. I suspect pretty.c:format_subject can be reused here.

> To paraphrase the last part of your sentence, if a tag is merely annotated
> and not signed, contents:signature would be empty (I am just making sure
> that I am reading the description correctly).

That is what I checked for in the tests I added.

> >  	while (*buf == '\n')
> >  		buf++; /* skip blank between subject and body */
> >  	*body = buf;
> > +	*signature = buf + parse_signature(buf, strlen(buf));
> 
> If there is no signature, parse_signature() would return (size_t) 0, no?

No, it returns strlen(buf) in that case, making signature the empty
string. It would perhaps better be called find_signature_in_body(),
since it is actually about parsing the rest of the body until we get to
the signature.

-Peff

Re: [PATCH] for-each-ref: add split message parts to %(contents:*).

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 805 bytes --]

On Wed, 31 Aug 2011 19:22:01 -0400
Jeff King <peff@peff.net> wrote:

> On Wed, Aug 31, 2011 at 03:54:35PM -0700, Junio C Hamano wrote:
> 
> > > +The complete message in a commit and tag object is `contents`.
> > > +Its first line is `contents:subject`, the remaining lines
> > > +are `contents:body` and the optional GPG signature
> > > +is `contents:signature`.
> > 
> > To match the parsing of commit objects, I would prefer to see
> > "subject" to mean "the first paragraph" (usually the first line
> > alone but that is purely from convention), but that probably is a
> > separate topic.
> 
> Good idea. I suspect pretty.c:format_subject can be reused here.

Should I fix regular 'subject' and 'body' as well, or just
the 'contents:' variants?

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 316 bytes --]

Re: [PATCH] for-each-ref: add split message parts to %(contents:*).

[Junio C Hamano]

Michał Górny <mgorny@gentoo.org> writes:

> On Wed, 31 Aug 2011 19:22:01 -0400
> Jeff King <peff@peff.net> wrote:
>
>> On Wed, Aug 31, 2011 at 03:54:35PM -0700, Junio C Hamano wrote:
>> 
>> > > +The complete message in a commit and tag object is `contents`.
>> > > +Its first line is `contents:subject`, the remaining lines
>> > > +are `contents:body` and the optional GPG signature
>> > > +is `contents:signature`.
>> > 
>> > To match the parsing of commit objects, I would prefer to see
>> > "subject" to mean "the first paragraph" (usually the first line
>> > alone but that is purely from convention), but that probably is a
>> > separate topic.
>> 
>> Good idea. I suspect pretty.c:format_subject can be reused here.
>
> Should I fix regular 'subject' and 'body' as well, or just
> the 'contents:' variants?

I thought you made them synonyms...

Re: [PATCH] for-each-ref: add split message parts to %(contents:*).

[Jeff King]

On Thu, Sep 01, 2011 at 09:00:39AM -0700, Junio C Hamano wrote:

> >> > To match the parsing of commit objects, I would prefer to see
> >> > "subject" to mean "the first paragraph" (usually the first line
> >> > alone but that is purely from convention), but that probably is a
> >> > separate topic.
> >> 
> >> Good idea. I suspect pretty.c:format_subject can be reused here.
> >
> > Should I fix regular 'subject' and 'body' as well, or just
> > the 'contents:' variants?
> 
> I thought you made them synonyms...

No, %(body) retains its historical usage as body+signature. If you think
it's OK to change that.

We could either leave %(subject) with its historical behavior, or fix it
to handle multi-line subjects. Although it's technically a regression to
change it, I tend to think it is simply a bug, as it doesn't match what
the rest of git (like "git log --format=%s") does.

-Peff

Re: [PATCH] for-each-ref: add split message parts to %(contents:*).

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 1341 bytes --]

On Thu, 1 Sep 2011 12:22:22 -0400
Jeff King <peff@peff.net> wrote:

> On Thu, Sep 01, 2011 at 09:00:39AM -0700, Junio C Hamano wrote:
> 
> > >> > To match the parsing of commit objects, I would prefer to see
> > >> > "subject" to mean "the first paragraph" (usually the first line
> > >> > alone but that is purely from convention), but that probably
> > >> > is a separate topic.
> > >> 
> > >> Good idea. I suspect pretty.c:format_subject can be reused here.
> > >
> > > Should I fix regular 'subject' and 'body' as well, or just
> > > the 'contents:' variants?
> > 
> > I thought you made them synonyms...
> 
> No, %(body) retains its historical usage as body+signature. If you
> think it's OK to change that.
> 
> We could either leave %(subject) with its historical behavior, or fix
> it to handle multi-line subjects. Although it's technically a
> regression to change it, I tend to think it is simply a bug, as it
> doesn't match what the rest of git (like "git log --format=%s") does.

Ok, I'll go with fixing it. If we want to have old behavior back, it's
as simple as putting the line copying function in the right place.

Sadly, I had to add a few magical '-1's and '+1's to get whitespace
in-place. It seems that signed, unannotated signatures glue to subject.

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 316 bytes --]

[PATCH] for-each-ref: add split message parts to %(contents:*).

[Michał Górny]

Now %(contents:subject) contains the message subject, %(contents:body)
main body part and %(contents:signature) GPG signature.
---
 Documentation/git-for-each-ref.txt |    7 +++--
 builtin/for-each-ref.c             |   42 ++++++++++++++++++++++++------------
 2 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 152e695..c872b88 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -101,9 +101,10 @@ Fields that have name-email-date tuple as its value (`author`,
 `committer`, and `tagger`) can be suffixed with `name`, `email`,
 and `date` to extract the named component.
 
-The first line of the message in a commit and tag object is
-`subject`, the remaining lines are `body`.  The whole message
-is `contents`.
+The complete message in a commit and tag object is `contents`.
+Its first line is `contents:subject`, the remaining lines
+are `contents:body` and the optional GPG signature
+is `contents:signature`.
 
 For sorting purposes, fields with numeric values sort in numeric
 order (`objectsize`, `authordate`, `committerdate`, `taggerdate`).
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 89e75c6..e320ba2 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -69,6 +69,9 @@ static struct {
 	{ "subject" },
 	{ "body" },
 	{ "contents" },
+	{ "contents:subject" },
+	{ "contents:body" },
+	{ "contents:signature" },
 	{ "upstream" },
 	{ "symref" },
 	{ "flag" },
@@ -458,8 +461,9 @@ static void grab_person(const char *who, struct atom_value *val, int deref, stru
 	}
 }
 
-static void find_subpos(const char *buf, unsigned long sz, const char **sub, const char **body)
+static void find_subpos(const char *buf, unsigned long sz, const char **sub, const char **body, const char **signature)
 {
+	*signature = buf + parse_signature(buf, sz);
 	while (*buf) {
 		const char *eol = strchr(buf, '\n');
 		if (!eol)
@@ -475,21 +479,21 @@ static void find_subpos(const char *buf, unsigned long sz, const char **sub, con
 	if (!*buf)
 		return;
 	*sub = buf; /* first non-empty line */
-	buf = strchr(buf, '\n');
-	if (!buf) {
-		*body = "";
-		return; /* no body */
-	}
-	while (*buf == '\n')
-		buf++; /* skip blank between subject and body */
-	*body = buf;
+	buf = format_subject(NULL, buf, NULL);
+
+	/* When having a signed tag without body, format_subject()
+	 * will start to eat the signature. */
+	if (buf > *signature)
+		*body = *signature;
+	else /* - 1 to get a trailing newline to strip */
+		*body = buf - 1;
 }
 
 /* See grab_values */
 static void grab_sub_body_contents(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
 {
 	int i;
-	const char *subpos = NULL, *bodypos = NULL;
+	const char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;
 
 	for (i = 0; i < used_atom_cnt; i++) {
 		const char *name = used_atom[i];
@@ -500,19 +504,29 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
 			name++;
 		if (strcmp(name, "subject") &&
 		    strcmp(name, "body") &&
-		    strcmp(name, "contents"))
+		    strcmp(name, "contents") &&
+		    strcmp(name, "contents:subject") &&
+		    strcmp(name, "contents:body") &&
+		    strcmp(name, "contents:signature"))
 			continue;
 		if (!subpos)
-			find_subpos(buf, sz, &subpos, &bodypos);
+			find_subpos(buf, sz, &subpos, &bodypos, &sigpos);
 		if (!subpos)
 			return;
 
-		if (!strcmp(name, "subject"))
-			v->s = copy_line(subpos);
+		if (!strcmp(name, "subject") || !strcmp(name, "contents:subject"))
+			v->s = xstrndup(subpos, bodypos - subpos - 1);
 		else if (!strcmp(name, "body"))
 			v->s = xstrdup(bodypos);
 		else if (!strcmp(name, "contents"))
 			v->s = xstrdup(subpos);
+		else if (!strcmp(name, "contents:body")) {
+			if (sigpos - bodypos > 0)
+				v->s = xstrndup(bodypos + 1, sigpos - bodypos - 1);
+			else
+				v->s = xstrdup("");
+		} else if (!strcmp(name, "contents:signature"))
+			v->s = xstrdup(sigpos);
 	}
 }
 
-- 
1.7.6.1

Re: [PATCH] for-each-ref: add split message parts to %(contents:*).

[Jeff King]

On Thu, Sep 01, 2011 at 06:50:01PM +0200, Michał Górny wrote:

> -	buf = strchr(buf, '\n');
> -	if (!buf) {
> -		*body = "";
> -		return; /* no body */
> -	}
> -	while (*buf == '\n')
> -		buf++; /* skip blank between subject and body */
> -	*body = buf;
> +	buf = format_subject(NULL, buf, NULL);
> +
> +	/* When having a signed tag without body, format_subject()
> +	 * will start to eat the signature. */
> +	if (buf > *signature)
> +		*body = *signature;
> +	else /* - 1 to get a trailing newline to strip */
> +		*body = buf - 1;

This last line is wrong if there is no trailing newline, no? Running
even the existing tests in t6300 against your new patch, I get:

expecting success:
  git for-each-ref --format='%(subject)' refs/heads/master >actual &&
  test_cmp expected actual

--- expected    2011-09-02 16:36:38.306058729 +0000
+++ actual      2011-09-02 16:36:38.318058729 +0000
@@ -1 +1 @@
-Initial
+Initia
not ok - 28 basic atom: head subject

-Peff

Re: [PATCH] for-each-ref: add split message parts to %(contents:*).

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 1213 bytes --]

On Fri, 2 Sep 2011 12:39:03 -0400
Jeff King <peff@peff.net> wrote:

> On Thu, Sep 01, 2011 at 06:50:01PM +0200, Michał Górny wrote:
> 
> > -	buf = strchr(buf, '\n');
> > -	if (!buf) {
> > -		*body = "";
> > -		return; /* no body */
> > -	}
> > -	while (*buf == '\n')
> > -		buf++; /* skip blank between subject and body */
> > -	*body = buf;
> > +	buf = format_subject(NULL, buf, NULL);
> > +
> > +	/* When having a signed tag without body, format_subject()
> > +	 * will start to eat the signature. */
> > +	if (buf > *signature)
> > +		*body = *signature;
> > +	else /* - 1 to get a trailing newline to strip */
> > +		*body = buf - 1;
> 
> This last line is wrong if there is no trailing newline, no? Running
> even the existing tests in t6300 against your new patch, I get:
> 
> expecting success:
>   git for-each-ref --format='%(subject)' refs/heads/master >actual &&
>   test_cmp expected actual
> 
> --- expected    2011-09-02 16:36:38.306058729 +0000
> +++ actual      2011-09-02 16:36:38.318058729 +0000
> @@ -1 +1 @@
> -Initial
> +Initia
> not ok - 28 basic atom: head subject

Any suggestion how to strip trailing newlines?

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 316 bytes --]

Re: [PATCH] for-each-ref: add split message parts to %(contents:*).

[Jeff King]

On Fri, Sep 02, 2011 at 07:39:26PM +0200, Michał Górny wrote:

> > > +	/* When having a signed tag without body, format_subject()
> > > +	 * will start to eat the signature. */
> > > +	if (buf > *signature)
> > > +		*body = *signature;
> > > +	else /* - 1 to get a trailing newline to strip */
> > > +		*body = buf - 1;
> > 
> > This last line is wrong if there is no trailing newline, no? Running
> > even the existing tests in t6300 against your new patch, I get:
> [...]
> 
> Any suggestion how to strip trailing newlines?

Just looking at your patch, it might work to do:

  else if (*buf == '\n')
          *body = buf - 1;
  else
          *body = buf;

But there may be other corner cases.  I need to read through the code
more carefully, which I should have time to do later today.

-Peff

Re: [PATCH] for-each-ref: add split message parts to %(contents:*).

[Jeff King]

On Fri, Sep 02, 2011 at 01:53:23PM -0400, Jeff King wrote:

> But there may be other corner cases.  I need to read through the code
> more carefully, which I should have time to do later today.

This ended up a little trickier than I expected, but I think the series
below is what we should do. I tried to add extensive tests, but let me
know if you can think of any other corner cases.

  [1/5]: t7004: factor out gpg setup
  [2/5]: t6300: add more body-parsing tests
  [3/5]: for-each-ref: refactor subject and body placeholder parsing
  [4/5]: for-each-ref: handle multiline subjects like --pretty
  [5/5]: for-each-ref: add split message parts to %(contents:*).

-Peff

Re: [PATCH] for-each-ref: add split message parts to %(contents:*).

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 918 bytes --]

On Wed, 7 Sep 2011 13:40:44 -0400
Jeff King <peff@peff.net> wrote:

> On Fri, Sep 02, 2011 at 01:53:23PM -0400, Jeff King wrote:
> 
> > But there may be other corner cases.  I need to read through the
> > code more carefully, which I should have time to do later today.
> 
> This ended up a little trickier than I expected, but I think the
> series below is what we should do. I tried to add extensive tests,
> but let me know if you can think of any other corner cases.
> 
>   [1/5]: t7004: factor out gpg setup
>   [2/5]: t6300: add more body-parsing tests
>   [3/5]: for-each-ref: refactor subject and body placeholder parsing
>   [4/5]: for-each-ref: handle multiline subjects like --pretty
>   [5/5]: for-each-ref: add split message parts to %(contents:*).

Thanks, it works great for me.

I tried multi-line subject + signature too and that works fine.

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 316 bytes --]

[PATCH 1/5] t7004: factor out gpg setup

[Jeff King]

Other test scripts may want to look at or verify signed
tags, and the setup is non-trivial. Let's factor this out
into lib-gpg.sh for other tests to use.

Signed-off-by: Jeff King <peff@peff.net>
---
Similar to the one I sent out earlier, but in that one I accidentally
did:

  . ../lib-gpg.sh

which is not right. If --root is used for the test script, then ".." is
not necessarily the right place to find the lib-gpg script. This fixes
it to use $TEST_DIRECTORY.

 t/lib-gpg.sh                     |   26 ++++++++++++++++++++++++++
 t/{t7004 => lib-gpg}/pubring.gpg |  Bin 1164 -> 1164 bytes
 t/{t7004 => lib-gpg}/random_seed |  Bin 600 -> 600 bytes
 t/{t7004 => lib-gpg}/secring.gpg |  Bin 1237 -> 1237 bytes
 t/{t7004 => lib-gpg}/trustdb.gpg |  Bin 1280 -> 1280 bytes
 t/t7004-tag.sh                   |   29 +----------------------------
 6 files changed, 27 insertions(+), 28 deletions(-)
 create mode 100755 t/lib-gpg.sh
 rename t/{t7004 => lib-gpg}/pubring.gpg (100%)
 rename t/{t7004 => lib-gpg}/random_seed (100%)
 rename t/{t7004 => lib-gpg}/secring.gpg (100%)
 rename t/{t7004 => lib-gpg}/trustdb.gpg (100%)

diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
new file mode 100755
index 0000000..28463fb
--- /dev/null
+++ b/t/lib-gpg.sh
@@ -0,0 +1,26 @@
+#!/bin/sh
+
+gpg_version=`gpg --version 2>&1`
+if test $? = 127; then
+	say "You do not seem to have gpg installed"
+else
+	# As said here: http://www.gnupg.org/documentation/faqs.html#q6.19
+	# the gpg version 1.0.6 didn't parse trust packets correctly, so for
+	# that version, creation of signed tags using the generated key fails.
+	case "$gpg_version" in
+	'gpg (GnuPG) 1.0.6'*)
+		say "Your version of gpg (1.0.6) is too buggy for testing"
+		;;
+	*)
+		# key generation info: gpg --homedir t/lib-gpg --gen-key
+		# Type DSA and Elgamal, size 2048 bits, no expiration date.
+		# Name and email: C O Mitter <committer@example.com>
+		# No password given, to enable non-interactive operation.
+		cp -R "$TEST_DIRECTORY"/lib-gpg ./gpghome
+		chmod 0700 gpghome
+		GNUPGHOME="$(pwd)/gpghome"
+		export GNUPGHOME
+		test_set_prereq GPG
+		;;
+	esac
+fi
diff --git a/t/t7004/pubring.gpg b/t/lib-gpg/pubring.gpg
similarity index 100%
rename from t/t7004/pubring.gpg
rename to t/lib-gpg/pubring.gpg
diff --git a/t/t7004/random_seed b/t/lib-gpg/random_seed
similarity index 100%
rename from t/t7004/random_seed
rename to t/lib-gpg/random_seed
diff --git a/t/t7004/secring.gpg b/t/lib-gpg/secring.gpg
similarity index 100%
rename from t/t7004/secring.gpg
rename to t/lib-gpg/secring.gpg
diff --git a/t/t7004/trustdb.gpg b/t/lib-gpg/trustdb.gpg
similarity index 100%
rename from t/t7004/trustdb.gpg
rename to t/lib-gpg/trustdb.gpg
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 097ce2b..e93ac73 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -8,6 +8,7 @@ test_description='git tag
 Tests for operations with tags.'
 
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-gpg.sh
 
 # creating and listing lightweight tags:
 
@@ -585,24 +586,6 @@ test_expect_success \
 	test_cmp expect actual
 '
 
-# subsequent tests require gpg; check if it is available
-gpg --version >/dev/null 2>/dev/null
-if [ $? -eq 127 ]; then
-	say "# gpg not found - skipping tag signing and verification tests"
-else
-	# As said here: http://www.gnupg.org/documentation/faqs.html#q6.19
-	# the gpg version 1.0.6 didn't parse trust packets correctly, so for
-	# that version, creation of signed tags using the generated key fails.
-	case "$(gpg --version)" in
-	'gpg (GnuPG) 1.0.6'*)
-		say "Skipping signed tag tests, because a bug in 1.0.6 version"
-		;;
-	*)
-		test_set_prereq GPG
-		;;
-	esac
-fi
-
 # trying to verify annotated non-signed tags:
 
 test_expect_success GPG \
@@ -625,16 +608,6 @@ test_expect_success GPG \
 
 # creating and verifying signed tags:
 
-# key generation info: gpg --homedir t/t7004 --gen-key
-# Type DSA and Elgamal, size 2048 bits, no expiration date.
-# Name and email: C O Mitter <committer@example.com>
-# No password given, to enable non-interactive operation.
-
-cp -R "$TEST_DIRECTORY"/t7004 ./gpghome
-chmod 0700 gpghome
-GNUPGHOME="$(pwd)/gpghome"
-export GNUPGHOME
-
 get_tag_header signed-tag $commit commit $time >expect
 echo 'A signed tag message' >>expect
 echo '-----BEGIN PGP SIGNATURE-----' >>expect
-- 
1.7.6.10.g62f04

[PATCH 2/5] t6300: add more body-parsing tests

[Jeff King]

The current tests don't actually check parsing commit and
tag messages that have both a subject and a body (they just
have single-line messages).

Signed-off-by: Jeff King <peff@peff.net>
---
This is mostly to help make sure the next patch doesn't regress this
case.

 t/t6300-for-each-ref.sh |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 7dc8a51..6fa4d52 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -37,6 +37,7 @@ test_atom() {
 	case "$1" in
 		head) ref=refs/heads/master ;;
 		 tag) ref=refs/tags/testtag ;;
+		   *) ref=$1 ;;
 	esac
 	printf '%s\n' "$3" >expected
 	test_expect_${4:-success} "basic atom: $1 $2" "
@@ -359,4 +360,23 @@ test_expect_success 'an unusual tag with an incomplete line' '
 
 '
 
+test_expect_success 'create tag with subject and body content' '
+	cat >>msg <<-\EOF &&
+		the subject line
+
+		first body line
+		second body line
+	EOF
+	git tag -F msg subject-body
+'
+test_atom refs/tags/subject-body subject 'the subject line'
+test_atom refs/tags/subject-body body 'first body line
+second body line
+'
+test_atom refs/tags/subject-body contents 'the subject line
+
+first body line
+second body line
+'
+
 test_done
-- 
1.7.6.10.g62f04

[PATCH 3/5] for-each-ref: refactor subject and body placeholder parsing

[Jeff King]

The find_subpos function was a little hard to use, as well
as to read. It would sometimes write into the subject and
body pointers, and sometimes not. The body pointer sometimes
could be compared to subject, and sometimes not. When
actually duplicating the subject, the caller was forced to
figure out again how long the subject is (which is not too
big a deal when the subject is a single line, but hard to
extend).

The refactoring makes the function more straightforward, both
to read and to use. We will always put something into the
subject and body pointers, and we return explicit lengths
for them, too.

This lays the groundwork both for more complex subject
parsing (e.g., multiline), as well as splitting the body
into subparts (like the text versus the signature).

Signed-off-by: Jeff King <peff@peff.net>
---
Sorry, the patch is a little bit hard to read. It's probably simpler to
just apply and read the resulting function.

 builtin/for-each-ref.c |   54 +++++++++++++++++++++++++----------------------
 1 files changed, 29 insertions(+), 25 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 89e75c6..bcea027 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -458,38 +458,42 @@ static void grab_person(const char *who, struct atom_value *val, int deref, stru
 	}
 }
 
-static void find_subpos(const char *buf, unsigned long sz, const char **sub, const char **body)
+static void find_subpos(const char *buf, unsigned long sz,
+			const char **sub, unsigned long *sublen,
+			const char **body, unsigned long *bodylen)
 {
-	while (*buf) {
-		const char *eol = strchr(buf, '\n');
-		if (!eol)
-			return;
-		if (eol[1] == '\n') {
-			buf = eol + 1;
-			break; /* found end of header */
-		}
-		buf = eol + 1;
+	const char *eol;
+	/* skip past header until we hit empty line */
+	while (*buf && *buf != '\n') {
+		eol = strchrnul(buf, '\n');
+		if (*eol)
+			eol++;
+		buf = eol;
 	}
+	/* skip any empty lines */
 	while (*buf == '\n')
 		buf++;
-	if (!*buf)
-		return;
-	*sub = buf; /* first non-empty line */
-	buf = strchr(buf, '\n');
-	if (!buf) {
-		*body = "";
-		return; /* no body */
-	}
+
+	/* subject is first non-empty line */
+	*sub = buf;
+	/* subject goes to end of line */
+	eol = strchrnul(buf, '\n');
+	*sublen = eol - buf;
+	buf = eol;
+
+	/* skip any empty lines */
 	while (*buf == '\n')
-		buf++; /* skip blank between subject and body */
+		buf++;
 	*body = buf;
+	*bodylen = strlen(buf);
 }
 
 /* See grab_values */
 static void grab_sub_body_contents(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
 {
 	int i;
-	const char *subpos = NULL, *bodypos = NULL;
+	const char *subpos = NULL, *bodypos;
+	unsigned long sublen, bodylen;
 
 	for (i = 0; i < used_atom_cnt; i++) {
 		const char *name = used_atom[i];
@@ -503,14 +507,14 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
 		    strcmp(name, "contents"))
 			continue;
 		if (!subpos)
-			find_subpos(buf, sz, &subpos, &bodypos);
-		if (!subpos)
-			return;
+			find_subpos(buf, sz,
+				    &subpos, &sublen,
+				    &bodypos, &bodylen);
 
 		if (!strcmp(name, "subject"))
-			v->s = copy_line(subpos);
+			v->s = xmemdupz(subpos, sublen);
 		else if (!strcmp(name, "body"))
-			v->s = xstrdup(bodypos);
+			v->s = xmemdupz(bodypos, bodylen);
 		else if (!strcmp(name, "contents"))
 			v->s = xstrdup(subpos);
 	}
-- 
1.7.6.10.g62f04

[PATCH 4/5] for-each-ref: handle multiline subjects like --pretty

[Jeff King]

Generally the format of a git tag or commit message is:

  subject

  body body body
  body body body

However, we occasionally see multiline subjects like:

  subject
  with multiple
  lines

  body body body
  body body body

The rest of git treats these multiline subjects as something
to be concatenated and shown as a single line (e.g., "git
log --pretty=format:%s" will do so since f53bd74). For
consistency, for-each-ref should do the same with its
"%(subject)".

Signed-off-by: Jeff King <peff@peff.net>
---
I split this out from the signature patch to make it more obvious what's
going on.

 builtin/for-each-ref.c  |   29 ++++++++++++++++++++++++-----
 t/t6300-for-each-ref.sh |   21 +++++++++++++++++++++
 2 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index bcea027..ea2112b 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -361,6 +361,18 @@ static const char *copy_email(const char *buf)
 	return xmemdupz(email, eoemail + 1 - email);
 }
 
+static char *copy_subject(const char *buf, unsigned long len)
+{
+	char *r = xmemdupz(buf, len);
+	int i;
+
+	for (i = 0; i < len; i++)
+		if (r[i] == '\n')
+			r[i] = ' ';
+
+	return r;
+}
+
 static void grab_date(const char *buf, struct atom_value *v, const char *atomname)
 {
 	const char *eoemail = strstr(buf, "> ");
@@ -476,10 +488,17 @@ static void find_subpos(const char *buf, unsigned long sz,
 
 	/* subject is first non-empty line */
 	*sub = buf;
-	/* subject goes to end of line */
-	eol = strchrnul(buf, '\n');
-	*sublen = eol - buf;
-	buf = eol;
+	/* subject goes to first empty line */
+	while (*buf && *buf != '\n') {
+		eol = strchrnul(buf, '\n');
+		if (*eol)
+			eol++;
+		buf = eol;
+	}
+	*sublen = buf - *sub;
+	/* drop trailing newline, if present */
+	if (*sublen && (*sub)[*sublen - 1] == '\n')
+		*sublen -= 1;
 
 	/* skip any empty lines */
 	while (*buf == '\n')
@@ -512,7 +531,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
 				    &bodypos, &bodylen);
 
 		if (!strcmp(name, "subject"))
-			v->s = xmemdupz(subpos, sublen);
+			v->s = copy_subject(subpos, sublen);
 		else if (!strcmp(name, "body"))
 			v->s = xmemdupz(bodypos, bodylen);
 		else if (!strcmp(name, "contents"))
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 6fa4d52..0c9ff96 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -379,4 +379,25 @@ first body line
 second body line
 '
 
+test_expect_success 'create tag with multiline subject' '
+	cat >msg <<-\EOF &&
+		first subject line
+		second subject line
+
+		first body line
+		second body line
+	EOF
+	git tag -F msg multiline
+'
+test_atom refs/tags/multiline subject 'first subject line second subject line'
+test_atom refs/tags/multiline body 'first body line
+second body line
+'
+test_atom refs/tags/multiline contents 'first subject line
+second subject line
+
+first body line
+second body line
+'
+
 test_done
-- 
1.7.6.10.g62f04

[PATCH 5/5] for-each-ref: add split message parts to %(contents:*).

[Jeff King]

From: Michał Górny <mgorny@gentoo.org>

The %(body) placeholder returns the whole body of a tag or
commit, including the signature. However, callers may want
to get just the body without signature, or just the
signature.

Rather than change the meaning of %(body), which might break
some scripts, this patch introduces a new set of
placeholders which break down the %(contents) placeholder
into its constituent parts.

[jk: initial patch by mg, rebased on top of my refactoring
and with tests by me]

Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/git-for-each-ref.txt |    7 ++--
 builtin/for-each-ref.c             |   32 +++++++++++++++---
 t/lib-gpg.sh                       |    8 +++++
 t/t6300-for-each-ref.sh            |   60 ++++++++++++++++++++++++++++++++++-
 4 files changed, 96 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 152e695..c872b88 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -101,9 +101,10 @@ Fields that have name-email-date tuple as its value (`author`,
 `committer`, and `tagger`) can be suffixed with `name`, `email`,
 and `date` to extract the named component.
 
-The first line of the message in a commit and tag object is
-`subject`, the remaining lines are `body`.  The whole message
-is `contents`.
+The complete message in a commit and tag object is `contents`.
+Its first line is `contents:subject`, the remaining lines
+are `contents:body` and the optional GPG signature
+is `contents:signature`.
 
 For sorting purposes, fields with numeric values sort in numeric
 order (`objectsize`, `authordate`, `committerdate`, `taggerdate`).
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index ea2112b..50fba65 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -69,6 +69,9 @@ static struct {
 	{ "subject" },
 	{ "body" },
 	{ "contents" },
+	{ "contents:subject" },
+	{ "contents:body" },
+	{ "contents:signature" },
 	{ "upstream" },
 	{ "symref" },
 	{ "flag" },
@@ -472,7 +475,9 @@ static void grab_person(const char *who, struct atom_value *val, int deref, stru
 
 static void find_subpos(const char *buf, unsigned long sz,
 			const char **sub, unsigned long *sublen,
-			const char **body, unsigned long *bodylen)
+			const char **body, unsigned long *bodylen,
+			unsigned long *nonsiglen,
+			const char **sig, unsigned long *siglen)
 {
 	const char *eol;
 	/* skip past header until we hit empty line */
@@ -486,10 +491,14 @@ static void find_subpos(const char *buf, unsigned long sz,
 	while (*buf == '\n')
 		buf++;
 
+	/* parse signature first; we might not even have a subject line */
+	*sig = buf + parse_signature(buf, strlen(buf));
+	*siglen = strlen(*sig);
+
 	/* subject is first non-empty line */
 	*sub = buf;
 	/* subject goes to first empty line */
-	while (*buf && *buf != '\n') {
+	while (buf < *sig && *buf && *buf != '\n') {
 		eol = strchrnul(buf, '\n');
 		if (*eol)
 			eol++;
@@ -505,14 +514,15 @@ static void find_subpos(const char *buf, unsigned long sz,
 		buf++;
 	*body = buf;
 	*bodylen = strlen(buf);
+	*nonsiglen = *sig - buf;
 }
 
 /* See grab_values */
 static void grab_sub_body_contents(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
 {
 	int i;
-	const char *subpos = NULL, *bodypos;
-	unsigned long sublen, bodylen;
+	const char *subpos = NULL, *bodypos, *sigpos;
+	unsigned long sublen, bodylen, nonsiglen, siglen;
 
 	for (i = 0; i < used_atom_cnt; i++) {
 		const char *name = used_atom[i];
@@ -523,17 +533,27 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
 			name++;
 		if (strcmp(name, "subject") &&
 		    strcmp(name, "body") &&
-		    strcmp(name, "contents"))
+		    strcmp(name, "contents") &&
+		    strcmp(name, "contents:subject") &&
+		    strcmp(name, "contents:body") &&
+		    strcmp(name, "contents:signature"))
 			continue;
 		if (!subpos)
 			find_subpos(buf, sz,
 				    &subpos, &sublen,
-				    &bodypos, &bodylen);
+				    &bodypos, &bodylen, &nonsiglen,
+				    &sigpos, &siglen);
 
 		if (!strcmp(name, "subject"))
 			v->s = copy_subject(subpos, sublen);
+		else if (!strcmp(name, "contents:subject"))
+			v->s = copy_subject(subpos, sublen);
 		else if (!strcmp(name, "body"))
 			v->s = xmemdupz(bodypos, bodylen);
+		else if (!strcmp(name, "contents:body"))
+			v->s = xmemdupz(bodypos, nonsiglen);
+		else if (!strcmp(name, "contents:signature"))
+			v->s = xmemdupz(sigpos, siglen);
 		else if (!strcmp(name, "contents"))
 			v->s = xstrdup(subpos);
 	}
diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index 28463fb..05824fa 100755
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -24,3 +24,11 @@ else
 		;;
 	esac
 fi
+
+sanitize_pgp() {
+	perl -ne '
+		/^-----END PGP/ and $in_pgp = 0;
+		print unless $in_pgp;
+		/^-----BEGIN PGP/ and $in_pgp = 1;
+	'
+}
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 0c9ff96..1721784 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -6,6 +6,7 @@
 test_description='for-each-ref test'
 
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-gpg.sh
 
 # Mon Jul 3 15:18:43 2006 +0000
 datestamp=1151939923
@@ -40,9 +41,10 @@ test_atom() {
 		   *) ref=$1 ;;
 	esac
 	printf '%s\n' "$3" >expected
-	test_expect_${4:-success} "basic atom: $1 $2" "
+	test_expect_${4:-success} $PREREQ "basic atom: $1 $2" "
 		git for-each-ref --format='%($2)' $ref >actual &&
-		test_cmp expected actual
+		sanitize_pgp <actual >actual.clean &&
+		test_cmp expected actual.clean
 	"
 }
 
@@ -72,7 +74,10 @@ test_atom head taggerdate ''
 test_atom head creator 'C O Mitter <committer@example.com> 1151939923 +0200'
 test_atom head creatordate 'Mon Jul 3 17:18:43 2006 +0200'
 test_atom head subject 'Initial'
+test_atom head contents:subject 'Initial'
 test_atom head body ''
+test_atom head contents:body ''
+test_atom head contents:signature ''
 test_atom head contents 'Initial
 '
 
@@ -102,7 +107,10 @@ test_atom tag taggerdate 'Mon Jul 3 17:18:45 2006 +0200'
 test_atom tag creator 'C O Mitter <committer@example.com> 1151939925 +0200'
 test_atom tag creatordate 'Mon Jul 3 17:18:45 2006 +0200'
 test_atom tag subject 'Tagging at 1151939927'
+test_atom tag contents:subject 'Tagging at 1151939927'
 test_atom tag body ''
+test_atom tag contents:body ''
+test_atom tag contents:signature ''
 test_atom tag contents 'Tagging at 1151939927
 '
 
@@ -390,9 +398,14 @@ test_expect_success 'create tag with multiline subject' '
 	git tag -F msg multiline
 '
 test_atom refs/tags/multiline subject 'first subject line second subject line'
+test_atom refs/tags/multiline contents:subject 'first subject line second subject line'
 test_atom refs/tags/multiline body 'first body line
 second body line
 '
+test_atom refs/tags/multiline contents:body 'first body line
+second body line
+'
+test_atom refs/tags/multiline contents:signature ''
 test_atom refs/tags/multiline contents 'first subject line
 second subject line
 
@@ -400,4 +413,47 @@ first body line
 second body line
 '
 
+test_expect_success GPG 'create signed tags' '
+	git tag -s -m "" signed-empty &&
+	git tag -s -m "subject line" signed-short &&
+	cat >msg <<-\EOF &&
+	subject line
+
+	body contents
+	EOF
+	git tag -s -F msg signed-long
+'
+
+sig='-----BEGIN PGP SIGNATURE-----
+-----END PGP SIGNATURE-----
+'
+
+PREREQ=GPG
+test_atom refs/tags/signed-empty subject ''
+test_atom refs/tags/signed-empty contents:subject ''
+test_atom refs/tags/signed-empty body "$sig"
+test_atom refs/tags/signed-empty contents:body ''
+test_atom refs/tags/signed-empty contents:signature "$sig"
+test_atom refs/tags/signed-empty contents "$sig"
+
+test_atom refs/tags/signed-short subject 'subject line'
+test_atom refs/tags/signed-short contents:subject 'subject line'
+test_atom refs/tags/signed-short body "$sig"
+test_atom refs/tags/signed-short contents:body ''
+test_atom refs/tags/signed-short contents:signature "$sig"
+test_atom refs/tags/signed-short contents "subject line
+$sig"
+
+test_atom refs/tags/signed-long subject 'subject line'
+test_atom refs/tags/signed-long contents:subject 'subject line'
+test_atom refs/tags/signed-long body "body contents
+$sig"
+test_atom refs/tags/signed-long contents:body 'body contents
+'
+test_atom refs/tags/signed-long contents:signature "$sig"
+test_atom refs/tags/signed-long contents "subject line
+
+body contents
+$sig"
+
 test_done
-- 
1.7.6.10.g62f04

Re: [PATCH] for-each-ref: add split message parts to %(contents:*).

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> We could either leave %(subject) with its historical behavior, or fix it
> to handle multi-line subjects. Although it's technically a regression to
> change it, I tend to think it is simply a bug, as it doesn't match what
> the rest of git (like "git log --format=%s") does.

I think %(subject) should be updated to match %(contents:subject) as a
bugfix, so %(body) should be adjusted to prevent "%(subject)%(body)" from
duplicating lines.

Thanks.

Side note. We probably would want to borrow from pretty-formats to allow
us to say %(subject)%(+body) or something...

Re: [PATCH] for-each-ref: add split message parts to %(contents:*).

[Jeff King]

On Thu, Sep 01, 2011 at 10:16:29AM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > We could either leave %(subject) with its historical behavior, or fix it
> > to handle multi-line subjects. Although it's technically a regression to
> > change it, I tend to think it is simply a bug, as it doesn't match what
> > the rest of git (like "git log --format=%s") does.
> 
> I think %(subject) should be updated to match %(contents:subject) as a
> bugfix, so %(body) should be adjusted to prevent "%(subject)%(body)" from
> duplicating lines.

OK. That makes it much easier to implement, too, I think. :)

> Side note. We probably would want to borrow from pretty-formats to allow
> us to say %(subject)%(+body) or something...

I have been meaning to take a closer look at Will Palmer's patches for
making the pretty-formats look more like the for-each-ref formats (which
I think would be the first step to unifying the features). But somehow
months have slipped by, and I haven't yet.

But I think unifying the formats and the code is better than trying to
port features across.

-Peff