On 09.02.2022 11:20, Todd Zullinger wrote:
Fabian Stelzer wrote:
On 07.02.2022 11:38, Todd Zullinger wrote:
How did it fail for you? It passes all the tests when I've
run it against Fedora and RHEL-based hosts. If it's flaky
on other systems, that would put a damper on doing it this
way. Though it _should_ work.
Sorry for the delays, I'm a bit busy with other things at the moment.
No apologies needed. This is something I worked on back in
November and had yet to send to the list, so I'm the last
person to rush another. :)
I did get an interactive popup asking if I would like to
trust the key when I ran the t4202 test. This never
happened with the old variant.
Interesting. I do have a patch in my gnupg-2.3 series to
reload the gpg agent after changing the trustlist, as the
changes were not picked up prior to that. In my case, I was
running the tests in an environment where gpg could not
prompt me. (It also seems like we should try harder to have
the test suite reject such prompts).
Yes, gpg-agent in general can be problematic for the tests. I'm not familiar
enough with gpg but I don't know if we can get by without it?
--- 8< ---
Subject: [PATCH] t/lib-gpg: reload gpg components after updating trustlist
With gpgsm from gnupg-2.3, the changes to the trustlist.txt do not
appear to be picked up without refreshing the gpg-agent. Use the 'all'
keyword to reload all of the gpg components. The scdaemon is started as
a child of gpg-agent, for example.
We used to have a --kill at this spot, but I removed it in 2e285e7803
(t/lib-gpg: drop redundant killing of gpg-agent, 2019-02-07). It seems
like it might be necessary (again) for 2.3.
Signed-off-by: Todd Zullinger <tmz@xxxxxxxxx>
---
Notes:
An alternative to doing this dance with the trustlist.txt and having to
kill and/or reload the gpg-agent to pick up the change right after the
import in each test might be to make this part of the steps used when
adding/updating/removing certificates in t/lib-gpg.
If not as a one-time affair when a cert is added/update/removed, then
perhaps as a step taken by/in t/lib-gpg.sh only once. It could populate
a gpghome to be copied into the trash dir for each test which used
gpg/gpgsm. I haven't measured the effect of the extra reload precisely,
but I'm sure it's not free.
(For what it's worth, it didn't add any noticeable amount of time to the
full builds/test runs I made while working on this, so it's a seemingly
small cost, at least.)
Also, hello Henning,
Way back, in February 2019¹, when I submitted 2e285e7803 to remove the
"redundant" killing of the gpg-agent, you said:
> Killing the agent once should be enough, i remember manually killing
> it many times as i was looking for a way to generate certs and trust
> (configure gpgsm for the test). That is probably why i copied it over
> in the first place.
As I wrote this patch to partially restore the gpg-agent killing (now
just a reload), I thought this might have been the sort of issue that
you hit while testing.
It could be unrelated, but it sounds quite similar to what I found with
gnupg-2.3 when trying to get it to pick up the trustlist.txt changes. I
thought you might at least enjoy seeing it come back around. :)
¹ <20190208093324.7b17f270@xxxxxxxxxxxxxxxxxxxxxxxxxx>
t/lib-gpg.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index 6bc083ca77..38e2c0f4fb 100644
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -75,6 +75,7 @@ test_lazy_prereq GPGSM '
gpgsm --homedir "${GNUPGHOME}" -K --with-colons |
awk -F ":" "/^fpr:/ {printf \"%s S relax\\n\", \$10}" \
>"${GNUPGHOME}/trustlist.txt" &&
+ (gpgconf --reload all || : ) &&
echo hello | gpgsm --homedir "${GNUPGHOME}" >/dev/null \
-u committer@xxxxxxxxxxx -o /dev/null --sign -
--- 8< ---
This patch fixes it for me.
I have another patch which changes the earlier gpgconf call
which kills gpg-agent to kill all gpg daemons, as there are
some others which could potentially interfere with the
tests:
--- 8< ---
Subject: [PATCH] t/lib-gpg: kill all gpg components, not just gpg-agent
The gpg-agent is one of several processes that newer releases of GnuPG
start automatically. Issue a kill to each of them to ensure they do not
affect separate tests. (Yes, the separate GNUPGHOME should do that
already. If we find that is case, we could drop the --kill entirely.)
In terms of compatibility, the 'all' keyword was added to the --kill &
--reload options in GnuPG 2.1.18. Debian and RHEL are often used as
indicators of how a change might affect older systems we often try to
support.
- Debian Strech (old old stable), which has limited security support
until June 2022, has GnuPG 2.1.18 (or 2.2.x in backports).
- CentOS/RHEL 7, which is supported until June 2024, has GnuPG
2.0.22, which lacks the --kill option, so the change won't have
any impact.
Signed-off-by: Todd Zullinger <tmz@xxxxxxxxx>
---
t/lib-gpg.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index d675698a2d..2bb309a8c1 100644
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -40,7 +40,7 @@ test_lazy_prereq GPG '
# > lib-gpg/ownertrust
mkdir "$GNUPGHOME" &&
chmod 0700 "$GNUPGHOME" &&
- (gpgconf --kill gpg-agent || : ) &&
+ (gpgconf --kill all || : ) &&
gpg --homedir "${GNUPGHOME}" --import \
"$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&
gpg --homedir "${GNUPGHOME}" --import-ownertrust \
--- 8< ---
I have the series in the gpg-misc-fixes branch:
https://github.com/tmzullinger/git/commits/gpg-misc-fixes
(That series does differ in that it has `string_list_split()`
instead of the simpler `strbuf_addch(&gpg_status, '\n');` to
fix the gpgsm parsing.)
Iff you think it would be useful or helpful, I can post that
series.
I have prepared the patch with the simple strstr() matching I can post in a
bit. I would add your two gpg test lib patches to it if thats ok?
Thanks
Thanks,
--
Todd
