diff mbox series

[bug#65343,v2] home: services: Add 'x11-display' service.

Message ID c423ece09843ab3ac580c2182e9628f067902f03.1697835966.git.ludo@gnu.org
State New
Headers show
Series [bug#65343,v2] home: services: Add 'x11-display' service. | expand

Commit Message

Ludovic Courtès Oct. 20, 2023, 9:09 p.m. UTC
* gnu/home/services/desktop.scm (x11-shepherd-service): New procedure.
(home-x11-service-type): New variable.
(redshift-shepherd-service): Add 'requirement' field.
(home-redshift-service-type): Extend 'home-x11-service-type'.
* doc/guix.texi (Desktop Home Services): Document it.
---
 doc/guix.texi                 | 34 ++++++++++++++
 gnu/home/services/desktop.scm | 87 +++++++++++++++++++++++++++++++++--
 2 files changed, 116 insertions(+), 5 deletions(-)

Hi!

Changes in this version:

  1. ‘x11-display’ defaults to the ‘DISPLAY’ value of the ‘shepherd’
     process, if any.  This makes it fully compatible with what we
     have now (processes basically inherit environment variables of
     the ‘shepherd’ process).

  2. One can specify a display: ‘herd start x11-display :42’.

WDYT?

Thanks,
Ludo’.


base-commit: 6b0a32196982a0a2f4dbb59d35e55833a5545ac6

Comments

Oleg Pykhalov Nov. 3, 2023, 4:58 p.m. UTC | #1
Hi Ludovic,

apologize for the slow response. I had some minor issues with setting a
'home shepherd' instance in a fresh testing virtual machine (not related
to the current issue).

Ludovic Courtès <ludo@gnu.org> writes:

> * gnu/home/services/desktop.scm (x11-shepherd-service): New procedure.
> (home-x11-service-type): New variable.
> (redshift-shepherd-service): Add 'requirement' field.
> (home-redshift-service-type): Extend 'home-x11-service-type'.
> * doc/guix.texi (Desktop Home Services): Document it.
> ---
>  doc/guix.texi                 | 34 ++++++++++++++
>  gnu/home/services/desktop.scm | 87 +++++++++++++++++++++++++++++++++--
>  2 files changed, 116 insertions(+), 5 deletions(-)
>
> Changes in this version:
>
>   1. ‘x11-display’ defaults to the ‘DISPLAY’ value of the ‘shepherd’
>      process, if any.  This makes it fully compatible with what we
>      have now (processes basically inherit environment variables of
>      the ‘shepherd’ process).
>
>   2. One can specify a display: ‘herd start x11-display :42’.
>
> WDYT?

I think we are going in the right direction, thank you for the
implementation!


During testing in a virtual machine I found an issue, which could be
reproduced with the steps bellow. It's not an emergency issue which
blocks the patch from merging, because we still have a workaround in our
pocket (stop and start ‘shepherd’ manually on a specific ‘DISPLAY’).

- start ‘shepherd’ manually on ‘127.0.0.1:5’ by typing ‘shepherd’ in a
  GUI terminal;
- stop ‘x11-display’ with ‘herd stop x11-display’;
- start ‘x11-display’ with ‘herd start x11-display :1’;
- start ‘redshift’ with ‘herd start redshift’.

The ‘redshift’ service starts a ‘redshift’ process.  The
/proc/REDSHIFT_PID/environ file has a ‘DISPLAY’ variable setted to
‘127.0.0.1:5’.

I expect that in /proc/REDSHIFT_PID/environ file the value of ‘DISPLAY’
environment variable should be ‘:1’, as in ‘x11-display’ service started
right before the ‘redshift’ service.

Following services could be used to get ‘:1’ and ‘127.0.0.1:5’:
--8<---------------cut here---------------start------------->8---
    (service lxqt-desktop-service-type)

    (service xvnc-service-type (xvnc-configuration
                                 (display-number 5)
                                 (localhost? #f)
                                 (xdmcp? #t)
                                 (inetd? #t)))

    (modify-services %desktop-services
                     (gdm-service-type config => (gdm-configuration
                                                  (inherit config)
                                                  (auto-suspend? #f)
                                                  (xdmcp? #t))))
--8<---------------cut here---------------end--------------->8---

A workaround for this issue to stop ‘shepherd’ with ‘herd stop root’,
and start it on ‘:1’ with ‘shepherd’ in a GUI terminal.


Regards,
Oleg.
diff mbox series

Patch

diff --git a/doc/guix.texi b/doc/guix.texi
index 91408b8e62..7984673b6a 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -44455,6 +44455,40 @@  Desktop Home Services
 may find useful on ``desktop'' systems running a graphical user
 environment such as Xorg.
 
+@cindex X Window, for Guix Home services
+@cindex X11, in Guix Home
+@defvar home-x11-service-type
+This is the service type representing the X Window graphical display
+server (also referred to as ``X11'').
+
+X Window is necessarily started by a system service; on Guix System,
+starting it is the responsibility of @code{gdm-service-type} and similar
+services (@pxref{X Window}).  At the level of Guix Home, as an
+unprivileged user, we cannot start X Window; all we can do is check
+whether it is running.  This is what this service does.
+
+As a user, you probably don't need to worry or explicitly instantiate
+@code{home-x11-service-type}.  Services that require an X Window
+graphical display, such as @code{home-redshift-service-type} below,
+instantiate it and depend on its corresponding @code{x11-display}
+Shepherd service (@pxref{Shepherd Home Service}).
+
+When X Window is running, the @code{x11-display} Shepherd service starts
+and sets the @env{DISPLAY} environment variable of the
+@command{shepherd} process, using its original value if it was already
+set; otherwise, it fails to start.
+
+The service can also be forced to use a given value for @env{DISPLAY},
+like so:
+
+@example
+herd start x11-display :3
+@end example
+
+In the example above, @code{x11-display} is instructed to set
+@env{DISPLAY} to @code{:3}.
+@end defvar
+
 @defvar home-redshift-service-type
 This is the service type for @uref{https://github.com/jonls/redshift,
 Redshift}, a program that adjusts the display color temperature
diff --git a/gnu/home/services/desktop.scm b/gnu/home/services/desktop.scm
index c4da116100..45a319c0f8 100644
--- a/gnu/home/services/desktop.scm
+++ b/gnu/home/services/desktop.scm
@@ -1,5 +1,5 @@ 
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2022 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2022-2023 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2022 ( <paren@disroot.org>
 ;;; Copyright © 2023 conses <contact@conses.eu>
 ;;; Copyright © 2023 Janneke Nieuwenhuizen <janneke@gnu.org>
@@ -30,7 +30,9 @@  (define-module (gnu home services desktop)
   #:use-module (guix gexp)
   #:use-module (srfi srfi-1)
   #:use-module (ice-9 match)
-  #:export (home-redshift-configuration
+  #:export (home-x11-service-type
+
+            home-redshift-configuration
             home-redshift-configuration?
             home-redshift-service-type
 
@@ -43,6 +45,74 @@  (define-module (gnu home services desktop)
             home-xmodmap-configuration
             home-xmodmap-service-type))
 
+
+;;;
+;;; Waiting for X11.
+;;;
+
+(define (x11-shepherd-service delay)
+  (list (shepherd-service
+         (provision '(x11-display))
+         (modules '((ice-9 ftw)
+                    (ice-9 match)
+                    (srfi srfi-1)))
+         (start
+          #~(lambda* (#:optional (display (getenv "DISPLAY")))
+              (define x11-directory
+                "/tmp/.X11-unix")
+
+              (define (find-display delay)
+                ;; Wait for an accessible socket to show up in X11-DIRECTORY,
+                ;; up to DELAY seconds.
+                (let loop ((attempts delay))
+                  (define socket
+                    (find (match-lambda
+                            ((or "." "..") #f)
+                            (name
+                             (let ((name (in-vicinity x11-directory
+                                                      name)))
+                               (access? name O_RDWR))))
+                          (or (scandir x11-directory) '())))
+
+                  (if (and socket (string-prefix? "X" socket))
+                      (let ((display (string-append
+                                      ":" (string-drop socket 1))))
+                        (format #t "X11 display server found at ~s.~%"
+                                display)
+                        display)
+                      (if (zero? attempts)
+                          (begin
+                            (format (current-error-port)
+                                    "X11 display server did not show up; \
+giving up.\n")
+                            #f)
+                          (begin
+                            (sleep 1)
+                            (loop (- attempts 1)))))))
+
+              (let ((display (or display (find-display #$delay))))
+                (when display
+                  (setenv "DISPLAY" display))
+                display)))
+         (stop #~(lambda (_)
+                   (unsetenv "DISPLAY")
+                   #f))
+         (respawn? #f))))
+
+(define home-x11-service-type
+  (service-type
+   (name 'home-x11-display)
+   (extensions (list (service-extension home-shepherd-service-type
+                                        x11-shepherd-service)))
+   (default-value 10)
+   (description
+    "Create a @code{x11-display} Shepherd service that waits for the X
+Window (or ``X11'') graphical display server to be up and running, up to a
+configurable delay, and sets the @code{DISPLAY} environment variable of
+@command{shepherd} itself accordingly.  If no accessible X11 server shows up
+during that time, the @code{x11-display} service is marked as failing to
+start.")))
+
 
 ;;;
 ;;; Redshift.
@@ -169,8 +239,11 @@  (define (redshift-shepherd-service config)
   (list (shepherd-service
          (documentation "Redshift program.")
          (provision '(redshift))
-         ;; FIXME: This fails to start if Home is first activated from a
-         ;; non-X11 session.
+
+         ;; Depend on 'x11-display', which sets 'DISPLAY' if an X11 server is
+         ;; available, and fails to start otherwise.
+         (requirement '(x11-display))
+
          (start #~(make-forkexec-constructor
                    (list #$(file-append (home-redshift-configuration-redshift config) "/bin/redshift")
                          "-c" #$config-file)))
@@ -181,7 +254,11 @@  (define home-redshift-service-type
   (service-type
    (name 'home-redshift)
    (extensions (list (service-extension home-shepherd-service-type
-                                        redshift-shepherd-service)))
+                                        redshift-shepherd-service)
+                     ;; Ensure 'home-x11-service-type' is instantiated so we
+                     ;; can depend on the Shepherd 'x11-display' service.
+                     (service-extension home-x11-service-type
+                                        (const #t))))
    (default-value (home-redshift-configuration))
    (description
     "Run Redshift, a program that adjusts the color temperature of display