Subject: Re: [PATCH v3 1/3] gpg-interface/gpgsm: fix for v2.3

From: Junio C Hamano <[email-masked]>
Date: Wed, 02 Mar 2022 11:18:33 -0800

Fabian Stelzer <[email-masked]> writes:

> Checking if signing was successful will now accept '[GNUPG]:
> SIG_CREATED' on any beginning of a line. Not just explictly the second
> one anymore.

"the second or subsequent one", I would think, but the code change
looks correct anyway.

> Switch to gpg's `--with-colons` output format to make
> parsing more robust.  This avoids issues where the
> human-readable output from gpg commands changes.

Does this refer only to how parsing in tests is done?

> Adjust error messages checking in tests for v2.3 specific output changes.

Does this refer only to the change to 4202 where "failed to find
the" and the colon after "certificate" are made optional, so that
the regexp can read messages from both pre- and post-2.3 versions?

> diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
> index 3e7ee1386a..6bc083ca77 100644
> --- a/t/lib-gpg.sh
> +++ b/t/lib-gpg.sh
> @@ -72,12 +72,10 @@ test_lazy_prereq GPGSM '
>  		--passphrase-fd 0 --pinentry-mode loopback \
>  		--import "$TEST_DIRECTORY"/lib-gpg/gpgsm_cert.p12 &&
>  
> -	gpgsm --homedir "${GNUPGHOME}" -K |
> -	grep fingerprint: |
> -	cut -d" " -f4 |
> -	tr -d "\\n" >"${GNUPGHOME}/trustlist.txt" &&
> +	gpgsm --homedir "${GNUPGHOME}" -K --with-colons |
> +	awk -F ":" "/^fpr:/ {printf \"%s S relax\\n\", \$10}" \
> +		>"${GNUPGHOME}/trustlist.txt" &&

The old iteration had (fpr|fingerprint) which appeared as if it were
catering to both pre- and post-2.3 versions, but "with colons", all
versions we care about would say "fpr" and that is the reason why we
no longer have such an alternative here?  Just checking my
understanding.

> -	echo " S relax" >>"${GNUPGHOME}/trustlist.txt" &&

This removal is because...?  I do not recall seeing the explanation
in the proposed log message.

>  	echo hello | gpgsm --homedir "${GNUPGHOME}" >/dev/null \
>  	       -u [email-masked] -o /dev/null --sign -
>  '
> diff --git a/t/t4202-log.sh b/t/t4202-log.sh
> index 55fac64446..d599bf4b11 100755
> --- a/t/t4202-log.sh
> +++ b/t/t4202-log.sh
> @@ -2037,7 +2037,7 @@ test_expect_success GPGSM 'log --graph --show-signature for merged tag x509 miss
>  	git merge --no-ff -m msg signed_tag_x509_nokey &&
>  	GNUPGHOME=. git log --graph --show-signature -n1 plain-x509-nokey >actual &&
>  	grep "^|\\\  merged tag" actual &&
> -	grep "^| | gpgsm: certificate not found" actual
> +	grep -Ei "^| | gpgsm:( failed to find the)? certificate:? not found" actual
>  '

OK.  It might be easier to read if we give two expressions
separately and say "we can take either of these", i.e.

	# the former is from pre-2.3, the latter is from 2.3 and later
	grep -e "^| | gpgsm: certificate not found" \
	     -e "^| | gpgsm: failed to find the certificate: not found" \
	     actual

Thanks for working on this update.