bug#54545: [Guix Home] ‘shepherd’ started twice?

["Ludovic Courtès" <ludo@gnu.org>]

Hi,

Andrew Tropin <andrew@trop.in> skribis:

> Prepared a patch series, which fixes the issues and sligthly adjusts the
> way home shepherd reload configuration logic works, now it happens only
> if configuration is changed and also it doesn't try to be smart and
> start a shepherd if it's not started yet.
>
> From d2578f8924217451ca20f0b61fd6f9b9d31c930d Mon Sep 17 00:00:00 2001
> From: Andrew Tropin <andrew@trop.in>
> Date: Tue, 12 Apr 2022 11:30:58 +0300
> Subject: [PATCH 1/3] home: shepherd: Prevent launching the second instance.
>
> * gnu/home/services/shepherd.scm: Prevent launching the second instance.

I applied this one with a convention commit log, including a “Fixes”
line, which is important for traceability.  (Please check the ChangeLog
convention for future patches.)

> From 56d16b4cd511f6837329b888dade0c6d6da4d89d Mon Sep 17 00:00:00 2001
> From: Andrew Tropin <andrew@trop.in>
> Date: Tue, 12 Apr 2022 12:19:50 +0300
> Subject: [PATCH 2/3] home: shepherd: Use run-on-change to reload shepherd
>  config.
>
> * gnu/home/services/shepherd.scm: Add shepherd configuration to
> XDG_CONFIG_HOME and use it instead of full path to the store. It's necessary
> to use run-on-change service.

[...]

> +++ b/gnu/home/services/shepherd.scm
> @@ -105,27 +105,30 @@ (define (launch-shepherd-gexp config)
>                  (system*
>                   #$(file-append shepherd "/bin/shepherd")
>                   "--logfile"
> -                 (string-append log-dir "/shepherd.log")
> -                 "--config"
> -                 #$(home-shepherd-configuration-file services shepherd)))))
> +                 (string-append log-dir "/shepherd.log")))))
>          #~"")))
>  
>  (define (reload-configuration-gexp config)
>    (let* ((shepherd (home-shepherd-configuration-shepherd config))
>           (services (home-shepherd-configuration-services config)))
> -    #~(system*
> -       #$(file-append shepherd "/bin/herd")
> -       "load" "root"
> -       #$(home-shepherd-configuration-file services shepherd))))
> +    #~(when (file-exists?
> +             (string-append
> +              (or (getenv "XDG_RUNTIME_DIR")
> +                  (format #f "/run/user/~a" (getuid)))
> +              "/shepherd/socket"))
> +        (system*
> +         #$(file-append shepherd "/bin/herd")
> +         "load" "root"
> +         #$(home-shepherd-configuration-file services shepherd)))))
>  
> -(define (ensure-shepherd-gexp config)
> -  #~(if (file-exists?
> -         (string-append
> -          (or (getenv "XDG_RUNTIME_DIR")
> -              (format #f "/run/user/~a" (getuid)))
> -          "/shepherd/socket"))
> -        #$(reload-configuration-gexp config)
> -        #$(launch-shepherd-gexp config)))
> +(define (add-shepherd-configuration config)
> +  (let* ((shepherd (home-shepherd-configuration-shepherd config))
> +         (services (home-shepherd-configuration-services config)))
> +    `(("shepherd/init.scm"
> +       ,(home-shepherd-configuration-file services shepherd)))))
> +
> +(define (home-shepherd-run-on-change config)
> +  `(("files/.config/shepherd/init.scm" ,(reload-configuration-gexp config))))

How does this relate to the bug at hand?

  https://issues.guix.gnu.org/54545

As discussed elsewhere, I find it less ambiguous to pass store file
names for configuration files.

That can be a drawback in some cases, for daemons that won’t be able to
load a new config from a different location, but shepherd is not in that
category: it can load a config file via ‘herd load root’ from any place.

Last, it would be nice if we could use the (guix scripts system
reconfigure) machinery like ‘guix system reconfigure’ and ‘guix deploy’
to upgrade services.  An idea for future work.  :-)

> From e80e9fae6f6bcd478fa904aad8eb426da3f42f10 Mon Sep 17 00:00:00 2001
> From: Andrew Tropin <andrew@trop.in>
> Date: Tue, 12 Apr 2022 12:23:26 +0300
> Subject: [PATCH 3/3] home: run-on-first-login: Add a startup message to the
>  script.
>
> gnu/home/services.scm: Add a startup message to the script to make it clear
> when it begins.
> ---
>  gnu/home/services.scm | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/gnu/home/services.scm b/gnu/home/services.scm
> index 49bd6e3555..e2c51910a8 100644
> --- a/gnu/home/services.scm
> +++ b/gnu/home/services.scm
> @@ -344,6 +344,7 @@ (define (compute-on-first-login-script _ gexps)
>       #~(begin
>         (use-modules (guix i18n))
>         #$%initialize-gettext
> +       (display (G_ "Starting run-on-first-login script.\n\n"))

I’m not fond of unconditional low-level logging.  In Guix there’s
relatively little logging and whatever logging there is is controlled by
‘-v’; I think we should follow that approach as much as possible.

Thank you!

Ludo’.