[bug#61982] [PATCH 2/2] home: services: xdg-base-directories: Deprecate XDG_LOG_HOME.

[Andrew Tropin <andrew@trop.in>]

[-- Attachment #1: Type: text/plain, Size: 9973 bytes --]

On 2023-03-05 15:19, Bruno Victal wrote:

> XDG_LOG_HOME is non-standard and log files should go to XDG_STATE_HOME
> according to the XDG Base Directory Specification.
>
> Fixes <https://issues.guix.gnu.org/61809>.
>
> * gnu/home/services/desktop.scm (home-dbus-shepherd-services): Log to XDG_STATE_HOME.
> * gnu/home/services/mcron.scm (home-mcron-shepherd-services): Ditto.
> * gnu/home/services/pm.scm (home-batsignal-shepherd-services): Ditto.
> * gnu/home/services/shepherd.scm (launch-shepherd-gexp): Ditto.
> * gnu/home/services/xdg.scm
> (home-xdg-base-directories-configuration)[log-home]: Deprecate and unset default value.
> (home-xdg-base-directories-environment-variables-service)
> (ensure-xdg-base-dirs-on-activation): Handle field deprecation.
> (home-xdg-base-directories-service-type): Update description.
> ---
>  gnu/home/services/desktop.scm  |  4 ++--
>  gnu/home/services/mcron.scm    |  4 ++--
>  gnu/home/services/pm.scm       |  6 +++---
>  gnu/home/services/shepherd.scm |  4 ++--
>  gnu/home/services/xdg.scm      | 36 +++++++++++++++++++++++++---------
>  5 files changed, 36 insertions(+), 18 deletions(-)
>
> diff --git a/gnu/home/services/desktop.scm b/gnu/home/services/desktop.scm
> index cb25b03b64..76df7b30a2 100644
> --- a/gnu/home/services/desktop.scm
> +++ b/gnu/home/services/desktop.scm
> @@ -206,8 +206,8 @@ (define (home-dbus-shepherd-services config)
>                           (default-environment-variables))
>                     #:log-file
>                     (format #f "~a/dbus.log"
> -                           (or (getenv "XDG_LOG_HOME")
> -                               (format #f "~a/.local/var/log"
> +                           (or (getenv "XDG_STATE_HOME")
> +                               (format #f "~a/.local/state"

If we deprecate XDG_LOG_HOME, maybe it's a good idea to make a helper
function, which can be reused across different services?  

Also, how about appending /log subdirectory by default?

WDYT?

>                                         (getenv "HOME"))))))
>           (stop #~(make-kill-destructor)))))
>  
> diff --git a/gnu/home/services/mcron.scm b/gnu/home/services/mcron.scm
> index 5f35bfe054..69c7f9faa6 100644
> --- a/gnu/home/services/mcron.scm
> +++ b/gnu/home/services/mcron.scm
> @@ -99,8 +99,8 @@ (define (home-mcron-shepherd-services config)
>                                          #~())
>                                   #$@files)
>                             #:log-file (string-append
> -                                       (or (getenv "XDG_LOG_HOME")
> -                                           (format #f "~a/.local/var/log"
> +                                       (or (getenv "XDG_STATE_HOME")
> +                                           (format #f "~a/.local/state"
>                                                     (getenv "HOME")))
>                                         "/mcron.log")))
>                   (stop #~(make-kill-destructor))
> diff --git a/gnu/home/services/pm.scm b/gnu/home/services/pm.scm
> index 5f09941827..274ab2303c 100644
> --- a/gnu/home/services/pm.scm
> +++ b/gnu/home/services/pm.scm
> @@ -128,9 +128,9 @@ (define (home-batsignal-shepherd-services config)
>                                   (list "-i")
>                                   (list)))
>                       #:log-file (string-append
> -                                 (or (getenv "XDG_LOG_HOME")
> -                                     (format #f "~a/.local/var/log"
> -                                               (getenv "HOME")))
> +                                 (or (getenv "XDG_STATE_HOME")
> +                                     (format #f "~a/.local/state"
> +                                             (getenv "HOME")))
>                                   "/batsignal.log")))
>             (stop #~(make-kill-destructor))))))
>  
> diff --git a/gnu/home/services/shepherd.scm b/gnu/home/services/shepherd.scm
> index 1a70a220f0..825cf66f05 100644
> --- a/gnu/home/services/shepherd.scm
> +++ b/gnu/home/services/shepherd.scm
> @@ -108,8 +108,8 @@ (define (launch-shepherd-gexp config)
>                        (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"
> +              (let ((log-dir (or (getenv "XDG_STATE_HOME")
> +                                 (format #f "~a/.local/state"
>                                           (getenv "HOME")))))
>                  ;; TODO: Remove it, 0.9.2 creates it automatically?
>                  ((@ (guix build utils) mkdir-p) log-dir)
> diff --git a/gnu/home/services/xdg.scm b/gnu/home/services/xdg.scm
> index ac557b4c3d..958772696b 100644
> --- a/gnu/home/services/xdg.scm
> +++ b/gnu/home/services/xdg.scm
> @@ -1,6 +1,7 @@
>  ;;; GNU Guix --- Functional package management for GNU
>  ;;; Copyright © 2021, 2022 Andrew Tropin <andrew@trop.in>
>  ;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
> +;;; Copyright © 2023 Bruno Victal <mirai@makinata.eu>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -22,6 +23,7 @@ (define-module (gnu home services xdg)
>    #:use-module (gnu home services)
>    #:use-module (gnu packages freedesktop)
>    #:use-module (gnu home services utils)
> +  #:use-module (guix deprecation)
>    #:use-module (guix gexp)
>    #:use-module (guix modules)
>    #:use-module (guix records)
> @@ -39,7 +41,7 @@ (define-module (gnu home services xdg)
>              home-xdg-base-directories-configuration-config-home
>              home-xdg-base-directories-configuration-data-home
>              home-xdg-base-directories-configuration-state-home
> -            home-xdg-base-directories-configuration-log-home
> +            home-xdg-base-directories-configuration-log-home  ; deprecated
>              home-xdg-base-directories-configuration-runtime-dir
>  
>              home-xdg-user-directories-service-type
> @@ -77,6 +79,7 @@ (define-module (gnu home services xdg)
>  
>  (define (serialize-path field-name val) "")
>  (define path? string?)
> +(define-maybe path)
>  
>  (define-configuration home-xdg-base-directories-configuration
>    (cache-home
> @@ -97,12 +100,17 @@ (define-configuration home-xdg-base-directories-configuration
>     (path "${XDG_RUNTIME_DIR:-/run/user/$UID}")
>     "Base directory for programs to store user-specific runtime files,
>  like sockets.")
> +  ;; TODO: deprecated field, use $XDG_STATE_HOME(/log) instead.
>    (log-home
> -   (path "$HOME/.local/var/log")
> +   maybe-path
>     "Base directory for programs to store log files, analogus to
>  @file{/var/log}, but for user.  It is not a part of XDG Base Directory
>  Specification, but helps to make implementation of home services more
> -consistent.")
> +consistent."
> +   (lambda (field-name val)
> +     (when (maybe-value-set? val)
> +       (warn-about-deprecation field-name #f #:replacement 'state-home))
> +     (serialize-path field-name val)))
>    (state-home
>     (path "$HOME/.local/state")
>     "Base directory for programs to store state data that should persist
> @@ -117,7 +125,13 @@ (define (home-xdg-base-directories-environment-variables-service config)
>              #f "XDG_~a"
>              (object->snake-case-string (configuration-field-name field) 'upper))
>             ((configuration-field-getter field) config)))
> -   home-xdg-base-directories-configuration-fields))
> +   ;; XXX: deprecated field, remove later
> +   (if (maybe-value-set?
> +        (home-xdg-base-directories-configuration-log-home config))
> +       home-xdg-base-directories-configuration-fields
> +       (filter-configuration-fields
> +        home-xdg-base-directories-configuration-fields
> +        '(log-home) #t))))
>  
>  (define (ensure-xdg-base-dirs-on-activation config)
>    (with-imported-modules '((guix build utils))
> @@ -138,7 +152,14 @@ (define (ensure-xdg-base-dirs-on-activation config)
>                       ;; and will be provided by elogind or other service.
>                       (and (not (string=? "XDG_RUNTIME_DIR" variable))
>                            variable)))
> -                 home-xdg-base-directories-configuration-fields)))))
> +                 ;; XXX: deprecated field, remove later
> +                 (if (maybe-value-set?
> +                      (home-xdg-base-directories-configuration-log-home
> +                       config))
> +                     home-xdg-base-directories-configuration-fields
> +                     (filter-configuration-fields
> +                      home-xdg-base-directories-configuration-fields
> +                      '(log-home) #t)))))))
>  
>  (define (last-extension-or-cfg config extensions)
>    "Picks configuration value from last provided extension.  If there
> @@ -157,10 +178,7 @@ (define home-xdg-base-directories-service-type
>                  (default-value (home-xdg-base-directories-configuration))
>                  (compose identity)
>                  (extend last-extension-or-cfg)
> -                (description "Configure XDG base directories.  This
> -service introduces an additional @env{XDG_LOG_HOME} variable.  It's not
> -a part of XDG specification, at least yet, but are convenient to have,
> -it improves the consistency between different home services.  The
> +                (description "Configure XDG base directories.  The
>  services of this service-type is instantiated by default, to provide
>  non-default value, extend the service-type (using @code{simple-service}
>  for example).")))

Overall LGTM, let me know what you think on a few comments above.

-- 
Best regards,
Andrew Tropin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]