git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH 2/2] tests(gpg): increase verbosity to allow debugging
Date: Mon, 23 Mar 2020 16:15:47 -0400
Message-ID: <20200323201547.GA35429@coredump.intra.peff.net> (raw)
In-Reply-To: <xmqq8sjq6fob.fsf@gitster.c.googlers.com>

On Mon, Mar 23, 2020 at 12:21:08PM -0700, Junio C Hamano wrote:

> > +		gpg --homedir "${GNUPGHOME}" \
> > +			--sign -u committer@example.com >/dev/null &&
> 
> We lost input redirection; I am assuming that it was unnecessary as
> the standard input of our tests are all redirected from /dev/null
> anyway?

Yes (though it also wouldn't hurt to leave it to explicitly document
that we're ignoring it for this command).

> We are not interested in seeing the signed output (we have nothing
> to compare to validate the correctness anyway), so discarding the
> standard output is fine.  We do not want to see it even while we are
> debugging, either.

Yes, exactly. I originally dropped it, but after looking at the "-v"
output I saw it was full of signed binary goo. :)

> Looking good.

There were actually a few subtle breakages in what I posted before. One
was just a dumb conversion: I forgot to set $gpg_version correctly. But
the other is much trickier: we have to hide our exit codes from
test_expect_success, since in the non-GPG case we want it to still
report success. So any breakage in the big &&-chain would say "test
failure", when it should just quietly continue without setting the GPG
prereq.

Here's what I came up with that I think is suitable for applying (though
if you find the GNUPGHOME thing below too gross, I can rework it as
indicated):

-- >8 --
Subject: [PATCH] t/lib-gpg: run setup code in test blocks

The steps to check the GPG prereq and set up GNUPGHOME are run in the
main script, with stdout and stderr redirected. This avoids spewing
useless output when GPG isn't available. But it also means that there's
no easy way to see what did happen if you're using "-v" or "-x".

Let's push this as much as possible into a lazy_prereq blocks, which
handle verbosity and tracing for us. There's one tricky thing here: part
of the setup involves setting $GNUPGHOME, but lazy_prereq blocks are
evaluated in a subshell in order to avoid accidental environment
contamination. Splitting the setup from the prereq is tricky; the prereq
is basically "did we successfully set things up".

We could run all of the GPG prereq code in its own test_expect_success
block. But that gets awkward because we _don't_ want to report failure
if a command fails (we just want to not set the prereq).

I've solved it here by pulling the GNUPGHOME setup into its own separate
setup step, that happens _before_ we check the prereq. That means we'd
set up the variable even if we don't have gpg, but that should be OK;
we'll be skipping any gpg tests in that case anyway. (If it's not, the
alternative is to put the big &&-chain into a separate function of "{}"
block).

Now that the code is inside test blocks, we can take advantage of this
to use &&-chaining and early returns, and avoid indenting everything
inside a big case statement.

Signed-off-by: Jeff King <peff@peff.net>
---
On top of Dscho's patch 1, since it uses $PWD/gpghome.

 t/lib-gpg.sh | 145 +++++++++++++++++++++++++++------------------------
 1 file changed, 76 insertions(+), 69 deletions(-)

diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index 11b83b8c24..56153b3123 100755
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -1,81 +1,88 @@
 #!/bin/sh
 
-gpg_version=$(gpg --version 2>&1)
-if test $? != 127
-then
+# This can't run as part of the lazy_prereq below because it has the side
+# effect of setting an environment variable.
+test_expect_success 'set up GNUPGHOME' '
+	mkdir ./gpghome &&
+	chmod 0700 ./gpghome &&
+	GNUPGHOME="$PWD/gpghome" &&
+	export GNUPGHOME
+'
+
+test_lazy_prereq GPG '
+	{
+		gpg_version=$(gpg --version)
+		test $? != 127
+	} &&
+
 	# As said here: http://www.gnupg.org/documentation/faqs.html#q6.19
-	# the gpg version 1.0.6 didn't parse trust packets correctly, so for
+	# the gpg version 1.0.6 did not parse trust packets correctly, so for
 	# that version, creation of signed tags using the generated key fails.
 	case "$gpg_version" in
-	'gpg (GnuPG) 1.0.6'*)
-		say "Your version of gpg (1.0.6) is too buggy for testing"
+		"gpg (GnuPG) 1.0.6"*)
+		echo >&2 "Your version of gpg (1.0.6) is too buggy for testing"
+		return 1
 		;;
-	*)
-		# Available key info:
-		# * Type DSA and Elgamal, size 2048 bits, no expiration date,
-		#   name and email: C O Mitter <committer@example.com>
-		# * Type RSA, size 2048 bits, no expiration date,
-		#   name and email: Eris Discordia <discord@example.net>
-		# No password given, to enable non-interactive operation.
-		# To generate new key:
-		#	gpg --homedir /tmp/gpghome --gen-key
-		# To write armored exported key to keyring:
-		#	gpg --homedir /tmp/gpghome --export-secret-keys \
-		#		--armor 0xDEADBEEF >> lib-gpg/keyring.gpg
-		#	gpg --homedir /tmp/gpghome --export \
-		#		--armor 0xDEADBEEF >> lib-gpg/keyring.gpg
-		# To export ownertrust:
-		#	gpg --homedir /tmp/gpghome --export-ownertrust \
-		#		> lib-gpg/ownertrust
-		mkdir ./gpghome &&
-		chmod 0700 ./gpghome &&
-		GNUPGHOME="$PWD/gpghome" &&
-		export GNUPGHOME &&
-		(gpgconf --kill gpg-agent >/dev/null 2>&1 || : ) &&
-		gpg --homedir "${GNUPGHOME}" 2>/dev/null --import \
-			"$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&
-		gpg --homedir "${GNUPGHOME}" 2>/dev/null --import-ownertrust \
-			"$TEST_DIRECTORY"/lib-gpg/ownertrust &&
-		gpg --homedir "${GNUPGHOME}" </dev/null >/dev/null 2>&1 \
-			--sign -u committer@example.com &&
-		test_set_prereq GPG &&
-		# Available key info:
-		# * see t/lib-gpg/gpgsm-gen-key.in
-		# To generate new certificate:
-		#  * no passphrase
-		#	gpgsm --homedir /tmp/gpghome/ \
-		#		-o /tmp/gpgsm.crt.user \
-		#		--generate-key \
-		#		--batch t/lib-gpg/gpgsm-gen-key.in
-		# To import certificate:
-		#	gpgsm --homedir /tmp/gpghome/ \
-		#		--import /tmp/gpgsm.crt.user
-		# To export into a .p12 we can later import:
-		#	gpgsm --homedir /tmp/gpghome/ \
-		#		-o t/lib-gpg/gpgsm_cert.p12 \
-		#		--export-secret-key-p12 "committer@example.com"
-		echo | gpgsm --homedir "${GNUPGHOME}" 2>/dev/null \
-			--passphrase-fd 0 --pinentry-mode loopback \
-			--import "$TEST_DIRECTORY"/lib-gpg/gpgsm_cert.p12 &&
+	esac &&
 
-		gpgsm --homedir "${GNUPGHOME}" 2>/dev/null -K |
-		grep fingerprint: |
-		cut -d" " -f4 |
-		tr -d '\n' >"${GNUPGHOME}/trustlist.txt" &&
+	# Available key info:
+	# * Type DSA and Elgamal, size 2048 bits, no expiration date,
+	#   name and email: C O Mitter <committer@example.com>
+	# * Type RSA, size 2048 bits, no expiration date,
+	#   name and email: Eris Discordia <discord@example.net>
+	# No password given, to enable non-interactive operation.
+	# To generate new key:
+	#	gpg --homedir /tmp/gpghome --gen-key
+	# To write armored exported key to keyring:
+	#	gpg --homedir /tmp/gpghome --export-secret-keys \
+	#		--armor 0xDEADBEEF >> lib-gpg/keyring.gpg
+	#	gpg --homedir /tmp/gpghome --export \
+	#		--armor 0xDEADBEEF >> lib-gpg/keyring.gpg
+	# To export ownertrust:
+	#	gpg --homedir /tmp/gpghome --export-ownertrust \
+	#		> lib-gpg/ownertrust
+	(gpgconf --kill gpg-agent || : ) &&
+	gpg --homedir "${GNUPGHOME}" --import \
+		"$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&
+	gpg --homedir "${GNUPGHOME}" --import-ownertrust \
+		"$TEST_DIRECTORY"/lib-gpg/ownertrust &&
+	gpg --homedir "${GNUPGHOME}" \
+		--sign -u committer@example.com >/dev/null
+'
 
-		echo " S relax" >>"${GNUPGHOME}/trustlist.txt" &&
-		echo hello | gpgsm --homedir "${GNUPGHOME}" >/dev/null \
-			-u committer@example.com -o /dev/null --sign - 2>&1 &&
-		test_set_prereq GPGSM
-		;;
-	esac
-fi
+test_have_prereq GPG &&
+test_lazy_prereq GPGSM '
+	# Available key info:
+	# * see t/lib-gpg/gpgsm-gen-key.in
+	# To generate new certificate:
+	#  * no passphrase
+	#	gpgsm --homedir /tmp/gpghome/ \
+	#		-o /tmp/gpgsm.crt.user \
+	#		--generate-key \
+	#		--batch t/lib-gpg/gpgsm-gen-key.in
+	# To import certificate:
+	#	gpgsm --homedir /tmp/gpghome/ \
+	#		--import /tmp/gpgsm.crt.user
+	# To export into a .p12 we can later import:
+	#	gpgsm --homedir /tmp/gpghome/ \
+	#		-o t/lib-gpg/gpgsm_cert.p12 \
+	#		--export-secret-key-p12 "committer@example.com"
+	echo | gpgsm --homedir "${GNUPGHOME}" \
+		--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" &&
+	echo " S relax" >>"${GNUPGHOME}/trustlist.txt" &&
+	echo hello | gpgsm --homedir "${GNUPGHOME}" \
+		-u committer@example.com -o /dev/null --sign -
+'
 
-if test_have_prereq GPG &&
-    echo | gpg --homedir "${GNUPGHOME}" -b --rfc1991 >/dev/null 2>&1
-then
-	test_set_prereq RFC1991
-fi
+test_have_prereq GPG &&
+test_lazy_prereq RFC1991 '
+    echo | gpg --homedir "${GNUPGHOME}" -b --rfc1991
+'
 
 sanitize_pgp() {
 	perl -ne '
-- 
2.26.0.561.g096102dc45


  reply index

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-23 13:09 [PATCH 0/2] Enable GPG in the Windows part of the CI/PR builds Johannes Schindelin via GitGitGadget
2020-03-23 13:09 ` [PATCH 1/2] tests(gpg): allow the gpg-agent to start on Windows Johannes Schindelin via GitGitGadget
2020-03-23 17:46   ` Junio C Hamano
2020-03-24 19:55     ` Johannes Schindelin
2020-03-24 20:59       ` Junio C Hamano
2020-03-24 22:26         ` Johannes Schindelin
2020-03-24 23:40           ` Junio C Hamano
2020-03-23 13:09 ` [PATCH 2/2] tests(gpg): increase verbosity to allow debugging Johannes Schindelin via GitGitGadget
2020-03-23 17:32   ` Jeff King
2020-03-23 18:04     ` Jeff King
2020-03-23 19:21       ` Junio C Hamano
2020-03-23 20:15         ` Jeff King [this message]
2020-03-23 21:28           ` Junio C Hamano
2020-03-23 21:31             ` Jeff King
2020-03-24 21:41               ` Johannes Schindelin
2020-03-24 22:05                 ` Jeff King
2020-03-24 22:25                   ` Johannes Schindelin
2020-03-24 22:33                     ` Jeff King
2020-03-25  5:41 ` [PATCH v2 0/5] Enable GPG in the Windows part of the CI/PR builds Johannes Schindelin via GitGitGadget
2020-03-25  5:41   ` [PATCH v2 1/5] tests(gpg): allow the gpg-agent to start on Windows Johannes Schindelin via GitGitGadget
2020-03-25  5:41   ` [PATCH v2 2/5] t/lib-gpg.sh: stop pretending to be a stand-alone script Johannes Schindelin via GitGitGadget
2020-03-26  8:21     ` Jeff King
2020-03-26 13:48       ` Johannes Schindelin
2020-03-26 19:31       ` Junio C Hamano
2020-03-25  5:41   ` [PATCH v2 3/5] tests: turn GPG, GPGSM and RFC1991 into lazy prereqs Johannes Schindelin via GitGitGadget
2020-03-25 17:25     ` Junio C Hamano
2020-03-26  8:35     ` Jeff King
2020-03-26 14:27       ` Johannes Schindelin
2020-03-27  9:10         ` Jeff King
2020-03-27 17:44           ` Junio C Hamano
2020-03-27 20:24             ` Eric Sunshine
2020-03-27 21:37               ` Junio C Hamano
2020-03-28 10:58                 ` Jeff King
2020-03-28 10:54             ` Jeff King
2020-03-28 23:49               ` [PATCH v2] t/README: suggest how to leave test early with failure Junio C Hamano
2020-03-29  7:23                 ` Eric Sunshine
2020-03-29 14:33                 ` Jeff King
2020-03-30 18:39           ` [PATCH v2 3/5] tests: turn GPG, GPGSM and RFC1991 into lazy prereqs Johannes Schindelin
2020-03-31  9:34             ` Jeff King
2020-03-25  5:41   ` [PATCH v2 4/5] tests: do not let lazy prereqs inside `test_expect_*` turn off tracing Johannes Schindelin via GitGitGadget
2020-03-25 17:23     ` Junio C Hamano
2020-03-26 13:45       ` Johannes Schindelin
2020-03-26  8:49     ` Jeff King
2020-03-26 14:34       ` Johannes Schindelin
2020-03-25  5:41   ` [PATCH v2 5/5] tests: increase the verbosity of the GPG-related prereqs Johannes Schindelin via GitGitGadget
2020-03-26  8:50     ` Jeff King
2020-03-26 14:36       ` Johannes Schindelin
2020-03-26 15:35   ` [PATCH v3 0/5] Enable GPG in the Windows part of the CI/PR builds Johannes Schindelin via GitGitGadget
2020-03-26 15:35     ` [PATCH v3 1/5] tests(gpg): allow the gpg-agent to start on Windows Johannes Schindelin via GitGitGadget
2020-03-26 15:35     ` [PATCH v3 2/5] t/lib-gpg.sh: stop pretending to be a stand-alone script Johannes Schindelin via GitGitGadget
2020-03-26 15:35     ` [PATCH v3 3/5] tests: do not let lazy prereqs inside `test_expect_*` turn off tracing Johannes Schindelin via GitGitGadget
2020-03-26 15:35     ` [PATCH v3 4/5] tests: turn GPG, GPGSM and RFC1991 into lazy prereqs Johannes Schindelin via GitGitGadget
2020-03-26 15:35     ` [PATCH v3 5/5] tests: increase the verbosity of the GPG-related prereqs Johannes Schindelin via GitGitGadget
2020-03-27  9:12     ` [PATCH v3 0/5] Enable GPG in the Windows part of the CI/PR builds Jeff King
2020-03-27 17:45       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200323201547.GA35429@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org list mirror (unofficial, one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Example config snippet for mirrors

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git