{"thread":{"id":"59006","subject":"[PATCH] ref-filter: add new atom \"signature\" atom","startedAt":"2022-12-27T01:00:00Z","lastAt":"2023-04-29T18:37:46Z","messageCount":19,"participants":["nsengaw4c via GitGitGadget","Junio C Hamano","Jeff King","NSENGIYUMVA WILBERFORCE","Christian Couder","Nsengiyumva Wilberforce","Kousik Sanagavarapu"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"469542","messageId":"pull.1452.git.1672102523902.gitgitgadget@gmail.com","threadId":"59006","inReplyTo":null,"subject":"[PATCH] ref-filter: add new atom \"signature\" atom","fromName":"nsengaw4c via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-12-27T00:55:23Z","receivedAt":"2022-12-27T01:00:00Z","isPatch":true,"sender":{"key":"name:nsengaw4c","avatar":null},"body":"From: Nsengiyumva Wilberforce \n\nThis only works for commits. Add \"signature\" atom with `grade`,\n`signer`, `key`, `fingerprint`, `primarykeyfingerprint`, `trustlevel`\nas arguments. This code and it's documentation are inspired by\nhow the %GG, %G?, %GS, %GK, %GF, %GP, and %GT pretty formats were\nimplemented.\n\nCo-authored-by: Hariom Verma \nCo-authored-by: Jaydeep Das \nMentored-by: Christian Couder \nMentored-by: Hariom Verma \nSigned-off-by: Nsengiyumva Wilberforce \n---\n ref-filter: add new atom \"signature\" atom\n \n This only works for commits. Add \"signature\" atom with grade, signer,\n key, fingerprint, primarykeyfingerprint, trustlevel as arguments. This\n code and it's documentation are inspired by how the %GG, %G?, %GS, %GK,\n %GF, %GP, and %GT pretty formats were implemented.\n \n Co-authored-by: Hariom Verma hariom18599@gmail.com Co-authored-by:\n Jaydeep Das jaydeepjd.8914@gmail.com Mentored-by: Christian Couder\n chriscool@tuxfamily.org Mentored-by: Hariom Verma hariom18599@gmail.com\n Signed-off-by: Nsengiyumva Wilberforce nsengiyumvawilberforce@gmail.com\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1452%2Fnsengiyumva-wilberforce%2Fsignature6-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1452/nsengiyumva-wilberforce/signature6-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1452\n\n Documentation/git-for-each-ref.txt | 27 +++++++\n ref-filter.c | 94 +++++++++++++++++++++++\n t/t6300-for-each-ref.sh | 116 +++++++++++++++++++++++++++++\n 3 files changed, 237 insertions(+)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 6da899c6296..9a0be85368b 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -212,6 +212,33 @@ symref::\n \t`:lstrip` and `:rstrip` options in the same way as `refname`\n \tabove.\n \n+signature::\n+\tThe GPG signature of a commit.\n+\n+signature:grade::\n+\tShow \"G\" for a good (valid) signature, \"B\" for a bad\n+\tsignature, \"U\" for a good signature with unknown validity, \"X\"\n+\tfor a good signature that has expired, \"Y\" for a good\n+\tsignature made by an expired key, \"R\" for a good signature\n+\tmade by a revoked key, \"E\" if the signature cannot be\n+\tchecked (e.g. missing key) and \"N\" for no signature.\n+\n+signature:signer::\n+\tThe signer of the GPG signature of a commit.\n+\n+signature:key::\n+\tThe key of the GPG signature of a commit.\n+\n+signature:fingerprint::\n+\tThe fingerprint of the GPG signature of a commit.\n+\n+signature:primarykeyfingerprint::\n+\tThe Primary Key fingerprint of the GPG signature of a commit.\n+\n+signature:trustlevel::\n+\tThe Trust level of the GPG signature of a commit. Possible\n+\toutputs are `ultimate`, `fully`, `marginal`, `never` and `undefined`.\n+\n worktreepath::\n \tThe absolute path to the worktree in which the ref is checked\n \tout, if it is checked out in any linked worktree. Empty string\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 9dc2cd14519..bb3624fb4e9 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -144,6 +144,7 @@ enum atom_type {\n \tATOM_BODY,\n \tATOM_TRAILERS,\n \tATOM_CONTENTS,\n+\tATOM_SIGNATURE,\n \tATOM_RAW,\n \tATOM_UPSTREAM,\n \tATOM_PUSH,\n@@ -208,6 +209,10 @@ static struct used_atom {\n \t\tstruct email_option {\n \t\t\tenum { EO_RAW, EO_TRIM, EO_LOCALPART } option;\n \t\t} email_option;\n+\t\tstruct {\n+\t\t\tenum { S_BARE, S_GRADE, S_SIGNER, S_KEY, S_PRI_KEY_FP,\n+\t\t\t S_FINGERPRINT, S_TRUST_LEVEL } option;\n+\t\t} signature;\n \t\tstruct refname_atom refname;\n \t\tchar *head;\n \t} u;\n@@ -378,6 +383,30 @@ static int subject_atom_parser(struct ref_format *format, struct used_atom *atom\n \treturn 0;\n }\n \n+static int signature_atom_parser(struct ref_format *format, struct used_atom *atom,\n+\t\t\t const char *arg, struct strbuf *err)\n+{\n+\tif (arg) {\n+\t\tif (!strcmp(arg, \"signer\"))\n+\t\t\tatom->u.signature.option = S_SIGNER;\n+\t\telse if (!strcmp(arg, \"grade\"))\n+\t\t\tatom->u.signature.option = S_GRADE;\n+\t\telse if (!strcmp(arg, \"key\"))\n+\t\t\tatom->u.signature.option = S_KEY;\n+\t\telse if (!strcmp(arg, \"fingerprint\"))\n+\t\t\tatom->u.signature.option = S_FINGERPRINT;\n+\t\telse if (!strcmp(arg, \"primarykeyfingerprint\"))\n+\t\t\tatom->u.signature.option = S_PRI_KEY_FP;\n+\t\telse if (!strcmp(arg, \"trustlevel\"))\n+\t\t\tatom->u.signature.option = S_TRUST_LEVEL;\n+\t\telse\n+\t\t\treturn strbuf_addf_ret(err, -1, _(\"unknown %%(signature) argument: %s\"), arg);\n+\t}\n+\telse\n+\t\tatom->u.signature.option = S_BARE;\n+\treturn 0;\n+}\n+\n static int trailers_atom_parser(struct ref_format *format, struct used_atom *atom,\n \t\t\t\tconst char *arg, struct strbuf *err)\n {\n@@ -613,6 +642,7 @@ static struct {\n \t[ATOM_BODY] = { \"body\", SOURCE_OBJ, FIELD_STR, body_atom_parser },\n \t[ATOM_TRAILERS] = { \"trailers\", SOURCE_OBJ, FIELD_STR, trailers_atom_parser },\n \t[ATOM_CONTENTS] = { \"contents\", SOURCE_OBJ, FIELD_STR, contents_atom_parser },\n+\t[ATOM_SIGNATURE] = { \"signature\", SOURCE_OBJ, FIELD_STR, signature_atom_parser },\n \t[ATOM_RAW] = { \"raw\", SOURCE_OBJ, FIELD_STR, raw_atom_parser },\n \t[ATOM_UPSTREAM] = { \"upstream\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n \t[ATOM_PUSH] = { \"push\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n@@ -1344,6 +1374,69 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void\n \t}\n }\n \n+static void grab_signature(struct atom_value *val, int deref, struct object *obj)\n+{\n+\tint i;\n+\tstruct commit *commit = (struct commit *) obj;\n+\n+\tfor (i = 0; i < used_atom_cnt; i++) {\n+\t\tstruct used_atom *atom = &used_atom[i];\n+\t\tconst char *name = atom->name;\n+\t\tstruct atom_value *v = &val[i];\n+\t\tstruct signature_check sigc = { 0 };\n+\n+\t\tif (!!deref != (*name == '*'))\n+\t\t\tcontinue;\n+\t\tif (deref)\n+\t\t\tname++;\n+\t\tif (strcmp(name, \"signature\") &&\n+\t\t\tstrcmp(name, \"signature:signer\") &&\n+\t\t\tstrcmp(name, \"signature:grade\") &&\n+\t\t\tstrcmp(name, \"signature:key\") &&\n+\t\t\tstrcmp(name, \"signature:fingerprint\") &&\n+\t\t\tstrcmp(name, \"signature:primarykeyfingerprint\") &&\n+\t\t\tstrcmp(name, \"signature:trustlevel\"))\n+\t\t\tcontinue;\n+\n+\t\tcheck_commit_signature(commit, &sigc);\n+\n+\t\tif (atom->u.signature.option == S_BARE)\n+\t\t\tv->s = xstrdup(sigc.output ? sigc.output: \"\");\n+\t\telse if (atom->u.signature.option == S_SIGNER)\n+\t\t\tv->s = xstrdup(sigc.signer ? sigc.signer : \"\");\n+\t\telse if (atom->u.signature.option == S_GRADE) {\n+\t\t\tswitch (sigc.result) {\n+\t\t\tcase 'G':\n+\t\t\t\tswitch (sigc.trust_level) {\n+\t\t\t\tcase TRUST_UNDEFINED:\n+\t\t\t\tcase TRUST_NEVER:\n+\t\t\t\t\tv->s = xstrfmt(\"%c\", (char)'U');\n+\t\t\t\t\tbreak;\n+\t\t\t\tdefault:\n+\t\t\t\t\tv->s = xstrfmt(\"%c\", (char)'G');\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n+\t\t\t\tbreak;\n+\t\t\tcase 'B':\n+\t\t\tcase 'E':\n+\t\t\tcase 'N':\n+\t\t\tcase 'X':\n+\t\t\tcase 'Y':\n+\t\t\tcase 'R':\n+\t\t\t\tv->s = xstrfmt(\"%c\", (char)sigc.result);\n+\t\t\t}\n+\t\t}\n+\t\telse if (atom->u.signature.option == S_KEY)\n+\t\t\tv->s = xstrdup(sigc.key ? sigc.key : \"\");\n+\t\telse if (atom->u.signature.option == S_FINGERPRINT)\n+\t\t\tv->s = xstrdup(sigc.fingerprint ? sigc.fingerprint : \"\");\n+\t\telse if (atom->u.signature.option == S_PRI_KEY_FP)\n+\t\t\tv->s = xstrdup(sigc.primary_key_fingerprint ? sigc.primary_key_fingerprint : \"\");\n+\t\telse if (atom->u.signature.option == S_TRUST_LEVEL)\n+\t\t\tv->s = xstrdup(gpg_trust_level_to_str(sigc.trust_level));\n+\t}\n+}\n+\n static void find_subpos(const char *buf,\n \t\t\tconst char **sub, size_t *sublen,\n \t\t\tconst char **body, size_t *bodylen,\n@@ -1536,6 +1629,7 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, s\n \t\tgrab_sub_body_contents(val, deref, data);\n \t\tgrab_person(\"author\", val, deref, buf);\n \t\tgrab_person(\"committer\", val, deref, buf);\n+\t\tgrab_signature(val, deref, obj);\n \t\tbreak;\n \tcase OBJ_TREE:\n \t\t/* grab_tree_values(val, deref, obj, buf, sz); */\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex fa38b874416..5726517cfda 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -6,6 +6,7 @@\n test_description='for-each-ref test'\n \n . ./test-lib.sh\n+GNUPGHOME_NOT_USED=$GNUPGHOME\n . \"$TEST_DIRECTORY\"/lib-gpg.sh\n . \"$TEST_DIRECTORY\"/lib-terminal.sh\n \n@@ -1446,4 +1447,119 @@ sig_crlf=\"$(printf \"%s\" \"$sig\" | append_cr; echo dummy)\"\n sig_crlf=${sig_crlf%dummy}\n test_atom refs/tags/fake-sig-crlf contents:signature \"$sig_crlf\"\n \n+GRADE_FORMAT=\"%(signature:grade)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)\"\n+TRUSTLEVEL_FORMAT=\"%(signature:trustlevel)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)\"\n+\n+test_expect_success GPG 'show good signature with custom format' '\n+\tgit checkout -b signed &&\n+\techo 1 >file && git add file &&\n+\ttest_tick && git commit -S -m initial &&\n+\tgit verify-commit signed 2>out &&\n+\tcat >expect <<-\\EOF &&\n+\tG\n+\t13B6F51ECDDE430D\n+\tC O Mitter \n+\t73D758744BE721698EC54E8713B6F51ECDDE430D\n+\t73D758744BE721698EC54E8713B6F51ECDDE430D\n+\tEOF\n+\tgit for-each-ref refs/heads/signed --format=\"$GRADE_FORMAT\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'test signature atom with grade option and bad signature' '\n+\tgit config commit.gpgsign true &&\n+\techo 3 >file && test_tick && git commit -a -m \"third\" --no-gpg-sign &&\n+\tgit tag third-unsigned &&\n+\n+\ttest_tick && git rebase -f HEAD^^ && git tag second-signed HEAD^ &&\n+\tgit tag third-signed &&\n+\n+\tgit cat-file commit third-signed >raw &&\n+\tsed -e \"s/^third/3rd forged/\" raw >forged1 &&\n+\tFORGED1=$(git hash-object -w -t commit forged1) &&\n+\tgit update-ref refs/tags/third-signed \"$FORGED1\" &&\n+\ttest_must_fail git verify-commit \"$FORGED1\" &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tB\n+\t13B6F51ECDDE430D\n+\tC O Mitter \n+\n+\n+\tEOF\n+\tgit for-each-ref refs/tags/third-signed --format=\"$GRADE_FORMAT\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'show untrusted signature with custom format' '\n+\techo 4 >file && test_tick && git commit -a -m fourth -SB7227189 &&\n+\tgit tag signed-fourth &&\n+\tcat >expect <<-\\EOF &&\n+\tU\n+\t65A0EEA02E30CAD7\n+\tEris Discordia \n+\tF8364A59E07FFE9F4D63005A65A0EEA02E30CAD7\n+\tD4BE22311AD3131E5EDA29A461092E85B7227189\n+\tEOF\n+\tgit for-each-ref refs/tags/signed-fourth --format=\"$GRADE_FORMAT\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'show untrusted signature with undefined trust level' '\n+\techo 5 >file && test_tick && git commit -a -m fifth -SB7227189 &&\n+\tgit tag fifth-signed &&\n+\tcat >expect <<-\\EOF &&\n+\tundefined\n+\t65A0EEA02E30CAD7\n+\tEris Discordia \n+\tF8364A59E07FFE9F4D63005A65A0EEA02E30CAD7\n+\tD4BE22311AD3131E5EDA29A461092E85B7227189\n+\tEOF\n+\tgit for-each-ref refs/tags/fifth-signed --format=\"$TRUSTLEVEL_FORMAT\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'show untrusted signature with ultimate trust level' '\n+\techo 7 >file && test_tick && git commit -a -m \"seventh\" --no-gpg-sign &&\n+\tgit tag seventh-unsigned &&\n+\n+\ttest_tick && git rebase -f HEAD^^ && git tag sixth-signed HEAD^ &&\n+\tgit tag seventh-signed &&\n+\tcat >expect <<-\\EOF &&\n+\tultimate\n+\t13B6F51ECDDE430D\n+\tC O Mitter \n+\t73D758744BE721698EC54E8713B6F51ECDDE430D\n+\t73D758744BE721698EC54E8713B6F51ECDDE430D\n+\tEOF\n+\tgit for-each-ref refs/tags/seventh-signed --format=\"$TRUSTLEVEL_FORMAT\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'show unknown signature with custom format' '\n+\tcat >expect <<-\\EOF &&\n+\tE\n+\t65A0EEA02E30CAD7\n+\n+\n+\n+\tEOF\n+\tGNUPGHOME=\"$GNUPGHOME_NOT_USED\" git for-each-ref refs/tags/fifth-signed --format=\"$GRADE_FORMAT\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'show lack of signature with custom format' '\n+\techo 8 >file && test_tick && git commit -a -m \"eigth unsigned\" --no-gpg-sign &&\n+\tgit tag eigth-unsigned &&\n+\tcat >expect <<-\\EOF &&\n+\tN\n+\n+\n+\n+\n+\tEOF\n+\tgit for-each-ref refs/tags/eigth-unsigned --format=\"$GRADE_FORMAT\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n\nbase-commit: c48035d29b4e524aed3a32f0403676f0d9128863\n-- \ngitgitgadget\n"},{"id":"469543","messageId":"xmqqo7rpvb83.fsf@gitster.g","threadId":"59006","inReplyTo":"pull.1452.git.1672102523902.gitgitgadget@gmail.com","subject":"Re: [PATCH] ref-filter: add new atom \"signature\" atom","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-12-27T02:20:28Z","receivedAt":"2022-12-27T02:20:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"nsengaw4c via GitGitGadget\" writes:\n\n> From: Nsengiyumva Wilberforce \n>\n> This only works for commits. Add \"signature\" atom with `grade`,\n> `signer`, `key`, `fingerprint`, `primarykeyfingerprint`, `trustlevel`\n> as arguments. This code and it's documentation are inspired by\n> how the %GG, %G?, %GS, %GK, %GF, %GP, and %GT pretty formats were\n> implemented.\n\nLacking motivation. Without explaining why somebody may want to\nhave the feature and what it would be used for, \"only works for\ncommits\" would invite a \"so what? does it even have to work?\" as a\nresponse, so start with a brief descrioption \"with the current set\nof atoms, $this_useful_thing cannot easily be achieved\" before\ndescribing its limitation.\n\nHaving said that, wouldn't it be natural to expect that the same\ncode can deal with signed tags? After all we use the same signature\nverification machinery at the lowest level in the callchain.\n\n> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\n> index 6da899c6296..9a0be85368b 100644\n> --- a/Documentation/git-for-each-ref.txt\n> +++ b/Documentation/git-for-each-ref.txt\n> @@ -212,6 +212,33 @@ symref::\n> \t`:lstrip` and `:rstrip` options in the same way as `refname`\n> \tabove.\n> \n> +signature::\n> +...\n> +signature:trustlevel::\n> +\tThe Trust level of the GPG signature of a commit. Possible\n> +\toutputs are `ultimate`, `fully`, `marginal`, `never` and `undefined`.\n\nA good list. How do these work for signature made with a tool other\nthan GPG (in other words, when \"gpg.format\" is set to something\nother than \"openpgp\")?\n\n> @@ -378,6 +383,30 @@ static int subject_atom_parser(struct ref_format *format, struct used_atom *atom\n> \treturn 0;\n> }\n> \n> +static int signature_atom_parser(struct ref_format *format, struct used_atom *atom,\n> +\t\t\t const char *arg, struct strbuf *err)\n> +{\n> +\tif (arg) {\n> +\t\tif (!strcmp(arg, \"signer\"))\n> +\t\t\tatom->u.signature.option = S_SIGNER;\n> +\t\telse if (!strcmp(arg, \"grade\"))\n> +\t\t\tatom->u.signature.option = S_GRADE;\n> +\t\telse if (!strcmp(arg, \"key\"))\n> +\t\t\tatom->u.signature.option = S_KEY;\n> +\t\telse if (!strcmp(arg, \"fingerprint\"))\n> +\t\t\tatom->u.signature.option = S_FINGERPRINT;\n> +\t\telse if (!strcmp(arg, \"primarykeyfingerprint\"))\n> +\t\t\tatom->u.signature.option = S_PRI_KEY_FP;\n> +\t\telse if (!strcmp(arg, \"trustlevel\"))\n> +\t\t\tatom->u.signature.option = S_TRUST_LEVEL;\n> +\t\telse\n> +\t\t\treturn strbuf_addf_ret(err, -1, _(\"unknown %%(signature) argument: %s\"), arg);\n> +\t}\n> +\telse\n> +\t\tatom->u.signature.option = S_BARE;\n> +\treturn 0;\n> +}\n\nHanding the !arg case first will make the if/else if/... cascade\neasier to follow, no? Also the body of the function may want to\nbecome a separate function that returns one of these S_FOO constants.\n\n\tstatic enum signatore_option signature_atom_parser(...)\n\t{\n enum signature_option opt = parse_signature_option(arg);\n if (opt < 0)\n return strbuf_addf_ret(err, opt, _(\"unknown ...\"), arg);\n return opt;\n\t}\n\nwhere parse_signature_option() would look like\n\n\tstatic enum signature_option parse_signature_option(const char *arg)\n\t{\n\t\tif (!arg)\n\t\t\treturn S_BARE;\n\t\telse if (!strcmp(arg, \"signer\"))\n\t\t\treturn S_SIGNER;\n\t\t...\n\t\telse\n\t\t\treturn -1;\n\t}\n\nor something like that?\n\n> @@ -1344,6 +1374,69 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void\n> \t}\n> }\n> \n> +static void grab_signature(struct atom_value *val, int deref, struct object *obj)\n\nTo be considerate for future developers, perhaps rename this to\ngrab_commit_signature(), so that they can add grab_tag_signature()\nwhen they lift the limitation of this implementaiton?\n\n> +{\n> +\tint i;\n> +\tstruct commit *commit = (struct commit *) obj;\n\nStyle? No SP between cast and value?\n\n> +\n> +\tfor (i = 0; i < used_atom_cnt; i++) {\n> +\t\tstruct used_atom *atom = &used_atom[i];\n> +\t\tconst char *name = atom->name;\n> +\t\tstruct atom_value *v = &val[i];\n> +\t\tstruct signature_check sigc = { 0 };\n> +\n> +\t\tif (!!deref != (*name == '*'))\n> +\t\t\tcontinue;\n> +\t\tif (deref)\n> +\t\t\tname++;\n> +\t\tif (strcmp(name, \"signature\") &&\n> +\t\t\tstrcmp(name, \"signature:signer\") &&\n> +\t\t\tstrcmp(name, \"signature:grade\") &&\n> +\t\t\tstrcmp(name, \"signature:key\") &&\n> +\t\t\tstrcmp(name, \"signature:fingerprint\") &&\n> +\t\t\tstrcmp(name, \"signature:primarykeyfingerprint\") &&\n> +\t\t\tstrcmp(name, \"signature:trustlevel\"))\n> +\t\t\tcontinue;\n\nAnd with the helper above, we can avoid the repetition here that can\ngo out of sync with the parser function.\n\n> +\t\tcheck_commit_signature(commit, &sigc);\n\nIf a format asks for signature:signer and signature:key, we\nshouldn't be running GPG twice. First check used_atom[] to see if\nwe even need to do _any_ signature processing (and leave if there is\nnot), populate the sigc just once and then enter the loop, perhaps?\n\nIn adddition, a call to check_commit_signature() should have a\nmatching call to signature_check_clear(); otherwise all the\nresources held by sigc would leak, wouldn't it?\n"},{"id":"469546","messageId":"Y6qMk3e+FqEThL5f@coredump.intra.peff.net","threadId":"59006","inReplyTo":"pull.1452.git.1672102523902.gitgitgadget@gmail.com","subject":"Re: [PATCH] ref-filter: add new atom \"signature\" atom","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-12-27T06:11:31Z","receivedAt":"2022-12-27T06:11:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Dec 27, 2022 at 12:55:23AM +0000, nsengaw4c via GitGitGadget wrote:\n\n> This only works for commits. Add \"signature\" atom with `grade`,\n> `signer`, `key`, `fingerprint`, `primarykeyfingerprint`, `trustlevel`\n> as arguments. This code and it's documentation are inspired by\n> how the %GG, %G?, %GS, %GK, %GF, %GP, and %GT pretty formats were\n> implemented.\n\nI don't have a real review for you, but rather two small requests since\nI was working in this area recently.\n\n> @@ -378,6 +383,30 @@ static int subject_atom_parser(struct ref_format *format, struct used_atom *atom\n> \treturn 0;\n> }\n> \n> +static int signature_atom_parser(struct ref_format *format, struct used_atom *atom,\n> +\t\t\t const char *arg, struct strbuf *err)\n\nCan you squash in an annotation for the unused parameter, like this:\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex a4c3f89f64..3b3592acb2 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -405,8 +405,9 @@ static int subject_atom_parser(struct ref_format *format UNUSED,\n \treturn 0;\n }\n \n-static int signature_atom_parser(struct ref_format *format, struct used_atom *atom,\n-\t\t\t const char *arg, struct strbuf *err)\n+static int signature_atom_parser(struct ref_format *format UNUSED,\n+\t\t\t\t struct used_atom *atom,\n+\t\t\t\t const char *arg, struct strbuf *err)\n {\n \tif (arg) {\n \t\tif (!strcmp(arg, \"signer\"))\n\nThis will eventually be necessary once we turn on -Wunused-parameter.\nI'm preparing a patch to convert all of the other parsers that need it,\nand I don't want to create a dependency between the two patches (it's OK\nfor you to add the UNUSED now, it's just not enforced yet).\n\nI can also circle back after your patch is merged and add it, but it's a\nbit easier to do it up front.\n\n> +{\n> +\tif (arg) {\n> +\t\tif (!strcmp(arg, \"signer\"))\n> +\t\t\tatom->u.signature.option = S_SIGNER;\n> +\t\telse if (!strcmp(arg, \"grade\"))\n> +\t\t\tatom->u.signature.option = S_GRADE;\n> +\t\telse if (!strcmp(arg, \"key\"))\n> +\t\t\tatom->u.signature.option = S_KEY;\n> +\t\telse if (!strcmp(arg, \"fingerprint\"))\n> +\t\t\tatom->u.signature.option = S_FINGERPRINT;\n> +\t\telse if (!strcmp(arg, \"primarykeyfingerprint\"))\n> +\t\t\tatom->u.signature.option = S_PRI_KEY_FP;\n> +\t\telse if (!strcmp(arg, \"trustlevel\"))\n> +\t\t\tatom->u.signature.option = S_TRUST_LEVEL;\n> +\t\telse\n> +\t\t\treturn strbuf_addf_ret(err, -1, _(\"unknown %%(signature) argument: %s\"), arg);\n> +\t}\n\nThe ref-filter code recently got a helper function to report this kind\nof argument error consistently, via dda4fc1a84 (ref-filter: factor out\n\"unrecognized %(foo) arg\" errors, 2022-12-14). If you rebase the patch\non the current master, you can just do:\n\n return err_bad_arg(err, \"signature\", arg);\n\nwhich will make the error message match the others (which in turn saves\nwork for translators).\n\n-Peff\n"},{"id":"469744","messageId":"CA+PPyiGPRztaLsty5LqT-7GfjPusyt=7hi22z1aPdm-G8pZpuQ@mail.gmail.com","threadId":"59006","inReplyTo":"xmqqo7rpvb83.fsf@gitster.g","subject":"Re: [PATCH] ref-filter: add new atom \"signature\" atom","fromName":"NSENGIYUMVA WILBERFORCE","fromEmail":"nsengiyumvawilberforce@gmail.com","sentAt":"2023-01-02T04:49:00Z","receivedAt":"2023-01-02T04:58:22Z","isPatch":true,"sender":{"key":"nsengiyumvawilberforce@gmail.com","avatar":"https://avatars.githubusercontent.com/u/65920105?v=4"},"body":"Hi\n>\n> > From: Nsengiyumva Wilberforce \n> >\n> > This only works for commits. Add \"signature\" atom with `grade`,\n> > `signer`, `key`, `fingerprint`, `primarykeyfingerprint`, `trustlevel`\n> > as arguments. This code and it's documentation are inspired by\n> > how the %GG, %G?, %GS, %GK, %GF, %GP, and %GT pretty formats were\n> > implemented.\n>\n> Lacking motivation. Without explaining why somebody may want to\n> have the feature and what it would be used for, \"only works for\n> commits\" would invite a \"so what? does it even have to work?\" as a\n> response, so start with a brief descrioption \"with the current set\n> of atoms, $this_useful_thing cannot easily be achieved\" before\n> describing its limitation.\n\nOk, I will edit the commit message. Thanks\n>\n> > diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\n> > index 6da899c6296..9a0be85368b 100644\n> > --- a/Documentation/git-for-each-ref.txt\n> > +++ b/Documentation/git-for-each-ref.txt\n> > @@ -212,6 +212,33 @@ symref::\n> > `:lstrip` and `:rstrip` options in the same way as `refname`\n> > above.\n> >\n> > +signature::\n> > +...\n> > +signature:trustlevel::\n> > + The Trust level of the GPG signature of a commit. Possible\n> > + outputs are `ultimate`, `fully`, `marginal`, `never` and `undefined`.\n>\n> A good list. How do these work for signature made with a tool other\n> than GPG (in other words, when \"gpg.format\" is set to something\n> other than \"openpgp\")?\n\nYou mean ssh and X509, right? honestly, I did not check the behavior.\nI am going to check\n\n> Having said that, wouldn't it be natural to expect that the same\n> code can deal with signed tags? After all we use the same signature\n> verification machinery at the lowest level in the callchain.\n\nVery right, it works for signed tags too.\n\n>\n> Handing the !arg case first will make the if/else if/... cascade\n> easier to follow, no? Also the body of the function may want to\n> become a separate function that returns one of these S_FOO constants.\n>\n> static enum signatore_option signature_atom_parser(...)\n> {\n> enum signature_option opt = parse_signature_option(arg);\n> if (opt < 0)\n> return strbuf_addf_ret(err, opt, _(\"unknown ...\"), arg);\n> return opt;\n> }\n>\n> where parse_signature_option() would look like\n>\n> static enum signature_option parse_signature_option(const char *arg)\n> {\n> if (!arg)\n> return S_BARE;\n> else if (!strcmp(arg, \"signer\"))\n> return S_SIGNER;\n> ...\n> else\n> return -1;\n> }\n>\n> or something like that?\n\n It makes more sense\n>\n> > +{\n> > + int i;\n> > + struct commit *commit = (struct commit *) obj;\n>\n> Style? No SP between cast and value?\n\nok, noted\n>\n> > +\n> > + for (i = 0; i < used_atom_cnt; i++) {\n> > + struct used_atom *atom = &used_atom[i];\n> > + const char *name = atom->name;\n> > + struct atom_value *v = &val[i];\n> > + struct signature_check sigc = { 0 };\n> > +\n> > + if (!!deref != (*name == '*'))\n> > + continue;\n> > + if (deref)\n> > + name++;\n> > + if (strcmp(name, \"signature\") &&\n> > + strcmp(name, \"signature:signer\") &&\n> > + strcmp(name, \"signature:grade\") &&\n> > + strcmp(name, \"signature:key\") &&\n> > + strcmp(name, \"signature:fingerprint\") &&\n> > + strcmp(name, \"signature:primarykeyfingerprint\") &&\n> > + strcmp(name, \"signature:trustlevel\"))\n> > + continue;\n>\n> And with the helper above, we can avoid the repetition here that can\n> go out of sync with the parser function.\n\nI am not sure I have understood this, which helper?\n\n> > + check_commit_signature(commit, &sigc);\n>\n> If a format asks for signature:signer and signature:key, we\n> shouldn't be running GPG twice. First check used_atom[] to see if\n> we even need to do _any_ signature processing (and leave if there is\n> not), populate the sigc just once and then enter the loop, perhaps?\n\nYeah, I think it was not right calling check_commit_signature() in the\nloop. Populating sigc at once looks more good to me\n\n\n>\n> In adddition, a call to check_commit_signature() should have a\n>\n> matching call to signature_check_clear(); otherwise all the\n>\n> resources held by sigc would leak, wouldn't it?\n\nYeah, it would.\n\n\nOn Mon, Dec 26, 2022 at 9:20 PM Junio C Hamano wrote:\n>\n> \"nsengaw4c via GitGitGadget\" writes:\n>\n> > From: Nsengiyumva Wilberforce \n> >\n> > This only works for commits. Add \"signature\" atom with `grade`,\n> > `signer`, `key`, `fingerprint`, `primarykeyfingerprint`, `trustlevel`\n> > as arguments. This code and it's documentation are inspired by\n> > how the %GG, %G?, %GS, %GK, %GF, %GP, and %GT pretty formats were\n> > implemented.\n>\n> Lacking motivation. Without explaining why somebody may want to\n> have the feature and what it would be used for, \"only works for\n> commits\" would invite a \"so what? does it even have to work?\" as a\n> response, so start with a brief descrioption \"with the current set\n> of atoms, $this_useful_thing cannot easily be achieved\" before\n> describing its limitation.\n>\n> Having said that, wouldn't it be natural to expect that the same\n> code can deal with signed tags? After all we use the same signature\n> verification machinery at the lowest level in the callchain.\n>\n> > diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\n> > index 6da899c6296..9a0be85368b 100644\n> > --- a/Documentation/git-for-each-ref.txt\n> > +++ b/Documentation/git-for-each-ref.txt\n> > @@ -212,6 +212,33 @@ symref::\n> > `:lstrip` and `:rstrip` options in the same way as `refname`\n> > above.\n> >\n> > +signature::\n> > +...\n> > +signature:trustlevel::\n> > + The Trust level of the GPG signature of a commit. Possible\n> > + outputs are `ultimate`, `fully`, `marginal`, `never` and `undefined`.\n>\n> A good list. How do these work for signature made with a tool other\n> than GPG (in other words, when \"gpg.format\" is set to something\n> other than \"openpgp\")?\n>\n> > @@ -378,6 +383,30 @@ static int subject_atom_parser(struct ref_format *format, struct used_atom *atom\n> > return 0;\n> > }\n> >\n> > +static int signature_atom_parser(struct ref_format *format, struct used_atom *atom,\n> > + const char *arg, struct strbuf *err)\n> > +{\n> > + if (arg) {\n> > + if (!strcmp(arg, \"signer\"))\n> > + atom->u.signature.option = S_SIGNER;\n> > + else if (!strcmp(arg, \"grade\"))\n> > + atom->u.signature.option = S_GRADE;\n> > + else if (!strcmp(arg, \"key\"))\n> > + atom->u.signature.option = S_KEY;\n> > + else if (!strcmp(arg, \"fingerprint\"))\n> > + atom->u.signature.option = S_FINGERPRINT;\n> > + else if (!strcmp(arg, \"primarykeyfingerprint\"))\n> > + atom->u.signature.option = S_PRI_KEY_FP;\n> > + else if (!strcmp(arg, \"trustlevel\"))\n> > + atom->u.signature.option = S_TRUST_LEVEL;\n> > + else\n> > + return strbuf_addf_ret(err, -1, _(\"unknown %%(signature) argument: %s\"), arg);\n> > + }\n> > + else\n> > + atom->u.signature.option = S_BARE;\n> > + return 0;\n> > +}\n>\n> Handing the !arg case first will make the if/else if/... cascade\n> easier to follow, no? Also the body of the function may want to\n> become a separate function that returns one of these S_FOO constants.\n>\n> static enum signatore_option signature_atom_parser(...)\n> {\n> enum signature_option opt = parse_signature_option(arg);\n> if (opt < 0)\n> return strbuf_addf_ret(err, opt, _(\"unknown ...\"), arg);\n> return opt;\n> }\n>\n> where parse_signature_option() would look like\n>\n> static enum signature_option parse_signature_option(const char *arg)\n> {\n> if (!arg)\n> return S_BARE;\n> else if (!strcmp(arg, \"signer\"))\n> return S_SIGNER;\n> ...\n> else\n> return -1;\n> }\n>\n> or something like that?\n>\n> > @@ -1344,6 +1374,69 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void\n> > }\n> > }\n> >\n> > +static void grab_signature(struct atom_value *val, int deref, struct object *obj)\n>\n> To be considerate for future developers, perhaps rename this to\n> grab_commit_signature(), so that they can add grab_tag_signature()\n> when they lift the limitation of this implementaiton?\n>\n> > +{\n> > + int i;\n> > + struct commit *commit = (struct commit *) obj;\n>\n> Style? No SP between cast and value?\n>\n> > +\n> > + for (i = 0; i < used_atom_cnt; i++) {\n> > + struct used_atom *atom = &used_atom[i];\n> > + const char *name = atom->name;\n> > + struct atom_value *v = &val[i];\n> > + struct signature_check sigc = { 0 };\n> > +\n> > + if (!!deref != (*name == '*'))\n> > + continue;\n> > + if (deref)\n> > + name++;\n> > + if (strcmp(name, \"signature\") &&\n> > + strcmp(name, \"signature:signer\") &&\n> > + strcmp(name, \"signature:grade\") &&\n> > + strcmp(name, \"signature:key\") &&\n> > + strcmp(name, \"signature:fingerprint\") &&\n> > + strcmp(name, \"signature:primarykeyfingerprint\") &&\n> > + strcmp(name, \"signature:trustlevel\"))\n> > + continue;\n>\n> And with the helper above, we can avoid the repetition here that can\n> go out of sync with the parser function.\n>\n> > + check_commit_signature(commit, &sigc);\n>\n> If a format asks for signature:signer and signature:key, we\n> shouldn't be running GPG twice. First check used_atom[] to see if\n> we even need to do _any_ signature processing (and leave if there is\n> not), populate the sigc just once and then enter the loop, perhaps?\n>\n> In adddition, a call to check_commit_signature() should have a\n> matching call to signature_check_clear(); otherwise all the\n> resources held by sigc would leak, wouldn't it?\n"},{"id":"469745","messageId":"CA+PPyiF3GzcnsuX6amUiaCa8onFVqMAO=naE4krpibip-c4bxw@mail.gmail.com","threadId":"59006","inReplyTo":"Y6qMk3e+FqEThL5f@coredump.intra.peff.net","subject":"Re: [PATCH] ref-filter: add new atom \"signature\" atom","fromName":"NSENGIYUMVA WILBERFORCE","fromEmail":"nsengiyumvawilberforce@gmail.com","sentAt":"2023-01-02T06:34:26Z","receivedAt":"2023-01-02T06:34:42Z","isPatch":true,"sender":{"key":"nsengiyumvawilberforce@gmail.com","avatar":"https://avatars.githubusercontent.com/u/65920105?v=4"},"body":">\n> diff --git a/ref-filter.c b/ref-filter.c\n> index a4c3f89f64..3b3592acb2 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -405,8 +405,9 @@ static int subject_atom_parser(struct ref_format *format UNUSED,\n> return 0;\n> }\n>\n> -static int signature_atom_parser(struct ref_format *format, struct used_atom *atom,\n> - const char *arg, struct strbuf *err)\n> +static int signature_atom_parser(struct ref_format *format UNUSED,\n> + struct used_atom *atom,\n> + const char *arg, struct strbuf *err)\n> {\n> if (arg) {\n> if (!strcmp(arg, \"signer\"))\n>\n> This will eventually be necessary once we turn on -Wunused-parameter.\n> I'm preparing a patch to convert all of the other parsers that need it,\n> and I don't want to create a dependency between the two patches (it's OK\n> for you to add the UNUSED now, it's just not enforced yet).\n>\n> I can also circle back after your patch is merged and add it, but it's a\n> bit easier to do it up front.\n\nThanks, worked on it\n\n> +{\n> + if (arg) {\n> + if (!strcmp(arg, \"signer\"))\n> + atom->u.signature.option = S_SIGNER;\n> + else if (!strcmp(arg, \"grade\"))\n> + atom->u.signature.option = S_GRADE;\n> + else if (!strcmp(arg, \"key\"))\n> + atom->u.signature.option = S_KEY;\n> + else if (!strcmp(arg, \"fingerprint\"))\n> + atom->u.signature.option = S_FINGERPRINT;\n> + else if (!strcmp(arg, \"primarykeyfingerprint\"))\n> + atom->u.signature.option = S_PRI_KEY_FP;\n> + else if (!strcmp(arg, \"trustlevel\"))\n> + atom->u.signature.option = S_TRUST_LEVEL;\n> + else\n> + return strbuf_addf_ret(err, -1, _(\"unknown %%(signature) argument: %s\"), arg);\n> + }\n\nThe ref-filter code recently got a helper function to report this kind\nof argument error consistently, via dda4fc1a84 (ref-filter: factor out\n\"unrecognized %(foo) arg\" errors, 2022-12-14). If you rebase the patch\non the current master, you can just do:\n\n return err_bad_arg(err, \"signature\", arg);\n\nwhich will make the error message match the others (which in turn saves\n>\n> work for translators).\n\nThanks for this, I have seen it too\n\n\nOn Tue, Dec 27, 2022 at 1:11 AM Jeff King wrote:\n>\n> On Tue, Dec 27, 2022 at 12:55:23AM +0000, nsengaw4c via GitGitGadget wrote:\n>\n> > This only works for commits. Add \"signature\" atom with `grade`,\n> > `signer`, `key`, `fingerprint`, `primarykeyfingerprint`, `trustlevel`\n> > as arguments. This code and it's documentation are inspired by\n> > how the %GG, %G?, %GS, %GK, %GF, %GP, and %GT pretty formats were\n> > implemented.\n>\n> I don't have a real review for you, but rather two small requests since\n> I was working in this area recently.\n>\n> > @@ -378,6 +383,30 @@ static int subject_atom_parser(struct ref_format *format, struct used_atom *atom\n> > return 0;\n> > }\n> >\n> > +static int signature_atom_parser(struct ref_format *format, struct used_atom *atom,\n> > + const char *arg, struct strbuf *err)\n>\n> Can you squash in an annotation for the unused parameter, like this:\n>\n> diff --git a/ref-filter.c b/ref-filter.c\n> index a4c3f89f64..3b3592acb2 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -405,8 +405,9 @@ static int subject_atom_parser(struct ref_format *format UNUSED,\n> return 0;\n> }\n>\n> -static int signature_atom_parser(struct ref_format *format, struct used_atom *atom,\n> - const char *arg, struct strbuf *err)\n> +static int signature_atom_parser(struct ref_format *format UNUSED,\n> + struct used_atom *atom,\n> + const char *arg, struct strbuf *err)\n> {\n> if (arg) {\n> if (!strcmp(arg, \"signer\"))\n>\n> This will eventually be necessary once we turn on -Wunused-parameter.\n> I'm preparing a patch to convert all of the other parsers that need it,\n> and I don't want to create a dependency between the two patches (it's OK\n> for you to add the UNUSED now, it's just not enforced yet).\n>\n> I can also circle back after your patch is merged and add it, but it's a\n> bit easier to do it up front.\n>\n> > +{\n> > + if (arg) {\n> > + if (!strcmp(arg, \"signer\"))\n> > + atom->u.signature.option = S_SIGNER;\n> > + else if (!strcmp(arg, \"grade\"))\n> > + atom->u.signature.option = S_GRADE;\n> > + else if (!strcmp(arg, \"key\"))\n> > + atom->u.signature.option = S_KEY;\n> > + else if (!strcmp(arg, \"fingerprint\"))\n> > + atom->u.signature.option = S_FINGERPRINT;\n> > + else if (!strcmp(arg, \"primarykeyfingerprint\"))\n> > + atom->u.signature.option = S_PRI_KEY_FP;\n> > + else if (!strcmp(arg, \"trustlevel\"))\n> > + atom->u.signature.option = S_TRUST_LEVEL;\n> > + else\n> > + return strbuf_addf_ret(err, -1, _(\"unknown %%(signature) argument: %s\"), arg);\n> > + }\n>\n> The ref-filter code recently got a helper function to report this kind\n> of argument error consistently, via dda4fc1a84 (ref-filter: factor out\n> \"unrecognized %(foo) arg\" errors, 2022-12-14). If you rebase the patch\n> on the current master, you can just do:\n>\n> return err_bad_arg(err, \"signature\", arg);\n>\n> which will make the error message match the others (which in turn saves\n> work for translators).\n>\n> -Peff\n"},{"id":"469746","messageId":"CAP8UFD3i7C2c79V2ORxh-Q-rNuwKVqkMRX0VoXy3iugw=u5K+A@mail.gmail.com","threadId":"59006","inReplyTo":"CA+PPyiGPRztaLsty5LqT-7GfjPusyt=7hi22z1aPdm-G8pZpuQ@mail.gmail.com","subject":"Re: [PATCH] ref-filter: add new atom \"signature\" atom","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2023-01-02T08:37:48Z","receivedAt":"2023-01-02T08:38:08Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Mon, Jan 2, 2023 at 6:01 AM NSENGIYUMVA WILBERFORCE\n wrote:\n\n> > Handing the !arg case first will make the if/else if/... cascade\n> > easier to follow, no? Also the body of the function may want to\n> > become a separate function that returns one of these S_FOO constants.\n> >\n> > static enum signatore_option signature_atom_parser(...)\n> > {\n> > enum signature_option opt = parse_signature_option(arg);\n> > if (opt < 0)\n> > return strbuf_addf_ret(err, opt, _(\"unknown ...\"), arg);\n> > return opt;\n> > }\n> >\n> > where parse_signature_option() would look like\n> >\n> > static enum signature_option parse_signature_option(const char *arg)\n> > {\n> > if (!arg)\n> > return S_BARE;\n> > else if (!strcmp(arg, \"signer\"))\n> > return S_SIGNER;\n> > ...\n> > else\n> > return -1;\n> > }\n> >\n> > or something like that?\n\n[...]\n\n> > > + if (strcmp(name, \"signature\") &&\n> > > + strcmp(name, \"signature:signer\") &&\n> > > + strcmp(name, \"signature:grade\") &&\n> > > + strcmp(name, \"signature:key\") &&\n> > > + strcmp(name, \"signature:fingerprint\") &&\n> > > + strcmp(name, \"signature:primarykeyfingerprint\") &&\n> > > + strcmp(name, \"signature:trustlevel\"))\n> > > + continue;\n> >\n> > And with the helper above, we can avoid the repetition here that can\n> > go out of sync with the parser function.\n>\n> I am not sure I have understood this, which helper?\n\nI think Junio is talking about the following function:\n\nstatic enum signature_option parse_signature_option(const char *arg)\n\nhe suggested above.\n\nWith this function the above code could be just something like:\n\nif (parse_signature_option(name) < 0)\n continue;\n"},{"id":"469753","messageId":"xmqqv8lov3hh.fsf@gitster.g","threadId":"59006","inReplyTo":"CAP8UFD3i7C2c79V2ORxh-Q-rNuwKVqkMRX0VoXy3iugw=u5K+A@mail.gmail.com","subject":"Re: [PATCH] ref-filter: add new atom \"signature\" atom","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-01-03T00:58:02Z","receivedAt":"2023-01-03T00:58:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n>> I am not sure I have understood this, which helper?\n>\n> I think Junio is talking about the following function:\n>\n> static enum signature_option parse_signature_option(const char *arg)\n>\n> he suggested above.\n\nCorrect.\n\n> With this function the above code could be just something like:\n>\n> if (parse_signature_option(name) < 0)\n> continue;\n\nMore or less so, but the first \"if\" in the helper I wrote in the\nmessage above is broken. It should be\n\n static enum signature_option parse_signature_option(const char *arg)\n {\n if (!*arg)\n return S_BARE;\n else if (!strcmp(arg, \"signer\"))\n return S_SIGNER;\n ...\n else\n return -1;\n }\n\nand then the code equivalent to the bunch of strcmp() would be\n\n\tif (skip_prefix(name, \"signature\", &name) &&\n (!*name || *name++ == ':') &&\n (0 <= parse_signature_option(name)))\n\t\t; /* we have \"signature\"-related atom */\n\telse\n\t\tcontinue; /* not a \"signature\" atom */\n\nor something like that.\n"},{"id":"469921","messageId":"CA+PPyiH5ANyHw-RSzMK+RXxio8gYk2DybY=XnDBvfD1M9s6Mmw@mail.gmail.com","threadId":"59006","inReplyTo":"CA+PPyiGd0-AiwhPa5e+fDdA9RybS+c5XeOYm5yycCZco3VHAxg@mail.gmail.com","subject":"Re: [PATCH] ref-filter: add new atom \"signature\" atom","fromName":"NSENGIYUMVA WILBERFORCE","fromEmail":"nsengiyumvawilberforce@gmail.com","sentAt":"2023-01-08T15:21:27Z","receivedAt":"2023-01-08T15:21:46Z","isPatch":true,"sender":{"key":"nsengiyumvawilberforce@gmail.com","avatar":"https://avatars.githubusercontent.com/u/65920105?v=4"},"body":"> >> I am not sure I have understood this, which helper?\n> >\n> > I think Junio is talking about the following function:\n> >\n> > static enum signature_option parse_signature_option(const char *arg)\n> >\n> > he suggested above.\n>\n> Correct.\n>\n> > With this function the above code could be just something like:\n> >\n> > if (parse_signature_option(name) < 0)\n> > continue;\n>\n> More or less so, but the first \"if\" in the helper I wrote in the\n> message above is broken. It should be\n>\n> static enum signature_option parse_signature_option(const char *arg)\n> {\n> if (!*arg)\n> return S_BARE;\n> else if (!strcmp(arg, \"signer\"))\n> return S_SIGNER;\n> ...\n> else\n> return -1;\n> }\n\nThis way, running git for-each-ref refs/heads/signature8\n--format=\"%(signature)\" raises a seg fault, I looked for the bug using\ngdb when I check the contents of *arg like this:p *arg, I get this:\nCannot access memory at 0x0.\n\nHowever, others like signature:key, signature:signer, etc are ok.\nLeaving it as arg makes everything fine. So I decided to leave it as\nyou had suggested first.\n\n\nI had actually forgotten to add the test for \"%(signature)\", so this\nscenario reminded me to do so.\n\n\nOn Mon, Jan 2, 2023 at 7:47 AM NSENGIYUMVA WILBERFORCE\n wrote:\n>\n> Hi\n>>\n>> > From: Nsengiyumva Wilberforce \n>> >\n>> > This only works for commits. Add \"signature\" atom with `grade`,\n>> > `signer`, `key`, `fingerprint`, `primarykeyfingerprint`, `trustlevel`\n>> > as arguments. This code and it's documentation are inspired by\n>> > how the %GG, %G?, %GS, %GK, %GF, %GP, and %GT pretty formats were\n>> > implemented.\n>>\n>> Lacking motivation. Without explaining why somebody may want to\n>> have the feature and what it would be used for, \"only works for\n>> commits\" would invite a \"so what? does it even have to work?\" as a\n>> response, so start with a brief descrioption \"with the current set\n>> of atoms, $this_useful_thing cannot easily be achieved\" before\n>> describing its limitation.\n>\n> Ok, I will edit the commit message. Thanks\n>>\n>> > diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\n>> > index 6da899c6296..9a0be85368b 100644\n>> > --- a/Documentation/git-for-each-ref.txt\n>> > +++ b/Documentation/git-for-each-ref.txt\n>> > @@ -212,6 +212,33 @@ symref::\n>> > `:lstrip` and `:rstrip` options in the same way as `refname`\n>> > above.\n>> >\n>> > +signature::\n>> > +...\n>> > +signature:trustlevel::\n>> > + The Trust level of the GPG signature of a commit. Possible\n>> > + outputs are `ultimate`, `fully`, `marginal`, `never` and `undefined`.\n>>\n>> A good list. How do these work for signature made with a tool other\n>> than GPG (in other words, when \"gpg.format\" is set to something\n>> other than \"openpgp\")?\n>\n> You mean ssh and X509, right? honestly, I did not check the behavior. I am going to check\n>\n>> Having said that, wouldn't it be natural to expect that the same\n>> code can deal with signed tags? After all we use the same signature\n>> verification machinery at the lowest level in the callchain.\n>\n> Very right, it works for signed tags too.\n>\n>>\n>> Handing the !arg case first will make the if/else if/... cascade\n>> easier to follow, no? Also the body of the function may want to\n>> become a separate function that returns one of these S_FOO constants.\n>>\n>> static enum signatore_option signature_atom_parser(...)\n>> {\n>> enum signature_option opt = parse_signature_option(arg);\n>> if (opt < 0)\n>> return strbuf_addf_ret(err, opt, _(\"unknown ...\"), arg);\n>> return opt;\n>> }\n>>\n>> where parse_signature_option() would look like\n>>\n>> static enum signature_option parse_signature_option(const char *arg)\n>> {\n>> if (!arg)\n>> return S_BARE;\n>> else if (!strcmp(arg, \"signer\"))\n>> return S_SIGNER;\n>> ...\n>> else\n>> return -1;\n>> }\n>>\n>> or something like that?\n>\n> It makes more sense\n>>\n>> > +{\n>> > + int i;\n>> > + struct commit *commit = (struct commit *) obj;\n>>\n>> Style? No SP between cast and value?\n>\n> ok, noted\n>>\n>> > +\n>> > + for (i = 0; i < used_atom_cnt; i++) {\n>> > + struct used_atom *atom = &used_atom[i];\n>> > + const char *name = atom->name;\n>> > + struct atom_value *v = &val[i];\n>> > + struct signature_check sigc = { 0 };\n>> > +\n>> > + if (!!deref != (*name == '*'))\n>> > + continue;\n>> > + if (deref)\n>> > + name++;\n>> > + if (strcmp(name, \"signature\") &&\n>> > + strcmp(name, \"signature:signer\") &&\n>> > + strcmp(name, \"signature:grade\") &&\n>> > + strcmp(name, \"signature:key\") &&\n>> > + strcmp(name, \"signature:fingerprint\") &&\n>> > + strcmp(name, \"signature:primarykeyfingerprint\") &&\n>> > + strcmp(name, \"signature:trustlevel\"))\n>> > + continue;\n>>\n>> And with the helper above, we can avoid the repetition here that can\n>> go out of sync with the parser function.\n>\n> I am not sure I have understood this, which helper?\n>\n>> > + check_commit_signature(commit, &sigc);\n>>\n>> If a format asks for signature:signer and signature:key, we\n>> shouldn't be running GPG twice. First check used_atom[] to see if\n>> we even need to do _any_ signature processing (and leave if there is\n>> not), populate the sigc just once and then enter the loop, perhaps?\n>\n> Yeah, I think it was not right calling check_commit_signature() in the loop. Populating sigc at once looks more good to me\n>\n>\n>>\n>> In adddition, a call to check_commit_signature() should have a\n>>\n>> matching call to signature_check_clear(); otherwise all the\n>>\n>> resources held by sigc would leak, wouldn't it?\n>\n> Yeah, it would.\n>\n> On Mon, Dec 26, 2022 at 9:20 PM Junio C Hamano wrote:\n>>\n>> \"nsengaw4c via GitGitGadget\" writes:\n>>\n>> > From: Nsengiyumva Wilberforce \n>> >\n>> > This only works for commits. Add \"signature\" atom with `grade`,\n>> > `signer`, `key`, `fingerprint`, `primarykeyfingerprint`, `trustlevel`\n>> > as arguments. This code and it's documentation are inspired by\n>> > how the %GG, %G?, %GS, %GK, %GF, %GP, and %GT pretty formats were\n>> > implemented.\n>>\n>> Lacking motivation. Without explaining why somebody may want to\n>> have the feature and what it would be used for, \"only works for\n>> commits\" would invite a \"so what? does it even have to work?\" as a\n>> response, so start with a brief descrioption \"with the current set\n>> of atoms, $this_useful_thing cannot easily be achieved\" before\n>> describing its limitation.\n>>\n>> Having said that, wouldn't it be natural to expect that the same\n>> code can deal with signed tags? After all we use the same signature\n>> verification machinery at the lowest level in the callchain.\n>>\n>> > diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\n>> > index 6da899c6296..9a0be85368b 100644\n>> > --- a/Documentation/git-for-each-ref.txt\n>> > +++ b/Documentation/git-for-each-ref.txt\n>> > @@ -212,6 +212,33 @@ symref::\n>> > `:lstrip` and `:rstrip` options in the same way as `refname`\n>> > above.\n>> >\n>> > +signature::\n>> > +...\n>> > +signature:trustlevel::\n>> > + The Trust level of the GPG signature of a commit. Possible\n>> > + outputs are `ultimate`, `fully`, `marginal`, `never` and `undefined`.\n>>\n>> A good list. How do these work for signature made with a tool other\n>> than GPG (in other words, when \"gpg.format\" is set to something\n>> other than \"openpgp\")?\n>>\n>> > @@ -378,6 +383,30 @@ static int subject_atom_parser(struct ref_format *format, struct used_atom *atom\n>> > return 0;\n>> > }\n>> >\n>> > +static int signature_atom_parser(struct ref_format *format, struct used_atom *atom,\n>> > + const char *arg, struct strbuf *err)\n>> > +{\n>> > + if (arg) {\n>> > + if (!strcmp(arg, \"signer\"))\n>> > + atom->u.signature.option = S_SIGNER;\n>> > + else if (!strcmp(arg, \"grade\"))\n>> > + atom->u.signature.option = S_GRADE;\n>> > + else if (!strcmp(arg, \"key\"))\n>> > + atom->u.signature.option = S_KEY;\n>> > + else if (!strcmp(arg, \"fingerprint\"))\n>> > + atom->u.signature.option = S_FINGERPRINT;\n>> > + else if (!strcmp(arg, \"primarykeyfingerprint\"))\n>> > + atom->u.signature.option = S_PRI_KEY_FP;\n>> > + else if (!strcmp(arg, \"trustlevel\"))\n>> > + atom->u.signature.option = S_TRUST_LEVEL;\n>> > + else\n>> > + return strbuf_addf_ret(err, -1, _(\"unknown %%(signature) argument: %s\"), arg);\n>> > + }\n>> > + else\n>> > + atom->u.signature.option = S_BARE;\n>> > + return 0;\n>> > +}\n>>\n>> Handing the !arg case first will make the if/else if/... cascade\n>> easier to follow, no? Also the body of the function may want to\n>> become a separate function that returns one of these S_FOO constants.\n>>\n>> static enum signatore_option signature_atom_parser(...)\n>> {\n>> enum signature_option opt = parse_signature_option(arg);\n>> if (opt < 0)\n>> return strbuf_addf_ret(err, opt, _(\"unknown ...\"), arg);\n>> return opt;\n>> }\n>>\n>> where parse_signature_option() would look like\n>>\n>> static enum signature_option parse_signature_option(const char *arg)\n>> {\n>> if (!arg)\n>> return S_BARE;\n>> else if (!strcmp(arg, \"signer\"))\n>> return S_SIGNER;\n>> ...\n>> else\n>> return -1;\n>> }\n>>\n>> or something like that?\n>>\n>> > @@ -1344,6 +1374,69 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void\n>> > }\n>> > }\n>> >\n>> > +static void grab_signature(struct atom_value *val, int deref, struct object *obj)\n>>\n>> To be considerate for future developers, perhaps rename this to\n>> grab_commit_signature(), so that they can add grab_tag_signature()\n>> when they lift the limitation of this implementaiton?\n>>\n>> > +{\n>> > + int i;\n>> > + struct commit *commit = (struct commit *) obj;\n>>\n>> Style? No SP between cast and value?\n>>\n>> > +\n>> > + for (i = 0; i < used_atom_cnt; i++) {\n>> > + struct used_atom *atom = &used_atom[i];\n>> > + const char *name = atom->name;\n>> > + struct atom_value *v = &val[i];\n>> > + struct signature_check sigc = { 0 };\n>> > +\n>> > + if (!!deref != (*name == '*'))\n>> > + continue;\n>> > + if (deref)\n>> > + name++;\n>> > + if (strcmp(name, \"signature\") &&\n>> > + strcmp(name, \"signature:signer\") &&\n>> > + strcmp(name, \"signature:grade\") &&\n>> > + strcmp(name, \"signature:key\") &&\n>> > + strcmp(name, \"signature:fingerprint\") &&\n>> > + strcmp(name, \"signature:primarykeyfingerprint\") &&\n>> > + strcmp(name, \"signature:trustlevel\"))\n>> > + continue;\n>>\n>> And with the helper above, we can avoid the repetition here that can\n>> go out of sync with the parser function.\n>>\n>> > + check_commit_signature(commit, &sigc);\n>>\n>> If a format asks for signature:signer and signature:key, we\n>> shouldn't be running GPG twice. First check used_atom[] to see if\n>> we even need to do _any_ signature processing (and leave if there is\n>> not), populate the sigc just once and then enter the loop, perhaps?\n>>\n>> In adddition, a call to check_commit_signature() should have a\n>> matching call to signature_check_clear(); otherwise all the\n>> resources held by sigc would leak, wouldn't it?\n"},{"id":"470017","messageId":"20230110005251.10539-1-nsengiyumvawilberforce@gmail.com","threadId":"59006","inReplyTo":"pull.1452.git.1672102523902.gitgitgadget@gmail.com","subject":"[PATCH v3 0/1] ref-filter: add new \"signature\" atom","fromName":"Nsengiyumva Wilberforce","fromEmail":"nsengiyumvawilberforce@gmail.com","sentAt":"2023-01-10T00:52:50Z","receivedAt":"2023-01-10T00:53:05Z","isPatch":true,"sender":{"key":"nsengiyumvawilberforce@gmail.com","avatar":"https://avatars.githubusercontent.com/u/65920105?v=4"},"body":"This commit duplicates the code for `signature` atom from pretty.c\nto ref-filter.c. This feature will help to get rid of current duplicate\nimplementation of `signature` atom when unifying implementations by\nusing ref-filter logic everywhere when ref-filter can do everything\npretty is doing.\n\nAdd \"signature\" atom with `grade`, `signer`, `key`,\n`fingerprint`, `primarykeyfingerprint`, `trustlevel` as arguments.\nThis code and its documentation are inspired by how the %GG, %G?,\n%GS, %GK, %GF, %GP, and %GT pretty formats were implemented.\n\nCo-authored-by: Hariom Verma \nCo-authored-by: Jaydeep Das \nMentored-by: Christian Couder \nMentored-by: Hariom Verma \nSigned-off-by: Nsengiyumva Wilberforce \n\nNsengiyumva Wilberforce (1):\n ref-filter: add new \"signature\" atom\n\n Documentation/git-for-each-ref.txt | 27 +++++++\n ref-filter.c | 101 +++++++++++++++++++++++++\n t/t6300-for-each-ref.sh | 116 +++++++++++++++++++++++++++++\n 3 files changed, 244 insertions(+)\n\nRange-diff against v2:\n1: ce51d8e79e = 1: ce51d8e79e ref-filter: add new \"signature\" atom\n-- \n2.39.0.138.gb334f1a8b9\n\n"},{"id":"470018","messageId":"20230110005251.10539-2-nsengiyumvawilberforce@gmail.com","threadId":"59006","inReplyTo":"20230110005251.10539-1-nsengiyumvawilberforce@gmail.com","subject":"[PATCH v3 1/1] ref-filter: add new \"signature\" atom","fromName":"Nsengiyumva Wilberforce","fromEmail":"nsengiyumvawilberforce@gmail.com","sentAt":"2023-01-10T00:52:51Z","receivedAt":"2023-01-10T00:53:36Z","isPatch":true,"sender":{"key":"nsengiyumvawilberforce@gmail.com","avatar":"https://avatars.githubusercontent.com/u/65920105?v=4"},"body":"This commit duplicates the code for `signature` atom from pretty.c\nto ref-filter.c. This feature will help to get rid of current duplicate\nimplementation of `signature` atom when unifying implementations by\nusing ref-filter logic everywhere when ref-filter can do everything\npretty is doing.\n\nAdd \"signature\" atom with `grade`, `signer`, `key`,\n`fingerprint`, `primarykeyfingerprint`, `trustlevel` as arguments.\nThis code and its documentation are inspired by how the %GG, %G?,\n%GS, %GK, %GF, %GP, and %GT pretty formats were implemented.\n\nCo-authored-by: Hariom Verma \nCo-authored-by: Jaydeep Das \nMentored-by: Christian Couder \nMentored-by: Hariom Verma \nSigned-off-by: Nsengiyumva Wilberforce \n---\n Documentation/git-for-each-ref.txt | 27 +++++++\n ref-filter.c | 101 +++++++++++++++++++++++++\n t/t6300-for-each-ref.sh | 116 +++++++++++++++++++++++++++++\n 3 files changed, 244 insertions(+)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 6da899c629..9a0be85368 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -212,6 +212,33 @@ symref::\n \t`:lstrip` and `:rstrip` options in the same way as `refname`\n \tabove.\n \n+signature::\n+\tThe GPG signature of a commit.\n+\n+signature:grade::\n+\tShow \"G\" for a good (valid) signature, \"B\" for a bad\n+\tsignature, \"U\" for a good signature with unknown validity, \"X\"\n+\tfor a good signature that has expired, \"Y\" for a good\n+\tsignature made by an expired key, \"R\" for a good signature\n+\tmade by a revoked key, \"E\" if the signature cannot be\n+\tchecked (e.g. missing key) and \"N\" for no signature.\n+\n+signature:signer::\n+\tThe signer of the GPG signature of a commit.\n+\n+signature:key::\n+\tThe key of the GPG signature of a commit.\n+\n+signature:fingerprint::\n+\tThe fingerprint of the GPG signature of a commit.\n+\n+signature:primarykeyfingerprint::\n+\tThe Primary Key fingerprint of the GPG signature of a commit.\n+\n+signature:trustlevel::\n+\tThe Trust level of the GPG signature of a commit. Possible\n+\toutputs are `ultimate`, `fully`, `marginal`, `never` and `undefined`.\n+\n worktreepath::\n \tThe absolute path to the worktree in which the ref is checked\n \tout, if it is checked out in any linked worktree. Empty string\ndiff --git a/ref-filter.c b/ref-filter.c\nindex a24324123e..0cba756b18 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -144,6 +144,7 @@ enum atom_type {\n \tATOM_BODY,\n \tATOM_TRAILERS,\n \tATOM_CONTENTS,\n+\tATOM_SIGNATURE,\n \tATOM_RAW,\n \tATOM_UPSTREAM,\n \tATOM_PUSH,\n@@ -208,6 +209,10 @@ static struct used_atom {\n \t\tstruct email_option {\n \t\t\tenum { EO_RAW, EO_TRIM, EO_LOCALPART } option;\n \t\t} email_option;\n+\t\tstruct {\n+\t\t\tenum { S_BARE, S_GRADE, S_SIGNER, S_KEY,\n+\t\t\t S_FINGERPRINT, S_PRI_KEY_FP, S_TRUST_LEVEL} option;\n+\t\t} signature;\n \t\tstruct refname_atom refname;\n \t\tchar *head;\n \t} u;\n@@ -394,6 +399,34 @@ static int subject_atom_parser(struct ref_format *format, struct used_atom *atom\n \treturn 0;\n }\n \n+static int parse_signature_option(const char *arg)\n+{\n+\tif (!arg)\n+\t\treturn S_BARE;\n+\telse if (!strcmp(arg, \"signer\"))\n+\t\treturn S_SIGNER;\n+\telse if (!strcmp(arg, \"grade\"))\n+\t\treturn S_GRADE;\n+\telse if (!strcmp(arg, \"key\"))\n+\t\treturn S_KEY;\n+\telse if (!strcmp(arg, \"fingerprint\"))\n+\t\treturn S_FINGERPRINT;\n+\telse if (!strcmp(arg, \"primarykeyfingerprint\"))\n+\t\treturn S_PRI_KEY_FP;\n+\telse if (!strcmp(arg, \"trustlevel\"))\n+\t\treturn S_TRUST_LEVEL;\n+\treturn -1;\n+}\n+\n+static int signature_atom_parser(struct ref_format *format UNUSED, struct used_atom *atom,\n+\t\t\t const char *arg, struct strbuf *err){\n+\tint opt = parse_signature_option(arg);\n+\tif (opt < 0)\n+\t\treturn err_bad_arg(err, \"signature\", arg);\n+\tatom->u.signature.option = opt;\n+\treturn 0;\n+}\n+\n static int trailers_atom_parser(struct ref_format *format, struct used_atom *atom,\n \t\t\t\tconst char *arg, struct strbuf *err)\n {\n@@ -631,6 +664,7 @@ static struct {\n \t[ATOM_BODY] = { \"body\", SOURCE_OBJ, FIELD_STR, body_atom_parser },\n \t[ATOM_TRAILERS] = { \"trailers\", SOURCE_OBJ, FIELD_STR, trailers_atom_parser },\n \t[ATOM_CONTENTS] = { \"contents\", SOURCE_OBJ, FIELD_STR, contents_atom_parser },\n+\t[ATOM_SIGNATURE] = { \"signature\", SOURCE_OBJ, FIELD_STR, signature_atom_parser },\n \t[ATOM_RAW] = { \"raw\", SOURCE_OBJ, FIELD_STR, raw_atom_parser },\n \t[ATOM_UPSTREAM] = { \"upstream\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n \t[ATOM_PUSH] = { \"push\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n@@ -1362,6 +1396,72 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void\n \t}\n }\n \n+static void grab_signature(struct atom_value *val, int deref, struct object *obj)\n+{\n+\tint i;\n+\tstruct commit *commit = (struct commit *) obj;\n+\tstruct signature_check sigc = { 0 };\n+\n+\tcheck_commit_signature(commit, &sigc);\n+\n+\tfor (i = 0; i < used_atom_cnt; i++) {\n+\t\tstruct used_atom *atom = &used_atom[i];\n+\t\tconst char *name = atom->name;\n+\t\tstruct atom_value *v = &val[i];\n+\n+\t\tif (!!deref != (*name == '*'))\n+\t\t\tcontinue;\n+\t\tif (deref)\n+\t\t\tname++;\n+\n+\t\tif (!skip_prefix(name, \"signature\", &name) || (*name &&\n+\t\t\t*name != ':'))\n+\t\t\tcontinue;\n+\t\tif (!*name)\n+\t\t\tname = NULL;\n+\t\telse\n+\t\t\tname++;\n+\t\tif (parse_signature_option(name) < 0)\n+\t\t\tcontinue;\n+\n+\t\tif (atom->u.signature.option == S_BARE)\n+\t\t\tv->s = xstrdup(sigc.output ? sigc.output: \"\");\n+\t\telse if (atom->u.signature.option == S_SIGNER)\n+\t\t\tv->s = xstrdup(sigc.signer ? sigc.signer : \"\");\n+\t\telse if (atom->u.signature.option == S_GRADE) {\n+\t\t\tswitch (sigc.result) {\n+\t\t\tcase 'G':\n+\t\t\t\tswitch (sigc.trust_level) {\n+\t\t\t\tcase TRUST_UNDEFINED:\n+\t\t\t\tcase TRUST_NEVER:\n+\t\t\t\t\tv->s = xstrfmt(\"%c\", (char)'U');\n+\t\t\t\t\tbreak;\n+\t\t\t\tdefault:\n+\t\t\t\t\tv->s = xstrfmt(\"%c\", (char)'G');\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n+\t\t\t\tbreak;\n+\t\t\tcase 'B':\n+\t\t\tcase 'E':\n+\t\t\tcase 'N':\n+\t\t\tcase 'X':\n+\t\t\tcase 'Y':\n+\t\t\tcase 'R':\n+\t\t\t\tv->s = xstrfmt(\"%c\", (char)sigc.result);\n+\t\t\t}\n+\t\t}\n+\t\telse if (atom->u.signature.option == S_KEY)\n+\t\t\tv->s = xstrdup(sigc.key ? sigc.key : \"\");\n+\t\telse if (atom->u.signature.option == S_FINGERPRINT)\n+\t\t\tv->s = xstrdup(sigc.fingerprint ? sigc.fingerprint : \"\");\n+\t\telse if (atom->u.signature.option == S_PRI_KEY_FP)\n+\t\t\tv->s = xstrdup(sigc.primary_key_fingerprint ? sigc.primary_key_fingerprint : \"\");\n+\t\telse if (atom->u.signature.option == S_TRUST_LEVEL)\n+\t\t\tv->s = xstrdup(gpg_trust_level_to_str(sigc.trust_level));\n+\t}\n+\tsignature_check_clear(&sigc);\n+}\n+\n static void find_subpos(const char *buf,\n \t\t\tconst char **sub, size_t *sublen,\n \t\t\tconst char **body, size_t *bodylen,\n@@ -1555,6 +1655,7 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, s\n \t\tgrab_sub_body_contents(val, deref, data);\n \t\tgrab_person(\"author\", val, deref, buf);\n \t\tgrab_person(\"committer\", val, deref, buf);\n+\t\tgrab_signature(val, deref, obj);\n \t\tbreak;\n \tcase OBJ_TREE:\n \t\t/* grab_tree_values(val, deref, obj, buf, sz); */\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 2ae1fc721b..47def9549d 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -6,6 +6,7 @@\n test_description='for-each-ref test'\n \n . ./test-lib.sh\n+GNUPGHOME_NOT_USED=$GNUPGHOME\n . \"$TEST_DIRECTORY\"/lib-gpg.sh\n . \"$TEST_DIRECTORY\"/lib-terminal.sh\n \n@@ -1464,4 +1465,119 @@ sig_crlf=\"$(printf \"%s\" \"$sig\" | append_cr; echo dummy)\"\n sig_crlf=${sig_crlf%dummy}\n test_atom refs/tags/fake-sig-crlf contents:signature \"$sig_crlf\"\n \n+GRADE_FORMAT=\"%(signature:grade)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)\"\n+TRUSTLEVEL_FORMAT=\"%(signature:trustlevel)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)\"\n+\n+test_expect_success GPG 'show good signature with custom format' '\n+\tgit checkout -b signed &&\n+\techo 2 >file && git add file &&\n+\ttest_tick && git commit -S -m initial &&\n+\tgit verify-commit signed 2>out &&\n+\tcat >expect <<-\\EOF &&\n+\tG\n+\t13B6F51ECDDE430D\n+\tC O Mitter \n+\t73D758744BE721698EC54E8713B6F51ECDDE430D\n+\t73D758744BE721698EC54E8713B6F51ECDDE430D\n+\tEOF\n+\tgit for-each-ref refs/heads/signed --format=\"$GRADE_FORMAT\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'test signature atom with grade option and bad signature' '\n+\tgit config commit.gpgsign true &&\n+\techo 3 >file && test_tick && git commit -a -m \"third\" --no-gpg-sign &&\n+\tgit tag third-unsigned &&\n+\n+\ttest_tick && git rebase -f HEAD^^ && git tag second-signed HEAD^ &&\n+\tgit tag third-signed &&\n+\n+\tgit cat-file commit third-signed >raw &&\n+\tsed -e \"s/^third/3rd forged/\" raw >forged1 &&\n+\tFORGED1=$(git hash-object -w -t commit forged1) &&\n+\tgit update-ref refs/tags/third-signed \"$FORGED1\" &&\n+\ttest_must_fail git verify-commit \"$FORGED1\" &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tB\n+\t13B6F51ECDDE430D\n+\tC O Mitter \n+\n+\n+\tEOF\n+\tgit for-each-ref refs/tags/third-signed --format=\"$GRADE_FORMAT\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'show untrusted signature with custom format' '\n+\techo 4 >file && test_tick && git commit -a -m fourth -SB7227189 &&\n+\tgit tag signed-fourth &&\n+\tcat >expect <<-\\EOF &&\n+\tU\n+\t65A0EEA02E30CAD7\n+\tEris Discordia \n+\tF8364A59E07FFE9F4D63005A65A0EEA02E30CAD7\n+\tD4BE22311AD3131E5EDA29A461092E85B7227189\n+\tEOF\n+\tgit for-each-ref refs/tags/signed-fourth --format=\"$GRADE_FORMAT\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'show untrusted signature with undefined trust level' '\n+\techo 5 >file && test_tick && git commit -a -m fifth -SB7227189 &&\n+\tgit tag fifth-signed &&\n+\tcat >expect <<-\\EOF &&\n+\tundefined\n+\t65A0EEA02E30CAD7\n+\tEris Discordia \n+\tF8364A59E07FFE9F4D63005A65A0EEA02E30CAD7\n+\tD4BE22311AD3131E5EDA29A461092E85B7227189\n+\tEOF\n+\tgit for-each-ref refs/tags/fifth-signed --format=\"$TRUSTLEVEL_FORMAT\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'show untrusted signature with ultimate trust level' '\n+\techo 7 >file && test_tick && git commit -a -m \"seventh\" --no-gpg-sign &&\n+\tgit tag seventh-unsigned &&\n+\n+\ttest_tick && git rebase -f HEAD^^ && git tag sixth-signed HEAD^ &&\n+\tgit tag seventh-signed &&\n+\tcat >expect <<-\\EOF &&\n+\tultimate\n+\t13B6F51ECDDE430D\n+\tC O Mitter \n+\t73D758744BE721698EC54E8713B6F51ECDDE430D\n+\t73D758744BE721698EC54E8713B6F51ECDDE430D\n+\tEOF\n+\tgit for-each-ref refs/tags/seventh-signed --format=\"$TRUSTLEVEL_FORMAT\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'show unknown signature with custom format' '\n+\tcat >expect <<-\\EOF &&\n+\tE\n+\t65A0EEA02E30CAD7\n+\n+\n+\n+\tEOF\n+\tGNUPGHOME=\"$GNUPGHOME_NOT_USED\" git for-each-ref refs/tags/fifth-signed --format=\"$GRADE_FORMAT\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'show lack of signature with custom format' '\n+\techo 8 >file && test_tick && git commit -a -m \"eigth unsigned\" --no-gpg-sign &&\n+\tgit tag eigth-unsigned &&\n+\tcat >expect <<-\\EOF &&\n+\tN\n+\n+\n+\n+\n+\tEOF\n+\tgit for-each-ref refs/tags/eigth-unsigned --format=\"$GRADE_FORMAT\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.39.0.138.gb334f1a8b9\n\n"},{"id":"470051","messageId":"CAP8UFD1A8BofxHZP5cJ-xcQA8rbfoLN50hMc-dfUYUOGKqA65A@mail.gmail.com","threadId":"59006","inReplyTo":"20230110005251.10539-1-nsengiyumvawilberforce@gmail.com","subject":"Re: [PATCH v3 0/1] ref-filter: add new \"signature\" atom","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2023-01-10T09:13:26Z","receivedAt":"2023-01-10T09:14:36Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Jan 10, 2023 at 1:53 AM Nsengiyumva Wilberforce\n wrote:\n\n> Documentation/git-for-each-ref.txt | 27 +++++++\n> ref-filter.c | 101 +++++++++++++++++++++++++\n> t/t6300-for-each-ref.sh | 116 +++++++++++++++++++++++++++++\n> 3 files changed, 244 insertions(+)\n>\n> Range-diff against v2:\n> 1: ce51d8e79e = 1: ce51d8e79e ref-filter: add new \"signature\" atom\n\nIt's strange that it's saying nothing changed compared to v2. I guess\nthe v2 it's talking about is not actually the v2 you sent to the list,\nright?\n\nAnyway it's better if you can describe a bit with your own words in\nthe cover letter what changed, and sometimes even what didn't change,\nsince the previous version you sent.\n\nFor example, here you can say that you decided to actually remove the\ntest that checked the %(signature) format and explain a bit the reason\nwhy you did that.\n\nYou can also send links in the cover letter to the branch(es) (on\nGitHub, GitLab or other such platforms) that you used for this and\nprevious versions of the patch (or patch series). It can help people\ntry it and look at the changes on their own system.\n\nThanks.\n"},{"id":"470447","messageId":"20230116173814.11338-1-nsengiyumvawilberforce@gmail.com","threadId":"59006","inReplyTo":"20230110005251.10539-2-nsengiyumvawilberforce@gmail.com","subject":"[PATCH v4 0/1] ref-filter: add new \"signature\" atom","fromName":"Nsengiyumva Wilberforce","fromEmail":"nsengiyumvawilberforce@gmail.com","sentAt":"2023-01-16T17:38:13Z","receivedAt":"2023-01-16T17:58:25Z","isPatch":true,"sender":{"key":"nsengiyumvawilberforce@gmail.com","avatar":"https://avatars.githubusercontent.com/u/65920105?v=4"},"body":"This patch is not different from previous version(3), this is because\nthe previous version(3) could not show the difference from version(2)\nof the patch. I will explain every change from the first version\n \nto version 3\n\nbecause I first had a big trouble in transitioning from gitgitgadget\nto using git send-mail\n\n***THE FOLLOWING ARE THE CHANGES***\na) From the first version to version 2\n\nVersion 2 addresses all Junio's comments for version 1, the comments are\nhere.\n\n->summary of the changes\ni) I changed the commit message to detail more about the feature I am\nintroducing.\nii) Introduced a new helper function in ref-filter.c called\nparse_signature_option() and handled !arg case first.\niii) Used the above helper function to eliminate the repetition that\nwas in grab_signature() for checking different signature option.\niv) I also moved check_commit_signature(commit, &sigc) out of the\nto avoid running GPG twice.still this change is in grab_signature()\nin ref-filter.c.\nv) add a new test in t6300 to test bare signature atom(%(signature))\nsince I had missed it\n\nNB: I did not change the parser function name as he suggested, I\nthink my commit message was misleading.\n\nb) from version 2 to version 3\n\n\n->summary of changes\ni) Got rid of test for bare signature atom. This is because the test was\npassing for some CI tests(different machines) and some others were\nfailing.\n\nBest Regards,\nWilberforce\n\nNsengiyumva Wilberforce (1):\n ref-filter: add new \"signature\" atom\n\n Documentation/git-for-each-ref.txt | 27 +++++++\n ref-filter.c | 101 +++++++++++++++++++++++++\n t/t6300-for-each-ref.sh | 116 +++++++++++++++++++++++++++++\n 3 files changed, 244 insertions(+)\n\nRange-diff against v3:\n1: ce51d8e79e = 1: ce51d8e79e ref-filter: add new \"signature\" atom\n-- \n2.39.0.138.gb334f1a8b9\n\n"},{"id":"470449","messageId":"20230116173814.11338-2-nsengiyumvawilberforce@gmail.com","threadId":"59006","inReplyTo":"20230116173814.11338-1-nsengiyumvawilberforce@gmail.com","subject":"[PATCH v4 1/1] ref-filter: add new \"signature\" atom","fromName":"Nsengiyumva Wilberforce","fromEmail":"nsengiyumvawilberforce@gmail.com","sentAt":"2023-01-16T17:38:14Z","receivedAt":"2023-01-16T17:58:58Z","isPatch":true,"sender":{"key":"nsengiyumvawilberforce@gmail.com","avatar":"https://avatars.githubusercontent.com/u/65920105?v=4"},"body":"This commit duplicates the code for `signature` atom from pretty.c\nto ref-filter.c. This feature will help to get rid of current duplicate\nimplementation of `signature` atom when unifying implementations by\nusing ref-filter logic everywhere when ref-filter can do everything\npretty is doing.\n\nAdd \"signature\" atom with `grade`, `signer`, `key`,\n`fingerprint`, `primarykeyfingerprint`, `trustlevel` as arguments.\nThis code and its documentation are inspired by how the %GG, %G?,\n%GS, %GK, %GF, %GP, and %GT pretty formats were implemented.\n\nCo-authored-by: Hariom Verma \nCo-authored-by: Jaydeep Das \nMentored-by: Christian Couder \nMentored-by: Hariom Verma \nSigned-off-by: Nsengiyumva Wilberforce \n---\n Documentation/git-for-each-ref.txt | 27 +++++++\n ref-filter.c | 101 +++++++++++++++++++++++++\n t/t6300-for-each-ref.sh | 116 +++++++++++++++++++++++++++++\n 3 files changed, 244 insertions(+)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 6da899c629..9a0be85368 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -212,6 +212,33 @@ symref::\n \t`:lstrip` and `:rstrip` options in the same way as `refname`\n \tabove.\n \n+signature::\n+\tThe GPG signature of a commit.\n+\n+signature:grade::\n+\tShow \"G\" for a good (valid) signature, \"B\" for a bad\n+\tsignature, \"U\" for a good signature with unknown validity, \"X\"\n+\tfor a good signature that has expired, \"Y\" for a good\n+\tsignature made by an expired key, \"R\" for a good signature\n+\tmade by a revoked key, \"E\" if the signature cannot be\n+\tchecked (e.g. missing key) and \"N\" for no signature.\n+\n+signature:signer::\n+\tThe signer of the GPG signature of a commit.\n+\n+signature:key::\n+\tThe key of the GPG signature of a commit.\n+\n+signature:fingerprint::\n+\tThe fingerprint of the GPG signature of a commit.\n+\n+signature:primarykeyfingerprint::\n+\tThe Primary Key fingerprint of the GPG signature of a commit.\n+\n+signature:trustlevel::\n+\tThe Trust level of the GPG signature of a commit. Possible\n+\toutputs are `ultimate`, `fully`, `marginal`, `never` and `undefined`.\n+\n worktreepath::\n \tThe absolute path to the worktree in which the ref is checked\n \tout, if it is checked out in any linked worktree. Empty string\ndiff --git a/ref-filter.c b/ref-filter.c\nindex a24324123e..0cba756b18 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -144,6 +144,7 @@ enum atom_type {\n \tATOM_BODY,\n \tATOM_TRAILERS,\n \tATOM_CONTENTS,\n+\tATOM_SIGNATURE,\n \tATOM_RAW,\n \tATOM_UPSTREAM,\n \tATOM_PUSH,\n@@ -208,6 +209,10 @@ static struct used_atom {\n \t\tstruct email_option {\n \t\t\tenum { EO_RAW, EO_TRIM, EO_LOCALPART } option;\n \t\t} email_option;\n+\t\tstruct {\n+\t\t\tenum { S_BARE, S_GRADE, S_SIGNER, S_KEY,\n+\t\t\t S_FINGERPRINT, S_PRI_KEY_FP, S_TRUST_LEVEL} option;\n+\t\t} signature;\n \t\tstruct refname_atom refname;\n \t\tchar *head;\n \t} u;\n@@ -394,6 +399,34 @@ static int subject_atom_parser(struct ref_format *format, struct used_atom *atom\n \treturn 0;\n }\n \n+static int parse_signature_option(const char *arg)\n+{\n+\tif (!arg)\n+\t\treturn S_BARE;\n+\telse if (!strcmp(arg, \"signer\"))\n+\t\treturn S_SIGNER;\n+\telse if (!strcmp(arg, \"grade\"))\n+\t\treturn S_GRADE;\n+\telse if (!strcmp(arg, \"key\"))\n+\t\treturn S_KEY;\n+\telse if (!strcmp(arg, \"fingerprint\"))\n+\t\treturn S_FINGERPRINT;\n+\telse if (!strcmp(arg, \"primarykeyfingerprint\"))\n+\t\treturn S_PRI_KEY_FP;\n+\telse if (!strcmp(arg, \"trustlevel\"))\n+\t\treturn S_TRUST_LEVEL;\n+\treturn -1;\n+}\n+\n+static int signature_atom_parser(struct ref_format *format UNUSED, struct used_atom *atom,\n+\t\t\t const char *arg, struct strbuf *err){\n+\tint opt = parse_signature_option(arg);\n+\tif (opt < 0)\n+\t\treturn err_bad_arg(err, \"signature\", arg);\n+\tatom->u.signature.option = opt;\n+\treturn 0;\n+}\n+\n static int trailers_atom_parser(struct ref_format *format, struct used_atom *atom,\n \t\t\t\tconst char *arg, struct strbuf *err)\n {\n@@ -631,6 +664,7 @@ static struct {\n \t[ATOM_BODY] = { \"body\", SOURCE_OBJ, FIELD_STR, body_atom_parser },\n \t[ATOM_TRAILERS] = { \"trailers\", SOURCE_OBJ, FIELD_STR, trailers_atom_parser },\n \t[ATOM_CONTENTS] = { \"contents\", SOURCE_OBJ, FIELD_STR, contents_atom_parser },\n+\t[ATOM_SIGNATURE] = { \"signature\", SOURCE_OBJ, FIELD_STR, signature_atom_parser },\n \t[ATOM_RAW] = { \"raw\", SOURCE_OBJ, FIELD_STR, raw_atom_parser },\n \t[ATOM_UPSTREAM] = { \"upstream\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n \t[ATOM_PUSH] = { \"push\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n@@ -1362,6 +1396,72 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void\n \t}\n }\n \n+static void grab_signature(struct atom_value *val, int deref, struct object *obj)\n+{\n+\tint i;\n+\tstruct commit *commit = (struct commit *) obj;\n+\tstruct signature_check sigc = { 0 };\n+\n+\tcheck_commit_signature(commit, &sigc);\n+\n+\tfor (i = 0; i < used_atom_cnt; i++) {\n+\t\tstruct used_atom *atom = &used_atom[i];\n+\t\tconst char *name = atom->name;\n+\t\tstruct atom_value *v = &val[i];\n+\n+\t\tif (!!deref != (*name == '*'))\n+\t\t\tcontinue;\n+\t\tif (deref)\n+\t\t\tname++;\n+\n+\t\tif (!skip_prefix(name, \"signature\", &name) || (*name &&\n+\t\t\t*name != ':'))\n+\t\t\tcontinue;\n+\t\tif (!*name)\n+\t\t\tname = NULL;\n+\t\telse\n+\t\t\tname++;\n+\t\tif (parse_signature_option(name) < 0)\n+\t\t\tcontinue;\n+\n+\t\tif (atom->u.signature.option == S_BARE)\n+\t\t\tv->s = xstrdup(sigc.output ? sigc.output: \"\");\n+\t\telse if (atom->u.signature.option == S_SIGNER)\n+\t\t\tv->s = xstrdup(sigc.signer ? sigc.signer : \"\");\n+\t\telse if (atom->u.signature.option == S_GRADE) {\n+\t\t\tswitch (sigc.result) {\n+\t\t\tcase 'G':\n+\t\t\t\tswitch (sigc.trust_level) {\n+\t\t\t\tcase TRUST_UNDEFINED:\n+\t\t\t\tcase TRUST_NEVER:\n+\t\t\t\t\tv->s = xstrfmt(\"%c\", (char)'U');\n+\t\t\t\t\tbreak;\n+\t\t\t\tdefault:\n+\t\t\t\t\tv->s = xstrfmt(\"%c\", (char)'G');\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n+\t\t\t\tbreak;\n+\t\t\tcase 'B':\n+\t\t\tcase 'E':\n+\t\t\tcase 'N':\n+\t\t\tcase 'X':\n+\t\t\tcase 'Y':\n+\t\t\tcase 'R':\n+\t\t\t\tv->s = xstrfmt(\"%c\", (char)sigc.result);\n+\t\t\t}\n+\t\t}\n+\t\telse if (atom->u.signature.option == S_KEY)\n+\t\t\tv->s = xstrdup(sigc.key ? sigc.key : \"\");\n+\t\telse if (atom->u.signature.option == S_FINGERPRINT)\n+\t\t\tv->s = xstrdup(sigc.fingerprint ? sigc.fingerprint : \"\");\n+\t\telse if (atom->u.signature.option == S_PRI_KEY_FP)\n+\t\t\tv->s = xstrdup(sigc.primary_key_fingerprint ? sigc.primary_key_fingerprint : \"\");\n+\t\telse if (atom->u.signature.option == S_TRUST_LEVEL)\n+\t\t\tv->s = xstrdup(gpg_trust_level_to_str(sigc.trust_level));\n+\t}\n+\tsignature_check_clear(&sigc);\n+}\n+\n static void find_subpos(const char *buf,\n \t\t\tconst char **sub, size_t *sublen,\n \t\t\tconst char **body, size_t *bodylen,\n@@ -1555,6 +1655,7 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, s\n \t\tgrab_sub_body_contents(val, deref, data);\n \t\tgrab_person(\"author\", val, deref, buf);\n \t\tgrab_person(\"committer\", val, deref, buf);\n+\t\tgrab_signature(val, deref, obj);\n \t\tbreak;\n \tcase OBJ_TREE:\n \t\t/* grab_tree_values(val, deref, obj, buf, sz); */\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 2ae1fc721b..47def9549d 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -6,6 +6,7 @@\n test_description='for-each-ref test'\n \n . ./test-lib.sh\n+GNUPGHOME_NOT_USED=$GNUPGHOME\n . \"$TEST_DIRECTORY\"/lib-gpg.sh\n . \"$TEST_DIRECTORY\"/lib-terminal.sh\n \n@@ -1464,4 +1465,119 @@ sig_crlf=\"$(printf \"%s\" \"$sig\" | append_cr; echo dummy)\"\n sig_crlf=${sig_crlf%dummy}\n test_atom refs/tags/fake-sig-crlf contents:signature \"$sig_crlf\"\n \n+GRADE_FORMAT=\"%(signature:grade)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)\"\n+TRUSTLEVEL_FORMAT=\"%(signature:trustlevel)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)\"\n+\n+test_expect_success GPG 'show good signature with custom format' '\n+\tgit checkout -b signed &&\n+\techo 2 >file && git add file &&\n+\ttest_tick && git commit -S -m initial &&\n+\tgit verify-commit signed 2>out &&\n+\tcat >expect <<-\\EOF &&\n+\tG\n+\t13B6F51ECDDE430D\n+\tC O Mitter \n+\t73D758744BE721698EC54E8713B6F51ECDDE430D\n+\t73D758744BE721698EC54E8713B6F51ECDDE430D\n+\tEOF\n+\tgit for-each-ref refs/heads/signed --format=\"$GRADE_FORMAT\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'test signature atom with grade option and bad signature' '\n+\tgit config commit.gpgsign true &&\n+\techo 3 >file && test_tick && git commit -a -m \"third\" --no-gpg-sign &&\n+\tgit tag third-unsigned &&\n+\n+\ttest_tick && git rebase -f HEAD^^ && git tag second-signed HEAD^ &&\n+\tgit tag third-signed &&\n+\n+\tgit cat-file commit third-signed >raw &&\n+\tsed -e \"s/^third/3rd forged/\" raw >forged1 &&\n+\tFORGED1=$(git hash-object -w -t commit forged1) &&\n+\tgit update-ref refs/tags/third-signed \"$FORGED1\" &&\n+\ttest_must_fail git verify-commit \"$FORGED1\" &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tB\n+\t13B6F51ECDDE430D\n+\tC O Mitter \n+\n+\n+\tEOF\n+\tgit for-each-ref refs/tags/third-signed --format=\"$GRADE_FORMAT\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'show untrusted signature with custom format' '\n+\techo 4 >file && test_tick && git commit -a -m fourth -SB7227189 &&\n+\tgit tag signed-fourth &&\n+\tcat >expect <<-\\EOF &&\n+\tU\n+\t65A0EEA02E30CAD7\n+\tEris Discordia \n+\tF8364A59E07FFE9F4D63005A65A0EEA02E30CAD7\n+\tD4BE22311AD3131E5EDA29A461092E85B7227189\n+\tEOF\n+\tgit for-each-ref refs/tags/signed-fourth --format=\"$GRADE_FORMAT\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'show untrusted signature with undefined trust level' '\n+\techo 5 >file && test_tick && git commit -a -m fifth -SB7227189 &&\n+\tgit tag fifth-signed &&\n+\tcat >expect <<-\\EOF &&\n+\tundefined\n+\t65A0EEA02E30CAD7\n+\tEris Discordia \n+\tF8364A59E07FFE9F4D63005A65A0EEA02E30CAD7\n+\tD4BE22311AD3131E5EDA29A461092E85B7227189\n+\tEOF\n+\tgit for-each-ref refs/tags/fifth-signed --format=\"$TRUSTLEVEL_FORMAT\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'show untrusted signature with ultimate trust level' '\n+\techo 7 >file && test_tick && git commit -a -m \"seventh\" --no-gpg-sign &&\n+\tgit tag seventh-unsigned &&\n+\n+\ttest_tick && git rebase -f HEAD^^ && git tag sixth-signed HEAD^ &&\n+\tgit tag seventh-signed &&\n+\tcat >expect <<-\\EOF &&\n+\tultimate\n+\t13B6F51ECDDE430D\n+\tC O Mitter \n+\t73D758744BE721698EC54E8713B6F51ECDDE430D\n+\t73D758744BE721698EC54E8713B6F51ECDDE430D\n+\tEOF\n+\tgit for-each-ref refs/tags/seventh-signed --format=\"$TRUSTLEVEL_FORMAT\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'show unknown signature with custom format' '\n+\tcat >expect <<-\\EOF &&\n+\tE\n+\t65A0EEA02E30CAD7\n+\n+\n+\n+\tEOF\n+\tGNUPGHOME=\"$GNUPGHOME_NOT_USED\" git for-each-ref refs/tags/fifth-signed --format=\"$GRADE_FORMAT\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'show lack of signature with custom format' '\n+\techo 8 >file && test_tick && git commit -a -m \"eigth unsigned\" --no-gpg-sign &&\n+\tgit tag eigth-unsigned &&\n+\tcat >expect <<-\\EOF &&\n+\tN\n+\n+\n+\n+\n+\tEOF\n+\tgit for-each-ref refs/tags/eigth-unsigned --format=\"$GRADE_FORMAT\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.39.0.138.gb334f1a8b9\n\n"},{"id":"471078","messageId":"xmqqmt65atu1.fsf@gitster.g","threadId":"59006","inReplyTo":"20230116173814.11338-1-nsengiyumvawilberforce@gmail.com","subject":"Re: [PATCH v4 0/1] ref-filter: add new \"signature\" atom","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-01-26T21:07:02Z","receivedAt":"2023-01-26T21:07:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nsengiyumva Wilberforce writes:\n\n> iv) I also moved check_commit_signature(commit, &sigc) out of the\n> to avoid running GPG twice.still this change is in grab_signature()\n> in ref-filter.c.\n\nIs there some code to avoid calling check_commit_signature() or\ngrab_signature() itself when no %(signature) atoms is used? When\nnobody asked to use signature atom at all, calling GPG once is\nalready once too many.\n\nThanks.\n\n"},{"id":"473410","messageId":"20230311210607.64927-1-nsengiyumvawilberforce@gmail.com","threadId":"59006","inReplyTo":"20230116173814.11338-2-nsengiyumvawilberforce@gmail.com","subject":"[PATCH v5 0/1] ref-filter: add new \"signature\" atom","fromName":"Nsengiyumva Wilberforce","fromEmail":"nsengiyumvawilberforce@gmail.com","sentAt":"2023-03-11T21:06:06Z","receivedAt":"2023-03-11T21:06:55Z","isPatch":true,"sender":{"key":"nsengiyumvawilberforce@gmail.com","avatar":"https://avatars.githubusercontent.com/u/65920105?v=4"},"body":"This patch addresses Junio's comment\nhere. I\nintroduced an integer flag called signature_checked to cater\nfor this.\n\nAlso, the fact that \"test bare signature atom\" test was failing on \ndifferent machines(CI on github) due to trustdb, I introduced a hack\nto cater for this.Something like this,\n`grep -v \"checking the trustdb\" out_orig >out &&`\n\nNsengiyumva Wilberforce (1):\n ref-filter: add new \"signature\" atom\n\n Documentation/git-for-each-ref.txt | 27 ++++++\n ref-filter.c | 106 ++++++++++++++++++++++++\n t/t6300-for-each-ref.sh | 128 +++++++++++++++++++++++++++++\n 3 files changed, 261 insertions(+)\n\nRange-diff against v4:\n1: d9ce65f411 = 1: d9ce65f411 ref-filter: add new \"signature\" atom\n-- \n2.39.GIT\n\n"},{"id":"473411","messageId":"20230311210607.64927-2-nsengiyumvawilberforce@gmail.com","threadId":"59006","inReplyTo":"20230311210607.64927-1-nsengiyumvawilberforce@gmail.com","subject":"[PATCH v5 1/1] ref-filter: add new \"signature\" atom","fromName":"Nsengiyumva Wilberforce","fromEmail":"nsengiyumvawilberforce@gmail.com","sentAt":"2023-03-11T21:06:07Z","receivedAt":"2023-03-11T21:07:06Z","isPatch":true,"sender":{"key":"nsengiyumvawilberforce@gmail.com","avatar":"https://avatars.githubusercontent.com/u/65920105?v=4"},"body":"This commit duplicates the code for `signature` atom from pretty.c\nto ref-filter.c. This feature will help to get rid of current duplicate\nimplementation of `signature` atom when unifying implementations by\nusing ref-filter logic everywhere when ref-filter can do everything\npretty is doing.\n\nAdd \"signature\" atom with `grade`, `signer`, `key`,\n`fingerprint`, `primarykeyfingerprint`, `trustlevel` as arguments.\nThis code and its documentation are inspired by how the %GG, %G?,\n%GS, %GK, %GF, %GP, and %GT pretty formats were implemented.\n\nCo-authored-by: Hariom Verma \nCo-authored-by: Jaydeep Das \nMentored-by: Christian Couder \nMentored-by: Hariom Verma \nSigned-off-by: Nsengiyumva Wilberforce \n---\n Documentation/git-for-each-ref.txt | 27 ++++++\n ref-filter.c | 106 ++++++++++++++++++++++++\n t/t6300-for-each-ref.sh | 128 +++++++++++++++++++++++++++++\n 3 files changed, 261 insertions(+)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 6da899c629..9a0be85368 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -212,6 +212,33 @@ symref::\n \t`:lstrip` and `:rstrip` options in the same way as `refname`\n \tabove.\n \n+signature::\n+\tThe GPG signature of a commit.\n+\n+signature:grade::\n+\tShow \"G\" for a good (valid) signature, \"B\" for a bad\n+\tsignature, \"U\" for a good signature with unknown validity, \"X\"\n+\tfor a good signature that has expired, \"Y\" for a good\n+\tsignature made by an expired key, \"R\" for a good signature\n+\tmade by a revoked key, \"E\" if the signature cannot be\n+\tchecked (e.g. missing key) and \"N\" for no signature.\n+\n+signature:signer::\n+\tThe signer of the GPG signature of a commit.\n+\n+signature:key::\n+\tThe key of the GPG signature of a commit.\n+\n+signature:fingerprint::\n+\tThe fingerprint of the GPG signature of a commit.\n+\n+signature:primarykeyfingerprint::\n+\tThe Primary Key fingerprint of the GPG signature of a commit.\n+\n+signature:trustlevel::\n+\tThe Trust level of the GPG signature of a commit. Possible\n+\toutputs are `ultimate`, `fully`, `marginal`, `never` and `undefined`.\n+\n worktreepath::\n \tThe absolute path to the worktree in which the ref is checked\n \tout, if it is checked out in any linked worktree. Empty string\ndiff --git a/ref-filter.c b/ref-filter.c\nindex a24324123e..2a5c5e9508 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -144,6 +144,7 @@ enum atom_type {\n \tATOM_BODY,\n \tATOM_TRAILERS,\n \tATOM_CONTENTS,\n+\tATOM_SIGNATURE,\n \tATOM_RAW,\n \tATOM_UPSTREAM,\n \tATOM_PUSH,\n@@ -208,6 +209,10 @@ static struct used_atom {\n \t\tstruct email_option {\n \t\t\tenum { EO_RAW, EO_TRIM, EO_LOCALPART } option;\n \t\t} email_option;\n+\t\tstruct {\n+\t\t\tenum { S_BARE, S_GRADE, S_SIGNER, S_KEY,\n+\t\t\t S_FINGERPRINT, S_PRI_KEY_FP, S_TRUST_LEVEL} option;\n+\t\t} signature;\n \t\tstruct refname_atom refname;\n \t\tchar *head;\n \t} u;\n@@ -394,6 +399,34 @@ static int subject_atom_parser(struct ref_format *format, struct used_atom *atom\n \treturn 0;\n }\n \n+static int parse_signature_option(const char *arg)\n+{\n+\tif (!arg)\n+\t\treturn S_BARE;\n+\telse if (!strcmp(arg, \"signer\"))\n+\t\treturn S_SIGNER;\n+\telse if (!strcmp(arg, \"grade\"))\n+\t\treturn S_GRADE;\n+\telse if (!strcmp(arg, \"key\"))\n+\t\treturn S_KEY;\n+\telse if (!strcmp(arg, \"fingerprint\"))\n+\t\treturn S_FINGERPRINT;\n+\telse if (!strcmp(arg, \"primarykeyfingerprint\"))\n+\t\treturn S_PRI_KEY_FP;\n+\telse if (!strcmp(arg, \"trustlevel\"))\n+\t\treturn S_TRUST_LEVEL;\n+\treturn -1;\n+}\n+\n+static int signature_atom_parser(struct ref_format *format UNUSED, struct used_atom *atom,\n+\t\t\t const char *arg, struct strbuf *err){\n+\tint opt = parse_signature_option(arg);\n+\tif (opt < 0)\n+\t\treturn err_bad_arg(err, \"signature\", arg);\n+\tatom->u.signature.option = opt;\n+\treturn 0;\n+}\n+\n static int trailers_atom_parser(struct ref_format *format, struct used_atom *atom,\n \t\t\t\tconst char *arg, struct strbuf *err)\n {\n@@ -631,6 +664,7 @@ static struct {\n \t[ATOM_BODY] = { \"body\", SOURCE_OBJ, FIELD_STR, body_atom_parser },\n \t[ATOM_TRAILERS] = { \"trailers\", SOURCE_OBJ, FIELD_STR, trailers_atom_parser },\n \t[ATOM_CONTENTS] = { \"contents\", SOURCE_OBJ, FIELD_STR, contents_atom_parser },\n+\t[ATOM_SIGNATURE] = { \"signature\", SOURCE_OBJ, FIELD_STR, signature_atom_parser },\n \t[ATOM_RAW] = { \"raw\", SOURCE_OBJ, FIELD_STR, raw_atom_parser },\n \t[ATOM_UPSTREAM] = { \"upstream\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n \t[ATOM_PUSH] = { \"push\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n@@ -1362,6 +1396,77 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void\n \t}\n }\n \n+static void grab_signature(struct atom_value *val, int deref, struct object *obj)\n+{\n+\tint i;\n+\tstruct commit *commit = (struct commit *) obj;\n+\tstruct signature_check sigc = { 0 };\n+\tint signature_checked = 0;\n+\n+\tfor (i = 0; i < used_atom_cnt; i++) {\n+\t\tstruct used_atom *atom = &used_atom[i];\n+\t\tconst char *name = atom->name;\n+\t\tstruct atom_value *v = &val[i];\n+\n+\t\tif (!!deref != (*name == '*'))\n+\t\t\tcontinue;\n+\t\tif (deref)\n+\t\t\tname++;\n+\n+\t\tif (!skip_prefix(name, \"signature\", &name) || (*name &&\n+\t\t\t*name != ':'))\n+\t\t\tcontinue;\n+\t\tif (!*name)\n+\t\t\tname = NULL;\n+\t\telse\n+\t\t\tname++;\n+\t\tif (parse_signature_option(name) < 0)\n+\t\t\tcontinue;\n+\n+\t\tif (!signature_checked) {\n+\t\t\tcheck_commit_signature(commit, &sigc);\n+\t\t\tsignature_checked = 1;\n+\t\t}\n+\n+\t\tif (atom->u.signature.option == S_BARE)\n+\t\t\tv->s = xstrdup(sigc.output ? sigc.output: \"\");\n+\t\telse if (atom->u.signature.option == S_SIGNER)\n+\t\t\tv->s = xstrdup(sigc.signer ? sigc.signer : \"\");\n+\t\telse if (atom->u.signature.option == S_GRADE) {\n+\t\t\tswitch (sigc.result) {\n+\t\t\tcase 'G':\n+\t\t\t\tswitch (sigc.trust_level) {\n+\t\t\t\tcase TRUST_UNDEFINED:\n+\t\t\t\tcase TRUST_NEVER:\n+\t\t\t\t\tv->s = xstrfmt(\"%c\", (char)'U');\n+\t\t\t\t\tbreak;\n+\t\t\t\tdefault:\n+\t\t\t\t\tv->s = xstrfmt(\"%c\", (char)'G');\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n+\t\t\t\tbreak;\n+\t\t\tcase 'B':\n+\t\t\tcase 'E':\n+\t\t\tcase 'N':\n+\t\t\tcase 'X':\n+\t\t\tcase 'Y':\n+\t\t\tcase 'R':\n+\t\t\t\tv->s = xstrfmt(\"%c\", (char)sigc.result);\n+\t\t\t}\n+\t\t}\n+\t\telse if (atom->u.signature.option == S_KEY)\n+\t\t\tv->s = xstrdup(sigc.key ? sigc.key : \"\");\n+\t\telse if (atom->u.signature.option == S_FINGERPRINT)\n+\t\t\tv->s = xstrdup(sigc.fingerprint ? sigc.fingerprint : \"\");\n+\t\telse if (atom->u.signature.option == S_PRI_KEY_FP)\n+\t\t\tv->s = xstrdup(sigc.primary_key_fingerprint ? sigc.primary_key_fingerprint : \"\");\n+\t\telse if (atom->u.signature.option == S_TRUST_LEVEL)\n+\t\t\tv->s = xstrdup(gpg_trust_level_to_str(sigc.trust_level));\n+\t}\n+\tif (signature_checked)\n+\t\tsignature_check_clear(&sigc);\n+}\n+\n static void find_subpos(const char *buf,\n \t\t\tconst char **sub, size_t *sublen,\n \t\t\tconst char **body, size_t *bodylen,\n@@ -1555,6 +1660,7 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, s\n \t\tgrab_sub_body_contents(val, deref, data);\n \t\tgrab_person(\"author\", val, deref, buf);\n \t\tgrab_person(\"committer\", val, deref, buf);\n+\t\tgrab_signature(val, deref, obj);\n \t\tbreak;\n \tcase OBJ_TREE:\n \t\t/* grab_tree_values(val, deref, obj, buf, sz); */\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 2ae1fc721b..c74e2ca169 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -6,6 +6,7 @@\n test_description='for-each-ref test'\n \n . ./test-lib.sh\n+GNUPGHOME_NOT_USED=$GNUPGHOME\n . \"$TEST_DIRECTORY\"/lib-gpg.sh\n . \"$TEST_DIRECTORY\"/lib-terminal.sh\n \n@@ -1464,4 +1465,131 @@ sig_crlf=\"$(printf \"%s\" \"$sig\" | append_cr; echo dummy)\"\n sig_crlf=${sig_crlf%dummy}\n test_atom refs/tags/fake-sig-crlf contents:signature \"$sig_crlf\"\n \n+GRADE_FORMAT=\"%(signature:grade)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)\"\n+TRUSTLEVEL_FORMAT=\"%(signature:trustlevel)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)\"\n+\n+test_expect_success GPG 'test bare signature atom' '\n+\tgit checkout -b signed &&\n+\techo 1 >file && git add file &&\n+\ttest_tick && git commit -S -m initial &&\n+\tgit verify-commit signed 2>out_orig &&\n+\tgrep -v \"checking the trustdb\" out_orig >out &&\n+\thead -3 out >expected &&\n+\ttail -1 out >>expected &&\n+\techo >>expected &&\n+\tgit for-each-ref refs/heads/signed --format=\"%(signature)\" >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success GPG 'show good signature with custom format' '\n+\techo 2 >file && git add file &&\n+\ttest_tick && git commit -S -m initial &&\n+\tgit verify-commit signed 2>out &&\n+\tcat >expect <<-\\EOF &&\n+\tG\n+\t13B6F51ECDDE430D\n+\tC O Mitter \n+\t73D758744BE721698EC54E8713B6F51ECDDE430D\n+\t73D758744BE721698EC54E8713B6F51ECDDE430D\n+\tEOF\n+\tgit for-each-ref refs/heads/signed --format=\"$GRADE_FORMAT\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'test signature atom with grade option and bad signature' '\n+\tgit config commit.gpgsign true &&\n+\techo 3 >file && test_tick && git commit -a -m \"third\" --no-gpg-sign &&\n+\tgit tag third-unsigned &&\n+\n+\ttest_tick && git rebase -f HEAD^^ && git tag second-signed HEAD^ &&\n+\tgit tag third-signed &&\n+\n+\tgit cat-file commit third-signed >raw &&\n+\tsed -e \"s/^third/3rd forged/\" raw >forged1 &&\n+\tFORGED1=$(git hash-object -w -t commit forged1) &&\n+\tgit update-ref refs/tags/third-signed \"$FORGED1\" &&\n+\ttest_must_fail git verify-commit \"$FORGED1\" &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tB\n+\t13B6F51ECDDE430D\n+\tC O Mitter \n+\n+\n+\tEOF\n+\tgit for-each-ref refs/tags/third-signed --format=\"$GRADE_FORMAT\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'show untrusted signature with custom format' '\n+\techo 4 >file && test_tick && git commit -a -m fourth -SB7227189 &&\n+\tgit tag signed-fourth &&\n+\tcat >expect <<-\\EOF &&\n+\tU\n+\t65A0EEA02E30CAD7\n+\tEris Discordia \n+\tF8364A59E07FFE9F4D63005A65A0EEA02E30CAD7\n+\tD4BE22311AD3131E5EDA29A461092E85B7227189\n+\tEOF\n+\tgit for-each-ref refs/tags/signed-fourth --format=\"$GRADE_FORMAT\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'show untrusted signature with undefined trust level' '\n+\techo 5 >file && test_tick && git commit -a -m fifth -SB7227189 &&\n+\tgit tag fifth-signed &&\n+\tcat >expect <<-\\EOF &&\n+\tundefined\n+\t65A0EEA02E30CAD7\n+\tEris Discordia \n+\tF8364A59E07FFE9F4D63005A65A0EEA02E30CAD7\n+\tD4BE22311AD3131E5EDA29A461092E85B7227189\n+\tEOF\n+\tgit for-each-ref refs/tags/fifth-signed --format=\"$TRUSTLEVEL_FORMAT\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'show untrusted signature with ultimate trust level' '\n+\techo 7 >file && test_tick && git commit -a -m \"seventh\" --no-gpg-sign &&\n+\tgit tag seventh-unsigned &&\n+\n+\ttest_tick && git rebase -f HEAD^^ && git tag sixth-signed HEAD^ &&\n+\tgit tag seventh-signed &&\n+\tcat >expect <<-\\EOF &&\n+\tultimate\n+\t13B6F51ECDDE430D\n+\tC O Mitter \n+\t73D758744BE721698EC54E8713B6F51ECDDE430D\n+\t73D758744BE721698EC54E8713B6F51ECDDE430D\n+\tEOF\n+\tgit for-each-ref refs/tags/seventh-signed --format=\"$TRUSTLEVEL_FORMAT\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'show unknown signature with custom format' '\n+\tcat >expect <<-\\EOF &&\n+\tE\n+\t65A0EEA02E30CAD7\n+\n+\n+\n+\tEOF\n+\tGNUPGHOME=\"$GNUPGHOME_NOT_USED\" git for-each-ref refs/tags/fifth-signed --format=\"$GRADE_FORMAT\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'show lack of signature with custom format' '\n+\techo 8 >file && test_tick && git commit -a -m \"eigth unsigned\" --no-gpg-sign &&\n+\tgit tag eigth-unsigned &&\n+\tcat >expect <<-\\EOF &&\n+\tN\n+\n+\n+\n+\n+\tEOF\n+\tgit for-each-ref refs/tags/eigth-unsigned --format=\"$GRADE_FORMAT\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.39.GIT\n\n"},{"id":"473510","messageId":"xmqqpm9bosjw.fsf@gitster.g","threadId":"59006","inReplyTo":"20230311210607.64927-2-nsengiyumvawilberforce@gmail.com","subject":"Re: [PATCH v5 1/1] ref-filter: add new \"signature\" atom","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-14T22:51:31Z","receivedAt":"2023-03-14T22:51:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nsengiyumva Wilberforce writes:\n\n> Co-authored-by: Hariom Verma \n> Co-authored-by: Jaydeep Das \n> Mentored-by: Christian Couder \n> Mentored-by: Hariom Verma \n> Signed-off-by: Nsengiyumva Wilberforce \n\nhttps://github.com/git/git/actions/runs/4410308954/jobs/7727652857#step:6:1898\n\n + git for-each-ref refs/heads/signed --format=%(signature)\n + test_cmp expected actual\n + test 2 -ne 2\n + eval diff -u \"$@\"\n + diff -u expected actual\n --- expected2023-03-13 23:08:51.495987174 +0000\n +++ actual2023-03-13 23:08:51.519986953 +0000\n @@ -1,5 +1,4 @@\n gpg: Signature made Mon Mar 13 23:08:51 2023 UTC\n gpg: using DSA key 13B6F51ECDDE430D\n -gpg: 3 marginal(s) needed, 1 complete(s) needed, PGP trust model\n gpg: Good signature from \"C O Mitter \"\n \n error: last command exited with $?=1\n not ok 342 - test bare signature atom\n + git for-each-ref refs/heads/signed --format=%(signature)\n + test_cmp expected actual\n + test 2 -ne 2\n + eval diff -u \"$@\"\n + diff -u expected actual\n --- expected\n +++ actual\n @@ -1,5 +1,4 @@\n gpg: Signature made Mon Mar 13 23:08:51 2023 UTC\n gpg: using DSA key 13B6F51ECDDE430D\n -gpg: 3 marginal(s) needed, 1 complete(s) needed, PGP trust model\n gpg: Good signature from \"C O Mitter \"\n \n error: last command exited with $?=1\n not ok 342 - test bare signature atom\n\nAre there differences between versions of GPG that, for the purpose\nof the testing, we do not care?\n"},{"id":"476281","messageId":"20230428182925.14975-1-five231003@gmail.com","threadId":"59006","inReplyTo":"20230311210607.64927-2-nsengiyumvawilberforce@gmail.com","subject":"Re: [PATCH v5 1/1] ref-filter: add new \"signature\" atom","fromName":"Kousik Sanagavarapu","fromEmail":"five231003@gmail.com","sentAt":"2023-04-28T18:29:25Z","receivedAt":"2023-04-28T18:29:35Z","isPatch":true,"sender":{"key":"five231003@gmail.com","avatar":"https://avatars.githubusercontent.com/u/75560439?v=4"},"body":"Nsengiyumva Wilberforce writes:\n\n> +test_expect_success GPG 'test bare signature atom' '\n\nI think this test is failing on CI because, as Junio said, there\nare different versions of gpg coming into play here. In particular,\nthis test is failing on (according to the logs) linux32 (daald/ubuntu32:xenial).\nThe version of GPG that xenial can use is at a maximum v1.4.20 (this is\nevident here https://packages.ubuntu.com/xenial/allpackages). But\naccording the code in lib-gpg.sh, we should be able to handle any GPG\nversion, except for v1.0.6.\n\nLooking at the logs that Junio posted, I think that the cuplrit is\n\n> +\tgrep -v \"checking the trustdb\" out_orig >out &&\n> +\thead -3 out >expected &&\n> +\ttail -1 out >>expected &&\n\nbut I'm not really sure. So I would really be grateful if you could\nexplain this hack to me, the one you mentioned in the cover-letter.\n\nThanks\n"},{"id":"476306","messageId":"20230429183736.3337-1-five231003@gmail.com","threadId":"59006","inReplyTo":"20230428182925.14975-1-five231003@gmail.com","subject":"Re: [PATCH v5 1/1] ref-filter: add new \"signature\" atom","fromName":"Kousik Sanagavarapu","fromEmail":"five231003@gmail.com","sentAt":"2023-04-29T18:37:36Z","receivedAt":"2023-04-29T18:37:46Z","isPatch":true,"sender":{"key":"five231003@gmail.com","avatar":"https://avatars.githubusercontent.com/u/75560439?v=4"},"body":"Kousik Sanagavarapu writes:\n\n>> +test_expect_success GPG 'test bare signature atom' '\n>\n> I think this test is failing on CI because, as Junio said, there\n> are different versions of gpg coming into play here. In particular,\n> this test is failing on (according to the logs) linux32 (daald/ubuntu32:xenial).\n> The version of GPG that xenial can use is at a maximum v1.4.20 (this is\n> evident here https://packages.ubuntu.com/xenial/allpackages). But\n> according the code in lib-gpg.sh, we should be able to handle any GPG\n> version, except for v1.0.6.\n\nAfter digging a little bit more into this, I found that when checking\nthe trustdb, the output is different for GPG v1 and GPG v2. So, I\nunderstand now why you did this\n\n>> +\tgrep -v \"checking the trustdb\" out_orig >out &&\n\nThe test is still failing because this is not the only line in the\noutput that is different, but also the line following it, which\nconveys information about the key trust levels\n\n gpg: 3 marginal(s) needed, 1 complete(s) needed, PGP trust model\n\n\nThis is also evident from the diff between \"expected\" and \"actual\" in\nthe logs[1].\n\n[1] https://github.com/git/git/actions/runs/4410308954/jobs/7727652857#step:6:1898\n\nThanks\n"}]}