Re: [PATCH] git-for-each-ref: move GPG sigs off %(body) to %(signature).
- From
Michael J Gruber <git@drmicha.warpmail.net>
- Date
- Aug 30, 2011, 09:43 UTC
- Message-ID
- <4E5CB0D0.7000905@drmicha.warpmail.net>
- In-Reply-To
- <1314694641-24148-1-git-send-email-mgorny@gentoo.org>
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);
> }
> }
>