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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 02.03.2022 11:18, Junio C Hamano wrote:
Fabian Stelzer <fs@xxxxxxxxxxxx> 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?

If only refers to the test prerequisite actually. I'll update the message.


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.

Correct. The `with-colons` always uses fpr pre and post 2.3


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

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

Switching to awk allows us to integrate this trailing info into the awk expression itself making this extra echo unnecessary.


 	echo hello | gpgsm --homedir "${GNUPGHOME}" >/dev/null \
 	       -u committer@xxxxxxxxxxx -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.

Easy enough. Initially I used a subshell and 2 grep calls but this is obviously easier. I prefer the static strings over the regex as well.

I'll send a new patch probably tomorrow and try to improve the commit message.

Thanks



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux