diff mbox series

[bug#73494,v6,1/3] services: activation: Continue on exceptions.

Message ID cffd22df3b0563065d0ec2f617ab883fc3ddc66b.1746283044.git.hako@ultrarare.space
State New
Headers
Series [bug#73494,v6,1/3] services: activation: Continue on exceptions. |

Commit Message

Hilton Chain May 3, 2025, 2:38 p.m. UTC 
* gnu/services.scm (activation-script): Reset environment before loading
activation script.
Catch exception and print the error.
Warn about failed activation script.

Change-Id: I89be31433fbb46d0c4a9dc6115ab167910840b6f
---
 gnu/services.scm | 39 +++++++++++++++++++++++++++++++++++----
 1 file changed, 35 insertions(+), 4 deletions(-)


base-commit: 415e3d98d6faf5fd3d1b7b3daa2f20636e4ff822
prerequisite-patch-id: cb3ac50fb870cf197a4e3ed2ccfb45a6a28400f1
prerequisite-patch-id: 2579801cc89f3e3e022074a0425fba3d38bbe1de
prerequisite-patch-id: 7174912f7b05011468bc47b2ba8167b0e617a70b
prerequisite-patch-id: 08fa38ca1e61c773052671339b78799f19fb7f60
prerequisite-patch-id: b68df6ad7528101d3fbb1fd44f94472a9c8f4f0a
prerequisite-patch-id: 58297e005e4ad0988bd5dc73cd0149637d5c7032
prerequisite-patch-id: 2c09773480fbfd323433986ec528fcb8019273f9
prerequisite-patch-id: e2846320c8b391ff3fec09e51c31578ee6057268
prerequisite-patch-id: 538047ea2f029dd135319c02ba569c4aab9af38d
prerequisite-patch-id: 4fb73799b5ea902d69ecf15b72e1fecc9903d0bc
prerequisite-patch-id: a3a0146e6686bf6f7804192e1a383239c118717b
prerequisite-patch-id: 8de6616d86785be0088bc210202d375b6d0cf5eb
prerequisite-patch-id: 2f32dcab8b97a4fd210ab5b29b6ebf327861be36
prerequisite-patch-id: 081be41a50854a1397ad6574d995ba444b927a7f
prerequisite-patch-id: 2a631555713d3b74f953673f94877e579045f99c
prerequisite-patch-id: b3d778b43f667e73692e1bd8b8945eaf69540fda

Comments

Ludovic Courtès May 3, 2025, 3:48 p.m. UTC | #1 
Hilton Chain <hako@ultrarare.space> writes:

> * gnu/services.scm (activation-script): Reset environment before loading
> activation script.
> Catch exception and print the error.
> Warn about failed activation script.
>
> Change-Id: I89be31433fbb46d0c4a9dc6115ab167910840b6f

[...]

> +                      (for-each (lambda (action)
> +                                  ;; Don't block activation process when one
> +                                  ;; action fails.
> +                                  (guard (condition
> +                                          (else
> +                                           (format (current-error-port) "~a~%"
> +                                                   condition)
> +                                           (warning
> +                                            (G_ "failed to activate '~a'~%")
> +                                            action)))
> +                                    (save-module-excursion
> +                                     (lambda ()
> +                                       (set-current-module
> +                                        (make-fresh-user-module))
> +                                       (primitive-load action)))))
> +                                '#$actions)))))

LGTM.

Ludo'.
Maxim Cournoyer May 8, 2025, 5:54 a.m. UTC | #2 
Hi,

Ludovic Courtès <ludo@gnu.org> writes:

> Hilton Chain <hako@ultrarare.space> writes:
>
>> * gnu/services.scm (activation-script): Reset environment before loading
>> activation script.
>> Catch exception and print the error.
>> Warn about failed activation script.
>>
>> Change-Id: I89be31433fbb46d0c4a9dc6115ab167910840b6f
>
> [...]
>
>> +                      (for-each (lambda (action)
>> +                                  ;; Don't block activation process when one
>> +                                  ;; action fails.
>> +                                  (guard (condition
>> +                                          (else
>> +                                           (format (current-error-port) "~a~%"
>> +                                                   condition)
>> +                                           (warning
>> +                                            (G_ "failed to activate '~a'~%")
>> +                                            action)))
>> +                                    (save-module-excursion
>> +                                     (lambda ()
>> +                                       (set-current-module
>> +                                        (make-fresh-user-module))
>> +                                       (primitive-load action)))))
>> +                                '#$actions)))))
>
> LGTM.

Thanks.  I've pushed just this 1/3 patch so far.  We still need to
understand why the use of shepherd-system-log-service hangs the jami
service (dbus daemon), *only* when /run is mounted on a tmpfs (!).
Ludovic Courtès May 10, 2025, 1:48 p.m. UTC | #3 
Hello,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> Thanks.  I've pushed just this 1/3 patch so far.  We still need to
> understand why the use of shepherd-system-log-service hangs the jami
> service (dbus daemon), *only* when /run is mounted on a tmpfs (!).

I missed that part of the discussion.  How do we know
shepherd-system-log-service is at fault?  Does Jami log over syslog?

Thanks,
Ludo’.
Z572 May 10, 2025, 3:03 p.m. UTC | #4 
Ludovic Courtès <ludo@gnu.org> writes:

> Hello,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>
>> Thanks.  I've pushed just this 1/3 patch so far.  We still need to
>> understand why the use of shepherd-system-log-service hangs the jami
>> service (dbus daemon), *only* when /run is mounted on a tmpfs (!).
>
> I missed that part of the discussion.  How do we know
> shepherd-system-log-service is at fault?  Does Jami log over syslog?

I checked the test log on ci and found that the Jami-related test failed
after switching to shepherd-system-log-service by default. After trying
to switch shepherd-system-log-service back to syslog, the test
succeeded.


>
> Thanks,
> Ludo’.
Maxim Cournoyer May 12, 2025, 7:56 a.m. UTC | #5 
Hi Ludovic,

Ludovic Courtès <ludo@gnu.org> writes:

> Hello,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>
>> Thanks.  I've pushed just this 1/3 patch so far.  We still need to
>> understand why the use of shepherd-system-log-service hangs the jami
>> service (dbus daemon), *only* when /run is mounted on a tmpfs (!).
>
> I missed that part of the discussion.  How do we know
> shepherd-system-log-service is at fault?  Does Jami log over syslog?

By trial and error (I assume?), Z572 discovered that if we replace
shepherd-system-log-service by shepherd-syslog-service the hang goes
away.  See the WIP patch 3/3 of this series that does this replacement
and where the problem disappears.

And yes, the Jami daemon logs to syslog by default with our current
service definition.
Z572 May 13, 2025, 4:23 a.m. UTC | #6 
Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> Hi Ludovic,
>
> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Hello,
>>
>> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>>
>>> Thanks.  I've pushed just this 1/3 patch so far.  We still need to
>>> understand why the use of shepherd-system-log-service hangs the jami
>>> service (dbus daemon), *only* when /run is mounted on a tmpfs (!).
>>
>> I missed that part of the discussion.  How do we know
>> shepherd-system-log-service is at fault?  Does Jami log over syslog?
>
> By trial and error (I assume?), Z572 discovered that if we replace

Thanks to ci.guix.gnu.org. I just checked the corresponding build. Jami
tests failed after switching to shepherd-system-log-service by default.

In the future, we should run system tests when sending patches for
improving services to avoid failures after merging without anyone
noticing

> shepherd-system-log-service by shepherd-syslog-service the hang goes
> away.  See the WIP patch 3/3 of this series that does this replacement
> and where the problem disappears.
>
> And yes, the Jami daemon logs to syslog by default with our current
> service definition.
diff mbox series

Patch

diff --git a/gnu/services.scm b/gnu/services.scm
index 8a4002e072..af054339fd 100644
--- a/gnu/services.scm
+++ b/gnu/services.scm
@@ -692,15 +692,31 @@  (define* (activation-service->script service)
 (define (activation-script gexps)
   "Return the system's activation script, which evaluates GEXPS."
   (define actions
-    (map (cut program-file "activate-service.scm" <>) gexps))
+    ;; TODO: Instead of importing modules here, let users of activation service
+    ;; add them explicitly.  See <https://issues.guix.gnu.org/76698>.
+    (map (lambda (action)
+           (program-file "activate-service.scm"
+                         (with-imported-modules (source-module-closure
+                                                 '((gnu build activation)
+                                                   (guix build utils)))
+                           #~(begin
+                               (use-modules (gnu build activation)
+                                            (guix build utils))
+                               #$action))))
+         gexps))
 
   (program-file "activate.scm"
                 (with-imported-modules (source-module-closure
                                         '((gnu build activation)
-                                          (guix build utils)))
+                                          (guix build utils)
+                                          (guix diagnostics)
+                                          (guix i18n)))
                   #~(begin
                       (use-modules (gnu build activation)
-                                   (guix build utils))
+                                   (guix build utils)
+                                   (guix diagnostics)
+                                   (guix i18n)
+                                   (srfi srfi-34))
 
                       (mkdir-p "/var/run")
                       ;; Make sure the user accounting database exists.  If it
@@ -720,7 +736,22 @@  (define (activation-script gexps)
 
                       ;; Run the services' activation snippets.
                       ;; TODO: Use 'load-compiled'.
-                      (for-each primitive-load '#$actions)))))
+                      (for-each (lambda (action)
+                                  ;; Don't block activation process when one
+                                  ;; action fails.
+                                  (guard (condition
+                                          (else
+                                           (format (current-error-port) "~a~%"
+                                                   condition)
+                                           (warning
+                                            (G_ "failed to activate '~a'~%")
+                                            action)))
+                                    (save-module-excursion
+                                     (lambda ()
+                                       (set-current-module
+                                        (make-fresh-user-module))
+                                       (primitive-load action)))))
+                                '#$actions)))))
 
 (define (gexps->activation-gexp gexps)
   "Return a gexp that runs the activation script containing GEXPS."