Re: [PATCHES] Add elogind service

[Thompson, David]

On Tue, Sep 1, 2015 at 7:42 AM, Andy Wingo <address@hidden> wrote:
> Attached are two patches.  The first updates elogind to a current
> release tarball.  The second adds a service.  These are the bottom two
> patches in wip-pam-elogind.  If you run the service, you'll at least
> have lid suspend support.  Some patches more patches to follow but these
> are the only elogind ones.  OK to commit?
>
> From fdd8893f287696fb016e9e78fbcba04d03840ee6 Mon Sep 17 00:00:00 2001
> From: Andy Wingo <address@hidden>
> Date: Tue, 18 Aug 2015 10:05:24 +0200
> Subject: [PATCH 1/7] gnu: elogind: Update to version 219.12.
>
> * gnu/packages/freedesktop.scm (elogind): Update to 219.12.  Use a tarball
>   instead of a git checkout.

LGTM.

> From 4f32d646cf14133a98899c448d588088c80d680d Mon Sep 17 00:00:00 2001
> From: Andy Wingo <address@hidden>
> Date: Tue, 18 Aug 2015 11:56:17 +0200
> Subject: [PATCH 2/7] gnu: Add elogind service.
>
> * gnu/services/desktop.scm (elogind-configuration-file, elogind-service): New
>   functions.
>   (%desktop-services): Add elogind-service.
> ---
>  gnu/services/desktop.scm | 172 
> ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 171 insertions(+), 1 deletion(-)
>
> diff --git a/gnu/services/desktop.scm b/gnu/services/desktop.scm
> index 4e4b49d..764954c 100644
> --- a/gnu/services/desktop.scm
> +++ b/gnu/services/desktop.scm
> @@ -26,6 +26,7 @@
>    #:use-module (gnu system shadow)
>    #:use-module (gnu packages glib)
>    #:use-module (gnu packages admin)
> +  #:use-module (gnu packages freedesktop)
>    #:use-module (gnu packages gnome)
>    #:use-module (gnu packages avahi)
>    #:use-module (gnu packages wicd)
> @@ -39,6 +40,7 @@
>              geoclue-application
>              %standard-geoclue-applications
>              geoclue-service
> +            elogind-service
>              %desktop-services))
>
>  ;;; Commentary:
> @@ -374,6 +376,173 @@ site} for more information."
>
>
>  ;;;
> +;;; Elogind login and seat management service.
> +;;;
> +
> +(define (missing-keyword-argument name)
> +  (error "missing keyword argument:" name))
> +
> +(define-syntax-rule (define-with-required-kwargs name (arg ...)
> +                      body ...)
> +  (define name (lambda* (#:key (arg (missing-keyword-argument 'arg)) ...)
> +                 body ...)))
> +
> +(define-with-required-kwargs elogind-configuration-file
> +  (kill-user-processes? kill-only-users kill-exclude-users
> +   inhibit-delay-max-seconds
> +   handle-power-key handle-suspend-key handle-hibernate-key
> +   handle-lid-switch handle-lid-switch-docked
> +   power-key-ignore-inhibited? suspend-key-ignore-inhibited?
> +   hibernate-key-ignore-inhibited? lid-switch-ignore-inhibited?
> +   holdoff-timeout-seconds
> +   idle-action idle-action-seconds
> +   runtime-directory-size-percent runtime-directory-size
> +   remove-ipc?
> +   suspend-state suspend-mode
> +   hibernate-state hibernate-mode
> +   hybrid-sleep-state hybrid-sleep-mode)
> +  (define (yesno x)
> +    (match x
> +      (#t "yes")
> +      (#f "no")
> +      (_ (error "expected #t or #f, instead got:" x))))
> +  (define char-set:user-name
> +    (string->char-set "abcdefghijklmnopqrstuvwxyz0123456789_-"))
> +  (define (valid-list? l pred)
> +    (and-map (lambda (x) (string-every pred x)) l))
> +  (define (user-name-list users)
> +    (unless (valid-list? users char-set:user-name)
> +      (error "invalid user list" users))
> +    (string-join users " "))
> +  (define (enum val allowed)
> +    (unless (memq val allowed)
> +      (error "invalid value" val allowed))
> +    (symbol->string val))
> +  (define (non-negative-integer x)
> +    (unless (exact-integer? x) (error "not an integer" x))
> +    (when (negative? x) (error "negative number not allowed" x))
> +    (number->string x))
> +  (define handle-actions
> +    '(ignore poweroff reboot halt kexec suspend hibernate hybrid-sleep lock))
> +  (define (handle-action x)
> +    (enum x handle-actions))
> +  (define (sleep-list tokens)
> +    (unless (valid-list? tokens char-set:user-name)
> +      (error "invalid sleep list" tokens))
> +    (string-join tokens " "))
> +  (text-file
> +   "logind.conf"
> +   (string-append
> +    "[Login]\n"
> +    "KillUserProcesses=" (yesno kill-user-processes?) "\n"
> +    "KillOnlyUsers=" (user-name-list kill-only-users) "\n"
> +    "KillExcludeUsers=" (user-name-list kill-exclude-users) "\n"
> +    "InhibitDelayMaxSecs=" (non-negative-integer inhibit-delay-max-seconds) 
> "\n"
> +    "HandlePowerKey=" (handle-action handle-power-key) "\n"
> +    "HandleSuspendKey=" (handle-action handle-suspend-key) "\n"
> +    "HandleHibernateKey=" (handle-action handle-hibernate-key) "\n"
> +    "HandleLidSwitch=" (handle-action handle-lid-switch) "\n"
> +    "HandleLidSwitchDocked=" (handle-action handle-lid-switch-docked) "\n"
> +    "PowerKeyIgnoreInhibited=" (yesno power-key-ignore-inhibited?) "\n"
> +    "SuspendKeyIgnoreInhibited=" (yesno suspend-key-ignore-inhibited?) "\n"
> +    "HibernateKeyIgnoreInhibited=" (yesno hibernate-key-ignore-inhibited?) 
> "\n"
> +    "LidSwitchIgnoreInhibited=" (yesno lid-switch-ignore-inhibited?) "\n"
> +    "HoldoffTimeoutSecs=" (non-negative-integer holdoff-timeout-seconds) "\n"
> +    "IdleAction=" (handle-action idle-action) "\n"
> +    "IdleActionSeconds=" (non-negative-integer idle-action-seconds) "\n"
> +    "RuntimeDirectorySize="
> +    (if runtime-directory-size-percent
> +        (string-append
> +         (non-negative-integer runtime-directory-size-percent)
> +         "%")
> +        (non-negative-integer runtime-directory-size)) "\n"
> +    "RemoveIpc=" (yesno remove-ipc?) "\n"
> +
> +    "[Sleep]\n"
> +    "SuspendState=" (sleep-list suspend-state) "\n"
> +    "SuspendMode=" (sleep-list suspend-mode) "\n"
> +    "HibernateState=" (sleep-list hibernate-state) "\n"
> +    "HibernateMode=" (sleep-list hibernate-mode) "\n"
> +    "HybridSleepState=" (sleep-list hybrid-sleep-state) "\n"
> +    "HybridSleepMode=" (sleep-list hybrid-sleep-mode) "\n")))

This procedure and the define-with-required-kwargs syntax seem a bit
awkward to me given that elogind-configuration-file is not part of the
public interface.  What advantage does this syntax bring?  Maybe it
would be better to use a record type for this structure or use a
nested alist?

    '((login (kill-user-processes? . #t) ...) (sleep (suspend-state "standby")))

Not sure what the best thing is.  Ludo?

The rest of the patch looks solid.  Very excited about this.  Thanks!

- Dave