Package: guix-patches;
Reported by: Thiago Jung Bauermann <bauermann <at> kolabnow.com>
Date: Tue, 14 Jun 2022 03:33:01 UTC
Severity: normal
Tags: patch
Done: Ludovic Courtès <ludo <at> gnu.org>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 55954 in the body.
You can then email your comments to 55954 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
View this report as an mbox folder, status mbox, maintainer mbox
guix-patches <at> gnu.org:bug#55954; Package guix-patches.
(Tue, 14 Jun 2022 03:33:01 GMT) Full text and rfc822 format available.Thiago Jung Bauermann <bauermann <at> kolabnow.com>:guix-patches <at> gnu.org.
(Tue, 14 Jun 2022 03:33:01 GMT) Full text and rfc822 format available.Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: Thiago Jung Bauermann <bauermann <at> kolabnow.com> To: guix-patches <at> gnu.org Cc: Thiago Jung Bauermann <bauermann <at> kolabnow.com> Subject: [PATCH] gnu: public-inbox: Fixes to allow the testsuite to run Date: Tue, 14 Jun 2022 00:32:23 -0300
This patch makes the public-inbox testsuite pass. Some tests are skipped,
so the test coverage could likely be increased with more massaging.
Perhaps the most significant change is using tini to run the testsuite so
that the testsuite's sub-processes are reaped. The ‘check’ phase is based on
the one from the mutter package. Thanks to Maxim Cournoyer for pointing out
this solution.
* gnu/packages/patches/public-inbox-fix-spawn-test.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add new patch.
* gnu/packages/mail.scm (public-inbox)[source]: Add new patch.
[arguments]<#:tests?>: Remove argument.
<#:imported-modules>: Add argument.
<#:modules>: Likewise.
<#:phases>{qualify-paths}: Substitute path for ‘/bin/cp’.
{pre-check}: Don't skip httpd-unix.t test. Remove unnecessary path
substitutions for “env” and “/bin/sh”.
{check}: Replace with custom version that launches the tests under tini.
[native-inputs]: Add tini.
---
gnu/local.mk | 1 +
gnu/packages/mail.scm | 60 +++++++++++++++----
.../patches/public-inbox-fix-spawn-test.patch | 43 +++++++++++++
3 files changed, 91 insertions(+), 13 deletions(-)
create mode 100644 gnu/packages/patches/public-inbox-fix-spawn-test.patch
diff --git a/gnu/local.mk b/gnu/local.mk
index 03e180cc8508..0653cbb3bc16 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1650,6 +1650,7 @@ dist_patch_DATA = \
%D%/packages/patches/psm-disable-memory-stats.patch \
%D%/packages/patches/psm-ldflags.patch \
%D%/packages/patches/psm-repro.patch \
+ %D%/packages/patches/public-inbox-fix-spawn-test.patch \
%D%/packages/patches/pulseaudio-fix-mult-test.patch \
%D%/packages/patches/pulseaudio-longer-test-timeout.patch \
%D%/packages/patches/pulseview-qt515-compat.patch \
diff --git a/gnu/packages/mail.scm b/gnu/packages/mail.scm
index 8596d0808beb..6f44c6c0f56a 100644
--- a/gnu/packages/mail.scm
+++ b/gnu/packages/mail.scm
@@ -87,6 +87,7 @@ (define-module (gnu packages mail)
#:use-module (gnu packages django)
#:use-module (gnu packages dns)
#:use-module (gnu packages docbook)
+ #:use-module (gnu packages docker)
#:use-module (gnu packages documentation)
#:use-module (gnu packages emacs)
#:use-module (gnu packages enchant)
@@ -4084,10 +4085,16 @@ (define-public public-inbox
(sha256
(base32
"0xni1l54v1z3p0zb52807maay0yqabp8jgf5iras5zmhgjyk3swz"))
- (file-name (git-file-name name version))))
+ (file-name (git-file-name name version))
+ (patches (search-patches "public-inbox-fix-spawn-test.patch"))))
(build-system perl-build-system)
(arguments
- '(#:tests? #f
+ `(#:imported-modules (,@%perl-build-system-modules
+ (guix build syscalls))
+ #:modules ((guix build perl-build-system)
+ (guix build syscalls)
+ (guix build utils)
+ (ice-9 match))
#:phases
(modify-phases %standard-phases
(add-before 'configure 'qualify-paths
@@ -4096,18 +4103,45 @@ (define-public public-inbox
(substitute* "lib/PublicInbox/Xapcmd.pm"
(("'xapian-compact'")
(format #f "'~a'" (search-input-file inputs
- "/bin/xapian-compact"))))))
+ "/bin/xapian-compact"))))
+ (substitute* "lib/PublicInbox/TestCommon.pm"
+ ;; This is only used for tests, but get it from ‘inputs’ so
+ ;; that cross builds won't hold a reference to a package built
+ ;; for another architecture.
+ (("/bin/cp") (search-input-file inputs "/bin/cp")))))
(add-before 'check 'pre-check
(lambda _
- (substitute* "t/spawn.t"
- (("\\['env'\\]") (string-append "['" (which "env") "']")))
- (substitute* "t/ds-leak.t"
- (("/bin/sh") (which "sh")))
- (invoke "./certs/create-certs.perl")
- ;; XXX: This test fails due to zombie process is not reaped by
- ;; the builder.
- (substitute* "t/httpd-unix.t"
- (("^SKIP: \\{") "SKIP: { skip('Guix');"))))
+ (invoke "./certs/create-certs.perl")))
+ (replace 'check
+ (lambda* (#:key target
+ (tests? (not target)) (test-flags '())
+ #:allow-other-keys)
+ (if tests?
+ (match (primitive-fork)
+ (0 ;child process
+ ;; lei tests build UNIX domain sockets in the temporary
+ ;; directory, but the path of those sockets can be at most
+ ;; 108 chars and Guix' default value for the variables
+ ;; below already use 47 chars. Use the shortest temporary
+ ;; path possible to avoid hitting the limit.
+ (setenv "TEMP" "/tmp")
+ (setenv "TEMPDIR" "/tmp")
+ (setenv "TMP" "/tmp")
+ (setenv "TMPDIR" "/tmp")
+
+ ;; Use tini so that signals are properly handled and
+ ;; doubly-forked processes get reaped; otherwise,
+ ;; lei-daemon is kept as a zombie and the testsuite
+ ;; fails thinking that it didn't quit as it should.
+ (set-child-subreaper!)
+ (apply execlp "tini" "--"
+ "make" "check" test-flags))
+ (pid
+ (match (waitpid pid)
+ ((_ . status)
+ (unless (zero? status)
+ (error "`make check' exited with status" status))))))
+ (format #t "test suite not run~%"))))
(add-after 'install 'wrap-programs
(lambda* (#:key inputs outputs #:allow-other-keys)
(let ((out (assoc-ref outputs "out")))
@@ -4126,7 +4160,7 @@ (define-public public-inbox
(find-files (string-append out "/bin")))))))))
(native-inputs
(list ;; For testing.
- lsof openssl))
+ lsof openssl tini))
(inputs
(list bash-minimal
curl
diff --git a/gnu/packages/patches/public-inbox-fix-spawn-test.patch b/gnu/packages/patches/public-inbox-fix-spawn-test.patch
new file mode 100644
index 000000000000..2739b1974de8
--- /dev/null
+++ b/gnu/packages/patches/public-inbox-fix-spawn-test.patch
@@ -0,0 +1,43 @@
+From 5593489d9c3ce22b1942f35c7ebb0e06fcf2bfa8 Mon Sep 17 00:00:00 2001
+From: Thiago Jung Bauermann <bauermann <at> kolabnow.com>
+Date: Fri, 10 Jun 2022 12:39:18 -0300
+Subject: [PATCH] t/spawn: Find invalid PID to try to join its process group
+
+In the container used to build packages of the GNU Guix distribution, PID 1
+runs as the same user as the test so this spawn that should fail actually
+succeeds.
+
+Fix the problem by going through different PIDs and picking one that
+either doesn't exist or we aren't allowed to signal.
+---
+
+This patch is taken from the public-inbox repository and will appear in the
+release after v1.8.
+
+ t/spawn.t | 13 ++++++++++++-
+ 1 file changed, 12 insertions(+), 1 deletion(-)
+
+diff --git a/t/spawn.t b/t/spawn.t
+index 6168c1f6171c..5fc99a2a101c 100644
+--- a/t/spawn.t
++++ b/t/spawn.t
+@@ -24,7 +24,18 @@ SKIP: {
+ is(waitpid($pid, 0), $pid, 'waitpid succeeds on spawned process');
+ is($?, 0, 'true exited successfully');
+ pipe(my ($r, $w)) or BAIL_OUT;
+- $pid = eval { spawn(['true'], undef, { pgid => 1, 2 => $w }) };
++
++ # Find invalid PID to try to join its process group.
++ my $wrong_pgid = 1;
++ for (my $i=0x7fffffff; $i >= 2; $i--) {
++ if (kill(0, $i) == 0) {
++ $wrong_pgid = $i;
++ last;
++ }
++ }
++
++ # Test spawn behavior when it can't join the requested process group.
++ $pid = eval { spawn(['true'], undef, { pgid => $wrong_pgid, 2 => $w }) };
+ close $w;
+ my $err = do { local $/; <$r> };
+ # diag "$err ($@)";
base-commit: 965d54e344dcd19adf3c32f8b9d2dcab62b91e6a
guix-patches <at> gnu.org:bug#55954; Package guix-patches.
(Thu, 16 Jun 2022 09:29:01 GMT) Full text and rfc822 format available.Message #8 received at 55954 <at> debbugs.gnu.org (full text, mbox):
From: Ludovic Courtès <ludo <at> gnu.org> To: Thiago Jung Bauermann <bauermann <at> kolabnow.com> Cc: 55954 <at> debbugs.gnu.org Subject: Re: bug#55954: [PATCH] gnu: public-inbox: Fixes to allow the testsuite to run Date: Thu, 16 Jun 2022 11:27:44 +0200
Hi,
Thiago Jung Bauermann <bauermann <at> kolabnow.com> skribis:
> This patch makes the public-inbox testsuite pass. Some tests are skipped,
> so the test coverage could likely be increased with more massaging.
>
> Perhaps the most significant change is using tini to run the testsuite so
> that the testsuite's sub-processes are reaped. The ‘check’ phase is based on
> the one from the mutter package. Thanks to Maxim Cournoyer for pointing out
> this solution.
>
> * gnu/packages/patches/public-inbox-fix-spawn-test.patch: New file.
> * gnu/local.mk (dist_patch_DATA): Add new patch.
> * gnu/packages/mail.scm (public-inbox)[source]: Add new patch.
> [arguments]<#:tests?>: Remove argument.
> <#:imported-modules>: Add argument.
> <#:modules>: Likewise.
> <#:phases>{qualify-paths}: Substitute path for ‘/bin/cp’.
> {pre-check}: Don't skip httpd-unix.t test. Remove unnecessary path
> substitutions for “env” and “/bin/sh”.
> {check}: Replace with custom version that launches the tests under tini.
> [native-inputs]: Add tini.
Applied, thanks!
[...]
> + (replace 'check
> + (lambda* (#:key target
> + (tests? (not target)) (test-flags '())
> + #:allow-other-keys)
> + (if tests?
> + (match (primitive-fork)
> + (0 ;child process
> + ;; lei tests build UNIX domain sockets in the temporary
> + ;; directory, but the path of those sockets can be at most
> + ;; 108 chars and Guix' default value for the variables
> + ;; below already use 47 chars. Use the shortest temporary
> + ;; path possible to avoid hitting the limit.
> + (setenv "TEMP" "/tmp")
> + (setenv "TEMPDIR" "/tmp")
> + (setenv "TMP" "/tmp")
> + (setenv "TMPDIR" "/tmp")
> +
> + ;; Use tini so that signals are properly handled and
> + ;; doubly-forked processes get reaped; otherwise,
> + ;; lei-daemon is kept as a zombie and the testsuite
> + ;; fails thinking that it didn't quit as it should.
> + (set-child-subreaper!)
> + (apply execlp "tini" "--"
> + "make" "check" test-flags))
> + (pid
> + (match (waitpid pid)
> + ((_ . status)
> + (unless (zero? status)
> + (error "`make check' exited with status" status))))))
It’s a bummer that we have to do all this. Would this work:
(replace 'check
(lambda _
(sigaction SIGINT SIG_DFL)
(sigaction SIGTERM SIG_DFL)
(sigaction SIGCHLD (lambda _ (waitpid WAIT_ANY WNOHANG)))
;; Ugly hack to make sure signal handler asyncs get a chance
;; to run while we’re in ‘waitpid’ during the “make check”
;; invocation.
(sigaction SIGALRM (lambda _ (alarm 1)))
(alarm 1)))
?
If not, the solution you propose LGTM.
Thanks,
Ludo’.
guix-patches <at> gnu.org:bug#55954; Package guix-patches.
(Fri, 17 Jun 2022 03:01:02 GMT) Full text and rfc822 format available.Message #11 received at 55954 <at> debbugs.gnu.org (full text, mbox):
From: Thiago Jung Bauermann <bauermann <at> kolabnow.com> To: Ludovic Courtès <ludo <at> gnu.org> Cc: 55954 <at> debbugs.gnu.org Subject: Re: bug#55954: [PATCH] gnu: public-inbox: Fixes to allow the testsuite to run Date: Thu, 16 Jun 2022 23:38:50 -0300
Hello Ludo,
Thank you for your review!
Ludovic Courtès <ludo <at> gnu.org> writes:
> Hi,
>
> Thiago Jung Bauermann <bauermann <at> kolabnow.com> skribis:
>
>> This patch makes the public-inbox testsuite pass. Some tests are skipped,
>> so the test coverage could likely be increased with more massaging.
>>
>> Perhaps the most significant change is using tini to run the testsuite so
>> that the testsuite's sub-processes are reaped. The ‘check’ phase is based on
>> the one from the mutter package. Thanks to Maxim Cournoyer for pointing out
>> this solution.
>>
>> * gnu/packages/patches/public-inbox-fix-spawn-test.patch: New file.
>> * gnu/local.mk (dist_patch_DATA): Add new patch.
>> * gnu/packages/mail.scm (public-inbox)[source]: Add new patch.
>> [arguments]<#:tests?>: Remove argument.
>> <#:imported-modules>: Add argument.
>> <#:modules>: Likewise.
>> <#:phases>{qualify-paths}: Substitute path for ‘/bin/cp’.
>> {pre-check}: Don't skip httpd-unix.t test. Remove unnecessary path
>> substitutions for “env” and “/bin/sh”.
>> {check}: Replace with custom version that launches the tests under tini.
>> [native-inputs]: Add tini.
>
> Applied, thanks!
I guess you had second thoughts. :-)
>> + (replace 'check
>> + (lambda* (#:key target
>> + (tests? (not target)) (test-flags '())
>> + #:allow-other-keys)
>> + (if tests?
>> + (match (primitive-fork)
>> + (0 ;child process
>> + ;; lei tests build UNIX domain sockets in the temporary
>> + ;; directory, but the path of those sockets can be at most
>> + ;; 108 chars and Guix' default value for the variables
>> + ;; below already use 47 chars. Use the shortest temporary
>> + ;; path possible to avoid hitting the limit.
>> + (setenv "TEMP" "/tmp")
>> + (setenv "TEMPDIR" "/tmp")
>> + (setenv "TMP" "/tmp")
>> + (setenv "TMPDIR" "/tmp")
>> +
>> + ;; Use tini so that signals are properly handled and
>> + ;; doubly-forked processes get reaped; otherwise,
>> + ;; lei-daemon is kept as a zombie and the testsuite
>> + ;; fails thinking that it didn't quit as it should.
>> + (set-child-subreaper!)
>> + (apply execlp "tini" "--"
>> + "make" "check" test-flags))
>> + (pid
>> + (match (waitpid pid)
>> + ((_ . status)
>> + (unless (zero? status)
>> + (error "`make check' exited with status" status))))))
>
> It’s a bummer that we have to do all this. Would this work:
>
> (replace 'check
> (lambda _
> (sigaction SIGINT SIG_DFL)
> (sigaction SIGTERM SIG_DFL)
> (sigaction SIGCHLD (lambda _ (waitpid WAIT_ANY WNOHANG)))
>
> ;; Ugly hack to make sure signal handler asyncs get a chance
> ;; to run while we’re in ‘waitpid’ during the “make check”
> ;; invocation.
> (sigaction SIGALRM (lambda _ (alarm 1)))
> (alarm 1)))
>
> ?
I assume there should be a "make check" in there somewhere. :-)
Unfortunately, it didn't work. This is the check phase I tried:
--8<---------------cut here---------------start------------->8---
(replace 'check
(lambda _
(setenv "TEMP" "/tmp")
(setenv "TEMPDIR" "/tmp")
(setenv "TMP" "/tmp")
(setenv "TMPDIR" "/tmp")
(sigaction SIGINT SIG_DFL)
(sigaction SIGTERM SIG_DFL)
(sigaction SIGCHLD (lambda _ (waitpid WAIT_ANY WNOHANG)))
;; Ugly hack to make sure signal handler asyncs get a chance
;; to run while we’re in ‘waitpid’ during the “make check”
;; invocation.
(sigaction SIGALRM (lambda _ (alarm 1)))
(alarm 1)
(unless (zero? (status:exit-val (system* "make" "check")))
(error "`make check' exited with error"))))
--8<---------------cut here---------------end--------------->8---
And this was the end of the output of the phase:
--8<---------------cut here---------------start------------->8---
⋮
Bailout called. Further testing stopped: daemon still running (PID:5282)
FAILED--Further testing stopped: daemon still running (PID:5282)
make: *** [Makefile:4141: check-each] Error 255
error: in phase 'check': uncaught exception:
misc-error #f "~A" ("`make check' exited with error") #f
phase `check' failed after 27.7 seconds
--8<---------------cut here---------------end--------------->8---
“daemon still running” means that lei's daemon process wasn't reaped.
> If not, the solution you propose LGTM.
Thanks. Hopefully this can be fixed in Guix's build machinery itself in
the core-updates branch and all the packages that are using this hack
can drop it then.
--
Thanks
Thiago
Ludovic Courtès <ludo <at> gnu.org>:Thiago Jung Bauermann <bauermann <at> kolabnow.com>:Message #16 received at 55954-done <at> debbugs.gnu.org (full text, mbox):
From: Ludovic Courtès <ludo <at> gnu.org> To: Thiago Jung Bauermann <bauermann <at> kolabnow.com> Cc: 55954-done <at> debbugs.gnu.org Subject: Re: bug#55954: [PATCH] gnu: public-inbox: Fixes to allow the testsuite to run Date: Fri, 17 Jun 2022 14:44:09 +0200
Hello,
Thiago Jung Bauermann <bauermann <at> kolabnow.com> skribis:
> Ludovic Courtès <ludo <at> gnu.org> writes:
[...]
>> Applied, thanks!
>
> I guess you had second thoughts. :-)
Yes, sorry; I wasn’t sure whether bothering you was a good idea, but I
was also dissatisfied with that whole situation (which is not
public-inbox’s fault!).
>> It’s a bummer that we have to do all this. Would this work:
>>
>> (replace 'check
>> (lambda _
>> (sigaction SIGINT SIG_DFL)
>> (sigaction SIGTERM SIG_DFL)
>> (sigaction SIGCHLD (lambda _ (waitpid WAIT_ANY WNOHANG)))
>>
>> ;; Ugly hack to make sure signal handler asyncs get a chance
>> ;; to run while we’re in ‘waitpid’ during the “make check”
>> ;; invocation.
>> (sigaction SIGALRM (lambda _ (alarm 1)))
>> (alarm 1)))
>>
>> ?
>
> I assume there should be a "make check" in there somewhere. :-)
>
> Unfortunately, it didn't work. This is the check phase I tried:
>
> (replace 'check
> (lambda _
> (setenv "TEMP" "/tmp")
> (setenv "TEMPDIR" "/tmp")
> (setenv "TMP" "/tmp")
> (setenv "TMPDIR" "/tmp")
>
> (sigaction SIGINT SIG_DFL)
> (sigaction SIGTERM SIG_DFL)
> (sigaction SIGCHLD (lambda _ (waitpid WAIT_ANY WNOHANG)))
>
> ;; Ugly hack to make sure signal handler asyncs get a chance
> ;; to run while we’re in ‘waitpid’ during the “make check”
> ;; invocation.
> (sigaction SIGALRM (lambda _ (alarm 1)))
> (alarm 1)
> (unless (zero? (status:exit-val (system* "make" "check")))
> (error "`make check' exited with error"))))
>
>
> And this was the end of the output of the phase:
>
> ⋮
> Bailout called. Further testing stopped: daemon still running (PID:5282)
> FAILED--Further testing stopped: daemon still running (PID:5282)
> make: *** [Makefile:4141: check-each] Error 255
> error: in phase 'check': uncaught exception:
> misc-error #f "~A" ("`make check' exited with error") #f
> phase `check' failed after 27.7 seconds
>
> “daemon still running” means that lei's daemon process wasn't reaped.
Damnit, I wonder how that can happen.
>> If not, the solution you propose LGTM.
>
> Thanks. Hopefully this can be fixed in Guix's build machinery itself in
> the core-updates branch and all the packages that are using this hack
> can drop it then.
Yeah.
Anyway, applied for good this time, thanks for taking extra time on
this!
Ludo’.
Debbugs Internal Request <help-debbugs <at> gnu.org>
to internal_control <at> debbugs.gnu.org.
(Sat, 16 Jul 2022 11:24:09 GMT) Full text and rfc822 format available.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.