bug#74082: guix-home: inconsistencies in log-files of shepherd services

bug#74082: guix-home: inconsistencies in log-files of shepherd services

[Dariqq]

Hi,

There seems to be an inconsistency with the log-files created by home 
shepherd service

- The main shepherd and some other services (dbus, batsignal, unclutter) 
  use .local/state/log/   (resp. $LOCAL_STATE_HOME/log)

- Many other services (kodi, znc, parcimonie, ssh-agent) use the 
shepherd-variable %user-log-dir (from (shepherd support))
which is .local/state/shepherd (resp. $LOCAL_STATE_HOME/shepherd)

which is certainly confusing.

In particular this overrides the default shepherd log file in 
$LOCAL_STATE_HOME/shepherd.

Some consistency would be nice here.

If guix wants to deviate from the shepherd defaults (should the defaults 
be changed instead?, not sure if there is a standard location for user 
level logs) it should probably export this somewhere such that all 
services can easily reuse it.

bug#74082: guix-home: inconsistencies in log-files of shepherd services

[Nicolas Graves]

On 2024-10-29 09:18, Dariqq wrote:

> Hi,
>
> There seems to be an inconsistency with the log-files created by home 
> shepherd service
>
> - The main shepherd and some other services (dbus, batsignal, unclutter) 
>   use .local/state/log/   (resp. $LOCAL_STATE_HOME/log)
>
> - Many other services (kodi, znc, parcimonie, ssh-agent) use the 
> shepherd-variable %user-log-dir (from (shepherd support))
> which is .local/state/shepherd (resp. $LOCAL_STATE_HOME/shepherd)
>
> which is certainly confusing.
>
> In particular this overrides the default shepherd log file in 
> $LOCAL_STATE_HOME/shepherd.
>
> Some consistency would be nice here.
>
> If guix wants to deviate from the shepherd defaults (should the defaults 
> be changed instead?, not sure if there is a standard location for user 
> level logs) it should probably export this somewhere such that all 
> services can easily reuse it.

IIRC, guix complies with shepherd defaults, and other services that use
log are not yet updated (can be verified with a git blame).  In RDE we
chose log instead.

I think this clarification is a welcome contribution, feel free to send
a patch, I'll happily review it!

-- 
Best regards,
Nicolas Graves

bug#74082: [PATCH] home: Use %user-log-dir as the log directory for all services.

[Dariqq]

* gnu/home/services/desktop.scm (home-dbus-shepherd-services): Log to
%user-log-dir.
* gnu/home/services/desktop.scm (home-unclutter-shepherd-services): Same.
* gnu/home/services/pm.scm (home-batsignal-shepherd-services): Same.
* gnu/home/services/shepherd.scm (launch-shepherd-gexp): Don't overwrite
default log-file.

Change-Id: I2742371cbddd1bf4d981efc41f3eae8f148336be
---

This patch fixes the inconsistent use of "LOCAL_STATE_HOME/log/" vs "LOCAL_STATE_HOME/shepherd/" for logfiles by changing the (remaining) shepherd services to the shepherd variable %user-log-dir i.e. LOCAL_STATE_DIR/shepherd.

I also removed the --log-file argument from the autostart shepherd invocation which makes it log to the default location LOCAL_STATE_DIR/shepherd/shepherd.log
and removed creation of the shepherd log dir. (comment says shepherd >= 0.9.2 should handle creation of the directory)
Have only tested it with dbus-service and the autostart shepherd because I dont use the other ones.

There is still another slight inconsistency with other home-services that let-bind the logdir, but this is less of an issue imo.


 gnu/home/services/desktop.scm  | 16 ++++++----------
 gnu/home/services/pm.scm       |  8 +++-----
 gnu/home/services/shepherd.scm | 18 +++++-------------
 3 files changed, 14 insertions(+), 28 deletions(-)

diff --git a/gnu/home/services/desktop.scm b/gnu/home/services/desktop.scm
index dc9de168b7..fc96ce9295 100644
--- a/gnu/home/services/desktop.scm
+++ b/gnu/home/services/desktop.scm
@@ -298,6 +298,7 @@ (define (home-dbus-shepherd-services config)
   (list (shepherd-service
          (documentation "Run the D-Bus daemon in session-specific mode.")
          (provision '(dbus))
+         (modules '((shepherd support)))      ;for '%user-log-dir'
          (start #~(make-forkexec-constructor
                    (list #$(file-append (home-dbus-dbus config)
                                         "/bin/dbus-daemon")
@@ -310,10 +311,7 @@ (define (home-dbus-shepherd-services config)
                    (cons "DBUS_VERBOSE=1"
                          (default-environment-variables))
                    #:log-file
-                   (format #f "~a/log/dbus.log"
-                           (or (getenv "XDG_STATE_HOME")
-                               (format #f "~a/.local/state"
-                                       (getenv "HOME"))))))
+                   (string-append %user-log-dir "/dbus.log")))
          (stop #~(make-kill-destructor)))))
 
 (define (home-dbus-environment-variables config)
@@ -352,7 +350,8 @@ (define (home-unclutter-shepherd-service config)
     ;; Depend on 'x11-display', which sets 'DISPLAY' if an X11 server is
     ;; available, and fails to start otherwise.
     (requirement '(x11-display))
-    (modules '((srfi srfi-1)
+    (modules '((shepherd support) ;for %user-log-dir
+               (srfi srfi-1)
                (srfi srfi-26)))
     (one-shot? #t)
     (start #~(lambda _
@@ -369,11 +368,8 @@ (define (home-unclutter-shepherd-service config)
                 (cons (string-append "DISPLAY=" (getenv "DISPLAY"))
                       (remove (cut string-prefix? "DISPLAY=" <>)
                               (default-environment-variables)))
-                #:log-file (string-append
-                            (or (getenv "XDG_STATE_HOME")
-                                (format #f "~a/.local/state"
-                                        (getenv "HOME")))
-                            "/log/unclutter.log")))))))
+                #:log-file
+                (string-append %user-log-dir "/unclutter.log")))))))
 
 (define home-unclutter-service-type
   (service-type
diff --git a/gnu/home/services/pm.scm b/gnu/home/services/pm.scm
index d8361fd214..00e3138508 100644
--- a/gnu/home/services/pm.scm
+++ b/gnu/home/services/pm.scm
@@ -88,6 +88,7 @@ (define (home-batsignal-shepherd-services config)
     (list (shepherd-service
            (provision '(batsignal))
            (documentation "Run the batsignal battery-watching daemon.")
+           (modules '((shepherd support)))      ;for '%user-log-dir'
            (start #~(make-forkexec-constructor
                      (append (list #$(file-append batsignal "/bin/batsignal")
                                    "-w" (number->string #$warning-level)
@@ -127,11 +128,8 @@ (define (home-batsignal-shepherd-services config)
                              (if #$ignore-missing?
                                  (list "-i")
                                  (list)))
-                     #:log-file (string-append
-                                 (or (getenv "XDG_STATE_HOME")
-                                     (format #f "~a/.local/state"
-                                             (getenv "HOME")))
-                                 "/log/batsignal.log")))
+                     #:log-file
+                     (string-append %user-log-dir "/batsignal.log")))
            (stop #~(make-kill-destructor))))))
 
 (define home-batsignal-service-type
diff --git a/gnu/home/services/shepherd.scm b/gnu/home/services/shepherd.scm
index 5ea8462020..034a7837ef 100644
--- a/gnu/home/services/shepherd.scm
+++ b/gnu/home/services/shepherd.scm
@@ -120,19 +120,11 @@ (define (launch-shepherd-gexp config)
                       (or (getenv "XDG_RUNTIME_DIR")
                           (format #f "/run/user/~a" (getuid)))
                       "/shepherd/socket"))
-              (let* ((state-dir (or (getenv "XDG_STATE_HOME")
-                                    (format #f "~a/.local/state"
-                                            (getenv "HOME"))))
-                     (log-dir (string-append state-dir "/log")))
-                ;; TODO: Remove it, 0.9.2 creates it automatically?
-                ((@ (guix build utils) mkdir-p) log-dir)
-                (system*
-                 #$(file-append shepherd "/bin/shepherd")
-                 "--logfile"
-                 (string-append log-dir "/shepherd.log")
-                 #$@(if silent? '("--silent") '())
-                 "--config"
-                 #$(home-shepherd-configuration-file config)))))
+              (system*
+               #$(file-append shepherd "/bin/shepherd")
+               #$@(if silent? '("--silent") '())
+               "--config"
+               #$(home-shepherd-configuration-file config))))
         #~"")))
 
 (define (reload-configuration-gexp config)

base-commit: 4491dec50a97dbdebd7dd6d41a5596358b155b79
-- 
2.46.0

bug#74082: guix-home: inconsistencies in log-files of shepherd services

[Ludovic Courtès]

Hi,

Dariqq <dariqq@posteo.net> skribis:

> * gnu/home/services/desktop.scm (home-dbus-shepherd-services): Log to
> %user-log-dir.
> * gnu/home/services/desktop.scm (home-unclutter-shepherd-services): Same.
> * gnu/home/services/pm.scm (home-batsignal-shepherd-services): Same.
> * gnu/home/services/shepherd.scm (launch-shepherd-gexp): Don't overwrite
> default log-file.
>
> Change-Id: I2742371cbddd1bf4d981efc41f3eae8f148336be

[...]

> +++ b/gnu/home/services/desktop.scm
> @@ -298,6 +298,7 @@ (define (home-dbus-shepherd-services config)
>    (list (shepherd-service
>           (documentation "Run the D-Bus daemon in session-specific mode.")
>           (provision '(dbus))
> +         (modules '((shepherd support)))      ;for '%user-log-dir'
>           (start #~(make-forkexec-constructor
>                     (list #$(file-append (home-dbus-dbus config)
>                                          "/bin/dbus-daemon")
> @@ -310,10 +311,7 @@ (define (home-dbus-shepherd-services config)
>                     (cons "DBUS_VERBOSE=1"
>                           (default-environment-variables))
>                     #:log-file
> -                   (format #f "~a/log/dbus.log"
> -                           (or (getenv "XDG_STATE_HOME")
> -                               (format #f "~a/.local/state"
> -                                       (getenv "HOME"))))))
> +                   (string-append %user-log-dir "/dbus.log")))

Nicolas, can you confirm that this is what you had in mind?

At first sight it LGTM.

Ludo’.

bug#74082: guix-home: inconsistencies in log-files of shepherd services

[Nicolas Graves]

On 2024-11-11 00:29, Ludovic Courtès wrote:
>
> Nicolas, can you confirm that this is what you had in mind?
>
> At first sight it LGTM.

Sorry for the late answer, this is indeed what I had in mind.  Untested
on my side, but LGTM too.

-- 
Best regards,
Nicolas Graves

bug#74082: guix-home: inconsistencies in log-files of shepherd services

[Ludovic Courtès]

Nicolas Graves <ngraves@ngraves.fr> skribis:

> On 2024-11-11 00:29, Ludovic Courtès wrote:
>>
>> Nicolas, can you confirm that this is what you had in mind?
>>
>> At first sight it LGTM.
>
> Sorry for the late answer, this is indeed what I had in mind.  Untested
> on my side, but LGTM too.

Alright.  Applied, thanks, Dariqq!