From 6c72121c346eb8999171585e50d6791a0dbc97fb Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 25 Mar 2020 05:41:17 +0000 Subject: tests(gpg): allow the gpg-agent to start on Windows In Git for Windows' SDK, we use the MSYS2 version of OpenSSH, meaning that the `gpg-agent` will fail horribly when being passed a `--homedir` that contains colons. Previously, we did pass the Windows version of the absolute path, though, which starts in the drive letter followed by, you guessed it, a colon. Let's use the same trick found elsewhere in our test suite where `$PWD` is used to refer to the pseudo-Unix path (which works only within the MSYS2 Bash/OpenSSH/Perl/etc, as opposed to `$(pwd)` which refers to the Windows path that `git.exe` understands, too). Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/lib-gpg.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 't') diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh index 8d28652b72..11b83b8c24 100755 --- a/t/lib-gpg.sh +++ b/t/lib-gpg.sh @@ -29,7 +29,7 @@ then # > lib-gpg/ownertrust mkdir ./gpghome && chmod 0700 ./gpghome && - GNUPGHOME="$(pwd)/gpghome" && + GNUPGHOME="$PWD/gpghome" && export GNUPGHOME && (gpgconf --kill gpg-agent >/dev/null 2>&1 || : ) && gpg --homedir "${GNUPGHOME}" 2>/dev/null --import \ -- cgit v1.2.3 From 975f45b6aa8161ad63ecaa4364a1f49c074de70c Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 26 Mar 2020 15:35:25 +0000 Subject: t/lib-gpg.sh: stop pretending to be a stand-alone script It makes no sense to call `./lib-gpg.sh`. Therefore the hash-bang line is unnecessary. There are other similar instances in `t/`, but they are too far from the context of the enclosing patch series, so they will be addressed separately. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/lib-gpg.sh | 2 -- 1 file changed, 2 deletions(-) (limited to 't') diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh index 11b83b8c24..4ead126835 100755 --- a/t/lib-gpg.sh +++ b/t/lib-gpg.sh @@ -1,5 +1,3 @@ -#!/bin/sh - gpg_version=$(gpg --version 2>&1) if test $? != 127 then -- cgit v1.2.3 From 477dcaddb6dceb0fc5b5064edef460d9d226386e Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 26 Mar 2020 15:35:26 +0000 Subject: tests: do not let lazy prereqs inside `test_expect_*` turn off tracing The `test_expect_*` functions use `test_eval_` and so does `test_run_lazy_prereq_`. If tracing is enabled via the `-x` option, `test_eval_` turns on tracing while evaluating the code block, and turns it off directly after it. This is unwanted for nested invocations. One somewhat surprising example of this is when running a test that calls `test_i18ngrep`: that function requires the `C_LOCALE_OUTPUT` prereq, and that prereq is a lazy one, so it is evaluated via `test_eval_`, the command tracing is turned off, and the test case continues to run _without tracing the commands_. Another somewhat surprising example is when one lazy prereq depends on another lazy prereq: the former will call `test_have_prereq` with the latter one, which in turn calls `test_eval_` and -- you guessed it -- tracing (if enabled) will be turned off _before_ returning to evaluating the other lazy prereq. As we will introduce just such a scenario with the GPG, GPGSM and RFC1991 prereqs, let's fix that by introducing a variable that keeps track of the current trace level: nested `test_eval_` calls will increment and then decrement the level, and only when it reaches 0, the tracing will _actually_ be turned off. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/t0000-basic.sh | 13 +++++++++++++ t/test-lib.sh | 6 ++++-- 2 files changed, 17 insertions(+), 2 deletions(-) (limited to 't') diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh index 3e440c078d..b859721620 100755 --- a/t/t0000-basic.sh +++ b/t/t0000-basic.sh @@ -833,6 +833,19 @@ then exit 1 fi +test_expect_success 'lazy prereqs do not turn off tracing' " + run_sub_test_lib_test lazy-prereq-and-tracing \ + 'lazy prereqs and -x' -v -x <<-\\EOF && + test_lazy_prereq LAZY true + + test_expect_success lazy 'test_have_prereq LAZY && echo trace' + + test_done + EOF + + grep 'echo trace' lazy-prereq-and-tracing/err +" + test_expect_success 'tests clean up even on failures' " run_sub_test_lib_test_err \ failing-cleanup 'Failing tests with cleanup commands' <<-\\EOF && diff --git a/t/test-lib.sh b/t/test-lib.sh index 0ea1e5a05e..529056be49 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -882,6 +882,7 @@ maybe_setup_valgrind () { fi } +trace_level_=0 want_trace () { test "$trace" = t && { test "$verbose" = t || test "$verbose_log" = t @@ -895,7 +896,7 @@ want_trace () { test_eval_inner_ () { # Do not add anything extra (including LF) after '$*' eval " - want_trace && set -x + want_trace && trace_level_=$(($trace_level_+1)) && set -x $*" } @@ -926,7 +927,8 @@ test_eval_ () { test_eval_ret_=$? if want_trace then - set +x + test 1 = $trace_level_ && set +x + trace_level_=$(($trace_level_-1)) fi } 2>/dev/null 4>&2 -- cgit v1.2.3 From b417ec5f22e05e4320b338cd48b280008314a691 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 26 Mar 2020 15:35:27 +0000 Subject: tests: turn GPG, GPGSM and RFC1991 into lazy prereqs The code to set those prereqs is executed completely outside of any `test_eval_` block. As a consequence, its output had to be suppressed so that it does not clutter the output of a regular test script run. Unfortunately, the output *stays* suppressed even when the `--verbose` option is in effect. This hid important output when debugging why the GPG prereq was not enabled in the Windows part of our CI builds. In preparation for fixing that, let's move all of this code into lazy prereqs. The only slightly tricky part is the global environment variable `GNUPGHOME`. Originally, it was configured only when we verified that there is a `gpg` in the `PATH` that we can use. This is now no longer possible, as lazy prereqs are evaluated in a subshell that changes the working directory to a temporary one. Therefore, we simply _always_ set that environment variable: it does not hurt anything because it does not indicate the presence of a working GPG. Side note: it was quite tempting to use a hack that is possible because we do not validate what is passed to `test_lazy_prereq` (and it is therefore possible to "break out" of the lazy_prereq subshell: test_lazy_prereq GPG '...) && GNUPGHOME=... && (...' However, this is rather tricksy hobbitses code, and the current patch is _much_ easier to understand. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/lib-gpg.sh | 102 +++++++++++++++++++++++++++++++++-------------------------- 1 file changed, 57 insertions(+), 45 deletions(-) (limited to 't') diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh index 4ead126835..7a78c562e8 100755 --- a/t/lib-gpg.sh +++ b/t/lib-gpg.sh @@ -1,12 +1,25 @@ -gpg_version=$(gpg --version 2>&1) -if test $? != 127 -then +# We always set GNUPGHOME, even if no usable GPG was found, as +# +# - It does not hurt, and +# +# - we cannot set global environment variables in lazy prereqs because they are +# executed in an eval'ed subshell that changes the working directory to a +# temporary one. + +GNUPGHOME="$PWD/gpghome" +export GNUPGHOME + +test_lazy_prereq GPG ' + gpg_version=$(gpg --version 2>&1) + test $? != 127 || exit 1 + # 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'*) + "gpg (GnuPG) 1.0.6"*) say "Your version of gpg (1.0.6) is too buggy for testing" + exit 1 ;; *) # Available key info: @@ -25,55 +38,54 @@ then # To export ownertrust: # gpg --homedir /tmp/gpghome --export-ownertrust \ # > lib-gpg/ownertrust - mkdir ./gpghome && - chmod 0700 ./gpghome && - GNUPGHOME="$PWD/gpghome" && - export GNUPGHOME && + mkdir "$GNUPGHOME" && + chmod 0700 "$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 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 && - - gpgsm --homedir "${GNUPGHOME}" 2>/dev/null -K | - grep fingerprint: | - cut -d" " -f4 | - tr -d '\n' >"${GNUPGHOME}/trustlist.txt" && - - 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 + --sign -u committer@example.com ;; esac -fi +' + +test_lazy_prereq GPGSM ' + test_have_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 && + + gpgsm --homedir "${GNUPGHOME}" 2>/dev/null -K | + grep fingerprint: | + cut -d" " -f4 | + tr -d "\\n" >"${GNUPGHOME}/trustlist.txt" && + + echo " S relax" >>"${GNUPGHOME}/trustlist.txt" && + echo hello | gpgsm --homedir "${GNUPGHOME}" >/dev/null \ + -u committer@example.com -o /dev/null --sign - 2>&1 +' -if test_have_prereq GPG && - echo | gpg --homedir "${GNUPGHOME}" -b --rfc1991 >/dev/null 2>&1 -then - test_set_prereq RFC1991 -fi +test_lazy_prereq RFC1991 ' + test_have_prereq GPG && + echo | gpg --homedir "${GNUPGHOME}" -b --rfc1991 >/dev/null 2>&1 +' sanitize_pgp() { perl -ne ' -- cgit v1.2.3 From 2b60649113b50d8dd288f4416d234ca27d31565f Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 26 Mar 2020 15:35:28 +0000 Subject: tests: increase the verbosity of the GPG-related prereqs Especially when debugging a test failure that can only be reproduced in the CI build (e.g. when the developer has no access to a macOS machine other than running the tests on a macOS build agent), output should not be suppressed. In the instance of `hi/gpg-prefer-check-signature`, where one GPG-related test failed for no apparent reason, the entire output of `gpg` and `gpgsm` was suppressed, even in verbose mode, leaving interested readers no clue what was going wrong. Let's fix this by no longer redirecting the output not to `/dev/null`. This is now possible because the affected prereqs were turned into lazy ones (and are therefore evaluated via `test_eval_` which respects the `--verbose` option). Note that we _still_ redirect `stdout` to `/dev/null` for those commands that sign their `stdin`, as the output would be binary (and useless anyway, because the reader would not have anything against which to compare the output). Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/lib-gpg.sh | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) (limited to 't') diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh index 7a78c562e8..9fc5241228 100755 --- a/t/lib-gpg.sh +++ b/t/lib-gpg.sh @@ -40,12 +40,12 @@ test_lazy_prereq GPG ' # > lib-gpg/ownertrust mkdir "$GNUPGHOME" && chmod 0700 "$GNUPGHOME" && - (gpgconf --kill gpg-agent >/dev/null 2>&1 || : ) && - gpg --homedir "${GNUPGHOME}" 2>/dev/null --import \ + (gpgconf --kill gpg-agent || : ) && + gpg --homedir "${GNUPGHOME}" --import \ "$TEST_DIRECTORY"/lib-gpg/keyring.gpg && - gpg --homedir "${GNUPGHOME}" 2>/dev/null --import-ownertrust \ + gpg --homedir "${GNUPGHOME}" --import-ownertrust \ "$TEST_DIRECTORY"/lib-gpg/ownertrust && - gpg --homedir "${GNUPGHOME}" /dev/null 2>&1 \ + gpg --homedir "${GNUPGHOME}" /dev/null \ --sign -u committer@example.com ;; esac @@ -68,23 +68,23 @@ test_lazy_prereq GPGSM ' # 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 && + echo | gpgsm --homedir "${GNUPGHOME}" \ + --passphrase-fd 0 --pinentry-mode loopback \ + --import "$TEST_DIRECTORY"/lib-gpg/gpgsm_cert.p12 && - gpgsm --homedir "${GNUPGHOME}" 2>/dev/null -K | - grep fingerprint: | - cut -d" " -f4 | + 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}" >/dev/null \ - -u committer@example.com -o /dev/null --sign - 2>&1 + echo " S relax" >>"${GNUPGHOME}/trustlist.txt" && + echo hello | gpgsm --homedir "${GNUPGHOME}" >/dev/null \ + -u committer@example.com -o /dev/null --sign - ' test_lazy_prereq RFC1991 ' test_have_prereq GPG && - echo | gpg --homedir "${GNUPGHOME}" -b --rfc1991 >/dev/null 2>&1 + echo | gpg --homedir "${GNUPGHOME}" -b --rfc1991 >/dev/null ' sanitize_pgp() { -- cgit v1.2.3