threads / patch / 50783

v2t/lib-gpg.sh: fix GPG keyring import options

Subject: [PATCH v2 1/1] t/lib-gpg.sh: fix GPG keyring import options

## tl;dr

3 messages between Mar 19, 2019 and Mar 20, 2019. Diffs are folded; open one to read it.

replies: 2people: 3as markdown or json

Dave Huseby· Mar 19, 2019, 23:20 UTC · lore

Fix the way GPG keyrings are imported during testing to prevent GPG from prompting for approval to change the default config. This appears to have no adverse affects on GPG users with "normal" configurations but fixes the always-interactive prompting I see with my multi-keyring setup.

v2: fixed the whitespace.
Signed-off-by: Dave Huseby <dhuseby@linuxfoundation.org>
---
 t/lib-gpg.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to t/lib-gpg.sh +2 −2
diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index 8d28652b72..e72e0554f1 100755
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -32,8 +32,8 @@ then
 		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 --import-options merge-only --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 \
-- 
2.11.0
Junio C Hamano· Mar 20, 2019, 01:15 UTC · re: Dave Huseby · lore

Re: [PATCH v2 1/1] t/lib-gpg.sh: fix GPG keyring import options

Dave Huseby <dhuseby@linuxfoundation.org> writes:
> Fix the way GPG keyrings are imported during testing to prevent GPG from 
> prompting for approval to change the default config. This appears to have no
> adverse affects on GPG users with "normal" configurations but fixes the
> always-interactive prompting I see with my multi-keyring setup.

I do not quite follow. If you are personally setting "always interactive" somehow, would it be possible to solve the issue more directly by unsetting "always interactive" here? After all, the "${GNUPGHOME}" we see in the test helper library is not referring to your setting but our throw-away setting used only during the test, so...

> v2: fixed the whitespace.
This line goes below "---"
Show 18 quoted lines
>
> Signed-off-by: Dave Huseby <dhuseby@linuxfoundation.org>
> ---
>  t/lib-gpg.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
> index 8d28652b72..e72e0554f1 100755
> --- a/t/lib-gpg.sh
> +++ b/t/lib-gpg.sh
> @@ -32,8 +32,8 @@ then
>  		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 --import-options merge-only --homedir "${GNUPGHOME}" 2>/dev/null \
> +			--import "$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&

This patch, without any explanation in the proposed log message, gives an impression that '--import-options merge-only' somehow must be inserted before '--homedir "${GNUPGHOME}"'. If that is truly required, the reason why it must be done before --homedir needs to be explained in the log message. If not, inserting before --homedir is making the patch unnecessarily misleading. If the lines were like so:

- gpg --homedir "${GNUPGHOME}" 2>/dev/null --import \ - "$TEST_DIRECTORY"/lib-gpg/keyring.gpg && + gpg --homedir "${GNUPGHOME}" 2>/dev/null \ + --import-options merge-only \ + --import "$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&

the patch wouldn't have forced readers wonder why the new option has to come first.

But as I said earlier, there needs a better explanation why it helps to have --import-options to squelch prompts---the cause-effect linkage is a bit too weak to make a convincing case to support why this is the right fix to a problem worth fixing.

Thanks.
>  		gpg --homedir "${GNUPGHOME}" 2>/dev/null --import-ownertrust \
>  			"$TEST_DIRECTORY"/lib-gpg/ownertrust &&
>  		gpg --homedir "${GNUPGHOME}" </dev/null >/dev/null 2>&1 \
SZEDER Gábor· Mar 20, 2019, 11:52 UTC · re: Dave Huseby · lore

Re: [PATCH v2 1/1] t/lib-gpg.sh: fix GPG keyring import options

On Tue, Mar 19, 2019 at 04:20:13PM -0700, Dave Huseby wrote:
Show 24 quoted lines
> Fix the way GPG keyrings are imported during testing to prevent GPG from 
> prompting for approval to change the default config. This appears to have no
> adverse affects on GPG users with "normal" configurations but fixes the
> always-interactive prompting I see with my multi-keyring setup.
> 
> v2: fixed the whitespace.
> 
> Signed-off-by: Dave Huseby <dhuseby@linuxfoundation.org>
> ---
>  t/lib-gpg.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
> index 8d28652b72..e72e0554f1 100755
> --- a/t/lib-gpg.sh
> +++ b/t/lib-gpg.sh
> @@ -32,8 +32,8 @@ then
>  		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 --import-options merge-only --homedir "${GNUPGHOME}" 2>/dev/null \
> +			--import "$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&

Normally an Ubuntu 16.04 LTS based box can run all tests with the GPG prereq just fine, but with this patch it can't.

After making 'lib-gpg.sh' a bit more informative by adding 'set -x' and removing the 'gpg ... 2>/dev/null' redirections it appears that with this '--import-options merge-only' GPG can't find the keys:

  $ ./t7612-merge-verify-signatures.sh -V -x
  + gpg --version
  + gpg_version=gpg (GnuPG) 1.4.20
  Copyright (C) 2015 Free Software Foundation, Inc.
  License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
  This is free software: you are free to change and redistribute it.
  There is NO WARRANTY, to the extent permitted by law.
  
  Home: /home/szeder/src/git/t/trash directory.t7612-merge-verify-signatures/gnupg-home-not-used
  Supported algorithms:
  Pubkey: RSA, RSA-E, RSA-S, ELG-E, DSA
  Cipher: IDEA, 3DES, CAST5, BLOWFISH, AES, AES192, AES256, TWOFISH,
          CAMELLIA128, CAMELLIA192, CAMELLIA256
  Hash: MD5, SHA1, RIPEMD160, SHA256, SHA384, SHA512, SHA224
  Compression: Uncompressed, ZIP, ZLIB, BZIP2
  + test 0 != 127
  + mkdir ./gpghome
  + chmod 0700 ./gpghome
  + pwd
  + GNUPGHOME=/home/szeder/src/git/t/trash directory.t7612-merge-verify-signatures/gpghome
  + export GNUPGHOME
  + gpgconf --kill gpg-agent
  + gpg --import-options merge-only --homedir /home/szeder/src/git/t/trash directory.t7612-merge-verify-signatures/gpghome --import /home/szeder/src/git/t/lib-gpg/keyring.gpg
  gpg: keyring `/home/szeder/src/git/t/trash directory.t7612-merge-verify-signatures/gpghome/secring.gpg' created
  gpg: keyring `/home/szeder/src/git/t/trash directory.t7612-merge-verify-signatures/gpghome/pubring.gpg' created
  gpg: key CDDE430D: secret key not found: secret key not available
  gpg: error reading `/home/szeder/src/git/t/lib-gpg/keyring.gpg': secret key not available
  gpg: import from `/home/szeder/src/git/t/lib-gpg/keyring.gpg' failed: secret key not available
  gpg: Total number processed: 0
  gpg:       secret keys read: 1
  + test_have_prereq GPG
  + save_IFS=     
  
  + IFS=,
  + set -- GPG
  + IFS=  
  
  + total_prereq=0
  + ok_prereq=0
  + missing_prereq=
  + negative_prereq=
  + total_prereq=1
  + satisfied_this_prereq=
  + prerequisite=GPG
  + test -z 
  + missing_prereq=GPG
  + test 1 = 0
  + set +x
  ok 1 # skip create signed commits (missing GPG)
  [....]

← back to recent threads