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

From: Andrew Tropin <[email-masked]>
Date: Tue, 12 Apr 2022 12:27:55 +0300

On 2022-04-04 22:16, Ludovic Courtès wrote:

> Hi,
>
> Andrew Tropin <[email-masked]> skribis:
>
>> Activation script tries to load latest shepherd configuration with `herd
>> load root ./path/to/config.scm` and it starts a shepherd process.  Login
>> shell starts the shepherd process as well.  Probably we need to do
>> config reload using on-change service and also not trigger on-change
>> stuff if user isn't logged in.
>
> Makes sense.
>
>> I can think on the proper solution and make a patch with a fix later
>> this week or beginning of the next week.
>
> Awesome, thanks for taking a look!
>
> Ludo’.

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 <[email-masked]>
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.
---
 gnu/home/services/shepherd.scm | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/gnu/home/services/shepherd.scm b/gnu/home/services/shepherd.scm
index 012585fea4..df6bbb30e6 100644
--- a/gnu/home/services/shepherd.scm
+++ b/gnu/home/services/shepherd.scm
@@ -93,17 +93,21 @@ (define (launch-shepherd-gexp config)
          (services (home-shepherd-configuration-services config)))
     (if (home-shepherd-configuration-auto-start? config)
         (with-imported-modules '((guix build utils))
-          #~(let ((log-dir (or (getenv "XDG_LOG_HOME")
-                               (format #f "~a/.local/var/log" (getenv "HOME")))))
-              ((@ (guix build utils) mkdir-p) log-dir)
-              (system*
-               #$(file-append shepherd "/bin/shepherd")
-               "--logfile"
-               (string-append
-                log-dir
-                "/shepherd.log")
-               "--config"
-               #$(home-shepherd-configuration-file services shepherd))))
+          #~(unless (file-exists?
+                     (string-append
+                      (or (getenv "XDG_RUNTIME_DIR")
+                          (format #f "/run/user/~a" (getuid)))
+                      "/shepherd/socket"))
+              (let ((log-dir (or (getenv "XDG_LOG_HOME")
+                                 (format #f "~a/.local/var/log"
+                                         (getenv "HOME")))))
+                ((@ (guix build utils) mkdir-p) log-dir)
+                (system*
+                 #$(file-append shepherd "/bin/shepherd")
+                 "--logfile"
+                 (string-append log-dir "/shepherd.log")
+                 "--config"
+                 #$(home-shepherd-configuration-file services shepherd)))))
         #~"")))
 
 (define (reload-configuration-gexp config)
-- 
2.35.1

From 56d16b4cd511f6837329b888dade0c6d6da4d89d Mon Sep 17 00:00:00 2001
From: Andrew Tropin <[email-masked]>
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.
---
 gnu/home/services/shepherd.scm | 40 +++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 17 deletions(-)

diff --git a/gnu/home/services/shepherd.scm b/gnu/home/services/shepherd.scm
index df6bbb30e6..9a99fed2b5 100644
--- a/gnu/home/services/shepherd.scm
+++ 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))))
 
 (define-public home-shepherd-service-type
   (service-type (name 'home-shepherd)
@@ -134,8 +137,11 @@ (define-public home-shepherd-service-type
                         home-run-on-first-login-service-type
                         launch-shepherd-gexp)
                        (service-extension
-                        home-activation-service-type
-                        ensure-shepherd-gexp)
+                        home-xdg-configuration-files-service-type
+                        add-shepherd-configuration)
+                       (service-extension
+                        home-run-on-change-service-type
+                        home-shepherd-run-on-change)
                        (service-extension
                         home-profile-service-type
                         (lambda (config)
-- 
2.35.1

From e80e9fae6f6bcd478fa904aad8eb426da3f42f10 Mon Sep 17 00:00:00 2001
From: Andrew Tropin <[email-masked]>
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"))
 
        (let* ((xdg-runtime-dir (or (getenv "XDG_RUNTIME_DIR")
                                    (format #f "/run/user/~a" (getuid))))
-- 
2.35.1

-- 
Best regards,
Andrew Tropin