guix-home: inconsistencies in log-files of shepherd services

  • Done
  • quality assurance status badge
Details
3 participants
  • Dariqq
  • Ludovic Courtès
  • Nicolas Graves
Owner
unassigned
Submitted by
Dariqq
Severity
normal
D
D
Dariqq wrote on 29 Oct 10:18 +0100
(address . bug-guix@gnu.org)
fb7b38d7-9227-4fcc-8222-d7f4426fc9df@posteo.net
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.
N
N
Nicolas Graves wrote on 29 Oct 11:36 +0100
877c9rnqlo.fsf@ngraves.fr
On 2024-10-29 09:18, Dariqq wrote:

Toggle quote (24 lines)
> 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
D
D
Dariqq wrote on 29 Oct 13:29 +0100
[PATCH] home: Use %user-log-dir as the log directory for all services.
(address . 74082@debbugs.gnu.org)
e4d92abd5455196228421a9baaf7061dec7dd947.1730204944.git.dariqq@posteo.net
* 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(-)

Toggle diff (107 lines)
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
D
D
Dariqq wrote on 2 Nov 11:24 +0100
control message for bug #74082
(address . control@debbugs.gnu.org)
afdb2b66-2822-41fa-8216-724e78c32785@posteo.net
tags 74082 + patch
quit
L
L
Ludovic Courtès wrote on 11 Nov 00:29 +0100
Re: bug#74082: guix-home: inconsistencies in log-files of shepherd services
87h68evfa6.fsf_-_@gnu.org
Hi,

Dariqq <dariqq@posteo.net> skribis:

Toggle quote (9 lines)
> * 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

[...]

Toggle quote (19 lines)
> +++ 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’.
N
N
Nicolas Graves wrote on 11 Nov 23:10 +0100
87frnx5snb.fsf@ngraves.fr
On 2024-11-11 00:29, Ludovic Courtès wrote:
Toggle quote (5 lines)
>
> 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
L
L
Ludovic Courtès wrote on 20 Nov 22:58 +0100
(name . Nicolas Graves)(address . ngraves@ngraves.fr)
87o729va7q.fsf@gnu.org
Nicolas Graves <ngraves@ngraves.fr> skribis:

Toggle quote (9 lines)
> 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!
Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 74082@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 74082
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch