Git Mailing List Archive mirror
 help / color / mirror / Atom feed
* [GSoC][PATCH 0/2] Add new "signature" atom
@ 2023-05-29 18:32 Kousik Sanagavarapu
  2023-05-29 18:32 ` [PATCH 1/2] t/lib-gpg: introduce new prereq GPG2 Kousik Sanagavarapu
                   ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: Kousik Sanagavarapu @ 2023-05-29 18:32 UTC (permalink / raw)
  To: git; +Cc: Kousik Sanagavarapu

Hi,
This series duplicates the code for signature related formats from
pretty to ref-filter, which is a step in the process of duplicating all
the pretty formats which are not present in ref-filter, with the end
goal of a single formatting interface for git in mind.

PATCH 1/2 introduces a new prereq GPG2 for the purpose of testing stuff
that breaks with GPG version <= v2. This is evident from the CI failure
in [1] that was sent with the same subject as this series (more info
below).

PATCH 2/2 adds a new "signature" atom, which is a duplication of the %G*
formats in pretty. This was sent before by Nsengiyumva Wilberforce and
was in "seen" until May and was removed because of CI failure. I have
built upon it and have done some minor changes to it. The CI jobs are
now successful which can be found at [2].

[1]: Patch

	https://lore.kernel.org/git/20230311210607.64927-1-nsengiyumvawilberforce@gmail.com/

     Junio's email about CI failure

	https://lore.kernel.org/git/xmqqpm9bosjw.fsf@gitster.g/

[2]: https://github.com/five-sh/git/actions/runs/5114306975/workflow

Here is the range-diff, compared to Nsengiyumva's final version posted
to the mailing list

-:  ---------- > 1:  5c97d11b79 t/lib-gpg: introduce new prereq GPG2
1:  8a49102b1f ! 2:  e89f14283d ref-filter: add new "signature" atom
    @@
      ## Metadata ##
    -Author: Nsengiyumva Wilberforce <nsengiyumvawilberforce@gmail.com>
    +Author: Kousik Sanagavarapu <five231003@gmail.com>
     
      ## Commit message ##
         ref-filter: add new "signature" atom
     
    -    This commit duplicates the code for `signature` atom from
         pretty.c
    -    to ref-filter.c. This feature will help to get rid of current
         duplicate
    -    implementation of `signature` atom when unifying
         implementations by
    -    using ref-filter logic everywhere when ref-filter can do
         everything
    -    pretty is doing.
    +    Duplicate the code for outputting the signature and it's other
    +    parameters for commits and tags in ref-filter from pretty. In
the
    +    future, this will help in getting rid of the current duplicate
    +    implementations of such logic everywhere, when ref-filter can
do
    +    everything that pretty is doing.
     
    -    Add "signature" atom with `grade`, `signer`, `key`,
    -    `fingerprint`, `primarykeyfingerprint`, `trustlevel` as
         arguments.
    -    This code and its documentation are inspired by how the %GG,
         %G?,
    -    %GS, %GK, %GF, %GP, and %GT pretty formats were implemented.
    +    The new atom "signature" and it's friends are equivalent to the
existing
    +    pretty formats as follows:
    +
    +            %(signature) = %GG
    +            %(signature:grade) = %G?
    +            %(siganture:signer) = %GS
    +            %(signature:key) = %GK
    +            %(signature:fingerprint) = %GF
    +            %(signature:primarykeyfingerprint) = %GP
    +            %(signature:trustlevel) = %GT
     
         Co-authored-by: Hariom Verma <hariom18599@gmail.com>
         Co-authored-by: Jaydeep Das <jaydeepjd.8914@gmail.com>
    -    Mentored-by: Christian Couder <chriscool@tuxfamily.org>
    +    Co-authored-by: Nsengiyumva Wilberforce
<nsengiyumvawilberforce@gmail.com>
    +    Mentored-by: Christian Couder <christian.couder@gmail.com>
         Mentored-by: Hariom Verma <hariom18599@gmail.com>
    -    Signed-off-by: Nsengiyumva Wilberforce
         <nsengiyumvawilberforce@gmail.com>
    +    Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
     
      ## Documentation/git-for-each-ref.txt ##
     @@ Documentation/git-for-each-ref.txt: symref::
    @@ ref-filter.c: static int subject_atom_parser(struct ref_format
*format UNUSED,
     +  return -1;
     +}
     +
    -+static int signature_atom_parser(struct ref_format *format UNUSED,
struct used_atom *atom,
    -+                         const char *arg, struct strbuf *err){
    ++static int signature_atom_parser(struct ref_format *format UNUSED,
    ++                           struct used_atom *atom,
    ++                           const char *arg, struct strbuf *err)
    ++{
     +  int opt = parse_signature_option(arg);
     +  if (opt < 0)
     +          return err_bad_arg(err, "signature", arg);
    @@ t/t6300-for-each-ref.sh: test_expect_success 'git for-each-ref
with non-existing
     +GRADE_FORMAT="%(signature:grade)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)"
     +TRUSTLEVEL_FORMAT="%(signature:trustlevel)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)"
     +
    -+test_expect_success GPG 'test bare signature atom' '
    ++test_expect_success GPG 'setup: signature gpg' '
     +  git checkout -b signed &&
    -+  echo 1 >file && git add file &&
    -+  test_tick && git commit -S -m initial &&
    -+  git verify-commit signed 2>out_orig &&
    -+  grep -v "checking the trustdb" out_orig >out &&
    -+  head -3 out >expected &&
    -+  tail -1 out >>expected &&
    -+  echo >>expected &&
    -+  git for-each-ref refs/heads/signed --format="%(signature)"
>actual &&
    -+  test_cmp expected actual
    ++
    ++  test_when_finished "test_unconfig commit.gpgSign" &&
    ++
    ++  echo "1" >file &&
    ++  git add file &&
    ++  test_tick &&
    ++  git commit -S -m "file: 1" &&
    ++  git tag first-signed &&
    ++
    ++  echo "2" >file &&
    ++  test_tick &&
    ++  git commit -a -m "file: 2" &&
    ++  git tag second-unsigned &&
    ++
    ++  git config commit.gpgSign 1 &&
    ++  echo "3" >file &&
    ++  test_tick &&
    ++  git commit -a --no-gpg-sign -m "file: 3" &&
    ++  git tag third-unsigned &&
    ++
    ++  test_tick &&
    ++  git rebase -f HEAD^^ && git tag second-signed HEAD^ &&
    ++  git tag third-signed &&
    ++
    ++  echo "4" >file &&
    ++  test_tick &&
    ++  git commit -a -SB7227189 -m "file: 4" &&
    ++  git tag fourth-signed &&
    ++
    ++  echo "5" >file &&
    ++  test_tick &&
    ++  git commit -a --no-gpg-sign -m "file: 5" &&
    ++  git tag fifth-unsigned &&
    ++
    ++  echo "6" >file &&
    ++  test_tick &&
    ++  git commit -a --no-gpg-sign -m "file: 6" &&
    ++
    ++  test_tick &&
    ++  git rebase -f HEAD^^ &&
    ++  git tag fifth-signed HEAD^ &&
    ++  git tag sixth-signed &&
    ++
    ++  echo "7" >file &&
    ++  test_tick &&
    ++  git commit -a --no-gpg-sign -m "file: 7" &&
    ++  git tag seventh-unsigned
    ++'
    ++
    ++test_expect_success GPGSSH 'setup: signature ssh' '
    ++  test_config gpg.format ssh &&
    ++  test_config user.signingkey "${GPGSSH_KEY_PRIMARY}" &&
    ++  echo "8" >file &&
    ++  test_tick &&
    ++  git commit -a -S -m "file: 8" &&
    ++  git tag eighth-signed-ssh
    ++'
    ++
    ++test_expect_success GPG2 'bare signature atom' '
    ++  git verify-commit first-signed 2>out.raw &&
    ++  grep -Ev "checking the trustdb|PGP trust model" out.raw >out &&
    ++  head -3 out >expect &&
    ++  tail -1 out >>expect &&
    ++  echo "" >>expect &&
    ++  git for-each-ref refs/tags/first-signed \
    ++          --format="%(signature)" >actual &&
    ++  test_cmp expect actual
     +'
     +
     +test_expect_success GPG 'show good signature with custom format' '
    -+  echo 2 >file && git add file &&
    -+  test_tick && git commit -S -m initial &&
    -+  git verify-commit signed 2>out &&
    ++  git verify-commit first-signed &&
     +  cat >expect <<-\EOF &&
     +  G
     +  13B6F51ECDDE430D
    @@ t/t6300-for-each-ref.sh: test_expect_success 'git for-each-ref
with non-existing
     +  73D758744BE721698EC54E8713B6F51ECDDE430D
     +  73D758744BE721698EC54E8713B6F51ECDDE430D
     +  EOF
    -+  git for-each-ref refs/heads/signed --format="$GRADE_FORMAT"
>actual &&
    ++  git for-each-ref refs/tags/first-signed \
    ++          --format="$GRADE_FORMAT" >actual &&
     +  test_cmp expect actual
     +'
    ++test_expect_success GPGSSH 'show good signature with custom format
    ++                      with ssh' '
    ++  test_config gpg.ssh.allowedSignersFile
"${GPGSSH_ALLOWED_SIGNERS}" &&
    ++  FINGERPRINT=$(ssh-keygen -lf "${GPGSSH_KEY_PRIMARY}" | awk
"{print \$2;}") &&
    ++  cat >expect.tmpl <<-\EOF &&
    ++  G
    ++  FINGERPRINT
    ++  principal with number 1
    ++  FINGERPRINT
     +
    -+test_expect_success GPG 'test signature atom with grade option and
bad signature' '
    -+  git config commit.gpgsign true &&
    -+  echo 3 >file && test_tick && git commit -a -m "third"
--no-gpg-sign &&
    -+  git tag third-unsigned &&
    -+
    -+  test_tick && git rebase -f HEAD^^ && git tag second-signed HEAD^
&&
    -+  git tag third-signed &&
    ++  EOF
    ++  sed "s|FINGERPRINT|$FINGERPRINT|g" expect.tmpl >expect &&
    ++  git for-each-ref refs/tags/eighth-signed-ssh \
    ++          --format="$GRADE_FORMAT" >actual &&
    ++  test_cmp expect actual
    ++'
     +
    ++test_expect_success GPG 'signature atom with grade option and bad
signature' '
     +  git cat-file commit third-signed >raw &&
    -+  sed -e "s/^third/3rd forged/" raw >forged1 &&
    ++  sed -e "s/^file: 3/file: 3 forged/" raw >forged1 &&
     +  FORGED1=$(git hash-object -w -t commit forged1) &&
     +  git update-ref refs/tags/third-signed "$FORGED1" &&
     +  test_must_fail git verify-commit "$FORGED1" &&
    @@ t/t6300-for-each-ref.sh: test_expect_success 'git for-each-ref
with non-existing
     +
     +
     +  EOF
    -+  git for-each-ref refs/tags/third-signed --format="$GRADE_FORMAT"
>actual &&
    ++  git for-each-ref refs/tags/third-signed \
    ++          --format="$GRADE_FORMAT" >actual &&
     +  test_cmp expect actual
     +'
     +
     +test_expect_success GPG 'show untrusted signature with custom
format' '
    -+  echo 4 >file && test_tick && git commit -a -m fourth -SB7227189
&&
    -+  git tag signed-fourth &&
     +  cat >expect <<-\EOF &&
     +  U
     +  65A0EEA02E30CAD7
    @@ t/t6300-for-each-ref.sh: test_expect_success 'git for-each-ref
with non-existing
     +  F8364A59E07FFE9F4D63005A65A0EEA02E30CAD7
     +  D4BE22311AD3131E5EDA29A461092E85B7227189
     +  EOF
    -+  git for-each-ref refs/tags/signed-fourth
--format="$GRADE_FORMAT" >actual &&
    ++  git for-each-ref refs/tags/fourth-signed \
    ++          --format="$GRADE_FORMAT" >actual &&
     +  test_cmp expect actual
     +'
     +
     +test_expect_success GPG 'show untrusted signature with undefined
trust level' '
    -+  echo 5 >file && test_tick && git commit -a -m fifth -SB7227189
&&
    -+  git tag fifth-signed &&
     +  cat >expect <<-\EOF &&
     +  undefined
     +  65A0EEA02E30CAD7
    @@ t/t6300-for-each-ref.sh: test_expect_success 'git for-each-ref
with non-existing
     +  F8364A59E07FFE9F4D63005A65A0EEA02E30CAD7
     +  D4BE22311AD3131E5EDA29A461092E85B7227189
     +  EOF
    -+  git for-each-ref refs/tags/fifth-signed
--format="$TRUSTLEVEL_FORMAT" >actual &&
    ++  git for-each-ref refs/tags/fourth-signed \
    ++          --format="$TRUSTLEVEL_FORMAT" >actual &&
     +  test_cmp expect actual
     +'
     +
     +test_expect_success GPG 'show untrusted signature with ultimate
trust level' '
    -+  echo 7 >file && test_tick && git commit -a -m "seventh"
--no-gpg-sign &&
    -+  git tag seventh-unsigned &&
    -+
    -+  test_tick && git rebase -f HEAD^^ && git tag sixth-signed HEAD^
&&
    -+  git tag seventh-signed &&
     +  cat >expect <<-\EOF &&
     +  ultimate
     +  13B6F51ECDDE430D
    @@ t/t6300-for-each-ref.sh: test_expect_success 'git for-each-ref
with non-existing
     +  73D758744BE721698EC54E8713B6F51ECDDE430D
     +  73D758744BE721698EC54E8713B6F51ECDDE430D
     +  EOF
    -+  git for-each-ref refs/tags/seventh-signed
--format="$TRUSTLEVEL_FORMAT" >actual &&
    ++  git for-each-ref refs/tags/sixth-signed \
    ++          --format="$TRUSTLEVEL_FORMAT" >actual &&
     +  test_cmp expect actual
     +'
     +
     +test_expect_success GPG 'show unknown signature with custom
format' '
     +  cat >expect <<-\EOF &&
     +  E
    -+  65A0EEA02E30CAD7
    ++  13B6F51ECDDE430D
     +
     +
     +
     +  EOF
    -+  GNUPGHOME="$GNUPGHOME_NOT_USED" git for-each-ref
refs/tags/fifth-signed --format="$GRADE_FORMAT" >actual &&
    ++  GNUPGHOME="$GNUPGHOME_NOT_USED" git for-each-ref \
    ++          refs/tags/sixth-signed --format="$GRADE_FORMAT" >actual
&&
     +  test_cmp expect actual
     +'
     +
     +test_expect_success GPG 'show lack of signature with custom
format' '
    -+  echo 8 >file && test_tick && git commit -a -m "eigth unsigned"
--no-gpg-sign &&
    -+  git tag eigth-unsigned &&
     +  cat >expect <<-\EOF &&
     +  N
     +
    @@ t/t6300-for-each-ref.sh: test_expect_success 'git for-each-ref
with non-existing
     +
     +
     +  EOF
    -+  git for-each-ref refs/tags/eigth-unsigned
--format="$GRADE_FORMAT" >actual &&
    ++  git for-each-ref refs/tags/seventh-unsigned \
    ++          --format="$GRADE_FORMAT" >actual &&
     +  test_cmp expect actual
     +'
     + 

Kousik Sanagavarapu (2):
  t/lib-gpg: introduce new prereq GPG2
  ref-filter: add new "signature" atom

 Documentation/git-for-each-ref.txt |  27 +++++
 ref-filter.c                       | 111 ++++++++++++++++-
 t/lib-gpg.sh                       |  21 ++++
 t/t6300-for-each-ref.sh            | 189 +++++++++++++++++++++++++++++
 t/t7510-signed-commit.sh           |   7 ++
 5 files changed, 353 insertions(+), 2 deletions(-)

-- 
2.41.0.rc0


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/2] t/lib-gpg: introduce new prereq GPG2
  2023-05-29 18:32 [GSoC][PATCH 0/2] Add new "signature" atom Kousik Sanagavarapu
@ 2023-05-29 18:32 ` Kousik Sanagavarapu
  2023-06-01  8:39   ` Christian Couder
  2023-05-29 18:32 ` [PATCH 2/2] ref-filter: add new "signature" atom Kousik Sanagavarapu
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 19+ messages in thread
From: Kousik Sanagavarapu @ 2023-05-29 18:32 UTC (permalink / raw)
  To: git; +Cc: Kousik Sanagavarapu, Christian Couder, Hariom Verma

GnuPG v2.0.0 released in 2006, which according to its release notes

	https://gnupg.org/download/release_notes.html

is the "First stable version of GnuPG integrating OpenPGP and S/MIME".

Use this version or it's successors for tests that will fail for
versions less than v2.0.0 because of the difference in the output on
stderr between the versions (v2.* vs v0.* or v2.* vs v1.*). Skip if
the GPG version detected is less than v2.0.0.

Do not, however, remove the existing prereq GPG yet since a lot of tests
still work with the prereq GPG (that is even with versions v0.* or v1.*)
and some systems still use these versions.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
 t/lib-gpg.sh             | 21 +++++++++++++++++++++
 t/t7510-signed-commit.sh |  7 +++++++
 2 files changed, 28 insertions(+)

diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index 114785586a..4287ea8621 100644
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -51,6 +51,27 @@ test_lazy_prereq GPG '
 	esac
 '
 
+test_lazy_prereq GPG2 '
+	gpg_version=$(gpg --version 2>&1)
+	test $? != 127 || exit 1
+
+	case "$gpg_version" in
+	!"gpg (GnuPG) 2."*)
+		say "This test requires a GPG version >= v2.0.0"
+		exit 1
+		;;
+	*)
+		(gpgconf --kill all || : ) &&
+		gpg --homedir "${GNUPGHOME}" --import \
+			"$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&
+		gpg --homedir "${GNUPGHOME}" --import-ownertrust \
+			"$TEST_DIRECTORY"/lib-gpg/ownertrust &&
+		gpg --homedir "${GNUPGHOME}" </dev/null >/dev/null \
+			--sign -u committer@example.com
+		;;
+	esac
+'
+
 test_lazy_prereq GPGSM '
 	test_have_prereq GPG &&
 	# Available key info:
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index ccbc416402..96b316ae01 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -218,6 +218,13 @@ test_expect_success GPG 'amending already signed commit' '
 	! grep "BAD signature from" actual
 '
 
+test_expect_success GPG2 'bare signature' '
+	git verify-commit fifth-signed 2>expect &&
+	echo "" >>expect &&
+	git log -1 --format="%GG" fifth-signed >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success GPG 'show good signature with custom format' '
 	cat >expect <<-\EOF &&
 	G
-- 
2.41.0.rc0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/2] ref-filter: add new "signature" atom
  2023-05-29 18:32 [GSoC][PATCH 0/2] Add new "signature" atom Kousik Sanagavarapu
  2023-05-29 18:32 ` [PATCH 1/2] t/lib-gpg: introduce new prereq GPG2 Kousik Sanagavarapu
@ 2023-05-29 18:32 ` Kousik Sanagavarapu
  2023-06-01  8:58   ` Christian Couder
  2023-06-01  9:11 ` [GSoC][PATCH 0/2] Add " Christian Couder
  2023-06-02  2:11 ` [PATCH v2 " Kousik Sanagavarapu
  3 siblings, 1 reply; 19+ messages in thread
From: Kousik Sanagavarapu @ 2023-05-29 18:32 UTC (permalink / raw)
  To: git
  Cc: Kousik Sanagavarapu, Hariom Verma, Jaydeep Das,
	Nsengiyumva Wilberforce, Christian Couder

Duplicate the code for outputting the signature and it's other
parameters for commits and tags in ref-filter from pretty. In the
future, this will help in getting rid of the current duplicate
implementations of such logic everywhere, when ref-filter can do
everything that pretty is doing.

The new atom "signature" and it's friends are equivalent to the existing
pretty formats as follows:

	%(signature) = %GG
	%(signature:grade) = %G?
	%(siganture:signer) = %GS
	%(signature:key) = %GK
	%(signature:fingerprint) = %GF
	%(signature:primarykeyfingerprint) = %GP
	%(signature:trustlevel) = %GT

Co-authored-by: Hariom Verma <hariom18599@gmail.com>
Co-authored-by: Jaydeep Das <jaydeepjd.8914@gmail.com>
Co-authored-by: Nsengiyumva Wilberforce <nsengiyumvawilberforce@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
 Documentation/git-for-each-ref.txt |  27 +++++
 ref-filter.c                       | 111 ++++++++++++++++-
 t/t6300-for-each-ref.sh            | 189 +++++++++++++++++++++++++++++
 3 files changed, 325 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 1e215d4e73..2dbb95252f 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -221,6 +221,33 @@ symref::
 	`:lstrip` and `:rstrip` options in the same way as `refname`
 	above.
 
+signature::
+	The GPG signature of a commit.
+
+signature:grade::
+	Show "G" for a good (valid) signature, "B" for a bad
+	signature, "U" for a good signature with unknown validity, "X"
+	for a good signature that has expired, "Y" for a good
+	signature made by an expired key, "R" for a good signature
+	made by a revoked key, "E" if the signature cannot be
+	checked (e.g. missing key) and "N" for no signature.
+
+signature:signer::
+	The signer of the GPG signature of a commit.
+
+signature:key::
+	The key of the GPG signature of a commit.
+
+signature:fingerprint::
+	The fingerprint of the GPG signature of a commit.
+
+signature:primarykeyfingerprint::
+	The Primary Key fingerprint of the GPG signature of a commit.
+
+signature:trustlevel::
+	The Trust level of the GPG signature of a commit. Possible
+	outputs are `ultimate`, `fully`, `marginal`, `never` and `undefined`.
+
 worktreepath::
 	The absolute path to the worktree in which the ref is checked
 	out, if it is checked out in any linked worktree. Empty string
diff --git a/ref-filter.c b/ref-filter.c
index 4991cd4f7a..bbab2d9528 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -150,6 +150,7 @@ enum atom_type {
 	ATOM_BODY,
 	ATOM_TRAILERS,
 	ATOM_CONTENTS,
+	ATOM_SIGNATURE,
 	ATOM_RAW,
 	ATOM_UPSTREAM,
 	ATOM_PUSH,
@@ -215,6 +216,10 @@ static struct used_atom {
 		struct email_option {
 			enum { EO_RAW, EO_TRIM, EO_LOCALPART } option;
 		} email_option;
+		struct {
+			enum { S_BARE, S_GRADE, S_SIGNER, S_KEY,
+			       S_FINGERPRINT, S_PRI_KEY_FP, S_TRUST_LEVEL} option;
+		} signature;
 		struct refname_atom refname;
 		char *head;
 	} u;
@@ -407,8 +412,37 @@ static int subject_atom_parser(struct ref_format *format UNUSED,
 	return 0;
 }
 
-static int trailers_atom_parser(struct ref_format *format UNUSED,
-				struct used_atom *atom,
+static int parse_signature_option(const char *arg)
+{
+	if (!arg)
+		return S_BARE;
+	else if (!strcmp(arg, "signer"))
+		return S_SIGNER;
+	else if (!strcmp(arg, "grade"))
+		return S_GRADE;
+	else if (!strcmp(arg, "key"))
+		return S_KEY;
+	else if (!strcmp(arg, "fingerprint"))
+		return S_FINGERPRINT;
+	else if (!strcmp(arg, "primarykeyfingerprint"))
+		return S_PRI_KEY_FP;
+	else if (!strcmp(arg, "trustlevel"))
+		return S_TRUST_LEVEL;
+	return -1;
+}
+
+static int signature_atom_parser(struct ref_format *format UNUSED,
+				 struct used_atom *atom,
+				 const char *arg, struct strbuf *err)
+{
+	int opt = parse_signature_option(arg);
+	if (opt < 0)
+		return err_bad_arg(err, "signature", arg);
+	atom->u.signature.option = opt;
+	return 0;
+}
+
+static int trailers_atom_parser(struct ref_format *format, struct used_atom *atom,
 				const char *arg, struct strbuf *err)
 {
 	atom->u.contents.trailer_opts.no_divider = 1;
@@ -668,6 +702,7 @@ static struct {
 	[ATOM_BODY] = { "body", SOURCE_OBJ, FIELD_STR, body_atom_parser },
 	[ATOM_TRAILERS] = { "trailers", SOURCE_OBJ, FIELD_STR, trailers_atom_parser },
 	[ATOM_CONTENTS] = { "contents", SOURCE_OBJ, FIELD_STR, contents_atom_parser },
+	[ATOM_SIGNATURE] = { "signature", SOURCE_OBJ, FIELD_STR, signature_atom_parser },
 	[ATOM_RAW] = { "raw", SOURCE_OBJ, FIELD_STR, raw_atom_parser },
 	[ATOM_UPSTREAM] = { "upstream", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },
 	[ATOM_PUSH] = { "push", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },
@@ -1405,6 +1440,77 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void
 	}
 }
 
+static void grab_signature(struct atom_value *val, int deref, struct object *obj)
+{
+	int i;
+	struct commit *commit = (struct commit *) obj;
+	struct signature_check sigc = { 0 };
+	int signature_checked = 0;
+
+	for (i = 0; i < used_atom_cnt; i++) {
+		struct used_atom *atom = &used_atom[i];
+		const char *name = atom->name;
+		struct atom_value *v = &val[i];
+
+		if (!!deref != (*name == '*'))
+			continue;
+		if (deref)
+			name++;
+
+		if (!skip_prefix(name, "signature", &name) || (*name &&
+			*name != ':'))
+			continue;
+		if (!*name)
+			name = NULL;
+		else
+			name++;
+		if (parse_signature_option(name) < 0)
+			continue;
+
+		if (!signature_checked) {
+			check_commit_signature(commit, &sigc);
+			signature_checked = 1;
+		}
+
+		if (atom->u.signature.option == S_BARE)
+			v->s = xstrdup(sigc.output ? sigc.output: "");
+		else if (atom->u.signature.option == S_SIGNER)
+			v->s = xstrdup(sigc.signer ? sigc.signer : "");
+		else if (atom->u.signature.option == S_GRADE) {
+			switch (sigc.result) {
+			case 'G':
+				switch (sigc.trust_level) {
+				case TRUST_UNDEFINED:
+				case TRUST_NEVER:
+					v->s = xstrfmt("%c", (char)'U');
+					break;
+				default:
+					v->s = xstrfmt("%c", (char)'G');
+					break;
+				}
+				break;
+			case 'B':
+			case 'E':
+			case 'N':
+			case 'X':
+			case 'Y':
+			case 'R':
+				v->s = xstrfmt("%c", (char)sigc.result);
+			}
+		}
+		else if (atom->u.signature.option == S_KEY)
+			v->s = xstrdup(sigc.key ? sigc.key : "");
+		else if (atom->u.signature.option == S_FINGERPRINT)
+			v->s = xstrdup(sigc.fingerprint ? sigc.fingerprint : "");
+		else if (atom->u.signature.option == S_PRI_KEY_FP)
+			v->s = xstrdup(sigc.primary_key_fingerprint ? sigc.primary_key_fingerprint : "");
+		else if (atom->u.signature.option == S_TRUST_LEVEL)
+			v->s = xstrdup(gpg_trust_level_to_str(sigc.trust_level));
+	}
+	if (signature_checked)
+		signature_check_clear(&sigc);
+}
+
 static void find_subpos(const char *buf,
 			const char **sub, size_t *sublen,
 			const char **body, size_t *bodylen,
@@ -1598,6 +1704,7 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, s
 		grab_sub_body_contents(val, deref, data);
 		grab_person("author", val, deref, buf);
 		grab_person("committer", val, deref, buf);
+		grab_signature(val, deref, obj);
 		break;
 	case OBJ_TREE:
 		/* grab_tree_values(val, deref, obj, buf, sz); */
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 5c00607608..98237beac7 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -6,6 +6,7 @@
 test_description='for-each-ref test'
 
 . ./test-lib.sh
+GNUPGHOME_NOT_USED=$GNUPGHOME
 . "$TEST_DIRECTORY"/lib-gpg.sh
 . "$TEST_DIRECTORY"/lib-terminal.sh
 
@@ -1522,4 +1523,192 @@ test_expect_success 'git for-each-ref with non-existing refs' '
 	test_must_be_empty actual
 '
 
+GRADE_FORMAT="%(signature:grade)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)"
+TRUSTLEVEL_FORMAT="%(signature:trustlevel)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)"
+
+test_expect_success GPG 'setup: signature gpg' '
+	git checkout -b signed &&
+
+	test_when_finished "test_unconfig commit.gpgSign" &&
+
+	echo "1" >file &&
+	git add file &&
+	test_tick &&
+	git commit -S -m "file: 1" &&
+	git tag first-signed &&
+
+	echo "2" >file &&
+	test_tick &&
+	git commit -a -m "file: 2" &&
+	git tag second-unsigned &&
+
+	git config commit.gpgSign 1 &&
+	echo "3" >file &&
+	test_tick &&
+	git commit -a --no-gpg-sign -m "file: 3" &&
+	git tag third-unsigned &&
+
+	test_tick &&
+	git rebase -f HEAD^^ && git tag second-signed HEAD^ &&
+	git tag third-signed &&
+
+	echo "4" >file &&
+	test_tick &&
+	git commit -a -SB7227189 -m "file: 4" &&
+	git tag fourth-signed &&
+
+	echo "5" >file &&
+	test_tick &&
+	git commit -a --no-gpg-sign -m "file: 5" &&
+	git tag fifth-unsigned &&
+
+	echo "6" >file &&
+	test_tick &&
+	git commit -a --no-gpg-sign -m "file: 6" &&
+
+	test_tick &&
+	git rebase -f HEAD^^ &&
+	git tag fifth-signed HEAD^ &&
+	git tag sixth-signed &&
+
+	echo "7" >file &&
+	test_tick &&
+	git commit -a --no-gpg-sign -m "file: 7" &&
+	git tag seventh-unsigned
+'
+
+test_expect_success GPGSSH 'setup: signature ssh' '
+	test_config gpg.format ssh &&
+	test_config user.signingkey "${GPGSSH_KEY_PRIMARY}" &&
+	echo "8" >file &&
+	test_tick &&
+	git commit -a -S -m "file: 8" &&
+	git tag eighth-signed-ssh
+'
+
+test_expect_success GPG2 'bare signature atom' '
+	git verify-commit first-signed 2>out.raw &&
+	grep -Ev "checking the trustdb|PGP trust model" out.raw >out &&
+	head -3 out >expect &&
+	tail -1 out >>expect &&
+	echo "" >>expect &&
+	git for-each-ref refs/tags/first-signed \
+		--format="%(signature)" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show good signature with custom format' '
+	git verify-commit first-signed &&
+	cat >expect <<-\EOF &&
+	G
+	13B6F51ECDDE430D
+	C O Mitter <committer@example.com>
+	73D758744BE721698EC54E8713B6F51ECDDE430D
+	73D758744BE721698EC54E8713B6F51ECDDE430D
+	EOF
+	git for-each-ref refs/tags/first-signed \
+		--format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+test_expect_success GPGSSH 'show good signature with custom format
+			    with ssh' '
+	test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+	FINGERPRINT=$(ssh-keygen -lf "${GPGSSH_KEY_PRIMARY}" | awk "{print \$2;}") &&
+	cat >expect.tmpl <<-\EOF &&
+	G
+	FINGERPRINT
+	principal with number 1
+	FINGERPRINT
+
+	EOF
+	sed "s|FINGERPRINT|$FINGERPRINT|g" expect.tmpl >expect &&
+	git for-each-ref refs/tags/eighth-signed-ssh \
+		--format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'signature atom with grade option and bad signature' '
+	git cat-file commit third-signed >raw &&
+	sed -e "s/^file: 3/file: 3 forged/" raw >forged1 &&
+	FORGED1=$(git hash-object -w -t commit forged1) &&
+	git update-ref refs/tags/third-signed "$FORGED1" &&
+	test_must_fail git verify-commit "$FORGED1" &&
+
+	cat >expect <<-\EOF &&
+	B
+	13B6F51ECDDE430D
+	C O Mitter <committer@example.com>
+
+
+	EOF
+	git for-each-ref refs/tags/third-signed \
+		--format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show untrusted signature with custom format' '
+	cat >expect <<-\EOF &&
+	U
+	65A0EEA02E30CAD7
+	Eris Discordia <discord@example.net>
+	F8364A59E07FFE9F4D63005A65A0EEA02E30CAD7
+	D4BE22311AD3131E5EDA29A461092E85B7227189
+	EOF
+	git for-each-ref refs/tags/fourth-signed \
+		--format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show untrusted signature with undefined trust level' '
+	cat >expect <<-\EOF &&
+	undefined
+	65A0EEA02E30CAD7
+	Eris Discordia <discord@example.net>
+	F8364A59E07FFE9F4D63005A65A0EEA02E30CAD7
+	D4BE22311AD3131E5EDA29A461092E85B7227189
+	EOF
+	git for-each-ref refs/tags/fourth-signed \
+		--format="$TRUSTLEVEL_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show untrusted signature with ultimate trust level' '
+	cat >expect <<-\EOF &&
+	ultimate
+	13B6F51ECDDE430D
+	C O Mitter <committer@example.com>
+	73D758744BE721698EC54E8713B6F51ECDDE430D
+	73D758744BE721698EC54E8713B6F51ECDDE430D
+	EOF
+	git for-each-ref refs/tags/sixth-signed \
+		--format="$TRUSTLEVEL_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show unknown signature with custom format' '
+	cat >expect <<-\EOF &&
+	E
+	13B6F51ECDDE430D
+
+
+
+	EOF
+	GNUPGHOME="$GNUPGHOME_NOT_USED" git for-each-ref \
+		refs/tags/sixth-signed --format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show lack of signature with custom format' '
+	cat >expect <<-\EOF &&
+	N
+
+
+
+
+	EOF
+	git for-each-ref refs/tags/seventh-unsigned \
+		--format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.41.0.rc0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] t/lib-gpg: introduce new prereq GPG2
  2023-05-29 18:32 ` [PATCH 1/2] t/lib-gpg: introduce new prereq GPG2 Kousik Sanagavarapu
@ 2023-06-01  8:39   ` Christian Couder
  0 siblings, 0 replies; 19+ messages in thread
From: Christian Couder @ 2023-06-01  8:39 UTC (permalink / raw)
  To: Kousik Sanagavarapu; +Cc: git, Hariom Verma

On Mon, May 29, 2023 at 9:23 PM Kousik Sanagavarapu
<five231003@gmail.com> wrote:
>
> GnuPG v2.0.0 released in 2006, which according to its release notes
>
>         https://gnupg.org/download/release_notes.html
>
> is the "First stable version of GnuPG integrating OpenPGP and S/MIME".
>
> Use this version or it's successors for tests that will fail for
> versions less than v2.0.0 because of the difference in the output on
> stderr between the versions (v2.* vs v0.* or v2.* vs v1.*). Skip if
> the GPG version detected is less than v2.0.0.

Yeah, I think it's reasonable to stop worrying about the output of GPG
versions that are 17 year old.

> +test_lazy_prereq GPG2 '
> +       gpg_version=$(gpg --version 2>&1)
> +       test $? != 127 || exit 1
> +
> +       case "$gpg_version" in
> +       !"gpg (GnuPG) 2."*)

Maybe something like `"gpg (GnuPG) 0."* | "gpg (GnuPG) 1."*)` would be
better, as it would allow versions 3.X, 4.X, etc if they are ever
released.

> +               say "This test requires a GPG version >= v2.0.0"
> +               exit 1
> +               ;;

> diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
> index ccbc416402..96b316ae01 100755
> --- a/t/t7510-signed-commit.sh
> +++ b/t/t7510-signed-commit.sh
> @@ -218,6 +218,13 @@ test_expect_success GPG 'amending already signed commit' '
>         ! grep "BAD signature from" actual
>  '
>
> +test_expect_success GPG2 'bare signature' '
> +       git verify-commit fifth-signed 2>expect &&
> +       echo "" >>expect &&

We sometimes use `echo "" >` to add a new line, but we much more often
use just `echo >` for that purpose:

$ git grep 'echo >' | wc -l
339
$ git grep 'echo "" >' | wc -l
16

> +       git log -1 --format="%GG" fifth-signed >actual &&
> +       test_cmp expect actual
> +'

Thanks!

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] ref-filter: add new "signature" atom
  2023-05-29 18:32 ` [PATCH 2/2] ref-filter: add new "signature" atom Kousik Sanagavarapu
@ 2023-06-01  8:58   ` Christian Couder
  0 siblings, 0 replies; 19+ messages in thread
From: Christian Couder @ 2023-06-01  8:58 UTC (permalink / raw)
  To: Kousik Sanagavarapu
  Cc: git, Hariom Verma, Jaydeep Das, Nsengiyumva Wilberforce

On Mon, May 29, 2023 at 9:23 PM Kousik Sanagavarapu
<five231003@gmail.com> wrote:

> +test_expect_success GPG 'setup: signature gpg' '

Maybe something like "setup: sign some commits using gpg" or "setup
for signature atom using gpg" would be a bit clearer.

> +test_expect_success GPGSSH 'setup: signature ssh' '

Here also something like "setup: sign some commits using ssh" or
"setup for signature atom using ssh" would be a bit clearer.

> +       test_config gpg.format ssh &&
> +       test_config user.signingkey "${GPGSSH_KEY_PRIMARY}" &&

I wonder if this test should try to clean up the config a bit after
itself. The previous test used:

`test_when_finished "test_unconfig commit.gpgSign"`

and maybe this one could do something similar.

> +       echo "8" >file &&
> +       test_tick &&
> +       git commit -a -S -m "file: 8" &&
> +       git tag eighth-signed-ssh
> +'
> +
> +test_expect_success GPG2 'bare signature atom' '
> +       git verify-commit first-signed 2>out.raw &&
> +       grep -Ev "checking the trustdb|PGP trust model" out.raw >out &&
> +       head -3 out >expect &&
> +       tail -1 out >>expect &&
> +       echo "" >>expect &&

`echo >>expect` is more common to add a new line.

> +       git for-each-ref refs/tags/first-signed \
> +               --format="%(signature)" >actual &&
> +       test_cmp expect actual
> +'

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [GSoC][PATCH 0/2] Add new "signature" atom
  2023-05-29 18:32 [GSoC][PATCH 0/2] Add new "signature" atom Kousik Sanagavarapu
  2023-05-29 18:32 ` [PATCH 1/2] t/lib-gpg: introduce new prereq GPG2 Kousik Sanagavarapu
  2023-05-29 18:32 ` [PATCH 2/2] ref-filter: add new "signature" atom Kousik Sanagavarapu
@ 2023-06-01  9:11 ` Christian Couder
  2023-06-02  2:11 ` [PATCH v2 " Kousik Sanagavarapu
  3 siblings, 0 replies; 19+ messages in thread
From: Christian Couder @ 2023-06-01  9:11 UTC (permalink / raw)
  To: Kousik Sanagavarapu; +Cc: git

On Mon, May 29, 2023 at 9:50 PM Kousik Sanagavarapu
<five231003@gmail.com> wrote:

> PATCH 1/2 introduces a new prereq GPG2 for the purpose of testing stuff
> that breaks with GPG version <= v2. This is evident from the CI failure
> in [1] that was sent with the same subject as this series (more info
> below).
>
> PATCH 2/2 adds a new "signature" atom, which is a duplication of the %G*
> formats in pretty. This was sent before by Nsengiyumva Wilberforce and
> was in "seen" until May and was removed because of CI failure. I have
> built upon it and have done some minor changes to it. The CI jobs are
> now successful which can be found at [2].
>
> [1]: Patch
>
>         https://lore.kernel.org/git/20230311210607.64927-1-nsengiyumvawilberforce@gmail.com/

I don't think it's a big issue, but, as there are only minor change,
it might have been easier to refer to the previous work by just
calling this a v6 of the same patch series and using the
--in-reply-to=<message id> option of git format-patch to send it so it
would be in the same email thread as the previous work.

Now that a new series was started though, I think it makes more sense
for any improvement to this series to just be called v2 and be sent
using --in-reply-to=<message id> so that it is in the same thread as
this series.

Thanks!

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 0/2] Add new "signature" atom
  2023-05-29 18:32 [GSoC][PATCH 0/2] Add new "signature" atom Kousik Sanagavarapu
                   ` (2 preceding siblings ...)
  2023-06-01  9:11 ` [GSoC][PATCH 0/2] Add " Christian Couder
@ 2023-06-02  2:11 ` Kousik Sanagavarapu
  2023-06-02  2:11   ` [PATCH v2 1/2] t/lib-gpg: introduce new prereq GPG2 Kousik Sanagavarapu
                     ` (3 more replies)
  3 siblings, 4 replies; 19+ messages in thread
From: Kousik Sanagavarapu @ 2023-06-02  2:11 UTC (permalink / raw)
  To: git; +Cc: Kousik Sanagavarapu

Hi,

Thanks for the review.

Changes since v1:

    PATCH 1/2 -
	Changed the condition so that prereq GPG2 will only fail
	if we have GPG v0.* or v1.* instead of failing when we
	don't have v2.* (this will have an effect if in the future
	GPG v3.*, v4.* were introduced).

    PATCH 2/2 -
	Renamed the setup tests to be more clear about their purpose.

    Common to both the patches is the change where we introduce a
    newline to a file. Use "echo >" instead of "echo "" >".

I have also rebased this to be on top of v2.41.0, the previous version
was on top of v2.41.0-rc0.

Range-diff against v1:

1:  5c97d11b79 ! 1:  87465ef1a8 t/lib-gpg: introduce new prereq GPG2
    @@ t/lib-gpg.sh: test_lazy_prereq GPG '
     +  test $? != 127 || exit 1
     +
     +  case "$gpg_version" in
    -+  !"gpg (GnuPG) 2."*)
    ++  "gpg (GnuPG) 0."* | "gpg (GnuPG) 1.*")
     +          say "This test requires a GPG version >= v2.0.0"
     +          exit 1
     +          ;;
    @@ t/t7510-signed-commit.sh: test_expect_success GPG 'amending
already signed commi
      
     +test_expect_success GPG2 'bare signature' '
     +  git verify-commit fifth-signed 2>expect &&
    -+  echo "" >>expect &&
    ++  echo >>expect &&
     +  git log -1 --format="%GG" fifth-signed >actual &&
     +  test_cmp expect actual
     +'
2:  e89f14283d ! 2:  690869aa47 ref-filter: add new "signature" atom
    @@ t/t6300-for-each-ref.sh: test_expect_success 'git for-each-ref
with non-existing
     +GRADE_FORMAT="%(signature:grade)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)"
     +TRUSTLEVEL_FORMAT="%(signature:trustlevel)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)"
     +
    -+test_expect_success GPG 'setup: signature gpg' '
    ++test_expect_success GPG 'setup for signature atom using gpg' '
     +  git checkout -b signed &&
     +
     +  test_when_finished "test_unconfig commit.gpgSign" &&
    @@ t/t6300-for-each-ref.sh: test_expect_success 'git for-each-ref
with non-existing
     +  git tag seventh-unsigned
     +'
     +
    -+test_expect_success GPGSSH 'setup: signature ssh' '
    ++test_expect_success GPGSSH 'setup for signature atom using ssh' '
    ++  test_when_finished "test_unconfig gpg.format user.signingkey" &&
    ++
     +  test_config gpg.format ssh &&
     +  test_config user.signingkey "${GPGSSH_KEY_PRIMARY}" &&
     +  echo "8" >file &&
    @@ t/t6300-for-each-ref.sh: test_expect_success 'git for-each-ref
with non-existing
     +  grep -Ev "checking the trustdb|PGP trust model" out.raw >out &&
     +  head -3 out >expect &&
     +  tail -1 out >>expect &&
    -+  echo "" >>expect &&
    ++  echo  >>expect &&
     +  git for-each-ref refs/tags/first-signed \
     +          --format="%(signature)" >actual &&
     +  test_cmp expect actual

Kousik Sanagavarapu (2):
  t/lib-gpg: introduce new prereq GPG2
  ref-filter: add new "signature" atom

 Documentation/git-for-each-ref.txt |  27 ++++
 ref-filter.c                       | 111 ++++++++++++++++-
 t/lib-gpg.sh                       |  21 ++++
 t/t6300-for-each-ref.sh            | 191 +++++++++++++++++++++++++++++
 t/t7510-signed-commit.sh           |   7 ++
 5 files changed, 355 insertions(+), 2 deletions(-)

-- 
2.41.0


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 1/2] t/lib-gpg: introduce new prereq GPG2
  2023-06-02  2:11 ` [PATCH v2 " Kousik Sanagavarapu
@ 2023-06-02  2:11   ` Kousik Sanagavarapu
  2023-06-02  6:50     ` Christian Couder
  2023-06-02  2:11   ` [PATCH v2 2/2] ref-filter: add new "signature" atom Kousik Sanagavarapu
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 19+ messages in thread
From: Kousik Sanagavarapu @ 2023-06-02  2:11 UTC (permalink / raw)
  To: git; +Cc: Kousik Sanagavarapu, Christian Couder, Hariom Verma

GnuPG v2.0.0 released in 2006, which according to its release notes

	https://gnupg.org/download/release_notes.html

is the "First stable version of GnuPG integrating OpenPGP and S/MIME".

Use this version or it's successors for tests that will fail for
versions less than v2.0.0 because of the difference in the output on
stderr between the versions (v2.* vs v0.* or v2.* vs v1.*). Skip if
the GPG version detected is less than v2.0.0.

Do not, however, remove the existing prereq GPG yet since a lot of tests
still work with the prereq GPG (that is even with versions v0.* or v1.*)
and some systems still use these versions.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
 t/lib-gpg.sh             | 21 +++++++++++++++++++++
 t/t7510-signed-commit.sh |  7 +++++++
 2 files changed, 28 insertions(+)

diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index 114785586a..aba8f861ed 100644
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -51,6 +51,27 @@ test_lazy_prereq GPG '
 	esac
 '
 
+test_lazy_prereq GPG2 '
+	gpg_version=$(gpg --version 2>&1)
+	test $? != 127 || exit 1
+
+	case "$gpg_version" in
+	"gpg (GnuPG) 0."* | "gpg (GnuPG) 1.*")
+		say "This test requires a GPG version >= v2.0.0"
+		exit 1
+		;;
+	*)
+		(gpgconf --kill all || : ) &&
+		gpg --homedir "${GNUPGHOME}" --import \
+			"$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&
+		gpg --homedir "${GNUPGHOME}" --import-ownertrust \
+			"$TEST_DIRECTORY"/lib-gpg/ownertrust &&
+		gpg --homedir "${GNUPGHOME}" </dev/null >/dev/null \
+			--sign -u committer@example.com
+		;;
+	esac
+'
+
 test_lazy_prereq GPGSM '
 	test_have_prereq GPG &&
 	# Available key info:
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index ccbc416402..0d2dd29fe6 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -218,6 +218,13 @@ test_expect_success GPG 'amending already signed commit' '
 	! grep "BAD signature from" actual
 '
 
+test_expect_success GPG2 'bare signature' '
+	git verify-commit fifth-signed 2>expect &&
+	echo >>expect &&
+	git log -1 --format="%GG" fifth-signed >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success GPG 'show good signature with custom format' '
 	cat >expect <<-\EOF &&
 	G
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 2/2] ref-filter: add new "signature" atom
  2023-06-02  2:11 ` [PATCH v2 " Kousik Sanagavarapu
  2023-06-02  2:11   ` [PATCH v2 1/2] t/lib-gpg: introduce new prereq GPG2 Kousik Sanagavarapu
@ 2023-06-02  2:11   ` Kousik Sanagavarapu
  2023-06-02  8:23     ` Oswald Buddenhagen
  2023-06-02  7:29   ` [PATCH v2 0/2] Add " Junio C Hamano
  2023-06-04 18:22   ` [PATCH v3 " Kousik Sanagavarapu
  3 siblings, 1 reply; 19+ messages in thread
From: Kousik Sanagavarapu @ 2023-06-02  2:11 UTC (permalink / raw)
  To: git
  Cc: Kousik Sanagavarapu, Hariom Verma, Jaydeep Das,
	Nsengiyumva Wilberforce, Christian Couder

Duplicate the code for outputting the signature and it's other
parameters for commits and tags in ref-filter from pretty. In the
future, this will help in getting rid of the current duplicate
implementations of such logic everywhere, when ref-filter can do
everything that pretty is doing.

The new atom "signature" and it's friends are equivalent to the existing
pretty formats as follows:

	%(signature) = %GG
	%(signature:grade) = %G?
	%(siganture:signer) = %GS
	%(signature:key) = %GK
	%(signature:fingerprint) = %GF
	%(signature:primarykeyfingerprint) = %GP
	%(signature:trustlevel) = %GT

Co-authored-by: Hariom Verma <hariom18599@gmail.com>
Co-authored-by: Jaydeep Das <jaydeepjd.8914@gmail.com>
Co-authored-by: Nsengiyumva Wilberforce <nsengiyumvawilberforce@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
 Documentation/git-for-each-ref.txt |  27 ++++
 ref-filter.c                       | 111 ++++++++++++++++-
 t/t6300-for-each-ref.sh            | 191 +++++++++++++++++++++++++++++
 3 files changed, 327 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 1e215d4e73..2dbb95252f 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -221,6 +221,33 @@ symref::
 	`:lstrip` and `:rstrip` options in the same way as `refname`
 	above.
 
+signature::
+	The GPG signature of a commit.
+
+signature:grade::
+	Show "G" for a good (valid) signature, "B" for a bad
+	signature, "U" for a good signature with unknown validity, "X"
+	for a good signature that has expired, "Y" for a good
+	signature made by an expired key, "R" for a good signature
+	made by a revoked key, "E" if the signature cannot be
+	checked (e.g. missing key) and "N" for no signature.
+
+signature:signer::
+	The signer of the GPG signature of a commit.
+
+signature:key::
+	The key of the GPG signature of a commit.
+
+signature:fingerprint::
+	The fingerprint of the GPG signature of a commit.
+
+signature:primarykeyfingerprint::
+	The Primary Key fingerprint of the GPG signature of a commit.
+
+signature:trustlevel::
+	The Trust level of the GPG signature of a commit. Possible
+	outputs are `ultimate`, `fully`, `marginal`, `never` and `undefined`.
+
 worktreepath::
 	The absolute path to the worktree in which the ref is checked
 	out, if it is checked out in any linked worktree. Empty string
diff --git a/ref-filter.c b/ref-filter.c
index 4991cd4f7a..bbab2d9528 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -150,6 +150,7 @@ enum atom_type {
 	ATOM_BODY,
 	ATOM_TRAILERS,
 	ATOM_CONTENTS,
+	ATOM_SIGNATURE,
 	ATOM_RAW,
 	ATOM_UPSTREAM,
 	ATOM_PUSH,
@@ -215,6 +216,10 @@ static struct used_atom {
 		struct email_option {
 			enum { EO_RAW, EO_TRIM, EO_LOCALPART } option;
 		} email_option;
+		struct {
+			enum { S_BARE, S_GRADE, S_SIGNER, S_KEY,
+			       S_FINGERPRINT, S_PRI_KEY_FP, S_TRUST_LEVEL} option;
+		} signature;
 		struct refname_atom refname;
 		char *head;
 	} u;
@@ -407,8 +412,37 @@ static int subject_atom_parser(struct ref_format *format UNUSED,
 	return 0;
 }
 
-static int trailers_atom_parser(struct ref_format *format UNUSED,
-				struct used_atom *atom,
+static int parse_signature_option(const char *arg)
+{
+	if (!arg)
+		return S_BARE;
+	else if (!strcmp(arg, "signer"))
+		return S_SIGNER;
+	else if (!strcmp(arg, "grade"))
+		return S_GRADE;
+	else if (!strcmp(arg, "key"))
+		return S_KEY;
+	else if (!strcmp(arg, "fingerprint"))
+		return S_FINGERPRINT;
+	else if (!strcmp(arg, "primarykeyfingerprint"))
+		return S_PRI_KEY_FP;
+	else if (!strcmp(arg, "trustlevel"))
+		return S_TRUST_LEVEL;
+	return -1;
+}
+
+static int signature_atom_parser(struct ref_format *format UNUSED,
+				 struct used_atom *atom,
+				 const char *arg, struct strbuf *err)
+{
+	int opt = parse_signature_option(arg);
+	if (opt < 0)
+		return err_bad_arg(err, "signature", arg);
+	atom->u.signature.option = opt;
+	return 0;
+}
+
+static int trailers_atom_parser(struct ref_format *format, struct used_atom *atom,
 				const char *arg, struct strbuf *err)
 {
 	atom->u.contents.trailer_opts.no_divider = 1;
@@ -668,6 +702,7 @@ static struct {
 	[ATOM_BODY] = { "body", SOURCE_OBJ, FIELD_STR, body_atom_parser },
 	[ATOM_TRAILERS] = { "trailers", SOURCE_OBJ, FIELD_STR, trailers_atom_parser },
 	[ATOM_CONTENTS] = { "contents", SOURCE_OBJ, FIELD_STR, contents_atom_parser },
+	[ATOM_SIGNATURE] = { "signature", SOURCE_OBJ, FIELD_STR, signature_atom_parser },
 	[ATOM_RAW] = { "raw", SOURCE_OBJ, FIELD_STR, raw_atom_parser },
 	[ATOM_UPSTREAM] = { "upstream", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },
 	[ATOM_PUSH] = { "push", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },
@@ -1405,6 +1440,77 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void
 	}
 }
 
+static void grab_signature(struct atom_value *val, int deref, struct object *obj)
+{
+	int i;
+	struct commit *commit = (struct commit *) obj;
+	struct signature_check sigc = { 0 };
+	int signature_checked = 0;
+
+	for (i = 0; i < used_atom_cnt; i++) {
+		struct used_atom *atom = &used_atom[i];
+		const char *name = atom->name;
+		struct atom_value *v = &val[i];
+
+		if (!!deref != (*name == '*'))
+			continue;
+		if (deref)
+			name++;
+
+		if (!skip_prefix(name, "signature", &name) || (*name &&
+			*name != ':'))
+			continue;
+		if (!*name)
+			name = NULL;
+		else
+			name++;
+		if (parse_signature_option(name) < 0)
+			continue;
+
+		if (!signature_checked) {
+			check_commit_signature(commit, &sigc);
+			signature_checked = 1;
+		}
+
+		if (atom->u.signature.option == S_BARE)
+			v->s = xstrdup(sigc.output ? sigc.output: "");
+		else if (atom->u.signature.option == S_SIGNER)
+			v->s = xstrdup(sigc.signer ? sigc.signer : "");
+		else if (atom->u.signature.option == S_GRADE) {
+			switch (sigc.result) {
+			case 'G':
+				switch (sigc.trust_level) {
+				case TRUST_UNDEFINED:
+				case TRUST_NEVER:
+					v->s = xstrfmt("%c", (char)'U');
+					break;
+				default:
+					v->s = xstrfmt("%c", (char)'G');
+					break;
+				}
+				break;
+			case 'B':
+			case 'E':
+			case 'N':
+			case 'X':
+			case 'Y':
+			case 'R':
+				v->s = xstrfmt("%c", (char)sigc.result);
+			}
+		}
+		else if (atom->u.signature.option == S_KEY)
+			v->s = xstrdup(sigc.key ? sigc.key : "");
+		else if (atom->u.signature.option == S_FINGERPRINT)
+			v->s = xstrdup(sigc.fingerprint ? sigc.fingerprint : "");
+		else if (atom->u.signature.option == S_PRI_KEY_FP)
+			v->s = xstrdup(sigc.primary_key_fingerprint ? sigc.primary_key_fingerprint : "");
+		else if (atom->u.signature.option == S_TRUST_LEVEL)
+			v->s = xstrdup(gpg_trust_level_to_str(sigc.trust_level));
+	}
+	if (signature_checked)
+		signature_check_clear(&sigc);
+}
+
 static void find_subpos(const char *buf,
 			const char **sub, size_t *sublen,
 			const char **body, size_t *bodylen,
@@ -1598,6 +1704,7 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, s
 		grab_sub_body_contents(val, deref, data);
 		grab_person("author", val, deref, buf);
 		grab_person("committer", val, deref, buf);
+		grab_signature(val, deref, obj);
 		break;
 	case OBJ_TREE:
 		/* grab_tree_values(val, deref, obj, buf, sz); */
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 5c00607608..6e6ec852b5 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -6,6 +6,7 @@
 test_description='for-each-ref test'
 
 . ./test-lib.sh
+GNUPGHOME_NOT_USED=$GNUPGHOME
 . "$TEST_DIRECTORY"/lib-gpg.sh
 . "$TEST_DIRECTORY"/lib-terminal.sh
 
@@ -1522,4 +1523,194 @@ test_expect_success 'git for-each-ref with non-existing refs' '
 	test_must_be_empty actual
 '
 
+GRADE_FORMAT="%(signature:grade)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)"
+TRUSTLEVEL_FORMAT="%(signature:trustlevel)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)"
+
+test_expect_success GPG 'setup for signature atom using gpg' '
+	git checkout -b signed &&
+
+	test_when_finished "test_unconfig commit.gpgSign" &&
+
+	echo "1" >file &&
+	git add file &&
+	test_tick &&
+	git commit -S -m "file: 1" &&
+	git tag first-signed &&
+
+	echo "2" >file &&
+	test_tick &&
+	git commit -a -m "file: 2" &&
+	git tag second-unsigned &&
+
+	git config commit.gpgSign 1 &&
+	echo "3" >file &&
+	test_tick &&
+	git commit -a --no-gpg-sign -m "file: 3" &&
+	git tag third-unsigned &&
+
+	test_tick &&
+	git rebase -f HEAD^^ && git tag second-signed HEAD^ &&
+	git tag third-signed &&
+
+	echo "4" >file &&
+	test_tick &&
+	git commit -a -SB7227189 -m "file: 4" &&
+	git tag fourth-signed &&
+
+	echo "5" >file &&
+	test_tick &&
+	git commit -a --no-gpg-sign -m "file: 5" &&
+	git tag fifth-unsigned &&
+
+	echo "6" >file &&
+	test_tick &&
+	git commit -a --no-gpg-sign -m "file: 6" &&
+
+	test_tick &&
+	git rebase -f HEAD^^ &&
+	git tag fifth-signed HEAD^ &&
+	git tag sixth-signed &&
+
+	echo "7" >file &&
+	test_tick &&
+	git commit -a --no-gpg-sign -m "file: 7" &&
+	git tag seventh-unsigned
+'
+
+test_expect_success GPGSSH 'setup for signature atom using ssh' '
+	test_when_finished "test_unconfig gpg.format user.signingkey" &&
+
+	test_config gpg.format ssh &&
+	test_config user.signingkey "${GPGSSH_KEY_PRIMARY}" &&
+	echo "8" >file &&
+	test_tick &&
+	git commit -a -S -m "file: 8" &&
+	git tag eighth-signed-ssh
+'
+
+test_expect_success GPG2 'bare signature atom' '
+	git verify-commit first-signed 2>out.raw &&
+	grep -Ev "checking the trustdb|PGP trust model" out.raw >out &&
+	head -3 out >expect &&
+	tail -1 out >>expect &&
+	echo  >>expect &&
+	git for-each-ref refs/tags/first-signed \
+		--format="%(signature)" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show good signature with custom format' '
+	git verify-commit first-signed &&
+	cat >expect <<-\EOF &&
+	G
+	13B6F51ECDDE430D
+	C O Mitter <committer@example.com>
+	73D758744BE721698EC54E8713B6F51ECDDE430D
+	73D758744BE721698EC54E8713B6F51ECDDE430D
+	EOF
+	git for-each-ref refs/tags/first-signed \
+		--format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+test_expect_success GPGSSH 'show good signature with custom format
+			    with ssh' '
+	test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+	FINGERPRINT=$(ssh-keygen -lf "${GPGSSH_KEY_PRIMARY}" | awk "{print \$2;}") &&
+	cat >expect.tmpl <<-\EOF &&
+	G
+	FINGERPRINT
+	principal with number 1
+	FINGERPRINT
+
+	EOF
+	sed "s|FINGERPRINT|$FINGERPRINT|g" expect.tmpl >expect &&
+	git for-each-ref refs/tags/eighth-signed-ssh \
+		--format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'signature atom with grade option and bad signature' '
+	git cat-file commit third-signed >raw &&
+	sed -e "s/^file: 3/file: 3 forged/" raw >forged1 &&
+	FORGED1=$(git hash-object -w -t commit forged1) &&
+	git update-ref refs/tags/third-signed "$FORGED1" &&
+	test_must_fail git verify-commit "$FORGED1" &&
+
+	cat >expect <<-\EOF &&
+	B
+	13B6F51ECDDE430D
+	C O Mitter <committer@example.com>
+
+
+	EOF
+	git for-each-ref refs/tags/third-signed \
+		--format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show untrusted signature with custom format' '
+	cat >expect <<-\EOF &&
+	U
+	65A0EEA02E30CAD7
+	Eris Discordia <discord@example.net>
+	F8364A59E07FFE9F4D63005A65A0EEA02E30CAD7
+	D4BE22311AD3131E5EDA29A461092E85B7227189
+	EOF
+	git for-each-ref refs/tags/fourth-signed \
+		--format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show untrusted signature with undefined trust level' '
+	cat >expect <<-\EOF &&
+	undefined
+	65A0EEA02E30CAD7
+	Eris Discordia <discord@example.net>
+	F8364A59E07FFE9F4D63005A65A0EEA02E30CAD7
+	D4BE22311AD3131E5EDA29A461092E85B7227189
+	EOF
+	git for-each-ref refs/tags/fourth-signed \
+		--format="$TRUSTLEVEL_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show untrusted signature with ultimate trust level' '
+	cat >expect <<-\EOF &&
+	ultimate
+	13B6F51ECDDE430D
+	C O Mitter <committer@example.com>
+	73D758744BE721698EC54E8713B6F51ECDDE430D
+	73D758744BE721698EC54E8713B6F51ECDDE430D
+	EOF
+	git for-each-ref refs/tags/sixth-signed \
+		--format="$TRUSTLEVEL_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show unknown signature with custom format' '
+	cat >expect <<-\EOF &&
+	E
+	13B6F51ECDDE430D
+
+
+
+	EOF
+	GNUPGHOME="$GNUPGHOME_NOT_USED" git for-each-ref \
+		refs/tags/sixth-signed --format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show lack of signature with custom format' '
+	cat >expect <<-\EOF &&
+	N
+
+
+
+
+	EOF
+	git for-each-ref refs/tags/seventh-unsigned \
+		--format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 1/2] t/lib-gpg: introduce new prereq GPG2
  2023-06-02  2:11   ` [PATCH v2 1/2] t/lib-gpg: introduce new prereq GPG2 Kousik Sanagavarapu
@ 2023-06-02  6:50     ` Christian Couder
  2023-06-02 12:58       ` Kousik Sanagavarapu
  0 siblings, 1 reply; 19+ messages in thread
From: Christian Couder @ 2023-06-02  6:50 UTC (permalink / raw)
  To: Kousik Sanagavarapu; +Cc: git, Hariom Verma

On Fri, Jun 2, 2023 at 4:31 AM Kousik Sanagavarapu <five231003@gmail.com> wrote:

> +test_lazy_prereq GPG2 '
> +       gpg_version=$(gpg --version 2>&1)
> +       test $? != 127 || exit 1
> +
> +       case "$gpg_version" in
> +       "gpg (GnuPG) 0."* | "gpg (GnuPG) 1.*")

s/"gpg (GnuPG) 1.*"/"gpg (GnuPG) 1."*/

I am not sure if it changes anything, but for testing if we have v0
here and v1.0.6 in the "test_lazy_prereq PGP", we put the '*'
character outside the double quoted string.

> +               say "This test requires a GPG version >= v2.0.0"
> +               exit 1
> +               ;;
> +       *)
> +               (gpgconf --kill all || : ) &&
> +               gpg --homedir "${GNUPGHOME}" --import \
> +                       "$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&
> +               gpg --homedir "${GNUPGHOME}" --import-ownertrust \
> +                       "$TEST_DIRECTORY"/lib-gpg/ownertrust &&
> +               gpg --homedir "${GNUPGHOME}" </dev/null >/dev/null \
> +                       --sign -u committer@example.com
> +               ;;
> +       esac
> +'

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/2] Add new "signature" atom
  2023-06-02  2:11 ` [PATCH v2 " Kousik Sanagavarapu
  2023-06-02  2:11   ` [PATCH v2 1/2] t/lib-gpg: introduce new prereq GPG2 Kousik Sanagavarapu
  2023-06-02  2:11   ` [PATCH v2 2/2] ref-filter: add new "signature" atom Kousik Sanagavarapu
@ 2023-06-02  7:29   ` Junio C Hamano
  2023-06-02  7:51     ` Eric Sunshine
  2023-06-02 13:13     ` Kousik Sanagavarapu
  2023-06-04 18:22   ` [PATCH v3 " Kousik Sanagavarapu
  3 siblings, 2 replies; 19+ messages in thread
From: Junio C Hamano @ 2023-06-02  7:29 UTC (permalink / raw)
  To: Kousik Sanagavarapu; +Cc: git

Kousik Sanagavarapu <five231003@gmail.com> writes:

> I have also rebased this to be on top of v2.41.0, the previous version
> was on top of v2.41.0-rc0.

I am still feverish and feeling weak so no real review from me yet,
but there is one thing that immediately jumped at me.

> Range-diff against v1:
>
> 1:  5c97d11b79 ! 1:  87465ef1a8 t/lib-gpg: introduce new prereq GPG2
>     @@ t/lib-gpg.sh: test_lazy_prereq GPG '
>      +  test $? != 127 || exit 1
>      +
>      +  case "$gpg_version" in
>     -+  !"gpg (GnuPG) 2."*)
>     ++  "gpg (GnuPG) 0."* | "gpg (GnuPG) 1.*")

The last '*' being inside double-quote would not be what you
intended, I suspect?

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/2] Add new "signature" atom
  2023-06-02  7:29   ` [PATCH v2 0/2] Add " Junio C Hamano
@ 2023-06-02  7:51     ` Eric Sunshine
  2023-06-03  0:16       ` Junio C Hamano
  2023-06-02 13:13     ` Kousik Sanagavarapu
  1 sibling, 1 reply; 19+ messages in thread
From: Eric Sunshine @ 2023-06-02  7:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Kousik Sanagavarapu, git

On Fri, Jun 2, 2023 at 3:33 AM Junio C Hamano <gitster@pobox.com> wrote:
> Kousik Sanagavarapu <five231003@gmail.com> writes:
> > I have also rebased this to be on top of v2.41.0, the previous version
> > was on top of v2.41.0-rc0.
>
> I am still feverish and feeling weak so no real review from me yet,
> but there is one thing that immediately jumped at me.
>
> > 1:  5c97d11b79 ! 1:  87465ef1a8 t/lib-gpg: introduce new prereq GPG2
> >     @@ t/lib-gpg.sh: test_lazy_prereq GPG '
> >      +  test $? != 127 || exit 1
> >      +
> >      +  case "$gpg_version" in
> >     -+  !"gpg (GnuPG) 2."*)
> >     ++  "gpg (GnuPG) 0."* | "gpg (GnuPG) 1.*")
>
> The last '*' being inside double-quote would not be what you
> intended, I suspect?

I noticed that, as well, when running my eye over the range-diff.
Moreover, I wondered if using `[01]` to avoid the repetition would be
worthwhile:

    case "$gpg_version" in
    "gpg (GnuPG) "[01].*)

though, of course, it's subjective whether that is clearer.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/2] ref-filter: add new "signature" atom
  2023-06-02  2:11   ` [PATCH v2 2/2] ref-filter: add new "signature" atom Kousik Sanagavarapu
@ 2023-06-02  8:23     ` Oswald Buddenhagen
  0 siblings, 0 replies; 19+ messages in thread
From: Oswald Buddenhagen @ 2023-06-02  8:23 UTC (permalink / raw)
  To: Kousik Sanagavarapu; +Cc: git, Junio C Hamano

just some nitpicks from me:

On Fri, Jun 02, 2023 at 07:41:55AM +0530, Kousik Sanagavarapu wrote:
>Duplicate the code for outputting the signature and it's other
>
"its", not "it's". this repeats all over the place, so please look up 
the rules.

>+static void grab_signature(struct atom_value *val, int deref, struct 
>object *obj)
>+{

>+		if (!skip_prefix(name, "signature", &name) || (*name &&
>+			*name != ':'))
>+			continue;
>
this looks confusing.

but there appears to be no exception to the "no excess braces" rule for 
multi-line conditions, unlike for bodies - maybe there should be?

also, i would wrap logically, not right at the margin, so

	if (!skip_prefix(name, "signature", &name) ||
	    (*name && *name != ':'))

>+		else if (atom->u.signature.option == S_GRADE) {
>
brace symmetry rule violated.

>+			case 'R':
>+				v->s = xstrfmt("%c", (char)sigc.result);
>+			}
>
omitting the final 'break' looks like bad style to me.

regards,
ossi

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 1/2] t/lib-gpg: introduce new prereq GPG2
  2023-06-02  6:50     ` Christian Couder
@ 2023-06-02 12:58       ` Kousik Sanagavarapu
  0 siblings, 0 replies; 19+ messages in thread
From: Kousik Sanagavarapu @ 2023-06-02 12:58 UTC (permalink / raw)
  To: Christian Couder; +Cc: git

On Fri, Jun 02, 2023 at 08:50:18AM +0200, Christian Couder wrote:
> On Fri, Jun 2, 2023 at 4:31 AM Kousik Sanagavarapu <five231003@gmail.com> wrote:
> 
> > +test_lazy_prereq GPG2 '
> > +       gpg_version=$(gpg --version 2>&1)
> > +       test $? != 127 || exit 1
> > +
> > +       case "$gpg_version" in
> > +       "gpg (GnuPG) 0."* | "gpg (GnuPG) 1.*")
> 
> s/"gpg (GnuPG) 1.*"/"gpg (GnuPG) 1."*/
> 
> I am not sure if it changes anything, but for testing if we have v0
> here and v1.0.6 in the "test_lazy_prereq PGP", we put the '*'
> character outside the double quoted string.

It does seem that it changes things, thanks for catching. I'll reroll
with the necessary changes (there are also some style issues that
Oswald pointed out).

Thanks

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/2] Add new "signature" atom
  2023-06-02  7:29   ` [PATCH v2 0/2] Add " Junio C Hamano
  2023-06-02  7:51     ` Eric Sunshine
@ 2023-06-02 13:13     ` Kousik Sanagavarapu
  1 sibling, 0 replies; 19+ messages in thread
From: Kousik Sanagavarapu @ 2023-06-02 13:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Fri, Jun 02, 2023 at 04:29:34PM +0900, Junio C Hamano wrote:
> Kousik Sanagavarapu <five231003@gmail.com> writes:
> 
> > I have also rebased this to be on top of v2.41.0, the previous version
> > was on top of v2.41.0-rc0.
> 
> I am still feverish and feeling weak so no real review from me yet,

Please take care and get well soon.

> but there is one thing that immediately jumped at me.
>
> > Range-diff against v1:
> >
> > 1:  5c97d11b79 ! 1:  87465ef1a8 t/lib-gpg: introduce new prereq GPG2
> >     @@ t/lib-gpg.sh: test_lazy_prereq GPG '
> >      +  test $? != 127 || exit 1
> >      +
> >      +  case "$gpg_version" in
> >     -+  !"gpg (GnuPG) 2."*)
> >     ++  "gpg (GnuPG) 0."* | "gpg (GnuPG) 1.*")
> 
> The last '*' being inside double-quote would not be what you
> intended, I suspect?

Yeah, that was kind of a typo, thanks for catching it.

Thanks

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/2] Add new "signature" atom
  2023-06-02  7:51     ` Eric Sunshine
@ 2023-06-03  0:16       ` Junio C Hamano
  0 siblings, 0 replies; 19+ messages in thread
From: Junio C Hamano @ 2023-06-03  0:16 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: Kousik Sanagavarapu, git

Eric Sunshine <sunshine@sunshineco.com> writes:

> Moreover, I wondered if using `[01]` to avoid the repetition would be
> worthwhile:
>
>     case "$gpg_version" in
>     "gpg (GnuPG) "[01].*)
>
> though, of course, it's subjective whether that is clearer.

Excellent.

I'd say that the value of your version is primarily that it is much
less error prone than repeating the constant string part that can be
misspelt.  The glob limiting "begins with either '0' or '1' followed
by a dot" might be slightly less easier to understand for less trained
eyes, but eyes will not remain untrained forever, so it is OK.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v3 0/2] Add new "signature" atom
  2023-06-02  2:11 ` [PATCH v2 " Kousik Sanagavarapu
                     ` (2 preceding siblings ...)
  2023-06-02  7:29   ` [PATCH v2 0/2] Add " Junio C Hamano
@ 2023-06-04 18:22   ` Kousik Sanagavarapu
  2023-06-04 18:22     ` [PATCH v3 1/2] t/lib-gpg: introduce new prereq GPG2 Kousik Sanagavarapu
  2023-06-04 18:22     ` [PATCH v3 2/2] ref-filter: add new "signature" atom Kousik Sanagavarapu
  3 siblings, 2 replies; 19+ messages in thread
From: Kousik Sanagavarapu @ 2023-06-04 18:22 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Christian Couder, Eric Sunshine,
	Oswald Buddenhagen, Kousik Sanagavarapu

Hi,

Thanks for the reviews.

Changes since v2:

  PATCH 1/2 -
    Changed "it's" to "its" in the commit message.

    Changed the `case` statement in prereq GPG2 to use the glob pattern
    instead. This has the advantage of being precise and less typo-prone.

  PATCH 2/2 -
    Changed "it's" to "its" in the commit message.

    Changed the `if else` to `switch` when handling options in
    grab_siganture(). This increases the readability of code unlike the
    previous `if else` checking for the type of option, which also
    didn't comply with the style.

    The same kind of refactoring of can be done in other parts of
    ref-filter as well. ZheNing Hu has done some work on it [1], but it
    looks like they were generated by some kind of a script and there
    are unnecessary braces around.

[1]: https://lore.kernel.org/git/2321b873d0c0223e553492d80ced2a51d8ce7281.1629189701.git.gitgitgadget@gmail.com/ 
Range-diff against v2:

1:  87465ef1a8 ! 1:  a7ed6628e0 t/lib-gpg: introduce new prereq GPG2
    @@ Commit message
     
         is the "First stable version of GnuPG integrating OpenPGP and
S/MIME".
     
    -    Use this version or it's successors for tests that will fail
         for
    +    Use this version or its successors for tests that will fail for
         versions less than v2.0.0 because of the difference in the
output on
         stderr between the versions (v2.* vs v0.* or v2.* vs v1.*).
Skip if
         the GPG version detected is less than v2.0.0.
    @@ t/lib-gpg.sh: test_lazy_prereq GPG '
     +  test $? != 127 || exit 1
     +
     +  case "$gpg_version" in
    -+  "gpg (GnuPG) 0."* | "gpg (GnuPG) 1.*")
    ++  "gpg (GnuPG) "[01].*)
     +          say "This test requires a GPG version >= v2.0.0"
     +          exit 1
     +          ;;
2:  690869aa47 ! 2:  b6da96dab2 ref-filter: add new "signature" atom
    @@ Metadata
      ## Commit message ##
         ref-filter: add new "signature" atom
     
    -    Duplicate the code for outputting the signature and it's other
    +    Duplicate the code for outputting the signature and its other
         parameters for commits and tags in ref-filter from pretty. In
the
         future, this will help in getting rid of the current duplicate
         implementations of such logic everywhere, when ref-filter can
do
         everything that pretty is doing.
     
    -    The new atom "signature" and it's friends are equivalent to the
         existing
    +    The new atom "signature" and its friends are equivalent to the
existing
         pretty formats as follows:
     
                 %(signature) = %GG
    @@ Documentation/git-for-each-ref.txt: symref::
     +  The fingerprint of the GPG signature of a commit.
     +
     +signature:primarykeyfingerprint::
    -+  The Primary Key fingerprint of the GPG signature of a commit.
    ++  The primary key fingerprint of the GPG signature of a commit.
     +
     +signature:trustlevel::
    -+  The Trust level of the GPG signature of a commit. Possible
    ++  The trust level of the GPG signature of a commit. Possible
     +  outputs are `ultimate`, `fully`, `marginal`, `never` and
`undefined`.
     +
      worktreepath::
    @@ ref-filter.c: static struct used_atom {
                } email_option;
     +          struct {
     +                  enum { S_BARE, S_GRADE, S_SIGNER, S_KEY,
    -+                         S_FINGERPRINT, S_PRI_KEY_FP,
S_TRUST_LEVEL} option;
    ++                         S_FINGERPRINT, S_PRI_KEY_FP,
S_TRUST_LEVEL } option;
     +          } signature;
                struct refname_atom refname;
                char *head;
    @@ ref-filter.c: static void grab_person(const char *who, struct
atom_value *val, i
     +          struct used_atom *atom = &used_atom[i];
     +          const char *name = atom->name;
     +          struct atom_value *v = &val[i];
    ++          int opt;
     +
     +          if (!!deref != (*name == '*'))
     +                  continue;
     +          if (deref)
     +                  name++;
     +
    -+          if (!skip_prefix(name, "signature", &name) || (*name &&
    -+                  *name != ':'))
    ++          if (!skip_prefix(name, "signature", &name) ||
    ++              (*name && *name != ':'))
     +                  continue;
     +          if (!*name)
     +                  name = NULL;
     +          else
     +                  name++;
    -+          if (parse_signature_option(name) < 0)
    ++
    ++          opt = parse_signature_option(name);
    ++          if (opt < 0)
     +                  continue;
     +
     +          if (!signature_checked) {
    @@ ref-filter.c: static void grab_person(const char *who, struct
atom_value *val, i
     +                  signature_checked = 1;
     +          }
     +
    -+          if (atom->u.signature.option == S_BARE)
    ++          switch (opt) {
    ++          case S_BARE:
     +                  v->s = xstrdup(sigc.output ? sigc.output: "");
    -+          else if (atom->u.signature.option == S_SIGNER)
    ++                  break;
    ++          case S_SIGNER:
     +                  v->s = xstrdup(sigc.signer ? sigc.signer : "");
    -+          else if (atom->u.signature.option == S_GRADE) {
    ++                  break;
    ++          case S_GRADE:
     +                  switch (sigc.result) {
     +                  case 'G':
     +                          switch (sigc.trust_level) {
    @@ ref-filter.c: static void grab_person(const char *who, struct
atom_value *val, i
     +                  case 'Y':
     +                  case 'R':
     +                          v->s = xstrfmt("%c", (char)sigc.result);
    ++                          break;
     +                  }
    -+          }
    -+          else if (atom->u.signature.option == S_KEY)
    ++                  break;
    ++          case S_KEY:
     +                  v->s = xstrdup(sigc.key ? sigc.key : "");
    -+          else if (atom->u.signature.option == S_FINGERPRINT)
    -+                  v->s = xstrdup(sigc.fingerprint ?
sigc.fingerprint : "");
    -+          else if (atom->u.signature.option == S_PRI_KEY_FP)
    -+                  v->s = xstrdup(sigc.primary_key_fingerprint ?
sigc.primary_key_fingerprint : "");
    -+          else if (atom->u.signature.option == S_TRUST_LEVEL)
    ++                  break;
    ++          case S_FINGERPRINT:
    ++                  v->s = xstrdup(sigc.fingerprint ?
    ++                                 sigc.fingerprint : "");
    ++                  break;
    ++          case S_PRI_KEY_FP:
    ++                  v->s = xstrdup(sigc.primary_key_fingerprint ?
    ++                                 sigc.primary_key_fingerprint :
"");
    ++                  break;
    ++          case S_TRUST_LEVEL:
     +                  v->s =
xstrdup(gpg_trust_level_to_str(sigc.trust_level));
    ++                  break;
    ++          }
     +  }
    ++
     +  if (signature_checked)
     +          signature_check_clear(&sigc);
     +}

Kousik Sanagavarapu (2):
  t/lib-gpg: introduce new prereq GPG2
  ref-filter: add new "signature" atom

 Documentation/git-for-each-ref.txt |  27 ++++
 ref-filter.c                       | 126 ++++++++++++++++++-
 t/lib-gpg.sh                       |  21 ++++
 t/t6300-for-each-ref.sh            | 191 +++++++++++++++++++++++++++++
 t/t7510-signed-commit.sh           |   7 ++
 5 files changed, 370 insertions(+), 2 deletions(-)

-- 
2.41.0


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v3 1/2] t/lib-gpg: introduce new prereq GPG2
  2023-06-04 18:22   ` [PATCH v3 " Kousik Sanagavarapu
@ 2023-06-04 18:22     ` Kousik Sanagavarapu
  2023-06-04 18:22     ` [PATCH v3 2/2] ref-filter: add new "signature" atom Kousik Sanagavarapu
  1 sibling, 0 replies; 19+ messages in thread
From: Kousik Sanagavarapu @ 2023-06-04 18:22 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Christian Couder, Eric Sunshine,
	Oswald Buddenhagen, Kousik Sanagavarapu, Hariom Verma

GnuPG v2.0.0 released in 2006, which according to its release notes

	https://gnupg.org/download/release_notes.html

is the "First stable version of GnuPG integrating OpenPGP and S/MIME".

Use this version or its successors for tests that will fail for
versions less than v2.0.0 because of the difference in the output on
stderr between the versions (v2.* vs v0.* or v2.* vs v1.*). Skip if
the GPG version detected is less than v2.0.0.

Do not, however, remove the existing prereq GPG yet since a lot of tests
still work with the prereq GPG (that is even with versions v0.* or v1.*)
and some systems still use these versions.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
 t/lib-gpg.sh             | 21 +++++++++++++++++++++
 t/t7510-signed-commit.sh |  7 +++++++
 2 files changed, 28 insertions(+)

diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index 114785586a..d219a09362 100644
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -51,6 +51,27 @@ test_lazy_prereq GPG '
 	esac
 '
 
+test_lazy_prereq GPG2 '
+	gpg_version=$(gpg --version 2>&1)
+	test $? != 127 || exit 1
+
+	case "$gpg_version" in
+	"gpg (GnuPG) "[01].*)
+		say "This test requires a GPG version >= v2.0.0"
+		exit 1
+		;;
+	*)
+		(gpgconf --kill all || : ) &&
+		gpg --homedir "${GNUPGHOME}" --import \
+			"$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&
+		gpg --homedir "${GNUPGHOME}" --import-ownertrust \
+			"$TEST_DIRECTORY"/lib-gpg/ownertrust &&
+		gpg --homedir "${GNUPGHOME}" </dev/null >/dev/null \
+			--sign -u committer@example.com
+		;;
+	esac
+'
+
 test_lazy_prereq GPGSM '
 	test_have_prereq GPG &&
 	# Available key info:
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index ccbc416402..0d2dd29fe6 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -218,6 +218,13 @@ test_expect_success GPG 'amending already signed commit' '
 	! grep "BAD signature from" actual
 '
 
+test_expect_success GPG2 'bare signature' '
+	git verify-commit fifth-signed 2>expect &&
+	echo >>expect &&
+	git log -1 --format="%GG" fifth-signed >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success GPG 'show good signature with custom format' '
 	cat >expect <<-\EOF &&
 	G
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 2/2] ref-filter: add new "signature" atom
  2023-06-04 18:22   ` [PATCH v3 " Kousik Sanagavarapu
  2023-06-04 18:22     ` [PATCH v3 1/2] t/lib-gpg: introduce new prereq GPG2 Kousik Sanagavarapu
@ 2023-06-04 18:22     ` Kousik Sanagavarapu
  1 sibling, 0 replies; 19+ messages in thread
From: Kousik Sanagavarapu @ 2023-06-04 18:22 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Christian Couder, Eric Sunshine,
	Oswald Buddenhagen, Kousik Sanagavarapu, Hariom Verma,
	Jaydeep Das, Nsengiyumva Wilberforce

Duplicate the code for outputting the signature and its other
parameters for commits and tags in ref-filter from pretty. In the
future, this will help in getting rid of the current duplicate
implementations of such logic everywhere, when ref-filter can do
everything that pretty is doing.

The new atom "signature" and its friends are equivalent to the existing
pretty formats as follows:

	%(signature) = %GG
	%(signature:grade) = %G?
	%(siganture:signer) = %GS
	%(signature:key) = %GK
	%(signature:fingerprint) = %GF
	%(signature:primarykeyfingerprint) = %GP
	%(signature:trustlevel) = %GT

Co-authored-by: Hariom Verma <hariom18599@gmail.com>
Co-authored-by: Jaydeep Das <jaydeepjd.8914@gmail.com>
Co-authored-by: Nsengiyumva Wilberforce <nsengiyumvawilberforce@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
 Documentation/git-for-each-ref.txt |  27 ++++
 ref-filter.c                       | 126 ++++++++++++++++++-
 t/t6300-for-each-ref.sh            | 191 +++++++++++++++++++++++++++++
 3 files changed, 342 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 1e215d4e73..2e0318770b 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -221,6 +221,33 @@ symref::
 	`:lstrip` and `:rstrip` options in the same way as `refname`
 	above.
 
+signature::
+	The GPG signature of a commit.
+
+signature:grade::
+	Show "G" for a good (valid) signature, "B" for a bad
+	signature, "U" for a good signature with unknown validity, "X"
+	for a good signature that has expired, "Y" for a good
+	signature made by an expired key, "R" for a good signature
+	made by a revoked key, "E" if the signature cannot be
+	checked (e.g. missing key) and "N" for no signature.
+
+signature:signer::
+	The signer of the GPG signature of a commit.
+
+signature:key::
+	The key of the GPG signature of a commit.
+
+signature:fingerprint::
+	The fingerprint of the GPG signature of a commit.
+
+signature:primarykeyfingerprint::
+	The primary key fingerprint of the GPG signature of a commit.
+
+signature:trustlevel::
+	The trust level of the GPG signature of a commit. Possible
+	outputs are `ultimate`, `fully`, `marginal`, `never` and `undefined`.
+
 worktreepath::
 	The absolute path to the worktree in which the ref is checked
 	out, if it is checked out in any linked worktree. Empty string
diff --git a/ref-filter.c b/ref-filter.c
index 4991cd4f7a..5c6924b367 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -150,6 +150,7 @@ enum atom_type {
 	ATOM_BODY,
 	ATOM_TRAILERS,
 	ATOM_CONTENTS,
+	ATOM_SIGNATURE,
 	ATOM_RAW,
 	ATOM_UPSTREAM,
 	ATOM_PUSH,
@@ -215,6 +216,10 @@ static struct used_atom {
 		struct email_option {
 			enum { EO_RAW, EO_TRIM, EO_LOCALPART } option;
 		} email_option;
+		struct {
+			enum { S_BARE, S_GRADE, S_SIGNER, S_KEY,
+			       S_FINGERPRINT, S_PRI_KEY_FP, S_TRUST_LEVEL } option;
+		} signature;
 		struct refname_atom refname;
 		char *head;
 	} u;
@@ -407,8 +412,37 @@ static int subject_atom_parser(struct ref_format *format UNUSED,
 	return 0;
 }
 
-static int trailers_atom_parser(struct ref_format *format UNUSED,
-				struct used_atom *atom,
+static int parse_signature_option(const char *arg)
+{
+	if (!arg)
+		return S_BARE;
+	else if (!strcmp(arg, "signer"))
+		return S_SIGNER;
+	else if (!strcmp(arg, "grade"))
+		return S_GRADE;
+	else if (!strcmp(arg, "key"))
+		return S_KEY;
+	else if (!strcmp(arg, "fingerprint"))
+		return S_FINGERPRINT;
+	else if (!strcmp(arg, "primarykeyfingerprint"))
+		return S_PRI_KEY_FP;
+	else if (!strcmp(arg, "trustlevel"))
+		return S_TRUST_LEVEL;
+	return -1;
+}
+
+static int signature_atom_parser(struct ref_format *format UNUSED,
+				 struct used_atom *atom,
+				 const char *arg, struct strbuf *err)
+{
+	int opt = parse_signature_option(arg);
+	if (opt < 0)
+		return err_bad_arg(err, "signature", arg);
+	atom->u.signature.option = opt;
+	return 0;
+}
+
+static int trailers_atom_parser(struct ref_format *format, struct used_atom *atom,
 				const char *arg, struct strbuf *err)
 {
 	atom->u.contents.trailer_opts.no_divider = 1;
@@ -668,6 +702,7 @@ static struct {
 	[ATOM_BODY] = { "body", SOURCE_OBJ, FIELD_STR, body_atom_parser },
 	[ATOM_TRAILERS] = { "trailers", SOURCE_OBJ, FIELD_STR, trailers_atom_parser },
 	[ATOM_CONTENTS] = { "contents", SOURCE_OBJ, FIELD_STR, contents_atom_parser },
+	[ATOM_SIGNATURE] = { "signature", SOURCE_OBJ, FIELD_STR, signature_atom_parser },
 	[ATOM_RAW] = { "raw", SOURCE_OBJ, FIELD_STR, raw_atom_parser },
 	[ATOM_UPSTREAM] = { "upstream", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },
 	[ATOM_PUSH] = { "push", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },
@@ -1405,6 +1440,92 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void
 	}
 }
 
+static void grab_signature(struct atom_value *val, int deref, struct object *obj)
+{
+	int i;
+	struct commit *commit = (struct commit *) obj;
+	struct signature_check sigc = { 0 };
+	int signature_checked = 0;
+
+	for (i = 0; i < used_atom_cnt; i++) {
+		struct used_atom *atom = &used_atom[i];
+		const char *name = atom->name;
+		struct atom_value *v = &val[i];
+		int opt;
+
+		if (!!deref != (*name == '*'))
+			continue;
+		if (deref)
+			name++;
+
+		if (!skip_prefix(name, "signature", &name) ||
+		    (*name && *name != ':'))
+			continue;
+		if (!*name)
+			name = NULL;
+		else
+			name++;
+
+		opt = parse_signature_option(name);
+		if (opt < 0)
+			continue;
+
+		if (!signature_checked) {
+			check_commit_signature(commit, &sigc);
+			signature_checked = 1;
+		}
+
+		switch (opt) {
+		case S_BARE:
+			v->s = xstrdup(sigc.output ? sigc.output: "");
+			break;
+		case S_SIGNER:
+			v->s = xstrdup(sigc.signer ? sigc.signer : "");
+			break;
+		case S_GRADE:
+			switch (sigc.result) {
+			case 'G':
+				switch (sigc.trust_level) {
+				case TRUST_UNDEFINED:
+				case TRUST_NEVER:
+					v->s = xstrfmt("%c", (char)'U');
+					break;
+				default:
+					v->s = xstrfmt("%c", (char)'G');
+					break;
+				}
+				break;
+			case 'B':
+			case 'E':
+			case 'N':
+			case 'X':
+			case 'Y':
+			case 'R':
+				v->s = xstrfmt("%c", (char)sigc.result);
+				break;
+			}
+			break;
+		case S_KEY:
+			v->s = xstrdup(sigc.key ? sigc.key : "");
+			break;
+		case S_FINGERPRINT:
+			v->s = xstrdup(sigc.fingerprint ?
+				       sigc.fingerprint : "");
+			break;
+		case S_PRI_KEY_FP:
+			v->s = xstrdup(sigc.primary_key_fingerprint ?
+				       sigc.primary_key_fingerprint : "");
+			break;
+		case S_TRUST_LEVEL:
+			v->s = xstrdup(gpg_trust_level_to_str(sigc.trust_level));
+			break;
+		}
+	}
+
+	if (signature_checked)
+		signature_check_clear(&sigc);
+}
+
 static void find_subpos(const char *buf,
 			const char **sub, size_t *sublen,
 			const char **body, size_t *bodylen,
@@ -1598,6 +1719,7 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, s
 		grab_sub_body_contents(val, deref, data);
 		grab_person("author", val, deref, buf);
 		grab_person("committer", val, deref, buf);
+		grab_signature(val, deref, obj);
 		break;
 	case OBJ_TREE:
 		/* grab_tree_values(val, deref, obj, buf, sz); */
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 5c00607608..6e6ec852b5 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -6,6 +6,7 @@
 test_description='for-each-ref test'
 
 . ./test-lib.sh
+GNUPGHOME_NOT_USED=$GNUPGHOME
 . "$TEST_DIRECTORY"/lib-gpg.sh
 . "$TEST_DIRECTORY"/lib-terminal.sh
 
@@ -1522,4 +1523,194 @@ test_expect_success 'git for-each-ref with non-existing refs' '
 	test_must_be_empty actual
 '
 
+GRADE_FORMAT="%(signature:grade)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)"
+TRUSTLEVEL_FORMAT="%(signature:trustlevel)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)"
+
+test_expect_success GPG 'setup for signature atom using gpg' '
+	git checkout -b signed &&
+
+	test_when_finished "test_unconfig commit.gpgSign" &&
+
+	echo "1" >file &&
+	git add file &&
+	test_tick &&
+	git commit -S -m "file: 1" &&
+	git tag first-signed &&
+
+	echo "2" >file &&
+	test_tick &&
+	git commit -a -m "file: 2" &&
+	git tag second-unsigned &&
+
+	git config commit.gpgSign 1 &&
+	echo "3" >file &&
+	test_tick &&
+	git commit -a --no-gpg-sign -m "file: 3" &&
+	git tag third-unsigned &&
+
+	test_tick &&
+	git rebase -f HEAD^^ && git tag second-signed HEAD^ &&
+	git tag third-signed &&
+
+	echo "4" >file &&
+	test_tick &&
+	git commit -a -SB7227189 -m "file: 4" &&
+	git tag fourth-signed &&
+
+	echo "5" >file &&
+	test_tick &&
+	git commit -a --no-gpg-sign -m "file: 5" &&
+	git tag fifth-unsigned &&
+
+	echo "6" >file &&
+	test_tick &&
+	git commit -a --no-gpg-sign -m "file: 6" &&
+
+	test_tick &&
+	git rebase -f HEAD^^ &&
+	git tag fifth-signed HEAD^ &&
+	git tag sixth-signed &&
+
+	echo "7" >file &&
+	test_tick &&
+	git commit -a --no-gpg-sign -m "file: 7" &&
+	git tag seventh-unsigned
+'
+
+test_expect_success GPGSSH 'setup for signature atom using ssh' '
+	test_when_finished "test_unconfig gpg.format user.signingkey" &&
+
+	test_config gpg.format ssh &&
+	test_config user.signingkey "${GPGSSH_KEY_PRIMARY}" &&
+	echo "8" >file &&
+	test_tick &&
+	git commit -a -S -m "file: 8" &&
+	git tag eighth-signed-ssh
+'
+
+test_expect_success GPG2 'bare signature atom' '
+	git verify-commit first-signed 2>out.raw &&
+	grep -Ev "checking the trustdb|PGP trust model" out.raw >out &&
+	head -3 out >expect &&
+	tail -1 out >>expect &&
+	echo  >>expect &&
+	git for-each-ref refs/tags/first-signed \
+		--format="%(signature)" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show good signature with custom format' '
+	git verify-commit first-signed &&
+	cat >expect <<-\EOF &&
+	G
+	13B6F51ECDDE430D
+	C O Mitter <committer@example.com>
+	73D758744BE721698EC54E8713B6F51ECDDE430D
+	73D758744BE721698EC54E8713B6F51ECDDE430D
+	EOF
+	git for-each-ref refs/tags/first-signed \
+		--format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+test_expect_success GPGSSH 'show good signature with custom format
+			    with ssh' '
+	test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+	FINGERPRINT=$(ssh-keygen -lf "${GPGSSH_KEY_PRIMARY}" | awk "{print \$2;}") &&
+	cat >expect.tmpl <<-\EOF &&
+	G
+	FINGERPRINT
+	principal with number 1
+	FINGERPRINT
+
+	EOF
+	sed "s|FINGERPRINT|$FINGERPRINT|g" expect.tmpl >expect &&
+	git for-each-ref refs/tags/eighth-signed-ssh \
+		--format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'signature atom with grade option and bad signature' '
+	git cat-file commit third-signed >raw &&
+	sed -e "s/^file: 3/file: 3 forged/" raw >forged1 &&
+	FORGED1=$(git hash-object -w -t commit forged1) &&
+	git update-ref refs/tags/third-signed "$FORGED1" &&
+	test_must_fail git verify-commit "$FORGED1" &&
+
+	cat >expect <<-\EOF &&
+	B
+	13B6F51ECDDE430D
+	C O Mitter <committer@example.com>
+
+
+	EOF
+	git for-each-ref refs/tags/third-signed \
+		--format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show untrusted signature with custom format' '
+	cat >expect <<-\EOF &&
+	U
+	65A0EEA02E30CAD7
+	Eris Discordia <discord@example.net>
+	F8364A59E07FFE9F4D63005A65A0EEA02E30CAD7
+	D4BE22311AD3131E5EDA29A461092E85B7227189
+	EOF
+	git for-each-ref refs/tags/fourth-signed \
+		--format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show untrusted signature with undefined trust level' '
+	cat >expect <<-\EOF &&
+	undefined
+	65A0EEA02E30CAD7
+	Eris Discordia <discord@example.net>
+	F8364A59E07FFE9F4D63005A65A0EEA02E30CAD7
+	D4BE22311AD3131E5EDA29A461092E85B7227189
+	EOF
+	git for-each-ref refs/tags/fourth-signed \
+		--format="$TRUSTLEVEL_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show untrusted signature with ultimate trust level' '
+	cat >expect <<-\EOF &&
+	ultimate
+	13B6F51ECDDE430D
+	C O Mitter <committer@example.com>
+	73D758744BE721698EC54E8713B6F51ECDDE430D
+	73D758744BE721698EC54E8713B6F51ECDDE430D
+	EOF
+	git for-each-ref refs/tags/sixth-signed \
+		--format="$TRUSTLEVEL_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show unknown signature with custom format' '
+	cat >expect <<-\EOF &&
+	E
+	13B6F51ECDDE430D
+
+
+
+	EOF
+	GNUPGHOME="$GNUPGHOME_NOT_USED" git for-each-ref \
+		refs/tags/sixth-signed --format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show lack of signature with custom format' '
+	cat >expect <<-\EOF &&
+	N
+
+
+
+
+	EOF
+	git for-each-ref refs/tags/seventh-unsigned \
+		--format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2023-06-04 18:59 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-29 18:32 [GSoC][PATCH 0/2] Add new "signature" atom Kousik Sanagavarapu
2023-05-29 18:32 ` [PATCH 1/2] t/lib-gpg: introduce new prereq GPG2 Kousik Sanagavarapu
2023-06-01  8:39   ` Christian Couder
2023-05-29 18:32 ` [PATCH 2/2] ref-filter: add new "signature" atom Kousik Sanagavarapu
2023-06-01  8:58   ` Christian Couder
2023-06-01  9:11 ` [GSoC][PATCH 0/2] Add " Christian Couder
2023-06-02  2:11 ` [PATCH v2 " Kousik Sanagavarapu
2023-06-02  2:11   ` [PATCH v2 1/2] t/lib-gpg: introduce new prereq GPG2 Kousik Sanagavarapu
2023-06-02  6:50     ` Christian Couder
2023-06-02 12:58       ` Kousik Sanagavarapu
2023-06-02  2:11   ` [PATCH v2 2/2] ref-filter: add new "signature" atom Kousik Sanagavarapu
2023-06-02  8:23     ` Oswald Buddenhagen
2023-06-02  7:29   ` [PATCH v2 0/2] Add " Junio C Hamano
2023-06-02  7:51     ` Eric Sunshine
2023-06-03  0:16       ` Junio C Hamano
2023-06-02 13:13     ` Kousik Sanagavarapu
2023-06-04 18:22   ` [PATCH v3 " Kousik Sanagavarapu
2023-06-04 18:22     ` [PATCH v3 1/2] t/lib-gpg: introduce new prereq GPG2 Kousik Sanagavarapu
2023-06-04 18:22     ` [PATCH v3 2/2] ref-filter: add new "signature" atom Kousik Sanagavarapu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).