threads / discuss / 28250

More formatting with 'git tag -l'

Subject: More formatting with 'git tag -l'

## tl;dr

32 messages between Aug 29, 2011 and Sep 15, 2011.

replies: 31people: 4as markdown or json

Michał Górny· Aug 29, 2011, 19:10 UTC · lore
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
Jeff King· Aug 29, 2011, 19:36 UTC · re: Michał Górny · lore

Re: More formatting with 'git tag -l'

On Mon, Aug 29, 2011 at 09:10:18PM +0200, Michał Górny wrote:
Show 9 quoted lines
> 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
Michał Górny· Aug 29, 2011, 21:20 UTC · re: Jeff King · lore

Re: More formatting with 'git tag -l'

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

Show 16 quoted lines
> 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
Jeff King· Aug 29, 2011, 21:37 UTC · re: Michał Górny · lore

Re: More formatting with 'git tag -l'

On Mon, Aug 29, 2011 at 11:20:11PM +0200, Michał Górny wrote:
Show 8 quoted lines
> > 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
Michał Górny· Aug 29, 2011, 21:50 UTC · re: Jeff King · lore

Re: More formatting with 'git tag -l'

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

Show 16 quoted lines
> 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
Michał Górny· Aug 30, 2011, 08:57 UTC · re: Jeff King · lore

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

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
Michael J Gruber· Aug 30, 2011, 09:43 UTC · re: Michał Górny · lore

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

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:

Show 64 quoted lines
> 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!

Show 50 quoted lines
> +	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);
>  	}
>  }
>  
Michał Górny· Aug 30, 2011, 15:58 UTC · re: Michael J Gruber · lore

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

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

Show 13 quoted lines
> > 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.
Show 8 quoted lines
> 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
Jeff King· Aug 30, 2011, 16:27 UTC · re: Michael J Gruber · lore

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

On Tue, Aug 30, 2011 at 11:43:44AM +0200, Michael J Gruber wrote:
Show 10 quoted lines
> > 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".

Show 6 quoted lines
> 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.
Show 8 quoted lines
> > +	/* 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
Michał Górny· Aug 31, 2011, 09:11 UTC · re: Michael J Gruber · lore

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

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
Jeff King· Aug 31, 2011, 16:42 UTC · re: Michał Górny · lore

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

On Wed, Aug 31, 2011 at 11:11:49AM +0200, Michał Górny wrote:
Show 11 quoted lines
> @@ -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
Jeff King· Aug 31, 2011, 16:44 UTC · re: Jeff King · lore

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

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
Jeff King· Aug 31, 2011, 16:44 UTC · re: Jeff King · lore

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

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
Junio C Hamano· Aug 31, 2011, 22:54 UTC · re: Michał Górny · lore

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

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?
Show 11 quoted lines
> @@ -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.

Show 35 quoted lines
>  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?
Jeff King· Aug 31, 2011, 23:22 UTC · re: Junio C Hamano · lore

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

On Wed, Aug 31, 2011 at 03:54:35PM -0700, Junio C Hamano wrote:
Show 8 quoted lines
> > +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.
Show 6 quoted lines
> >  	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
Michał Górny· Sep 1, 2011, 07:34 UTC · re: Jeff King · lore

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

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

Show 13 quoted lines
> 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
Junio C Hamano· Sep 1, 2011, 16:00 UTC · re: Michał Górny · lore

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

Michał Górny <mgorny@gentoo.org> writes:
Show 19 quoted lines
> 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...
Jeff King· Sep 1, 2011, 16:22 UTC · re: Junio C Hamano · lore

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

On Thu, Sep 01, 2011 at 09:00:39AM -0700, Junio C Hamano wrote:
Show 11 quoted lines
> >> > 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
Michał Górny· Sep 1, 2011, 16:48 UTC · re: Jeff King · lore

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

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

Show 21 quoted lines
> 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
Michał Górny· Sep 1, 2011, 16:50 UTC · re: Michał Górny · lore

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

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
Jeff King· Sep 2, 2011, 16:39 UTC · re: Michał Górny · lore

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

On Thu, Sep 01, 2011 at 06:50:01PM +0200, Michał Górny wrote:
Show 16 quoted lines
> -	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
Michał Górny· Sep 2, 2011, 17:39 UTC · re: Jeff King · lore

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

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

Show 32 quoted lines
> 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
Jeff King· Sep 2, 2011, 17:53 UTC · re: Michał Górny · lore

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

On Fri, Sep 02, 2011 at 07:39:26PM +0200, Michał Górny wrote:
Show 12 quoted lines
> > > +	/* 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
Jeff King· Sep 7, 2011, 17:40 UTC · re: Jeff King · lore

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

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
Michał Górny· Sep 15, 2011, 08:18 UTC · re: Jeff King · lore

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

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

Show 14 quoted lines
> 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
Jeff King· Sep 7, 2011, 17:42 UTC · re: Jeff King · lore

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

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
Jeff King· Sep 7, 2011, 17:43 UTC · re: Jeff King · lore

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

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
Jeff King· Sep 7, 2011, 17:44 UTC · re: Jeff King · lore

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

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
Jeff King· Sep 7, 2011, 17:44 UTC · re: Jeff King · lore

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

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
Jeff King· Sep 7, 2011, 17:46 UTC · re: Jeff King · lore

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

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
Junio C Hamano· Sep 1, 2011, 17:16 UTC · re: Jeff King · lore

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

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

Jeff King· Sep 1, 2011, 18:19 UTC · re: Junio C Hamano · lore

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

On Thu, Sep 01, 2011 at 10:16:29AM -0700, Junio C Hamano wrote:
Show 10 quoted lines
> 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

← back to recent threads