Subject: Re: [bug#66525] [PATCH 1/7] gnu: mutter: Remove dependency on (guix build syscalls).

From: Maxim Cournoyer <[email-masked]>
Date: Sat, 14 Oct 2023 08:52:45 -0400

Hi Ludo,

Ludovic Courtès <[email-masked]> writes:

> * gnu/packages/gnome.scm (mutter)[arguments]: Remove #:imported-modules.
> Remove (guix build syscalls) from #:modules.
> Rewrite ‘check’ phase to reap processes from the build process.
> [native-inputs]: Remove TINI.
> ---
>  gnu/packages/gnome.scm | 25 +++++++++++--------------
>  1 file changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/gnu/packages/gnome.scm b/gnu/packages/gnome.scm
> index 908b5782b5..a4993b7aa9 100644
> --- a/gnu/packages/gnome.scm
> +++ b/gnu/packages/gnome.scm
> @@ -7831,10 +7831,7 @@ (define-public mutter
>      (build-system meson-build-system)
>      (arguments
>       (list
> -      #:imported-modules `(,@%meson-build-system-modules
> -                           (guix build syscalls))
>        #:modules '((guix build meson-build-system)
> -                  (guix build syscalls)
>                    (guix build utils)
>                    (ice-9 match))
>        #:glib-or-gtk? #t
> @@ -7928,22 +7925,23 @@ (define-public mutter
>                              "1"))
>                  (match (primitive-fork)
>                    (0                    ;child process
> -                   (set-child-subreaper!)
>                     ;; Use tini so that signals are properly handled and
>                     ;; doubly-forked processes get reaped; otherwise,
>                     ;; python-dbusmock would waste time polling for the dbus
>                     ;; processes it spawns to be reaped, in vain.

As Bruno mentioned, the comment above has gone stale.

> -                   (apply execlp "tini" "--"
> -                          "dbus-run-session" "--"
> +                   (apply execlp "dbus-run-session" "dbus-run-session"
>                            "xvfb-run" "-a" "-s" (getenv "XVFB_SERVER_ARGS")
>                            "meson" "test" "-t" "0" "--print-errorlogs"
>                            test-options))
> -                  (pid
> -                   (match (waitpid pid)
> -                     ((_ . status)
> -                      (unless (zero? status)
> -                        (error "`meson test' exited with status"
> -                               status))))))))))))
> +                  (dbus-pid
> +                   (let loop ()
> +                     (match (waitpid WAIT_ANY)
> +                       ((pid . status)
> +                        (if (= pid dbus-pid)
> +                            (unless (zero? status)
> +                              (error "`meson test' exited with status"
> +                                     status))
> +                            (loop)))))))))))))

Interesting simplification!  I obviously wasn't aware this could work
instead of the more intricate set-child-subreaper! + fake init (tini).
I guess it solves a very narrow subset signal handling behavior compared
to a real init, but that it is sufficient here.

LGTM with comments from Bruno taken into account.

I think it could go to core-updates since we're already prepping the
branch.  It may give some respite to the berlin aarch64 workers, which
have been working non-stop for days or weeks.

-- 
Thanks,
Maxim