[PATCH] ref-filter: add new atom "signature" atom

[PATCH] ref-filter: add new atom "signature" atom

[nsengaw4c via GitGitGadget]

From: Nsengiyumva Wilberforce <nsengiyumvawilberforce@gmail.com>

This only works for commits. Add "signature" atom with `grade`,
`signer`, `key`, `fingerprint`, `primarykeyfingerprint`, `trustlevel`
as arguments. This code and it's documentation are inspired by
how the %GG, %G?, %GS, %GK, %GF, %GP, and %GT pretty formats were
implemented.

Co-authored-by: Hariom Verma <hariom18599@gmail.com>
Co-authored-by: Jaydeep Das <jaydeepjd.8914@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Nsengiyumva Wilberforce <nsengiyumvawilberforce@gmail.com>
---
    ref-filter: add new atom "signature" atom
    
    This only works for commits. Add "signature" atom with grade, signer,
    key, fingerprint, primarykeyfingerprint, trustlevel as arguments. This
    code and it's documentation are inspired by how the %GG, %G?, %GS, %GK,
    %GF, %GP, and %GT pretty formats were implemented.
    
    Co-authored-by: Hariom Verma hariom18599@gmail.com Co-authored-by:
    Jaydeep Das jaydeepjd.8914@gmail.com Mentored-by: Christian Couder
    chriscool@tuxfamily.org Mentored-by: Hariom Verma hariom18599@gmail.com
    Signed-off-by: Nsengiyumva Wilberforce nsengiyumvawilberforce@gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1452%2Fnsengiyumva-wilberforce%2Fsignature6-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1452/nsengiyumva-wilberforce/signature6-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1452

 Documentation/git-for-each-ref.txt |  27 +++++++
 ref-filter.c                       |  94 +++++++++++++++++++++++
 t/t6300-for-each-ref.sh            | 116 +++++++++++++++++++++++++++++
 3 files changed, 237 insertions(+)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 6da899c6296..9a0be85368b 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -212,6 +212,33 @@ symref::
 	`:lstrip` and `:rstrip` options in the same way as `refname`
 	above.
 
+signature::
+	The GPG signature of a commit.
+
+signature:grade::
+	Show "G" for a good (valid) signature, "B" for a bad
+	signature, "U" for a good signature with unknown validity, "X"
+	for a good signature that has expired, "Y" for a good
+	signature made by an expired key, "R" for a good signature
+	made by a revoked key, "E" if the signature cannot be
+	checked (e.g. missing key) and "N" for no signature.
+
+signature:signer::
+	The signer of the GPG signature of a commit.
+
+signature:key::
+	The key of the GPG signature of a commit.
+
+signature:fingerprint::
+	The fingerprint of the GPG signature of a commit.
+
+signature:primarykeyfingerprint::
+	The Primary Key fingerprint of the GPG signature of a commit.
+
+signature:trustlevel::
+	The Trust level of the GPG signature of a commit. Possible
+	outputs are `ultimate`, `fully`, `marginal`, `never` and `undefined`.
+
 worktreepath::
 	The absolute path to the worktree in which the ref is checked
 	out, if it is checked out in any linked worktree. Empty string
diff --git a/ref-filter.c b/ref-filter.c
index 9dc2cd14519..bb3624fb4e9 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -144,6 +144,7 @@ enum atom_type {
 	ATOM_BODY,
 	ATOM_TRAILERS,
 	ATOM_CONTENTS,
+	ATOM_SIGNATURE,
 	ATOM_RAW,
 	ATOM_UPSTREAM,
 	ATOM_PUSH,
@@ -208,6 +209,10 @@ static struct used_atom {
 		struct email_option {
 			enum { EO_RAW, EO_TRIM, EO_LOCALPART } option;
 		} email_option;
+		struct {
+			enum { S_BARE, S_GRADE, S_SIGNER, S_KEY, S_PRI_KEY_FP,
+			       S_FINGERPRINT, S_TRUST_LEVEL } option;
+		} signature;
 		struct refname_atom refname;
 		char *head;
 	} u;
@@ -378,6 +383,30 @@ static int subject_atom_parser(struct ref_format *format, struct used_atom *atom
 	return 0;
 }
 
+static int signature_atom_parser(struct ref_format *format, struct used_atom *atom,
+			       const char *arg, struct strbuf *err)
+{
+	if (arg) {
+		if (!strcmp(arg, "signer"))
+			atom->u.signature.option = S_SIGNER;
+		else if (!strcmp(arg, "grade"))
+			atom->u.signature.option = S_GRADE;
+		else if (!strcmp(arg, "key"))
+			atom->u.signature.option = S_KEY;
+		else if (!strcmp(arg, "fingerprint"))
+			atom->u.signature.option = S_FINGERPRINT;
+		else if (!strcmp(arg, "primarykeyfingerprint"))
+			atom->u.signature.option = S_PRI_KEY_FP;
+		else if (!strcmp(arg, "trustlevel"))
+			atom->u.signature.option = S_TRUST_LEVEL;
+		else
+			return strbuf_addf_ret(err, -1, _("unknown %%(signature) argument: %s"), arg);
+	}
+	else
+		atom->u.signature.option = S_BARE;
+	return 0;
+}
+
 static int trailers_atom_parser(struct ref_format *format, struct used_atom *atom,
 				const char *arg, struct strbuf *err)
 {
@@ -613,6 +642,7 @@ static struct {
 	[ATOM_BODY] = { "body", SOURCE_OBJ, FIELD_STR, body_atom_parser },
 	[ATOM_TRAILERS] = { "trailers", SOURCE_OBJ, FIELD_STR, trailers_atom_parser },
 	[ATOM_CONTENTS] = { "contents", SOURCE_OBJ, FIELD_STR, contents_atom_parser },
+	[ATOM_SIGNATURE] = { "signature", SOURCE_OBJ, FIELD_STR, signature_atom_parser },
 	[ATOM_RAW] = { "raw", SOURCE_OBJ, FIELD_STR, raw_atom_parser },
 	[ATOM_UPSTREAM] = { "upstream", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },
 	[ATOM_PUSH] = { "push", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },
@@ -1344,6 +1374,69 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void
 	}
 }
 
+static void grab_signature(struct atom_value *val, int deref, struct object *obj)
+{
+	int i;
+	struct commit *commit = (struct commit *) obj;
+
+	for (i = 0; i < used_atom_cnt; i++) {
+		struct used_atom *atom = &used_atom[i];
+		const char *name = atom->name;
+		struct atom_value *v = &val[i];
+		struct signature_check sigc = { 0 };
+
+		if (!!deref != (*name == '*'))
+			continue;
+		if (deref)
+			name++;
+		if (strcmp(name, "signature") &&
+			strcmp(name, "signature:signer") &&
+			strcmp(name, "signature:grade") &&
+			strcmp(name, "signature:key") &&
+			strcmp(name, "signature:fingerprint") &&
+			strcmp(name, "signature:primarykeyfingerprint") &&
+			strcmp(name, "signature:trustlevel"))
+			continue;
+
+		check_commit_signature(commit, &sigc);
+
+		if (atom->u.signature.option == S_BARE)
+			v->s = xstrdup(sigc.output ? sigc.output: "");
+		else if (atom->u.signature.option == S_SIGNER)
+			v->s = xstrdup(sigc.signer ? sigc.signer : "");
+		else if (atom->u.signature.option == S_GRADE) {
+			switch (sigc.result) {
+			case 'G':
+				switch (sigc.trust_level) {
+				case TRUST_UNDEFINED:
+				case TRUST_NEVER:
+					v->s = xstrfmt("%c", (char)'U');
+					break;
+				default:
+					v->s = xstrfmt("%c", (char)'G');
+					break;
+				}
+				break;
+			case 'B':
+			case 'E':
+			case 'N':
+			case 'X':
+			case 'Y':
+			case 'R':
+				v->s = xstrfmt("%c", (char)sigc.result);
+			}
+		}
+		else if (atom->u.signature.option == S_KEY)
+			v->s = xstrdup(sigc.key ? sigc.key : "");
+		else if (atom->u.signature.option == S_FINGERPRINT)
+			v->s = xstrdup(sigc.fingerprint ? sigc.fingerprint : "");
+		else if (atom->u.signature.option == S_PRI_KEY_FP)
+			v->s = xstrdup(sigc.primary_key_fingerprint ? sigc.primary_key_fingerprint : "");
+		else if (atom->u.signature.option == S_TRUST_LEVEL)
+			v->s = xstrdup(gpg_trust_level_to_str(sigc.trust_level));
+	}
+}
+
 static void find_subpos(const char *buf,
 			const char **sub, size_t *sublen,
 			const char **body, size_t *bodylen,
@@ -1536,6 +1629,7 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, s
 		grab_sub_body_contents(val, deref, data);
 		grab_person("author", val, deref, buf);
 		grab_person("committer", val, deref, buf);
+		grab_signature(val, deref, obj);
 		break;
 	case OBJ_TREE:
 		/* grab_tree_values(val, deref, obj, buf, sz); */
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index fa38b874416..5726517cfda 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
+GNUPGHOME_NOT_USED=$GNUPGHOME
 . "$TEST_DIRECTORY"/lib-gpg.sh
 . "$TEST_DIRECTORY"/lib-terminal.sh
 
@@ -1446,4 +1447,119 @@ sig_crlf="$(printf "%s" "$sig" | append_cr; echo dummy)"
 sig_crlf=${sig_crlf%dummy}
 test_atom refs/tags/fake-sig-crlf contents:signature "$sig_crlf"
 
+GRADE_FORMAT="%(signature:grade)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)"
+TRUSTLEVEL_FORMAT="%(signature:trustlevel)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)"
+
+test_expect_success GPG 'show good signature with custom format' '
+	git checkout -b signed &&
+	echo 1 >file && git add file &&
+	test_tick && git commit -S -m initial &&
+	git verify-commit signed 2>out &&
+	cat >expect <<-\EOF &&
+	G
+	13B6F51ECDDE430D
+	C O Mitter <committer@example.com>
+	73D758744BE721698EC54E8713B6F51ECDDE430D
+	73D758744BE721698EC54E8713B6F51ECDDE430D
+	EOF
+	git for-each-ref refs/heads/signed --format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'test signature atom with grade option and bad signature' '
+	git config commit.gpgsign true &&
+	echo 3 >file && test_tick && git commit -a -m "third" --no-gpg-sign &&
+	git tag third-unsigned &&
+
+	test_tick && git rebase -f HEAD^^ && git tag second-signed HEAD^ &&
+	git tag third-signed &&
+
+	git cat-file commit third-signed >raw &&
+	sed -e "s/^third/3rd forged/" raw >forged1 &&
+	FORGED1=$(git hash-object -w -t commit forged1) &&
+	git update-ref refs/tags/third-signed "$FORGED1" &&
+	test_must_fail git verify-commit "$FORGED1" &&
+
+	cat >expect <<-\EOF &&
+	B
+	13B6F51ECDDE430D
+	C O Mitter <committer@example.com>
+
+
+	EOF
+	git for-each-ref refs/tags/third-signed --format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show untrusted signature with custom format' '
+	echo 4 >file && test_tick && git commit -a -m fourth -SB7227189 &&
+	git tag signed-fourth &&
+	cat >expect <<-\EOF &&
+	U
+	65A0EEA02E30CAD7
+	Eris Discordia <discord@example.net>
+	F8364A59E07FFE9F4D63005A65A0EEA02E30CAD7
+	D4BE22311AD3131E5EDA29A461092E85B7227189
+	EOF
+	git for-each-ref refs/tags/signed-fourth --format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show untrusted signature with undefined trust level' '
+	echo 5 >file && test_tick && git commit -a -m fifth -SB7227189 &&
+	git tag fifth-signed &&
+	cat >expect <<-\EOF &&
+	undefined
+	65A0EEA02E30CAD7
+	Eris Discordia <discord@example.net>
+	F8364A59E07FFE9F4D63005A65A0EEA02E30CAD7
+	D4BE22311AD3131E5EDA29A461092E85B7227189
+	EOF
+	git for-each-ref refs/tags/fifth-signed --format="$TRUSTLEVEL_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show untrusted signature with ultimate trust level' '
+	echo 7 >file && test_tick && git commit -a -m "seventh" --no-gpg-sign &&
+	git tag seventh-unsigned &&
+
+	test_tick && git rebase -f HEAD^^ && git tag sixth-signed HEAD^ &&
+	git tag seventh-signed &&
+	cat >expect <<-\EOF &&
+	ultimate
+	13B6F51ECDDE430D
+	C O Mitter <committer@example.com>
+	73D758744BE721698EC54E8713B6F51ECDDE430D
+	73D758744BE721698EC54E8713B6F51ECDDE430D
+	EOF
+	git for-each-ref refs/tags/seventh-signed --format="$TRUSTLEVEL_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show unknown signature with custom format' '
+	cat >expect <<-\EOF &&
+	E
+	65A0EEA02E30CAD7
+
+
+
+	EOF
+	GNUPGHOME="$GNUPGHOME_NOT_USED" git for-each-ref refs/tags/fifth-signed --format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show lack of signature with custom format' '
+	echo 8 >file && test_tick && git commit -a -m "eigth unsigned" --no-gpg-sign &&
+	git tag eigth-unsigned &&
+	cat >expect <<-\EOF &&
+	N
+
+
+
+
+	EOF
+	git for-each-ref refs/tags/eigth-unsigned --format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
 test_done

base-commit: c48035d29b4e524aed3a32f0403676f0d9128863
-- 
gitgitgadget

Re: [PATCH] ref-filter: add new atom "signature" atom

[Junio C Hamano]

"nsengaw4c via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Nsengiyumva Wilberforce <nsengiyumvawilberforce@gmail.com>
>
> This only works for commits. Add "signature" atom with `grade`,
> `signer`, `key`, `fingerprint`, `primarykeyfingerprint`, `trustlevel`
> as arguments. This code and it's documentation are inspired by
> how the %GG, %G?, %GS, %GK, %GF, %GP, and %GT pretty formats were
> implemented.

Lacking motivation.  Without explaining why somebody may want to
have the feature and what it would be used for, "only works for
commits" would invite a "so what?  does it even have to work?"  as a
response, so start with a brief descrioption "with the current set
of atoms, $this_useful_thing cannot easily be achieved" before
describing its limitation.

Having said that, wouldn't it be natural to expect that the same
code can deal with signed tags?  After all we use the same signature
verification machinery at the lowest level in the callchain.

> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
> index 6da899c6296..9a0be85368b 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -212,6 +212,33 @@ symref::
>  	`:lstrip` and `:rstrip` options in the same way as `refname`
>  	above.
>  
> +signature::
> +...
> +signature:trustlevel::
> +	The Trust level of the GPG signature of a commit. Possible
> +	outputs are `ultimate`, `fully`, `marginal`, `never` and `undefined`.

A good list.  How do these work for signature made with a tool other
than GPG (in other words, when "gpg.format" is set to something
other than "openpgp")?

> @@ -378,6 +383,30 @@ static int subject_atom_parser(struct ref_format *format, struct used_atom *atom
>  	return 0;
>  }
>  
> +static int signature_atom_parser(struct ref_format *format, struct used_atom *atom,
> +			       const char *arg, struct strbuf *err)
> +{
> +	if (arg) {
> +		if (!strcmp(arg, "signer"))
> +			atom->u.signature.option = S_SIGNER;
> +		else if (!strcmp(arg, "grade"))
> +			atom->u.signature.option = S_GRADE;
> +		else if (!strcmp(arg, "key"))
> +			atom->u.signature.option = S_KEY;
> +		else if (!strcmp(arg, "fingerprint"))
> +			atom->u.signature.option = S_FINGERPRINT;
> +		else if (!strcmp(arg, "primarykeyfingerprint"))
> +			atom->u.signature.option = S_PRI_KEY_FP;
> +		else if (!strcmp(arg, "trustlevel"))
> +			atom->u.signature.option = S_TRUST_LEVEL;
> +		else
> +			return strbuf_addf_ret(err, -1, _("unknown %%(signature) argument: %s"), arg);
> +	}
> +	else
> +		atom->u.signature.option = S_BARE;
> +	return 0;
> +}

Handing the !arg case first will make the if/else if/... cascade
easier to follow, no?  Also the body of the function may want to
become a separate function that returns one of these S_FOO constants.

	static enum signatore_option signature_atom_parser(...)
	{
                enum signature_option opt = parse_signature_option(arg);
                if (opt < 0)
                        return strbuf_addf_ret(err, opt, _("unknown ..."), arg);
                return opt;
	}

where parse_signature_option() would look like

	static enum signature_option parse_signature_option(const char *arg)
	{
		if (!arg)
			return S_BARE;
		else if (!strcmp(arg, "signer"))
			return S_SIGNER;
		...
		else
			return -1;
	}

or something like that?

> @@ -1344,6 +1374,69 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void
>  	}
>  }
>  
> +static void grab_signature(struct atom_value *val, int deref, struct object *obj)

To be considerate for future developers, perhaps rename this to
grab_commit_signature(), so that they can add grab_tag_signature()
when they lift the limitation of this implementaiton?

> +{
> +	int i;
> +	struct commit *commit = (struct commit *) obj;

Style?  No SP between cast and value?

> +
> +	for (i = 0; i < used_atom_cnt; i++) {
> +		struct used_atom *atom = &used_atom[i];
> +		const char *name = atom->name;
> +		struct atom_value *v = &val[i];
> +		struct signature_check sigc = { 0 };
> +
> +		if (!!deref != (*name == '*'))
> +			continue;
> +		if (deref)
> +			name++;
> +		if (strcmp(name, "signature") &&
> +			strcmp(name, "signature:signer") &&
> +			strcmp(name, "signature:grade") &&
> +			strcmp(name, "signature:key") &&
> +			strcmp(name, "signature:fingerprint") &&
> +			strcmp(name, "signature:primarykeyfingerprint") &&
> +			strcmp(name, "signature:trustlevel"))
> +			continue;

And with the helper above, we can avoid the repetition here that can
go out of sync with the parser function.

> +		check_commit_signature(commit, &sigc);

If a format asks for signature:signer and signature:key, we
shouldn't be running GPG twice.  First check used_atom[] to see if
we even need to do _any_ signature processing (and leave if there is
not), populate the sigc just once and then enter the loop, perhaps?

In adddition, a call to check_commit_signature() should have a
matching call to signature_check_clear(); otherwise all the
resources held by sigc would leak, wouldn't it?

Re: [PATCH] ref-filter: add new atom "signature" atom

[Jeff King]

On Tue, Dec 27, 2022 at 12:55:23AM +0000, nsengaw4c via GitGitGadget wrote:

> This only works for commits. Add "signature" atom with `grade`,
> `signer`, `key`, `fingerprint`, `primarykeyfingerprint`, `trustlevel`
> as arguments. This code and it's documentation are inspired by
> how the %GG, %G?, %GS, %GK, %GF, %GP, and %GT pretty formats were
> implemented.

I don't have a real review for you, but rather two small requests since
I was working in this area recently.

> @@ -378,6 +383,30 @@ static int subject_atom_parser(struct ref_format *format, struct used_atom *atom
>  	return 0;
>  }
>  
> +static int signature_atom_parser(struct ref_format *format, struct used_atom *atom,
> +			       const char *arg, struct strbuf *err)

Can you squash in an annotation for the unused parameter, like this:

diff --git a/ref-filter.c b/ref-filter.c
index a4c3f89f64..3b3592acb2 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -405,8 +405,9 @@ static int subject_atom_parser(struct ref_format *format UNUSED,
 	return 0;
 }
 
-static int signature_atom_parser(struct ref_format *format, struct used_atom *atom,
-			       const char *arg, struct strbuf *err)
+static int signature_atom_parser(struct ref_format *format UNUSED,
+				 struct used_atom *atom,
+				 const char *arg, struct strbuf *err)
 {
 	if (arg) {
 		if (!strcmp(arg, "signer"))

This will eventually be necessary once we turn on -Wunused-parameter.
I'm preparing a patch to convert all of the other parsers that need it,
and I don't want to create a dependency between the two patches (it's OK
for you to add the UNUSED now, it's just not enforced yet).

I can also circle back after your patch is merged and add it, but it's a
bit easier to do it up front.

> +{
> +	if (arg) {
> +		if (!strcmp(arg, "signer"))
> +			atom->u.signature.option = S_SIGNER;
> +		else if (!strcmp(arg, "grade"))
> +			atom->u.signature.option = S_GRADE;
> +		else if (!strcmp(arg, "key"))
> +			atom->u.signature.option = S_KEY;
> +		else if (!strcmp(arg, "fingerprint"))
> +			atom->u.signature.option = S_FINGERPRINT;
> +		else if (!strcmp(arg, "primarykeyfingerprint"))
> +			atom->u.signature.option = S_PRI_KEY_FP;
> +		else if (!strcmp(arg, "trustlevel"))
> +			atom->u.signature.option = S_TRUST_LEVEL;
> +		else
> +			return strbuf_addf_ret(err, -1, _("unknown %%(signature) argument: %s"), arg);
> +	}

The ref-filter code recently got a helper function to report this kind
of argument error consistently, via dda4fc1a84 (ref-filter: factor out
"unrecognized %(foo) arg" errors, 2022-12-14). If you rebase the patch
on the current master, you can just do:

  return err_bad_arg(err, "signature", arg);

which will make the error message match the others (which in turn saves
work for translators).

-Peff

Re: [PATCH] ref-filter: add new atom "signature" atom

[NSENGIYUMVA WILBERFORCE]

Hi
>
> > From: Nsengiyumva Wilberforce <nsengiyumvawilberforce@gmail.com>
> >
> > This only works for commits. Add "signature" atom with `grade`,
> > `signer`, `key`, `fingerprint`, `primarykeyfingerprint`, `trustlevel`
> > as arguments. This code and it's documentation are inspired by
> > how the %GG, %G?, %GS, %GK, %GF, %GP, and %GT pretty formats were
> > implemented.
>
> Lacking motivation.  Without explaining why somebody may want to
> have the feature and what it would be used for, "only works for
> commits" would invite a "so what?  does it even have to work?"  as a
> response, so start with a brief descrioption "with the current set
> of atoms, $this_useful_thing cannot easily be achieved" before
> describing its limitation.

Ok, I will edit the commit message. Thanks
>
> > diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
> > index 6da899c6296..9a0be85368b 100644
> > --- a/Documentation/git-for-each-ref.txt
> > +++ b/Documentation/git-for-each-ref.txt
> > @@ -212,6 +212,33 @@ symref::
> >       `:lstrip` and `:rstrip` options in the same way as `refname`
> >       above.
> >
> > +signature::
> > +...
> > +signature:trustlevel::
> > +     The Trust level of the GPG signature of a commit. Possible
> > +     outputs are `ultimate`, `fully`, `marginal`, `never` and `undefined`.
>
> A good list.  How do these work for signature made with a tool other
> than GPG (in other words, when "gpg.format" is set to something
> other than "openpgp")?

You mean ssh and X509, right? honestly, I did not check the behavior.
I am going to check

> Having said that, wouldn't it be natural to expect that the same
> code can deal with signed tags?  After all we use the same signature
> verification machinery at the lowest level in the callchain.

Very right, it works for signed tags too.

>
> Handing the !arg case first will make the if/else if/... cascade
> easier to follow, no?  Also the body of the function may want to
> become a separate function that returns one of these S_FOO constants.
>
>         static enum signatore_option signature_atom_parser(...)
>         {
>                 enum signature_option opt = parse_signature_option(arg);
>                 if (opt < 0)
>                         return strbuf_addf_ret(err, opt, _("unknown ..."), arg);
>                 return opt;
>         }
>
> where parse_signature_option() would look like
>
>         static enum signature_option parse_signature_option(const char *arg)
>         {
>                 if (!arg)
>                         return S_BARE;
>                 else if (!strcmp(arg, "signer"))
>                         return S_SIGNER;
>                 ...
>                 else
>                         return -1;
>         }
>
> or something like that?

 It makes more sense
>
> > +{
> > +     int i;
> > +     struct commit *commit = (struct commit *) obj;
>
> Style?  No SP between cast and value?

ok, noted
>
> > +
> > +     for (i = 0; i < used_atom_cnt; i++) {
> > +             struct used_atom *atom = &used_atom[i];
> > +             const char *name = atom->name;
> > +             struct atom_value *v = &val[i];
> > +             struct signature_check sigc = { 0 };
> > +
> > +             if (!!deref != (*name == '*'))
> > +                     continue;
> > +             if (deref)
> > +                     name++;
> > +             if (strcmp(name, "signature") &&
> > +                     strcmp(name, "signature:signer") &&
> > +                     strcmp(name, "signature:grade") &&
> > +                     strcmp(name, "signature:key") &&
> > +                     strcmp(name, "signature:fingerprint") &&
> > +                     strcmp(name, "signature:primarykeyfingerprint") &&
> > +                     strcmp(name, "signature:trustlevel"))
> > +                     continue;
>
> And with the helper above, we can avoid the repetition here that can
> go out of sync with the parser function.

I am not sure I have understood this, which helper?

> > +             check_commit_signature(commit, &sigc);
>
> If a format asks for signature:signer and signature:key, we
> shouldn't be running GPG twice.  First check used_atom[] to see if
> we even need to do _any_ signature processing (and leave if there is
> not), populate the sigc just once and then enter the loop, perhaps?

Yeah, I think it was not right calling check_commit_signature() in the
loop. Populating sigc at once looks more good to me


>
>  In adddition, a call to check_commit_signature() should have a
>
> matching call to signature_check_clear(); otherwise all the
>
> resources held by sigc would leak, wouldn't it?

Yeah, it would.


On Mon, Dec 26, 2022 at 9:20 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "nsengaw4c via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Nsengiyumva Wilberforce <nsengiyumvawilberforce@gmail.com>
> >
> > This only works for commits. Add "signature" atom with `grade`,
> > `signer`, `key`, `fingerprint`, `primarykeyfingerprint`, `trustlevel`
> > as arguments. This code and it's documentation are inspired by
> > how the %GG, %G?, %GS, %GK, %GF, %GP, and %GT pretty formats were
> > implemented.
>
> Lacking motivation.  Without explaining why somebody may want to
> have the feature and what it would be used for, "only works for
> commits" would invite a "so what?  does it even have to work?"  as a
> response, so start with a brief descrioption "with the current set
> of atoms, $this_useful_thing cannot easily be achieved" before
> describing its limitation.
>
> Having said that, wouldn't it be natural to expect that the same
> code can deal with signed tags?  After all we use the same signature
> verification machinery at the lowest level in the callchain.
>
> > diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
> > index 6da899c6296..9a0be85368b 100644
> > --- a/Documentation/git-for-each-ref.txt
> > +++ b/Documentation/git-for-each-ref.txt
> > @@ -212,6 +212,33 @@ symref::
> >       `:lstrip` and `:rstrip` options in the same way as `refname`
> >       above.
> >
> > +signature::
> > +...
> > +signature:trustlevel::
> > +     The Trust level of the GPG signature of a commit. Possible
> > +     outputs are `ultimate`, `fully`, `marginal`, `never` and `undefined`.
>
> A good list.  How do these work for signature made with a tool other
> than GPG (in other words, when "gpg.format" is set to something
> other than "openpgp")?
>
> > @@ -378,6 +383,30 @@ static int subject_atom_parser(struct ref_format *format, struct used_atom *atom
> >       return 0;
> >  }
> >
> > +static int signature_atom_parser(struct ref_format *format, struct used_atom *atom,
> > +                            const char *arg, struct strbuf *err)
> > +{
> > +     if (arg) {
> > +             if (!strcmp(arg, "signer"))
> > +                     atom->u.signature.option = S_SIGNER;
> > +             else if (!strcmp(arg, "grade"))
> > +                     atom->u.signature.option = S_GRADE;
> > +             else if (!strcmp(arg, "key"))
> > +                     atom->u.signature.option = S_KEY;
> > +             else if (!strcmp(arg, "fingerprint"))
> > +                     atom->u.signature.option = S_FINGERPRINT;
> > +             else if (!strcmp(arg, "primarykeyfingerprint"))
> > +                     atom->u.signature.option = S_PRI_KEY_FP;
> > +             else if (!strcmp(arg, "trustlevel"))
> > +                     atom->u.signature.option = S_TRUST_LEVEL;
> > +             else
> > +                     return strbuf_addf_ret(err, -1, _("unknown %%(signature) argument: %s"), arg);
> > +     }
> > +     else
> > +             atom->u.signature.option = S_BARE;
> > +     return 0;
> > +}
>
> Handing the !arg case first will make the if/else if/... cascade
> easier to follow, no?  Also the body of the function may want to
> become a separate function that returns one of these S_FOO constants.
>
>         static enum signatore_option signature_atom_parser(...)
>         {
>                 enum signature_option opt = parse_signature_option(arg);
>                 if (opt < 0)
>                         return strbuf_addf_ret(err, opt, _("unknown ..."), arg);
>                 return opt;
>         }
>
> where parse_signature_option() would look like
>
>         static enum signature_option parse_signature_option(const char *arg)
>         {
>                 if (!arg)
>                         return S_BARE;
>                 else if (!strcmp(arg, "signer"))
>                         return S_SIGNER;
>                 ...
>                 else
>                         return -1;
>         }
>
> or something like that?
>
> > @@ -1344,6 +1374,69 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void
> >       }
> >  }
> >
> > +static void grab_signature(struct atom_value *val, int deref, struct object *obj)
>
> To be considerate for future developers, perhaps rename this to
> grab_commit_signature(), so that they can add grab_tag_signature()
> when they lift the limitation of this implementaiton?
>
> > +{
> > +     int i;
> > +     struct commit *commit = (struct commit *) obj;
>
> Style?  No SP between cast and value?
>
> > +
> > +     for (i = 0; i < used_atom_cnt; i++) {
> > +             struct used_atom *atom = &used_atom[i];
> > +             const char *name = atom->name;
> > +             struct atom_value *v = &val[i];
> > +             struct signature_check sigc = { 0 };
> > +
> > +             if (!!deref != (*name == '*'))
> > +                     continue;
> > +             if (deref)
> > +                     name++;
> > +             if (strcmp(name, "signature") &&
> > +                     strcmp(name, "signature:signer") &&
> > +                     strcmp(name, "signature:grade") &&
> > +                     strcmp(name, "signature:key") &&
> > +                     strcmp(name, "signature:fingerprint") &&
> > +                     strcmp(name, "signature:primarykeyfingerprint") &&
> > +                     strcmp(name, "signature:trustlevel"))
> > +                     continue;
>
> And with the helper above, we can avoid the repetition here that can
> go out of sync with the parser function.
>
> > +             check_commit_signature(commit, &sigc);
>
> If a format asks for signature:signer and signature:key, we
> shouldn't be running GPG twice.  First check used_atom[] to see if
> we even need to do _any_ signature processing (and leave if there is
> not), populate the sigc just once and then enter the loop, perhaps?
>
> In adddition, a call to check_commit_signature() should have a
> matching call to signature_check_clear(); otherwise all the
> resources held by sigc would leak, wouldn't it?

Re: [PATCH] ref-filter: add new atom "signature" atom

[NSENGIYUMVA WILBERFORCE]

>
> diff --git a/ref-filter.c b/ref-filter.c
> index a4c3f89f64..3b3592acb2 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -405,8 +405,9 @@ static int subject_atom_parser(struct ref_format *format UNUSED,
>         return 0;
>  }
>
> -static int signature_atom_parser(struct ref_format *format, struct used_atom *atom,
> -                              const char *arg, struct strbuf *err)
> +static int signature_atom_parser(struct ref_format *format UNUSED,
> +                                struct used_atom *atom,
> +                                const char *arg, struct strbuf *err)
>  {
>         if (arg) {
>                 if (!strcmp(arg, "signer"))
>
> This will eventually be necessary once we turn on -Wunused-parameter.
> I'm preparing a patch to convert all of the other parsers that need it,
> and I don't want to create a dependency between the two patches (it's OK
> for you to add the UNUSED now, it's just not enforced yet).
>
> I can also circle back after your patch is merged and add it, but it's a
> bit easier to do it up front.

Thanks, worked on it

> +{
> +     if (arg) {
> +             if (!strcmp(arg, "signer"))
> +                     atom->u.signature.option = S_SIGNER;
> +             else if (!strcmp(arg, "grade"))
> +                     atom->u.signature.option = S_GRADE;
> +             else if (!strcmp(arg, "key"))
> +                     atom->u.signature.option = S_KEY;
> +             else if (!strcmp(arg, "fingerprint"))
> +                     atom->u.signature.option = S_FINGERPRINT;
> +             else if (!strcmp(arg, "primarykeyfingerprint"))
> +                     atom->u.signature.option = S_PRI_KEY_FP;
> +             else if (!strcmp(arg, "trustlevel"))
> +                     atom->u.signature.option = S_TRUST_LEVEL;
> +             else
> +                     return strbuf_addf_ret(err, -1, _("unknown %%(signature) argument: %s"), arg);
> +     }

The ref-filter code recently got a helper function to report this kind
of argument error consistently, via dda4fc1a84 (ref-filter: factor out
"unrecognized %(foo) arg" errors, 2022-12-14). If you rebase the patch
on the current master, you can just do:

  return err_bad_arg(err, "signature", arg);

which will make the error message match the others (which in turn saves
>
> work for translators).

Thanks for this, I have seen it too


On Tue, Dec 27, 2022 at 1:11 AM Jeff King <peff@peff.net> wrote:
>
> On Tue, Dec 27, 2022 at 12:55:23AM +0000, nsengaw4c via GitGitGadget wrote:
>
> > This only works for commits. Add "signature" atom with `grade`,
> > `signer`, `key`, `fingerprint`, `primarykeyfingerprint`, `trustlevel`
> > as arguments. This code and it's documentation are inspired by
> > how the %GG, %G?, %GS, %GK, %GF, %GP, and %GT pretty formats were
> > implemented.
>
> I don't have a real review for you, but rather two small requests since
> I was working in this area recently.
>
> > @@ -378,6 +383,30 @@ static int subject_atom_parser(struct ref_format *format, struct used_atom *atom
> >       return 0;
> >  }
> >
> > +static int signature_atom_parser(struct ref_format *format, struct used_atom *atom,
> > +                            const char *arg, struct strbuf *err)
>
> Can you squash in an annotation for the unused parameter, like this:
>
> diff --git a/ref-filter.c b/ref-filter.c
> index a4c3f89f64..3b3592acb2 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -405,8 +405,9 @@ static int subject_atom_parser(struct ref_format *format UNUSED,
>         return 0;
>  }
>
> -static int signature_atom_parser(struct ref_format *format, struct used_atom *atom,
> -                              const char *arg, struct strbuf *err)
> +static int signature_atom_parser(struct ref_format *format UNUSED,
> +                                struct used_atom *atom,
> +                                const char *arg, struct strbuf *err)
>  {
>         if (arg) {
>                 if (!strcmp(arg, "signer"))
>
> This will eventually be necessary once we turn on -Wunused-parameter.
> I'm preparing a patch to convert all of the other parsers that need it,
> and I don't want to create a dependency between the two patches (it's OK
> for you to add the UNUSED now, it's just not enforced yet).
>
> I can also circle back after your patch is merged and add it, but it's a
> bit easier to do it up front.
>
> > +{
> > +     if (arg) {
> > +             if (!strcmp(arg, "signer"))
> > +                     atom->u.signature.option = S_SIGNER;
> > +             else if (!strcmp(arg, "grade"))
> > +                     atom->u.signature.option = S_GRADE;
> > +             else if (!strcmp(arg, "key"))
> > +                     atom->u.signature.option = S_KEY;
> > +             else if (!strcmp(arg, "fingerprint"))
> > +                     atom->u.signature.option = S_FINGERPRINT;
> > +             else if (!strcmp(arg, "primarykeyfingerprint"))
> > +                     atom->u.signature.option = S_PRI_KEY_FP;
> > +             else if (!strcmp(arg, "trustlevel"))
> > +                     atom->u.signature.option = S_TRUST_LEVEL;
> > +             else
> > +                     return strbuf_addf_ret(err, -1, _("unknown %%(signature) argument: %s"), arg);
> > +     }
>
> The ref-filter code recently got a helper function to report this kind
> of argument error consistently, via dda4fc1a84 (ref-filter: factor out
> "unrecognized %(foo) arg" errors, 2022-12-14). If you rebase the patch
> on the current master, you can just do:
>
>   return err_bad_arg(err, "signature", arg);
>
> which will make the error message match the others (which in turn saves
> work for translators).
>
> -Peff

Re: [PATCH] ref-filter: add new atom "signature" atom

[Christian Couder]

On Mon, Jan 2, 2023 at 6:01 AM NSENGIYUMVA WILBERFORCE
<nsengiyumvawilberforce@gmail.com> wrote:

> > Handing the !arg case first will make the if/else if/... cascade
> > easier to follow, no?  Also the body of the function may want to
> > become a separate function that returns one of these S_FOO constants.
> >
> >         static enum signatore_option signature_atom_parser(...)
> >         {
> >                 enum signature_option opt = parse_signature_option(arg);
> >                 if (opt < 0)
> >                         return strbuf_addf_ret(err, opt, _("unknown ..."), arg);
> >                 return opt;
> >         }
> >
> > where parse_signature_option() would look like
> >
> >         static enum signature_option parse_signature_option(const char *arg)
> >         {
> >                 if (!arg)
> >                         return S_BARE;
> >                 else if (!strcmp(arg, "signer"))
> >                         return S_SIGNER;
> >                 ...
> >                 else
> >                         return -1;
> >         }
> >
> > or something like that?

[...]

> > > +             if (strcmp(name, "signature") &&
> > > +                     strcmp(name, "signature:signer") &&
> > > +                     strcmp(name, "signature:grade") &&
> > > +                     strcmp(name, "signature:key") &&
> > > +                     strcmp(name, "signature:fingerprint") &&
> > > +                     strcmp(name, "signature:primarykeyfingerprint") &&
> > > +                     strcmp(name, "signature:trustlevel"))
> > > +                     continue;
> >
> > And with the helper above, we can avoid the repetition here that can
> > go out of sync with the parser function.
>
> I am not sure I have understood this, which helper?

I think Junio is talking about the following function:

static enum signature_option parse_signature_option(const char *arg)

he suggested above.

With this function the above code could be just something like:

if (parse_signature_option(name) < 0)
                    continue;

Re: [PATCH] ref-filter: add new atom "signature" atom

[Junio C Hamano]

Christian Couder <christian.couder@gmail.com> writes:

>> I am not sure I have understood this, which helper?
>
> I think Junio is talking about the following function:
>
> static enum signature_option parse_signature_option(const char *arg)
>
> he suggested above.

Correct.

> With this function the above code could be just something like:
>
> if (parse_signature_option(name) < 0)
>                     continue;

More or less so, but the first "if" in the helper I wrote in the
message above is broken.  It should be

         static enum signature_option parse_signature_option(const char *arg)
         {
                 if (!*arg)
                         return S_BARE;
                 else if (!strcmp(arg, "signer"))
                         return S_SIGNER;
                 ...
                 else
                         return -1;
         }

and then the code equivalent to the bunch of strcmp() would be

	if (skip_prefix(name, "signature", &name) &&
            (!*name || *name++ == ':') &&
            (0 <= parse_signature_option(name)))
		; /* we have "signature"-related atom */
	else
		continue; /* not a "signature" atom */

or something like that.

Re: [PATCH] ref-filter: add new atom "signature" atom

[NSENGIYUMVA WILBERFORCE]

> >> I am not sure I have understood this, which helper?
> >
> > I think Junio is talking about the following function:
> >
> > static enum signature_option parse_signature_option(const char *arg)
> >
> > he suggested above.
>
> Correct.
>
> > With this function the above code could be just something like:
> >
> > if (parse_signature_option(name) < 0)
> >                     continue;
>
> More or less so, but the first "if" in the helper I wrote in the
> message above is broken.  It should be
>
>          static enum signature_option parse_signature_option(const char *arg)
>          {
>                  if (!*arg)
>                          return S_BARE;
>                  else if (!strcmp(arg, "signer"))
>                          return S_SIGNER;
>                  ...
>                  else
>                          return -1;
>          }

This way, running git for-each-ref refs/heads/signature8
--format="%(signature)" raises a seg fault, I looked for the bug using
gdb when I check the contents of *arg like this:p *arg, I get this:
Cannot access memory at 0x0.

However, others like signature:key, signature:signer, etc are ok.
Leaving it as arg makes everything fine. So I decided to leave it as
you had suggested first.


I had actually forgotten to add the test for "%(signature)", so this
scenario reminded me to do so.


On Mon, Jan 2, 2023 at 7:47 AM NSENGIYUMVA WILBERFORCE
<nsengiyumvawilberforce@gmail.com> wrote:
>
> Hi
>>
>> > From: Nsengiyumva Wilberforce <nsengiyumvawilberforce@gmail.com>
>> >
>> > This only works for commits. Add "signature" atom with `grade`,
>> > `signer`, `key`, `fingerprint`, `primarykeyfingerprint`, `trustlevel`
>> > as arguments. This code and it's documentation are inspired by
>> > how the %GG, %G?, %GS, %GK, %GF, %GP, and %GT pretty formats were
>> > implemented.
>>
>> Lacking motivation.  Without explaining why somebody may want to
>> have the feature and what it would be used for, "only works for
>> commits" would invite a "so what?  does it even have to work?"  as a
>> response, so start with a brief descrioption "with the current set
>> of atoms, $this_useful_thing cannot easily be achieved" before
>> describing its limitation.
>
> Ok, I will edit the commit message. Thanks
>>
>> > diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
>> > index 6da899c6296..9a0be85368b 100644
>> > --- a/Documentation/git-for-each-ref.txt
>> > +++ b/Documentation/git-for-each-ref.txt
>> > @@ -212,6 +212,33 @@ symref::
>> >       `:lstrip` and `:rstrip` options in the same way as `refname`
>> >       above.
>> >
>> > +signature::
>> > +...
>> > +signature:trustlevel::
>> > +     The Trust level of the GPG signature of a commit. Possible
>> > +     outputs are `ultimate`, `fully`, `marginal`, `never` and `undefined`.
>>
>> A good list.  How do these work for signature made with a tool other
>> than GPG (in other words, when "gpg.format" is set to something
>> other than "openpgp")?
>
> You mean ssh and X509, right? honestly, I did not check the behavior. I am going to check
>
>> Having said that, wouldn't it be natural to expect that the same
>> code can deal with signed tags?  After all we use the same signature
>> verification machinery at the lowest level in the callchain.
>
> Very right, it works for signed tags too.
>
>>
>> Handing the !arg case first will make the if/else if/... cascade
>> easier to follow, no?  Also the body of the function may want to
>> become a separate function that returns one of these S_FOO constants.
>>
>>         static enum signatore_option signature_atom_parser(...)
>>         {
>>                 enum signature_option opt = parse_signature_option(arg);
>>                 if (opt < 0)
>>                         return strbuf_addf_ret(err, opt, _("unknown ..."), arg);
>>                 return opt;
>>         }
>>
>> where parse_signature_option() would look like
>>
>>         static enum signature_option parse_signature_option(const char *arg)
>>         {
>>                 if (!arg)
>>                         return S_BARE;
>>                 else if (!strcmp(arg, "signer"))
>>                         return S_SIGNER;
>>                 ...
>>                 else
>>                         return -1;
>>         }
>>
>> or something like that?
>
>  It makes more sense
>>
>> > +{
>> > +     int i;
>> > +     struct commit *commit = (struct commit *) obj;
>>
>> Style?  No SP between cast and value?
>
> ok, noted
>>
>> > +
>> > +     for (i = 0; i < used_atom_cnt; i++) {
>> > +             struct used_atom *atom = &used_atom[i];
>> > +             const char *name = atom->name;
>> > +             struct atom_value *v = &val[i];
>> > +             struct signature_check sigc = { 0 };
>> > +
>> > +             if (!!deref != (*name == '*'))
>> > +                     continue;
>> > +             if (deref)
>> > +                     name++;
>> > +             if (strcmp(name, "signature") &&
>> > +                     strcmp(name, "signature:signer") &&
>> > +                     strcmp(name, "signature:grade") &&
>> > +                     strcmp(name, "signature:key") &&
>> > +                     strcmp(name, "signature:fingerprint") &&
>> > +                     strcmp(name, "signature:primarykeyfingerprint") &&
>> > +                     strcmp(name, "signature:trustlevel"))
>> > +                     continue;
>>
>> And with the helper above, we can avoid the repetition here that can
>> go out of sync with the parser function.
>
> I am not sure I have understood this, which helper?
>
>> > +             check_commit_signature(commit, &sigc);
>>
>> If a format asks for signature:signer and signature:key, we
>> shouldn't be running GPG twice.  First check used_atom[] to see if
>> we even need to do _any_ signature processing (and leave if there is
>> not), populate the sigc just once and then enter the loop, perhaps?
>
> Yeah, I think it was not right calling check_commit_signature() in the loop. Populating sigc at once looks more good to me
>
>
>>
>>  In adddition, a call to check_commit_signature() should have a
>>
>> matching call to signature_check_clear(); otherwise all the
>>
>> resources held by sigc would leak, wouldn't it?
>
> Yeah, it would.
>
> On Mon, Dec 26, 2022 at 9:20 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> "nsengaw4c via GitGitGadget" <gitgitgadget@gmail.com> writes:
>>
>> > From: Nsengiyumva Wilberforce <nsengiyumvawilberforce@gmail.com>
>> >
>> > This only works for commits. Add "signature" atom with `grade`,
>> > `signer`, `key`, `fingerprint`, `primarykeyfingerprint`, `trustlevel`
>> > as arguments. This code and it's documentation are inspired by
>> > how the %GG, %G?, %GS, %GK, %GF, %GP, and %GT pretty formats were
>> > implemented.
>>
>> Lacking motivation.  Without explaining why somebody may want to
>> have the feature and what it would be used for, "only works for
>> commits" would invite a "so what?  does it even have to work?"  as a
>> response, so start with a brief descrioption "with the current set
>> of atoms, $this_useful_thing cannot easily be achieved" before
>> describing its limitation.
>>
>> Having said that, wouldn't it be natural to expect that the same
>> code can deal with signed tags?  After all we use the same signature
>> verification machinery at the lowest level in the callchain.
>>
>> > diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
>> > index 6da899c6296..9a0be85368b 100644
>> > --- a/Documentation/git-for-each-ref.txt
>> > +++ b/Documentation/git-for-each-ref.txt
>> > @@ -212,6 +212,33 @@ symref::
>> >       `:lstrip` and `:rstrip` options in the same way as `refname`
>> >       above.
>> >
>> > +signature::
>> > +...
>> > +signature:trustlevel::
>> > +     The Trust level of the GPG signature of a commit. Possible
>> > +     outputs are `ultimate`, `fully`, `marginal`, `never` and `undefined`.
>>
>> A good list.  How do these work for signature made with a tool other
>> than GPG (in other words, when "gpg.format" is set to something
>> other than "openpgp")?
>>
>> > @@ -378,6 +383,30 @@ static int subject_atom_parser(struct ref_format *format, struct used_atom *atom
>> >       return 0;
>> >  }
>> >
>> > +static int signature_atom_parser(struct ref_format *format, struct used_atom *atom,
>> > +                            const char *arg, struct strbuf *err)
>> > +{
>> > +     if (arg) {
>> > +             if (!strcmp(arg, "signer"))
>> > +                     atom->u.signature.option = S_SIGNER;
>> > +             else if (!strcmp(arg, "grade"))
>> > +                     atom->u.signature.option = S_GRADE;
>> > +             else if (!strcmp(arg, "key"))
>> > +                     atom->u.signature.option = S_KEY;
>> > +             else if (!strcmp(arg, "fingerprint"))
>> > +                     atom->u.signature.option = S_FINGERPRINT;
>> > +             else if (!strcmp(arg, "primarykeyfingerprint"))
>> > +                     atom->u.signature.option = S_PRI_KEY_FP;
>> > +             else if (!strcmp(arg, "trustlevel"))
>> > +                     atom->u.signature.option = S_TRUST_LEVEL;
>> > +             else
>> > +                     return strbuf_addf_ret(err, -1, _("unknown %%(signature) argument: %s"), arg);
>> > +     }
>> > +     else
>> > +             atom->u.signature.option = S_BARE;
>> > +     return 0;
>> > +}
>>
>> Handing the !arg case first will make the if/else if/... cascade
>> easier to follow, no?  Also the body of the function may want to
>> become a separate function that returns one of these S_FOO constants.
>>
>>         static enum signatore_option signature_atom_parser(...)
>>         {
>>                 enum signature_option opt = parse_signature_option(arg);
>>                 if (opt < 0)
>>                         return strbuf_addf_ret(err, opt, _("unknown ..."), arg);
>>                 return opt;
>>         }
>>
>> where parse_signature_option() would look like
>>
>>         static enum signature_option parse_signature_option(const char *arg)
>>         {
>>                 if (!arg)
>>                         return S_BARE;
>>                 else if (!strcmp(arg, "signer"))
>>                         return S_SIGNER;
>>                 ...
>>                 else
>>                         return -1;
>>         }
>>
>> or something like that?
>>
>> > @@ -1344,6 +1374,69 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void
>> >       }
>> >  }
>> >
>> > +static void grab_signature(struct atom_value *val, int deref, struct object *obj)
>>
>> To be considerate for future developers, perhaps rename this to
>> grab_commit_signature(), so that they can add grab_tag_signature()
>> when they lift the limitation of this implementaiton?
>>
>> > +{
>> > +     int i;
>> > +     struct commit *commit = (struct commit *) obj;
>>
>> Style?  No SP between cast and value?
>>
>> > +
>> > +     for (i = 0; i < used_atom_cnt; i++) {
>> > +             struct used_atom *atom = &used_atom[i];
>> > +             const char *name = atom->name;
>> > +             struct atom_value *v = &val[i];
>> > +             struct signature_check sigc = { 0 };
>> > +
>> > +             if (!!deref != (*name == '*'))
>> > +                     continue;
>> > +             if (deref)
>> > +                     name++;
>> > +             if (strcmp(name, "signature") &&
>> > +                     strcmp(name, "signature:signer") &&
>> > +                     strcmp(name, "signature:grade") &&
>> > +                     strcmp(name, "signature:key") &&
>> > +                     strcmp(name, "signature:fingerprint") &&
>> > +                     strcmp(name, "signature:primarykeyfingerprint") &&
>> > +                     strcmp(name, "signature:trustlevel"))
>> > +                     continue;
>>
>> And with the helper above, we can avoid the repetition here that can
>> go out of sync with the parser function.
>>
>> > +             check_commit_signature(commit, &sigc);
>>
>> If a format asks for signature:signer and signature:key, we
>> shouldn't be running GPG twice.  First check used_atom[] to see if
>> we even need to do _any_ signature processing (and leave if there is
>> not), populate the sigc just once and then enter the loop, perhaps?
>>
>> In adddition, a call to check_commit_signature() should have a
>> matching call to signature_check_clear(); otherwise all the
>> resources held by sigc would leak, wouldn't it?

[PATCH] ref-filter: add new atom "signature" atom

[nsengaw4c via GitGitGadget]

From: Nsengiyumva Wilberforce <nsengiyumvawilberforce@gmail.com>

This commit duplicates the code for `signature` atom from pretty.c
to ref-filter.c. This feature will help to get rid of current duplicate
implementation of `signature` atom when unifying implemenations by
using ref-filter logic everywhere when ref-filter can do everything
pretty is doing.

Add "signature" atom with `grade`, `signer`, `key`,
`fingerprint`, `primarykeyfingerprint`, `trustlevel` as arguments.
This code and its documentation are inspired by how the %GG, %G?,
%GS, %GK, %GF, %GP, and %GT pretty formats were implemented.

Co-authored-by: Hariom Verma <hariom18599@gmail.com>
Co-authored-by: Jaydeep Das <jaydeepjd.8914@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Nsengiyumva Wilberforce <nsengiyumvawilberforce@gmail.com>
---
    ref-filter: add new atom "signature" atom
    
    This commit duplicates the code for signature atom from pretty.c to
    ref-filter.c. This feature will help to get rid of current duplicate
    implementation of signature atom when unifying implemenations by using
    ref-filter logic everywhere when ref-filter can do everything pretty is
    doing.
    
    Add "signature" atom with grade, signer, key, fingerprint,
    primarykeyfingerprint, trustlevel as arguments. This code and its
    documentation are inspired by how the %GG, %G?, %GS, %GK, %GF, %GP, and
    %GT pretty formats were implemented.
    
    Co-authored-by: Hariom Verma hariom18599@gmail.com Co-authored-by:
    Jaydeep Das jaydeepjd.8914@gmail.com Mentored-by: Christian Couder
    chriscool@tuxfamily.org Mentored-by: Hariom Verma hariom18599@gmail.com
    Signed-off-by: Nsengiyumva Wilberforce nsengiyumvawilberforce@gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1428%2Fnsengiyumva-wilberforce%2Fsignature10-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1428/nsengiyumva-wilberforce/signature10-v1
Pull-Request: https://github.com/git/git/pull/1428

 Documentation/git-for-each-ref.txt |  27 ++++++
 ref-filter.c                       | 101 +++++++++++++++++++++++
 t/t6300-for-each-ref.sh            | 127 +++++++++++++++++++++++++++++
 3 files changed, 255 insertions(+)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 6da899c6296..9a0be85368b 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -212,6 +212,33 @@ symref::
 	`:lstrip` and `:rstrip` options in the same way as `refname`
 	above.
 
+signature::
+	The GPG signature of a commit.
+
+signature:grade::
+	Show "G" for a good (valid) signature, "B" for a bad
+	signature, "U" for a good signature with unknown validity, "X"
+	for a good signature that has expired, "Y" for a good
+	signature made by an expired key, "R" for a good signature
+	made by a revoked key, "E" if the signature cannot be
+	checked (e.g. missing key) and "N" for no signature.
+
+signature:signer::
+	The signer of the GPG signature of a commit.
+
+signature:key::
+	The key of the GPG signature of a commit.
+
+signature:fingerprint::
+	The fingerprint of the GPG signature of a commit.
+
+signature:primarykeyfingerprint::
+	The Primary Key fingerprint of the GPG signature of a commit.
+
+signature:trustlevel::
+	The Trust level of the GPG signature of a commit. Possible
+	outputs are `ultimate`, `fully`, `marginal`, `never` and `undefined`.
+
 worktreepath::
 	The absolute path to the worktree in which the ref is checked
 	out, if it is checked out in any linked worktree. Empty string
diff --git a/ref-filter.c b/ref-filter.c
index a24324123e7..0cba756b186 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -144,6 +144,7 @@ enum atom_type {
 	ATOM_BODY,
 	ATOM_TRAILERS,
 	ATOM_CONTENTS,
+	ATOM_SIGNATURE,
 	ATOM_RAW,
 	ATOM_UPSTREAM,
 	ATOM_PUSH,
@@ -208,6 +209,10 @@ static struct used_atom {
 		struct email_option {
 			enum { EO_RAW, EO_TRIM, EO_LOCALPART } option;
 		} email_option;
+		struct {
+			enum { S_BARE, S_GRADE, S_SIGNER, S_KEY,
+			       S_FINGERPRINT, S_PRI_KEY_FP, S_TRUST_LEVEL} option;
+		} signature;
 		struct refname_atom refname;
 		char *head;
 	} u;
@@ -394,6 +399,34 @@ static int subject_atom_parser(struct ref_format *format, struct used_atom *atom
 	return 0;
 }
 
+static int parse_signature_option(const char *arg)
+{
+	if (!arg)
+		return S_BARE;
+	else if (!strcmp(arg, "signer"))
+		return S_SIGNER;
+	else if (!strcmp(arg, "grade"))
+		return S_GRADE;
+	else if (!strcmp(arg, "key"))
+		return S_KEY;
+	else if (!strcmp(arg, "fingerprint"))
+		return S_FINGERPRINT;
+	else if (!strcmp(arg, "primarykeyfingerprint"))
+		return S_PRI_KEY_FP;
+	else if (!strcmp(arg, "trustlevel"))
+		return S_TRUST_LEVEL;
+	return -1;
+}
+
+static int signature_atom_parser(struct ref_format *format UNUSED, struct used_atom *atom,
+			       const char *arg, struct strbuf *err){
+	int opt = parse_signature_option(arg);
+	if (opt < 0)
+		return err_bad_arg(err, "signature", arg);
+	atom->u.signature.option = opt;
+	return 0;
+}
+
 static int trailers_atom_parser(struct ref_format *format, struct used_atom *atom,
 				const char *arg, struct strbuf *err)
 {
@@ -631,6 +664,7 @@ static struct {
 	[ATOM_BODY] = { "body", SOURCE_OBJ, FIELD_STR, body_atom_parser },
 	[ATOM_TRAILERS] = { "trailers", SOURCE_OBJ, FIELD_STR, trailers_atom_parser },
 	[ATOM_CONTENTS] = { "contents", SOURCE_OBJ, FIELD_STR, contents_atom_parser },
+	[ATOM_SIGNATURE] = { "signature", SOURCE_OBJ, FIELD_STR, signature_atom_parser },
 	[ATOM_RAW] = { "raw", SOURCE_OBJ, FIELD_STR, raw_atom_parser },
 	[ATOM_UPSTREAM] = { "upstream", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },
 	[ATOM_PUSH] = { "push", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },
@@ -1362,6 +1396,72 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void
 	}
 }
 
+static void grab_signature(struct atom_value *val, int deref, struct object *obj)
+{
+	int i;
+	struct commit *commit = (struct commit *) obj;
+	struct signature_check sigc = { 0 };
+
+	check_commit_signature(commit, &sigc);
+
+	for (i = 0; i < used_atom_cnt; i++) {
+		struct used_atom *atom = &used_atom[i];
+		const char *name = atom->name;
+		struct atom_value *v = &val[i];
+
+		if (!!deref != (*name == '*'))
+			continue;
+		if (deref)
+			name++;
+
+		if (!skip_prefix(name, "signature", &name) || (*name &&
+			*name != ':'))
+			continue;
+		if (!*name)
+			name = NULL;
+		else
+			name++;
+		if (parse_signature_option(name) < 0)
+			continue;
+
+		if (atom->u.signature.option == S_BARE)
+			v->s = xstrdup(sigc.output ? sigc.output: "");
+		else if (atom->u.signature.option == S_SIGNER)
+			v->s = xstrdup(sigc.signer ? sigc.signer : "");
+		else if (atom->u.signature.option == S_GRADE) {
+			switch (sigc.result) {
+			case 'G':
+				switch (sigc.trust_level) {
+				case TRUST_UNDEFINED:
+				case TRUST_NEVER:
+					v->s = xstrfmt("%c", (char)'U');
+					break;
+				default:
+					v->s = xstrfmt("%c", (char)'G');
+					break;
+				}
+				break;
+			case 'B':
+			case 'E':
+			case 'N':
+			case 'X':
+			case 'Y':
+			case 'R':
+				v->s = xstrfmt("%c", (char)sigc.result);
+			}
+		}
+		else if (atom->u.signature.option == S_KEY)
+			v->s = xstrdup(sigc.key ? sigc.key : "");
+		else if (atom->u.signature.option == S_FINGERPRINT)
+			v->s = xstrdup(sigc.fingerprint ? sigc.fingerprint : "");
+		else if (atom->u.signature.option == S_PRI_KEY_FP)
+			v->s = xstrdup(sigc.primary_key_fingerprint ? sigc.primary_key_fingerprint : "");
+		else if (atom->u.signature.option == S_TRUST_LEVEL)
+			v->s = xstrdup(gpg_trust_level_to_str(sigc.trust_level));
+	}
+	signature_check_clear(&sigc);
+}
+
 static void find_subpos(const char *buf,
 			const char **sub, size_t *sublen,
 			const char **body, size_t *bodylen,
@@ -1555,6 +1655,7 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, s
 		grab_sub_body_contents(val, deref, data);
 		grab_person("author", val, deref, buf);
 		grab_person("committer", val, deref, buf);
+		grab_signature(val, deref, obj);
 		break;
 	case OBJ_TREE:
 		/* grab_tree_values(val, deref, obj, buf, sz); */
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 2ae1fc721b1..a8efe6f58ec 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
+GNUPGHOME_NOT_USED=$GNUPGHOME
 . "$TEST_DIRECTORY"/lib-gpg.sh
 . "$TEST_DIRECTORY"/lib-terminal.sh
 
@@ -1464,4 +1465,130 @@ sig_crlf="$(printf "%s" "$sig" | append_cr; echo dummy)"
 sig_crlf=${sig_crlf%dummy}
 test_atom refs/tags/fake-sig-crlf contents:signature "$sig_crlf"
 
+GRADE_FORMAT="%(signature:grade)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)"
+TRUSTLEVEL_FORMAT="%(signature:trustlevel)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)"
+
+test_expect_success GPG 'test bare signature atom' '
+	git checkout -b signed &&
+	echo 1 >file && git add file &&
+	test_tick && git commit -S -m initial &&
+	git verify-commit signed 2>out &&
+	head -3 out >expected &&
+	tail -1 out >>expected &&
+	echo >>expected &&
+	git for-each-ref refs/heads/signed --format="%(signature)" >actual &&
+	test_cmp actual expected
+'
+
+test_expect_success GPG 'show good signature with custom format' '
+	echo 2 >file && git add file &&
+	test_tick && git commit -S -m initial &&
+	git verify-commit signed 2>out &&
+	cat >expect <<-\EOF &&
+	G
+	13B6F51ECDDE430D
+	C O Mitter <committer@example.com>
+	73D758744BE721698EC54E8713B6F51ECDDE430D
+	73D758744BE721698EC54E8713B6F51ECDDE430D
+	EOF
+	git for-each-ref refs/heads/signed --format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'test signature atom with grade option and bad signature' '
+	git config commit.gpgsign true &&
+	echo 3 >file && test_tick && git commit -a -m "third" --no-gpg-sign &&
+	git tag third-unsigned &&
+
+	test_tick && git rebase -f HEAD^^ && git tag second-signed HEAD^ &&
+	git tag third-signed &&
+
+	git cat-file commit third-signed >raw &&
+	sed -e "s/^third/3rd forged/" raw >forged1 &&
+	FORGED1=$(git hash-object -w -t commit forged1) &&
+	git update-ref refs/tags/third-signed "$FORGED1" &&
+	test_must_fail git verify-commit "$FORGED1" &&
+
+	cat >expect <<-\EOF &&
+	B
+	13B6F51ECDDE430D
+	C O Mitter <committer@example.com>
+
+
+	EOF
+	git for-each-ref refs/tags/third-signed --format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show untrusted signature with custom format' '
+	echo 4 >file && test_tick && git commit -a -m fourth -SB7227189 &&
+	git tag signed-fourth &&
+	cat >expect <<-\EOF &&
+	U
+	65A0EEA02E30CAD7
+	Eris Discordia <discord@example.net>
+	F8364A59E07FFE9F4D63005A65A0EEA02E30CAD7
+	D4BE22311AD3131E5EDA29A461092E85B7227189
+	EOF
+	git for-each-ref refs/tags/signed-fourth --format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show untrusted signature with undefined trust level' '
+	echo 5 >file && test_tick && git commit -a -m fifth -SB7227189 &&
+	git tag fifth-signed &&
+	cat >expect <<-\EOF &&
+	undefined
+	65A0EEA02E30CAD7
+	Eris Discordia <discord@example.net>
+	F8364A59E07FFE9F4D63005A65A0EEA02E30CAD7
+	D4BE22311AD3131E5EDA29A461092E85B7227189
+	EOF
+	git for-each-ref refs/tags/fifth-signed --format="$TRUSTLEVEL_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show untrusted signature with ultimate trust level' '
+	echo 7 >file && test_tick && git commit -a -m "seventh" --no-gpg-sign &&
+	git tag seventh-unsigned &&
+
+	test_tick && git rebase -f HEAD^^ && git tag sixth-signed HEAD^ &&
+	git tag seventh-signed &&
+	cat >expect <<-\EOF &&
+	ultimate
+	13B6F51ECDDE430D
+	C O Mitter <committer@example.com>
+	73D758744BE721698EC54E8713B6F51ECDDE430D
+	73D758744BE721698EC54E8713B6F51ECDDE430D
+	EOF
+	git for-each-ref refs/tags/seventh-signed --format="$TRUSTLEVEL_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show unknown signature with custom format' '
+	cat >expect <<-\EOF &&
+	E
+	65A0EEA02E30CAD7
+
+
+
+	EOF
+	GNUPGHOME="$GNUPGHOME_NOT_USED" git for-each-ref refs/tags/fifth-signed --format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show lack of signature with custom format' '
+	echo 8 >file && test_tick && git commit -a -m "eigth unsigned" --no-gpg-sign &&
+	git tag eigth-unsigned &&
+	cat >expect <<-\EOF &&
+	N
+
+
+
+
+	EOF
+	git for-each-ref refs/tags/eigth-unsigned --format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
 test_done

base-commit: 6bae53b138a1f38d8887f6b46d17661357a1468b
-- 
gitgitgadget

Re: [PATCH] ref-filter: add new atom "signature" atom

[Christian Couder]

On Mon, Jan 9, 2023 at 10:15 AM nsengaw4c via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Nsengiyumva Wilberforce <nsengiyumvawilberforce@gmail.com>

This patch should be marked as a V2 in its subject:

[PATCH v2] ref-filter: add new atom "signature" atom

not sure how to do that using GitGitGadget though.

Also "atom" appears twice in the subject, so the following would be even better:

[PATCH v2] ref-filter: add new "signature" atom

I am not sure how, but GitGitGadget should allow to set the
"In-Reply-To:" to the message ID of your previous version of this
patch, so that your email with the version 2 of the patch would appear
in the same email thread as the email of your previous version of this
patch on lore.kernel.org/git:

https://lore.kernel.org/git/pull.1452.git.1672102523902.gitgitgadget@gmail.com/

(Please don't resend this patch as a v2, but as a v3, if you make any change.)

> This commit duplicates the code for `signature` atom from pretty.c
> to ref-filter.c. This feature will help to get rid of current duplicate
> implementation of `signature` atom when unifying implemenations by

s/implemenations/implementations/

> using ref-filter logic everywhere when ref-filter can do everything
> pretty is doing.
>
> Add "signature" atom with `grade`, `signer`, `key`,
> `fingerprint`, `primarykeyfingerprint`, `trustlevel` as arguments.
> This code and its documentation are inspired by how the %GG, %G?,
> %GS, %GK, %GF, %GP, and %GT pretty formats were implemented.
>
> Co-authored-by: Hariom Verma <hariom18599@gmail.com>
> Co-authored-by: Jaydeep Das <jaydeepjd.8914@gmail.com>
> Mentored-by: Christian Couder <chriscool@tuxfamily.org>
> Mentored-by: Hariom Verma <hariom18599@gmail.com>
> Signed-off-by: Nsengiyumva Wilberforce <nsengiyumvawilberforce@gmail.com>
> ---
>     ref-filter: add new atom "signature" atom
>
>     This commit duplicates the code for signature atom from pretty.c to
>     ref-filter.c. This feature will help to get rid of current duplicate
>     implementation of signature atom when unifying implemenations by using

s/implemenations/implementations/

>     ref-filter logic everywhere when ref-filter can do everything pretty is
>     doing.
>
>     Add "signature" atom with grade, signer, key, fingerprint,
>     primarykeyfingerprint, trustlevel as arguments. This code and its
>     documentation are inspired by how the %GG, %G?, %GS, %GK, %GF, %GP, and
>     %GT pretty formats were implemented.
>
>     Co-authored-by: Hariom Verma hariom18599@gmail.com Co-authored-by:
>     Jaydeep Das jaydeepjd.8914@gmail.com Mentored-by: Christian Couder
>     chriscool@tuxfamily.org Mentored-by: Hariom Verma hariom18599@gmail.com
>     Signed-off-by: Nsengiyumva Wilberforce nsengiyumvawilberforce@gmail.com

Not sure you can do something about it, but the above lines aren't
properly wrapped.

> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1428%2Fnsengiyumva-wilberforce%2Fsignature10-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1428/nsengiyumva-wilberforce/signature10-v1
> Pull-Request: https://github.com/git/git/pull/1428
>
>  Documentation/git-for-each-ref.txt |  27 ++++++
>  ref-filter.c                       | 101 +++++++++++++++++++++++
>  t/t6300-for-each-ref.sh            | 127 +++++++++++++++++++++++++++++
>  3 files changed, 255 insertions(+)
>
> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
> index 6da899c6296..9a0be85368b 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -212,6 +212,33 @@ symref::
>         `:lstrip` and `:rstrip` options in the same way as `refname`
>         above.
>
> +signature::
> +       The GPG signature of a commit.
> +
> +signature:grade::
> +       Show "G" for a good (valid) signature, "B" for a bad
> +       signature, "U" for a good signature with unknown validity, "X"
> +       for a good signature that has expired, "Y" for a good
> +       signature made by an expired key, "R" for a good signature
> +       made by a revoked key, "E" if the signature cannot be
> +       checked (e.g. missing key) and "N" for no signature.
> +
> +signature:signer::
> +       The signer of the GPG signature of a commit.
> +
> +signature:key::
> +       The key of the GPG signature of a commit.
> +
> +signature:fingerprint::
> +       The fingerprint of the GPG signature of a commit.
> +
> +signature:primarykeyfingerprint::
> +       The Primary Key fingerprint of the GPG signature of a commit.
> +
> +signature:trustlevel::
> +       The Trust level of the GPG signature of a commit. Possible
> +       outputs are `ultimate`, `fully`, `marginal`, `never` and `undefined`.
> +
>  worktreepath::
>         The absolute path to the worktree in which the ref is checked
>         out, if it is checked out in any linked worktree. Empty string
> diff --git a/ref-filter.c b/ref-filter.c
> index a24324123e7..0cba756b186 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -144,6 +144,7 @@ enum atom_type {
>         ATOM_BODY,
>         ATOM_TRAILERS,
>         ATOM_CONTENTS,
> +       ATOM_SIGNATURE,
>         ATOM_RAW,
>         ATOM_UPSTREAM,
>         ATOM_PUSH,
> @@ -208,6 +209,10 @@ static struct used_atom {
>                 struct email_option {
>                         enum { EO_RAW, EO_TRIM, EO_LOCALPART } option;
>                 } email_option;
> +               struct {
> +                       enum { S_BARE, S_GRADE, S_SIGNER, S_KEY,
> +                              S_FINGERPRINT, S_PRI_KEY_FP, S_TRUST_LEVEL} option;
> +               } signature;
>                 struct refname_atom refname;
>                 char *head;
>         } u;
> @@ -394,6 +399,34 @@ static int subject_atom_parser(struct ref_format *format, struct used_atom *atom
>         return 0;
>  }
>
> +static int parse_signature_option(const char *arg)
> +{
> +       if (!arg)
> +               return S_BARE;
> +       else if (!strcmp(arg, "signer"))
> +               return S_SIGNER;
> +       else if (!strcmp(arg, "grade"))
> +               return S_GRADE;
> +       else if (!strcmp(arg, "key"))
> +               return S_KEY;
> +       else if (!strcmp(arg, "fingerprint"))
> +               return S_FINGERPRINT;
> +       else if (!strcmp(arg, "primarykeyfingerprint"))
> +               return S_PRI_KEY_FP;
> +       else if (!strcmp(arg, "trustlevel"))
> +               return S_TRUST_LEVEL;
> +       return -1;
> +}
> +
> +static int signature_atom_parser(struct ref_format *format UNUSED, struct used_atom *atom,
> +                              const char *arg, struct strbuf *err){
> +       int opt = parse_signature_option(arg);
> +       if (opt < 0)
> +               return err_bad_arg(err, "signature", arg);
> +       atom->u.signature.option = opt;
> +       return 0;
> +}
> +
>  static int trailers_atom_parser(struct ref_format *format, struct used_atom *atom,
>                                 const char *arg, struct strbuf *err)
>  {
> @@ -631,6 +664,7 @@ static struct {
>         [ATOM_BODY] = { "body", SOURCE_OBJ, FIELD_STR, body_atom_parser },
>         [ATOM_TRAILERS] = { "trailers", SOURCE_OBJ, FIELD_STR, trailers_atom_parser },
>         [ATOM_CONTENTS] = { "contents", SOURCE_OBJ, FIELD_STR, contents_atom_parser },
> +       [ATOM_SIGNATURE] = { "signature", SOURCE_OBJ, FIELD_STR, signature_atom_parser },
>         [ATOM_RAW] = { "raw", SOURCE_OBJ, FIELD_STR, raw_atom_parser },
>         [ATOM_UPSTREAM] = { "upstream", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },
>         [ATOM_PUSH] = { "push", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },
> @@ -1362,6 +1396,72 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void
>         }
>  }
>
> +static void grab_signature(struct atom_value *val, int deref, struct object *obj)
> +{
> +       int i;
> +       struct commit *commit = (struct commit *) obj;
> +       struct signature_check sigc = { 0 };
> +
> +       check_commit_signature(commit, &sigc);
> +
> +       for (i = 0; i < used_atom_cnt; i++) {
> +               struct used_atom *atom = &used_atom[i];
> +               const char *name = atom->name;
> +               struct atom_value *v = &val[i];
> +
> +               if (!!deref != (*name == '*'))
> +                       continue;
> +               if (deref)
> +                       name++;
> +
> +               if (!skip_prefix(name, "signature", &name) || (*name &&
> +                       *name != ':'))
> +                       continue;
> +               if (!*name)
> +                       name = NULL;
> +               else
> +                       name++;
> +               if (parse_signature_option(name) < 0)
> +                       continue;
> +
> +               if (atom->u.signature.option == S_BARE)
> +                       v->s = xstrdup(sigc.output ? sigc.output: "");
> +               else if (atom->u.signature.option == S_SIGNER)
> +                       v->s = xstrdup(sigc.signer ? sigc.signer : "");
> +               else if (atom->u.signature.option == S_GRADE) {
> +                       switch (sigc.result) {
> +                       case 'G':
> +                               switch (sigc.trust_level) {
> +                               case TRUST_UNDEFINED:
> +                               case TRUST_NEVER:
> +                                       v->s = xstrfmt("%c", (char)'U');
> +                                       break;
> +                               default:
> +                                       v->s = xstrfmt("%c", (char)'G');
> +                                       break;
> +                               }
> +                               break;
> +                       case 'B':
> +                       case 'E':
> +                       case 'N':
> +                       case 'X':
> +                       case 'Y':
> +                       case 'R':
> +                               v->s = xstrfmt("%c", (char)sigc.result);
> +                       }
> +               }
> +               else if (atom->u.signature.option == S_KEY)
> +                       v->s = xstrdup(sigc.key ? sigc.key : "");
> +               else if (atom->u.signature.option == S_FINGERPRINT)
> +                       v->s = xstrdup(sigc.fingerprint ? sigc.fingerprint : "");
> +               else if (atom->u.signature.option == S_PRI_KEY_FP)
> +                       v->s = xstrdup(sigc.primary_key_fingerprint ? sigc.primary_key_fingerprint : "");
> +               else if (atom->u.signature.option == S_TRUST_LEVEL)
> +                       v->s = xstrdup(gpg_trust_level_to_str(sigc.trust_level));
> +       }
> +       signature_check_clear(&sigc);
> +}
> +
>  static void find_subpos(const char *buf,
>                         const char **sub, size_t *sublen,
>                         const char **body, size_t *bodylen,
> @@ -1555,6 +1655,7 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, s
>                 grab_sub_body_contents(val, deref, data);
>                 grab_person("author", val, deref, buf);
>                 grab_person("committer", val, deref, buf);
> +               grab_signature(val, deref, obj);
>                 break;
>         case OBJ_TREE:
>                 /* grab_tree_values(val, deref, obj, buf, sz); */
> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> index 2ae1fc721b1..a8efe6f58ec 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
> +GNUPGHOME_NOT_USED=$GNUPGHOME
>  . "$TEST_DIRECTORY"/lib-gpg.sh
>  . "$TEST_DIRECTORY"/lib-terminal.sh
>
> @@ -1464,4 +1465,130 @@ sig_crlf="$(printf "%s" "$sig" | append_cr; echo dummy)"
>  sig_crlf=${sig_crlf%dummy}
>  test_atom refs/tags/fake-sig-crlf contents:signature "$sig_crlf"
>
> +GRADE_FORMAT="%(signature:grade)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)"
> +TRUSTLEVEL_FORMAT="%(signature:trustlevel)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)"
> +
> +test_expect_success GPG 'test bare signature atom' '
> +       git checkout -b signed &&
> +       echo 1 >file && git add file &&
> +       test_tick && git commit -S -m initial &&
> +       git verify-commit signed 2>out &&
> +       head -3 out >expected &&
> +       tail -1 out >>expected &&
> +       echo >>expected &&
> +       git for-each-ref refs/heads/signed --format="%(signature)" >actual &&
> +       test_cmp actual expected
> +'

(I already commented about this test in a previous email related to
how it fails on GitHub CI.)

Re: [PATCH] ref-filter: add new atom "signature" atom

[NSENGIYUMVA WILBERFORCE]

> <gitgitgadget@gmail.com> wrote:
> >
> > From: Nsengiyumva Wilberforce <nsengiyumvawilberforce@gmail.com>
>
> This patch should be marked as a V2 in its subject:
>
> [PATCH v2] ref-filter: add new atom "signature" atom
>
> not sure how to do that using GitGitGadget though.
>
> Also "atom" appears twice in the subject, so the following would be even better:
>
> [PATCH v2] ref-filter: add new "signature" atom
>
> I am not sure how, but GitGitGadget should allow to set the
> "In-Reply-To:" to the message ID of your previous version of this
> patch, so that your email with the version 2 of the patch would appear
> in the same email thread as the email of your previous version of this
> patch on lore.kernel.org/git:
>
> https://lore.kernel.org/git/pull.1452.git.1672102523902.gitgitgadget@gmail.com/
>
> (Please don't resend this patch as a v2, but as a v3, if you make any change.)

I do not see any option for changing the ID, I think I need some
directions on how to change the patch version



On Mon, Jan 9, 2023 at 4:45 AM Christian Couder
<christian.couder@gmail.com> wrote:
>
> On Mon, Jan 9, 2023 at 10:15 AM nsengaw4c via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> >
> > From: Nsengiyumva Wilberforce <nsengiyumvawilberforce@gmail.com>
>
> This patch should be marked as a V2 in its subject:
>
> [PATCH v2] ref-filter: add new atom "signature" atom
>
> not sure how to do that using GitGitGadget though.
>
> Also "atom" appears twice in the subject, so the following would be even better:
>
> [PATCH v2] ref-filter: add new "signature" atom
>
> I am not sure how, but GitGitGadget should allow to set the
> "In-Reply-To:" to the message ID of your previous version of this
> patch, so that your email with the version 2 of the patch would appear
> in the same email thread as the email of your previous version of this
> patch on lore.kernel.org/git:
>
> https://lore.kernel.org/git/pull.1452.git.1672102523902.gitgitgadget@gmail.com/
>
> (Please don't resend this patch as a v2, but as a v3, if you make any change.)
>
> > This commit duplicates the code for `signature` atom from pretty.c
> > to ref-filter.c. This feature will help to get rid of current duplicate
> > implementation of `signature` atom when unifying implemenations by
>
> s/implemenations/implementations/
>
> > using ref-filter logic everywhere when ref-filter can do everything
> > pretty is doing.
> >
> > Add "signature" atom with `grade`, `signer`, `key`,
> > `fingerprint`, `primarykeyfingerprint`, `trustlevel` as arguments.
> > This code and its documentation are inspired by how the %GG, %G?,
> > %GS, %GK, %GF, %GP, and %GT pretty formats were implemented.
> >
> > Co-authored-by: Hariom Verma <hariom18599@gmail.com>
> > Co-authored-by: Jaydeep Das <jaydeepjd.8914@gmail.com>
> > Mentored-by: Christian Couder <chriscool@tuxfamily.org>
> > Mentored-by: Hariom Verma <hariom18599@gmail.com>
> > Signed-off-by: Nsengiyumva Wilberforce <nsengiyumvawilberforce@gmail.com>
> > ---
> >     ref-filter: add new atom "signature" atom
> >
> >     This commit duplicates the code for signature atom from pretty.c to
> >     ref-filter.c. This feature will help to get rid of current duplicate
> >     implementation of signature atom when unifying implemenations by using
>
> s/implemenations/implementations/
>
> >     ref-filter logic everywhere when ref-filter can do everything pretty is
> >     doing.
> >
> >     Add "signature" atom with grade, signer, key, fingerprint,
> >     primarykeyfingerprint, trustlevel as arguments. This code and its
> >     documentation are inspired by how the %GG, %G?, %GS, %GK, %GF, %GP, and
> >     %GT pretty formats were implemented.
> >
> >     Co-authored-by: Hariom Verma hariom18599@gmail.com Co-authored-by:
> >     Jaydeep Das jaydeepjd.8914@gmail.com Mentored-by: Christian Couder
> >     chriscool@tuxfamily.org Mentored-by: Hariom Verma hariom18599@gmail.com
> >     Signed-off-by: Nsengiyumva Wilberforce nsengiyumvawilberforce@gmail.com
>
> Not sure you can do something about it, but the above lines aren't
> properly wrapped.
>
> > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1428%2Fnsengiyumva-wilberforce%2Fsignature10-v1
> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1428/nsengiyumva-wilberforce/signature10-v1
> > Pull-Request: https://github.com/git/git/pull/1428
> >
> >  Documentation/git-for-each-ref.txt |  27 ++++++
> >  ref-filter.c                       | 101 +++++++++++++++++++++++
> >  t/t6300-for-each-ref.sh            | 127 +++++++++++++++++++++++++++++
> >  3 files changed, 255 insertions(+)
> >
> > diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
> > index 6da899c6296..9a0be85368b 100644
> > --- a/Documentation/git-for-each-ref.txt
> > +++ b/Documentation/git-for-each-ref.txt
> > @@ -212,6 +212,33 @@ symref::
> >         `:lstrip` and `:rstrip` options in the same way as `refname`
> >         above.
> >
> > +signature::
> > +       The GPG signature of a commit.
> > +
> > +signature:grade::
> > +       Show "G" for a good (valid) signature, "B" for a bad
> > +       signature, "U" for a good signature with unknown validity, "X"
> > +       for a good signature that has expired, "Y" for a good
> > +       signature made by an expired key, "R" for a good signature
> > +       made by a revoked key, "E" if the signature cannot be
> > +       checked (e.g. missing key) and "N" for no signature.
> > +
> > +signature:signer::
> > +       The signer of the GPG signature of a commit.
> > +
> > +signature:key::
> > +       The key of the GPG signature of a commit.
> > +
> > +signature:fingerprint::
> > +       The fingerprint of the GPG signature of a commit.
> > +
> > +signature:primarykeyfingerprint::
> > +       The Primary Key fingerprint of the GPG signature of a commit.
> > +
> > +signature:trustlevel::
> > +       The Trust level of the GPG signature of a commit. Possible
> > +       outputs are `ultimate`, `fully`, `marginal`, `never` and `undefined`.
> > +
> >  worktreepath::
> >         The absolute path to the worktree in which the ref is checked
> >         out, if it is checked out in any linked worktree. Empty string
> > diff --git a/ref-filter.c b/ref-filter.c
> > index a24324123e7..0cba756b186 100644
> > --- a/ref-filter.c
> > +++ b/ref-filter.c
> > @@ -144,6 +144,7 @@ enum atom_type {
> >         ATOM_BODY,
> >         ATOM_TRAILERS,
> >         ATOM_CONTENTS,
> > +       ATOM_SIGNATURE,
> >         ATOM_RAW,
> >         ATOM_UPSTREAM,
> >         ATOM_PUSH,
> > @@ -208,6 +209,10 @@ static struct used_atom {
> >                 struct email_option {
> >                         enum { EO_RAW, EO_TRIM, EO_LOCALPART } option;
> >                 } email_option;
> > +               struct {
> > +                       enum { S_BARE, S_GRADE, S_SIGNER, S_KEY,
> > +                              S_FINGERPRINT, S_PRI_KEY_FP, S_TRUST_LEVEL} option;
> > +               } signature;
> >                 struct refname_atom refname;
> >                 char *head;
> >         } u;
> > @@ -394,6 +399,34 @@ static int subject_atom_parser(struct ref_format *format, struct used_atom *atom
> >         return 0;
> >  }
> >
> > +static int parse_signature_option(const char *arg)
> > +{
> > +       if (!arg)
> > +               return S_BARE;
> > +       else if (!strcmp(arg, "signer"))
> > +               return S_SIGNER;
> > +       else if (!strcmp(arg, "grade"))
> > +               return S_GRADE;
> > +       else if (!strcmp(arg, "key"))
> > +               return S_KEY;
> > +       else if (!strcmp(arg, "fingerprint"))
> > +               return S_FINGERPRINT;
> > +       else if (!strcmp(arg, "primarykeyfingerprint"))
> > +               return S_PRI_KEY_FP;
> > +       else if (!strcmp(arg, "trustlevel"))
> > +               return S_TRUST_LEVEL;
> > +       return -1;
> > +}
> > +
> > +static int signature_atom_parser(struct ref_format *format UNUSED, struct used_atom *atom,
> > +                              const char *arg, struct strbuf *err){
> > +       int opt = parse_signature_option(arg);
> > +       if (opt < 0)
> > +               return err_bad_arg(err, "signature", arg);
> > +       atom->u.signature.option = opt;
> > +       return 0;
> > +}
> > +
> >  static int trailers_atom_parser(struct ref_format *format, struct used_atom *atom,
> >                                 const char *arg, struct strbuf *err)
> >  {
> > @@ -631,6 +664,7 @@ static struct {
> >         [ATOM_BODY] = { "body", SOURCE_OBJ, FIELD_STR, body_atom_parser },
> >         [ATOM_TRAILERS] = { "trailers", SOURCE_OBJ, FIELD_STR, trailers_atom_parser },
> >         [ATOM_CONTENTS] = { "contents", SOURCE_OBJ, FIELD_STR, contents_atom_parser },
> > +       [ATOM_SIGNATURE] = { "signature", SOURCE_OBJ, FIELD_STR, signature_atom_parser },
> >         [ATOM_RAW] = { "raw", SOURCE_OBJ, FIELD_STR, raw_atom_parser },
> >         [ATOM_UPSTREAM] = { "upstream", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },
> >         [ATOM_PUSH] = { "push", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },
> > @@ -1362,6 +1396,72 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void
> >         }
> >  }
> >
> > +static void grab_signature(struct atom_value *val, int deref, struct object *obj)
> > +{
> > +       int i;
> > +       struct commit *commit = (struct commit *) obj;
> > +       struct signature_check sigc = { 0 };
> > +
> > +       check_commit_signature(commit, &sigc);
> > +
> > +       for (i = 0; i < used_atom_cnt; i++) {
> > +               struct used_atom *atom = &used_atom[i];
> > +               const char *name = atom->name;
> > +               struct atom_value *v = &val[i];
> > +
> > +               if (!!deref != (*name == '*'))
> > +                       continue;
> > +               if (deref)
> > +                       name++;
> > +
> > +               if (!skip_prefix(name, "signature", &name) || (*name &&
> > +                       *name != ':'))
> > +                       continue;
> > +               if (!*name)
> > +                       name = NULL;
> > +               else
> > +                       name++;
> > +               if (parse_signature_option(name) < 0)
> > +                       continue;
> > +
> > +               if (atom->u.signature.option == S_BARE)
> > +                       v->s = xstrdup(sigc.output ? sigc.output: "");
> > +               else if (atom->u.signature.option == S_SIGNER)
> > +                       v->s = xstrdup(sigc.signer ? sigc.signer : "");
> > +               else if (atom->u.signature.option == S_GRADE) {
> > +                       switch (sigc.result) {
> > +                       case 'G':
> > +                               switch (sigc.trust_level) {
> > +                               case TRUST_UNDEFINED:
> > +                               case TRUST_NEVER:
> > +                                       v->s = xstrfmt("%c", (char)'U');
> > +                                       break;
> > +                               default:
> > +                                       v->s = xstrfmt("%c", (char)'G');
> > +                                       break;
> > +                               }
> > +                               break;
> > +                       case 'B':
> > +                       case 'E':
> > +                       case 'N':
> > +                       case 'X':
> > +                       case 'Y':
> > +                       case 'R':
> > +                               v->s = xstrfmt("%c", (char)sigc.result);
> > +                       }
> > +               }
> > +               else if (atom->u.signature.option == S_KEY)
> > +                       v->s = xstrdup(sigc.key ? sigc.key : "");
> > +               else if (atom->u.signature.option == S_FINGERPRINT)
> > +                       v->s = xstrdup(sigc.fingerprint ? sigc.fingerprint : "");
> > +               else if (atom->u.signature.option == S_PRI_KEY_FP)
> > +                       v->s = xstrdup(sigc.primary_key_fingerprint ? sigc.primary_key_fingerprint : "");
> > +               else if (atom->u.signature.option == S_TRUST_LEVEL)
> > +                       v->s = xstrdup(gpg_trust_level_to_str(sigc.trust_level));
> > +       }
> > +       signature_check_clear(&sigc);
> > +}
> > +
> >  static void find_subpos(const char *buf,
> >                         const char **sub, size_t *sublen,
> >                         const char **body, size_t *bodylen,
> > @@ -1555,6 +1655,7 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, s
> >                 grab_sub_body_contents(val, deref, data);
> >                 grab_person("author", val, deref, buf);
> >                 grab_person("committer", val, deref, buf);
> > +               grab_signature(val, deref, obj);
> >                 break;
> >         case OBJ_TREE:
> >                 /* grab_tree_values(val, deref, obj, buf, sz); */
> > diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> > index 2ae1fc721b1..a8efe6f58ec 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
> > +GNUPGHOME_NOT_USED=$GNUPGHOME
> >  . "$TEST_DIRECTORY"/lib-gpg.sh
> >  . "$TEST_DIRECTORY"/lib-terminal.sh
> >
> > @@ -1464,4 +1465,130 @@ sig_crlf="$(printf "%s" "$sig" | append_cr; echo dummy)"
> >  sig_crlf=${sig_crlf%dummy}
> >  test_atom refs/tags/fake-sig-crlf contents:signature "$sig_crlf"
> >
> > +GRADE_FORMAT="%(signature:grade)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)"
> > +TRUSTLEVEL_FORMAT="%(signature:trustlevel)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)"
> > +
> > +test_expect_success GPG 'test bare signature atom' '
> > +       git checkout -b signed &&
> > +       echo 1 >file && git add file &&
> > +       test_tick && git commit -S -m initial &&
> > +       git verify-commit signed 2>out &&
> > +       head -3 out >expected &&
> > +       tail -1 out >>expected &&
> > +       echo >>expected &&
> > +       git for-each-ref refs/heads/signed --format="%(signature)" >actual &&
> > +       test_cmp actual expected
> > +'
>
> (I already commented about this test in a previous email related to
> how it fails on GitHub CI.)