diff mbox series

[bug#35305,WIP] LightDM service

Message ID 87zhooso9g.fsf@lprndn.info
State Accepted
Headers show
Series [bug#35305,WIP] LightDM service | expand

Checks

Context Check Description
cbaines/applying patch fail Apply failed

Commit Message

L p R n d n April 17, 2019, 2:24 p.m. UTC 
Hello,

Wanted to work on Guix's website but fate led me to try my way on a
lightdm service...
So, here is what I got for now.
It fails to start a window manager but I don't think I'll be able to
debug the last mile with my sole knowledge.

Beside that, here is a list of possible improvements before merging:

* lightdm-greeter-gtk configuration is a part of lightdm's service.
It might be a good idea to give it its own service but it would
mean we need to write a service for each lightdm greeter.

* lightdm complains about the lack of
  org.freedesktop.DisplayManager.AccountsService interface.
  The lightdm package provides the relevent files but it seems
  accountsservice doesn't find them. I think it searches them in
  $XDG_DATA_DIRS/accountservices . See
  https://github.com/NixOS/nixpkgs/issues/45059

* lightdm-gtk-greeter's wrapper is handmade and ugly.

* General refining.

A thorough review would also be welcome as I'm not really sure I know
what I'm doing.

Thanks!

L  p R n  d n

Comments

Jonathan Brielmaier April 18, 2019, 11:20 a.m. UTC | #1 
Please add gnu/services/lightdm.scm to gnu/local.mk
L p R n d n Aug. 26, 2019, 3:58 p.m. UTC | #2 
ping
Nicolò Balzarotti March 15, 2020, 9:50 p.m. UTC | #3 
Cool, I was wondering why the lightdm package was available but no
service was provided for it.  Anything blocking this patch?

Thanks, Nicolò

L  p R n  d n    <guix@lprndn.info> writes:

> ping
Efraim Flashner March 16, 2020, 7:34 a.m. UTC | #4 
On Sun, Mar 15, 2020 at 10:50:07PM +0100, Nicolò Balzarotti wrote:
> Cool, I was wondering why the lightdm package was available but no
> service was provided for it.  Anything blocking this patch?
> 
> Thanks, Nicolò
> 
> L  p R n  d n    <guix@lprndn.info> writes:
> 
> > ping

I don't think anyone got around to reviewing it :/
L p R n d n March 16, 2020, 8:36 a.m. UTC | #5 
Hello,

Also a rebase on latest master might be necessary. I was waiting for
staging to be merged so if I got time, I'll probably take care of this
during the week. ;)

Have a nice day,

L  p R n  d n
Brice Waegeneire April 7, 2020, 5:06 p.m. UTC | #6 
Hello L p R n d n,

I never did a review before but I would like to see this patch merged, 
so
bear with me.

The indentation of lightdm's origin should probably be done in the 
commit
01 not 03.

> `("XDG_DATA_DIRS" ":" prefix (,(string-append (assoc-ref inputs 
> "hicolor-icon-theme")
>                                               "/share")
>                               ,(string-append (assoc-ref inputs "glib")
>                                               "/share")
>                               ,(string-append (assoc-ref inputs 
> "shared-mime-info")
>                                               "/share")
>                               ,(string-append (assoc-ref inputs "gtk+")
>                                               "/share")
>                               ,(string-append (assoc-ref inputs "exo")
>                                               "/share")
>                               ,(string-append (assoc-ref outputs "out")
>                                               "/share")
>                               "/run/current-system/profile/share"))
This part can use a map procedure.

It would be nice if “lightdm-service-type” support 
“set-xorg-configuration”
like the other login manager now does by using 
“handle-xorg-configuration”
see 50be0da7bfd5c108697679effeb2a893d2f37598 for how it's done in GDM, 
SLIM
and co.

> +         (comment "LighDM user")
                           ^ a “t” is missing here

> +(define (lightdm-shepherd-service config)
> +  "Return a <lightdm-service> for LightDM with CONFIG."
> +
> +  (define lightdm-command
> +    #~(list (string-append #$(lightdm-configuration-lightdm config) 
> "/sbin/lightdm")))
[…]
> + (fork+exec-command
> + (list #$(file-append
> + (lightdm-configuration-lightdm config)
> + "/sbin/lightdm"))

“lightdm-command” isn't used, I get the hint it ought to be the argument 
of
“fork+exec-command.”


> +(define (lightdm-etc-service config)
> +  (list `("xdg/lightdm/lightdm.conf.d/lightdm.conf"
> +          ,(lightdm-configuration-file config))
> +        `(,(string-append "xdg/lightdm/"
> +                          (computed-file-name
> +                           
> (lightdm-configuration-greeter-configuration config)))
> +          ,(lightdm-configuration-greeter-configuration config))))

I've been told, in Guix, it's better to avoid putting configuration in
“/etc” since it cause edge case during rollback and such, specifying the
configuration by passing the “--config” argument to “lightdm” would be 
more
appropriate.

> +        (define %user
> +          (getpw "lightdm"))
> +        (let ((directory "/var/lib/lightdm-data"))
> +          (mkdir-p directory)
> +          (chown directory (passwd:uid %user) (passwd:gid %user))))))

“%user” could go in the “let”. BTW can't lightdm use its user home
directory instead of “/var/lib/lightdm-data” or the reverse; IOW does it
need to have to own two directories in “/var/lib”?

Several lines in “gnu/services/lightdm.scm” exceed the maximal line 
length.


Thank you very much for this patch series. I'm impatient seeing it in 
Guix
proper.

- Brice
Ricardo Wurmus Aug. 31, 2022, 7:13 a.m. UTC | #7 
A variation of this has been pushed to the master branch with commit
0ea62e84a787fe94cfeadf67ef27ea995a382b84.
diff mbox series

Patch

From 3106b950f70aba2851091731bff4030087c6eca4 Mon Sep 17 00:00:00 2001
From: L  p R n  d n <guix@lprndn.info>
Date: Wed, 17 Apr 2019 15:47:52 +0200
Subject: [PATCH 10/10] services: Add lightDM service.

* gnu/services/lightdm.scm: New file.
---
 gnu/services/lightdm.scm | 259 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 259 insertions(+)
 create mode 100644 gnu/services/lightdm.scm

diff --git a/gnu/services/lightdm.scm b/gnu/services/lightdm.scm
new file mode 100644
index 0000000000..b280df49ae
--- /dev/null
+++ b/gnu/services/lightdm.scm
@@ -0,0 +1,259 @@ 
+(define-module (gnu services lightdm)
+  #:use-module (guix gexp)
+  #:use-module (guix records)
+
+  #:use-module (gnu system pam)
+  #:use-module (gnu system shadow)
+
+  #:use-module (gnu services)
+  #:use-module (gnu services dbus)
+  #:use-module (gnu services desktop)
+  #:use-module (gnu services shepherd)
+  #:use-module (gnu services xorg)
+
+  #:use-module (gnu packages admin)
+  #:use-module (gnu packages display-managers)
+  #:use-module (gnu packages freedesktop)
+  #:use-module (gnu packages gnome)
+  #:use-module (gnu packages xorg)
+
+  #:export (lightdm-configuration
+            lightdm-configuration?
+            lightdm-service-type))
+
+(define (lightdm-pam-service)
+  "Return a PAM service for @command{lightdm}."
+  (unix-pam-service
+   "lightdm"
+   #:allow-empty-passwords? #t)
+  ;; (pam-service
+  ;; (name "lightdm")
+  ;; (auth
+  ;; (list
+  ;; Block login if they are globally disabled
+  ;; (pam-entry (control "required") (module "pam_nologin.so"))
+  ;; Load environment from /etc/environment and ~/.pam_environment
+  ;; (pam-entry (control "required") (module "pam_env.so"))
+  ;; Use /etc/passwd and /etc/shadow for passwords
+  ;; (pam-entry (control "required") (module "pam_unix.so"))
+  ;; https://wiki.gentoo.org/wiki/LightDM#Unlock_GNOME_Keyring
+  ;; (pam-entry (control "optional") (module "pam_gnome_keyring.so"))
+  ;; ))
+  ;; Check account is active, change password if required
+  ;; (account
+  ;; (list
+  ;; (pam-entry (control "required") (module "pam_unix.so"))))
+  ;; Allow password to be changed
+  ;; (password
+  ;; (list
+  ;; (pam-entry (control "required") (module "pam_unix.so"))))
+  ;; Setup session
+  ;; (session
+  ;; (list
+  ;; (pam-entry (control "required") (module "pam_unix.so"))
+  ;; https://wiki.gentoo.org/wiki/LightDM#Unlock_GNOME_Keyring
+  ;; (pam-entry (control "optional") (module "pam_gnome_keyring.so")
+  ;; (arguments (list "auto_start")))
+  ;; )))
+  )
+
+(define (lightdm-greeter-pam-service)
+  "Return a PAM service for @command{lightdm-greeter}}."
+  (pam-service
+   (name "lightdm-greeter")
+   (auth
+    (list
+     ;; Load environment from /etc/environment and ~/.pam_environment
+     (pam-entry (control "required") (module "pam_env.so"))
+     ;; Always let the greeter start without authentication
+     (pam-entry (control "required") (module "pam_permit.so"))))
+   ;; No action required for account management
+   (account
+    (list
+     (pam-entry (control "required") (module "pam_permit.so"))))
+   ;; Can't change password
+   (password
+    (list
+     (pam-entry (control "required") (module "pam_deny.so"))))
+   ;; Setup session
+   (session
+    (list
+     (pam-entry (control "required") (module "pam_unix.so"))
+     (pam-entry (control "required") (module "pam_env.so"))))))
+
+(define (lightdm-autologin-pam-service)
+  "Return a PAM service for @command{lightdm-autologin}}."
+  (pam-service
+   (name "lightdm-autologin")
+   (auth
+    (list
+     ;; Block login if they are globally disabled
+     (pam-entry (control "required") (module "pam_nologin.so"))
+     ;; Load environment from /etc/environment and ~/.pam_environment
+     (pam-entry (control "required") (module "pam_env.so"))
+     ;; Allow access without authentication
+     (pam-entry (control "required") (module "pam_permit.so"))))
+   ;; Stop autologin if account requires action
+   (account
+    (list
+     (pam-entry (control "required") (module "pam_unix.so"))))
+   ;; Can't change password
+   (password
+    (list
+     (pam-entry (control "required") (module "pam_deny.so"))))
+   ;; Setup session
+   (session
+    (list
+     (pam-entry (control "required") (module "pam_unix.so"))))))
+
+(define-record-type* <lightdm-configuration>
+  lightdm-configuration make-lightdm-configuration
+  lightdm-configuration?
+
+  (lightdm lightdm-configuration-lightdm
+           (default lightdm))
+  (user lightdm-configuration-user
+        (default "lightdm"))
+  (greeters-directory lightdm-configuration-greeters-directory
+                      (default "/run/current-system/profile/share/xgreeters"))
+  (sessions-directory lightdm-configuration-sessions-directory
+                      (default (string-append
+                                "/run/current-system/profile/share/xsessions"
+                                ":/run/current-system/profile/share/wayland-sessions")))
+
+  ;; Seat configuration
+  (greeter-session lightdm-configuration-greeter-session
+                   (default "lightdm-gtk-greeter"))
+  (xserver-command lightdm-configuration-xserver-command
+                   (default (xorg-start-command)))
+  (pam-service lightdm-configuration-pam-service
+               (default (lightdm-pam-service)))
+  (pam-autologin-service lightdm-configuration-autologin-pam-service
+                         (default (lightdm-autologin-pam-service)))
+  (pam-greeter-service lightdm-configuration-greeter-pam-service
+                       (default (lightdm-greeter-pam-service)))
+  (autologin-user lightdm-configuration-autologin-user
+                  (default ""))
+  (default-session-name lightdm-configuration-default-session
+    (default ""))
+  (autologin-timeout lightdm-configuration-autologin-timeout
+                     (default ""))
+  ;; lightdm-gtk-greeter specifics
+  ;; Maybe it should have its own service
+  (gtk-greeter-theming-packages lightdm-configuration-gtk-greeter-theming-packages
+                                (default (list adwaita-icon-theme)))
+  (gtk-greeter-theme-name lightdm-configuration-gtk-greeter-theme-name
+                          (default ""))
+  (gtk-greeter-icon-theme-name
+   lightdm-configuration-gtk-greeter-icon-theme-name
+   (default "Adwaita"))
+  (gtk-greeter-cursor-theme-name
+   lightdm-configuration-gtk-greeter-cursor-theme-name
+   (default "Adwaita"))
+  (gtk-greeter-cursor-size lightdm-configuration-gtk-greeter-cursor-size
+                           (default 16))
+  (gtk-greeter-background lightdm-configuration-gtk-greeter-background
+                          (default "")))
+
+(define %lightdm-accounts
+  (list (user-group (name "lightdm") (system? #t))
+        (user-account
+         (name "lightdm")
+         (group "lightdm")
+         (system? #t)
+         (comment "LighDM user")
+         (home-directory "/var/lib/lightdm")
+         (shell (file-append shadow "/sbin/nologin")))))
+
+(define (lightdm-configuration-file config)
+  (mixed-text-file "lightdm.conf" "
+[LightDM]
+greeter-user = "          (lightdm-configuration-user config) "
+greeters-directory = "    (lightdm-configuration-greeters-directory config) "
+sessions-directory = "    (lightdm-configuration-sessions-directory config) "
+
+
+[Seat:*]
+xserver-command = "       (lightdm-configuration-xserver-command config) "
+greeter-session = "       (lightdm-configuration-greeter-session config) "
+user-session = "          (lightdm-configuration-default-session config) "
+autologin-user = "        (lightdm-configuration-autologin-user config) "
+autologin-session = "     (lightdm-configuration-default-session config) "
+autologin-user-timeout = " (lightdm-configuration-autologin-timeout config)))
+
+(define (lightdm-gtk-greeter-configuration-file config)
+  (mixed-text-file "lightdm-gtk-greeter.conf" "
+[greeter]
+theme-name = "        (lightdm-configuration-gtk-greeter-theme-name config) "
+icon-theme-name = "   (lightdm-configuration-gtk-greeter-icon-theme-name config) "
+cursor-theme-name = " (lightdm-configuration-gtk-greeter-cursor-theme-name config) "
+cursor-theme-size = " (number->string (lightdm-configuration-gtk-greeter-cursor-size config)) "
+background = "        (lightdm-configuration-gtk-greeter-background config)))
+
+(define (lightdm-shepherd-service config)
+  "Return a <lightdm-service> for LightDM with CONFIG."
+
+  (define lightdm-command
+    #~(list (string-append #$(lightdm-configuration-lightdm config) "/sbin/lightdm")))
+
+  (list (shepherd-service
+         (documentation "LightDM display manager.")
+         (requirement '(dbus-system user-processes host-name))
+         (provision '(display-manager))
+         (respawn? #f)
+         (start #~(lambda ()
+                    (fork+exec-command
+                     (list #$(file-append
+                              (lightdm-configuration-lightdm config)
+                              "/sbin/lightdm"))
+                     #:environment-variables
+                     (list
+                      (string-append
+                       "PATH=/run/current-system/profile/sbin"
+                       ":/run/current-system/profile/bin")))))
+         (stop #~(make-kill-destructor)))))
+
+(define (lightdm-etc-service config)
+  (list `("xdg/lightdm/lightdm.conf.d/lightdm.conf"
+          ,(lightdm-configuration-file config))
+        `("xdg/lightdm/lightdm-gtk-greeter.conf"
+          ,(lightdm-gtk-greeter-configuration-file config))))
+
+(define (lightdm-pam-services config)
+  (list (lightdm-configuration-pam-service config)
+        (lightdm-configuration-greeter-pam-service config)
+        (lightdm-configuration-autologin-pam-service config)))
+
+(define (lightdm-profile-service config)
+  (append (list lightdm-gtk-greeter lightdm)
+          (lightdm-configuration-gtk-greeter-theming-packages config)))
+
+(define (lightdm-activation-service config)
+  (with-imported-modules '((guix build utils))
+    #~(begin
+        (use-modules (guix build utils))
+        (define %user
+          (getpw #$(lightdm-configuration-user config)))
+        (let ((directory "/var/lib/lightdm-data"))
+          (mkdir-p directory)
+          (chown directory (passwd:uid %user) (passwd:gid %user))))))
+
+(define lightdm-service-type
+  (service-type (name 'lightdm)
+                (extensions
+                 (list
+                  (service-extension shepherd-root-service-type
+                                     lightdm-shepherd-service)
+                  (service-extension activation-service-type
+                                     lightdm-activation-service)
+                  (service-extension pam-root-service-type
+                                     lightdm-pam-services)
+                  (service-extension etc-service-type
+                                     lightdm-etc-service)
+                  (service-extension dbus-root-service-type
+                                     (compose list lightdm-configuration-lightdm))
+                  (service-extension account-service-type
+                                     (const %lightdm-accounts))
+                  (service-extension profile-service-type
+                                     lightdm-profile-service)))
+                (default-value (lightdm-configuration))))
-- 
2.21.0