[PATCH] home: services: Add 'x11-display' service.

  • Done
  • quality assurance status badge
Details
4 participants
  • Andrew Tropin
  • Brian Cully
  • Oleg Pykhalov
  • Ludovic Courtès
Owner
unassigned
Submitted by
Ludovic Courtès
Severity
normal
L
L
Ludovic Courtès wrote on 16 Aug 2023 19:43
(address . guix-patches@gnu.org)(name . Ludovic Courtès)(address . ludo@gnu.org)
b9fa2dae291ec797b1869cce7d74d59cf5299a03.1692207625.git.ludo@gnu.org
* 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 | 22 ++++++++++
gnu/home/services/desktop.scm | 82 ++++++++++++++++++++++++++++++++---
2 files changed, 99 insertions(+), 5 deletions(-)

Hello Guix!

This is an attempt to fix a longstanding issue with Home services
that depend on X11: how can we make sure that (1) they are not started
when X is not running, and (2) they get the correct ‘DISPLAY’
variable.

It’s a bit of a hack (the idea came up during a discussion on IRC
a few days ago), but it does the job. I guess it could be
extended to Wayland as well, but I’m not familiar with it.

Thoughts?

Ludo’.

Toggle diff (156 lines)
diff --git a/doc/guix.texi b/doc/guix.texi
index 22590b4f9c..a99ef8e5e8 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -44067,6 +44067,28 @@ 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
+@code{DISPLAY} environment variable of the @command{shepherd} process;
+otherwise, it fails to start.
+@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 626918fd9e..b293031fd1 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,69 @@ (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 ()
+ (define x11-directory
+ "/tmp/.X11-unix")
+
+ ;; 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)
+ (setenv "DISPLAY" display)
+ display)
+ (if (zero? attempts)
+ (begin
+ (display
+ "X11 display server did not show up; \
+giving up.\n"
+ (current-error-port))
+ #f)
+ (begin
+ (sleep 1)
+ (loop (- attempts 1))))))))
+ (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 5)
+ (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 +234,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 redshift "/bin/redshift")
"-c" #$config-file)))
@@ -181,7 +249,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

base-commit: 880ada0bdb9e694573ec42200d48658b27744b9b
--
2.41.0
O
O
Oleg Pykhalov wrote on 16 Aug 2023 21:03
(name . Ludovic Courtès)(address . ludo@gnu.org)
877cpuernh.fsf@gmail.com
Hi Ludovic,

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

Toggle quote (19 lines)
> * 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 | 22 ++++++++++
> gnu/home/services/desktop.scm | 82 ++++++++++++++++++++++++++++++++---
> 2 files changed, 99 insertions(+), 5 deletions(-)
>
> This is an attempt to fix a longstanding issue with Home services
> that depend on X11: how can we make sure that (1) they are not started
> when X is not running, and (2) they get the correct ‘DISPLAY’
> variable.
>
> […]
>
> Thoughts?

If I understood the patch correctly, the ‘x11-shepherd-service’
procedure finds a first file like ‘/tmp/.X11-unix/X0’ or
‘/tmp/.X11-unix/X2’ which is readable by a user which runs the Shepherd.

Is it possible to allow a user to exactly specify a list of files in
‘/tmp/.X11-unix’ directory, which will be checked? It will be useful
for VNC users to make sure X11 services are running on a specified
DISPLAY. Otherwise X11 services will be running on a DISPLAY handled by
VNC after boot and never on DISPLAY which becomes available after
authentication (XWayland, GDM , SLIM, LightDM, etc).

Regards,
Oleg.
-----BEGIN PGP SIGNATURE-----

iQJIBAEBCgAyFiEEcjhxI46s62NFSFhXFn+OpQAa+pwFAmTdHYMUHGdvLndpZ3Vz
dEBnbWFpbC5jb20ACgkQFn+OpQAa+pzVzA//QTaSDnAz2wcmgLZOU6iD8az9BZ5S
1cImUIxJl7ZP3MArUjntJYVvZH41lRxUcwcxipXhNANDcVTvnfynA7w3zQQqpbP5
tgrAprR9SpSwJpOjIhYPXKUnrMkLfeqhzIdfETAjRlrO2Ldlbb3YXquQwVNqkp+m
BVCeq57cFjpvKeLDti2gmraC2ds8uTSgAeySoL7qP5eRYkg/We+45w6osmcKoig4
Wul2VPhY2em673pA6oEA86CqzRUiLfNGG3jLcK3Fk1JDlIht57yZ9s7a3+xNe626
OG4uSK99RnGy3+7jpx82hS2mmFYZCXt+AOevZtvwON7Gi8CKGINXxyQhpvYuhj2d
JY31IgpfgRlfGU2IujpviXEnso14qCA8eAYzzQ9lZ4Fp/K5yMp6LK+mRPPrAoY2/
KMxQyXo0evlcLgRv2hcGwvnmiZ4tFASy3CQirhNhWSoW1kQQ3I0Fd6whbCtnjTKB
zTlZLKUN1wkd3QoSdevKh2zIQTzASgXGbzu8gpQ/hno8EEIqI2K61myfNL23FgPU
7sKhLL80T+pvNUm7NE8hf1J2ghmoMxVeUvNH1VJX5l3ZNNEkmrSYAOBexJ7HfyBJ
UoSsEW0ltWSRlI1Mx4mjGrHPocj24iXLvE5hnbEts1o6aJLf7NeKCgr1Egok+v0Q
iFjGcFnEWkIn+Ec=
=RRQ7
-----END PGP SIGNATURE-----

B
B
Brian Cully wrote on 16 Aug 2023 22:55
(name . Oleg Pykhalov)(address . go.wigust@gmail.com)
87a5uqwvva.fsf@spork.org
The largest issue I see with this patch is that it doesn't correlate the
X11 socket with the session being used in cases where there's more than
one X11 display.

If, for instance, I start an X session on the console, allocating the
first display (:0), everything will start up correctly. If I then log in
from a remote host with SSH using X forwarding, I'll get another display
allocated (:1), but this isn't accounted for. If I do these operations
in reverse, first starting my X-forwarded SSH session, then logging in
via console, it will almost certainly not do what just about anyone
wants.

This does presume the Shepherd can be started multiple times for a given
user, and run concurrently, though this does not appear to actually be
the case, since there's a single global location for the socket, which
isn't differentiated by session. But that's a separate issue.

This also doesn't handle the case of the X11 server going away, either
by crash or user request. If we're starting stuff on behalf of users
when it comes up, it seems to me we should also be stopping stuff on
users' behalf when it comes down. The lack of handling this could easily
lead to resource-churning loops where X11 goes away, but Shepherd
services continuously restart themselves trying to connect to a display
that no longer exists.

If this is only meant to be used when using a display manager, a la gdm,
then it might be ok. I'm not sure, since I don't use them. When logging
out of an X session started from gdm, is the user's shepherd process
stopped? Is it stopped gracefully? What about sddm? lightdm?
A
A
Andrew Tropin wrote on 5 Sep 2023 14:00
87o7igzv6u.fsf@trop.in
On 2023-08-16 16:55, Brian Cully wrote:

Toggle quote (30 lines)
> The largest issue I see with this patch is that it doesn't correlate the
> X11 socket with the session being used in cases where there's more than
> one X11 display.
>
> If, for instance, I start an X session on the console, allocating the
> first display (:0), everything will start up correctly. If I then log in
> from a remote host with SSH using X forwarding, I'll get another display
> allocated (:1), but this isn't accounted for. If I do these operations
> in reverse, first starting my X-forwarded SSH session, then logging in
> via console, it will almost certainly not do what just about anyone
> wants.
>
> This does presume the Shepherd can be started multiple times for a given
> user, and run concurrently, though this does not appear to actually be
> the case, since there's a single global location for the socket, which
> isn't differentiated by session. But that's a separate issue.
>
> This also doesn't handle the case of the X11 server going away, either
> by crash or user request. If we're starting stuff on behalf of users
> when it comes up, it seems to me we should also be stopping stuff on
> users' behalf when it comes down. The lack of handling this could easily
> lead to resource-churning loops where X11 goes away, but Shepherd
> services continuously restart themselves trying to connect to a display
> that no longer exists.
>
> If this is only meant to be used when using a display manager, a la gdm,
> then it might be ok. I'm not sure, since I don't use them. When logging
> out of an X session started from gdm, is the user's shepherd process
> stopped? Is it stopped gracefully? What about sddm? lightdm?

I have been seeking for the solution for this for some time and also
tried similiar thing as Ludo's patch does and I'm agree with concerns
mentioned above by Brian.

At the end in rde we decided to start shepherd by Sway (wayland
compositor), so all the services have proper environment variables. It
has other downsides, but overall works well for usual desktop use cases.

I don't have a complete generic answer to this problem yet.

--
Best regards,
Andrew Tropin
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEKEGaxlA4dEDH6S/6IgjSCVjB3rAFAmT3GFkACgkQIgjSCVjB
3rCvVA/+JGJaNzrKRqcALzPNx7KmRH2GWzuKAg704S2arHZ+wyJ8wfTiWnxXM+lE
sb7Y5WLFNHHTO6DbE9DyHFX2IMpkjE939JeP7FJhcOWzlALlisPCmXd2FQDgYgNk
R/97aACHhT+dxXF0M7BaWXX0bf4yeYoU0i2/zD1n6yuuGTCFPB23PFidQnd2TMuF
Z8ou8kcOhh+JeZPA3PNeqsvcSRu0MXbzFroliXjkPK0szpTiXNwW/2g5X/kreP46
WCoH7AQ6hjdOqaqkIHZJdgCt5JaCDPmlPUJdyL34Xn8NXjro4XtoOW8z2l/LIg68
gwg7H6cxTSV2CCXieAFpWw7VQjZKHymWMbaxjaSCmZLOQ9KgQ1ZRNyMgyiY/eh8D
a7MzCAJ323qbQQk/30YZYZA5u/QFyUfFaVtBgn5LLrS0m2BG5IBFGGnQIuC/Zrve
tfYCsR/6BQ8YtcZTH6w6KL14OQh2A5x25k3yKG50mrj7fQa+lCcB8Kn1rCK+zlgl
VuT7uXVaNcaUmroujy+pqHSJnufqmD2YHe9JZtVmfP2T1ve+KJiCyCa7vFSM3Jxw
zVF7otTI0PyLvK8hRAGnOiNA95o8TIzCYbpp9VgS1bqUntggUiw5ooOaI32k3FT9
21fXKbJ8DmRAssTLAGTcn53MkJLdMK/JDGU5wcWVOSbjnRwIrss=
=dQjN
-----END PGP SIGNATURE-----

L
L
Ludovic Courtès wrote on 14 Sep 2023 22:38
Re: bug#65343: [PATCH] home: services: Add 'x11-display' service.
(name . Brian Cully)(address . bjc@spork.org)
87msxo5w2x.fsf_-_@gnu.org
Hello Brian & Oleg,

Brian Cully <bjc@spork.org> skribis:

Toggle quote (8 lines)
> If, for instance, I start an X session on the console, allocating the
> first display (:0), everything will start up correctly. If I then log in
> from a remote host with SSH using X forwarding, I'll get another display
> allocated (:1), but this isn't accounted for. If I do these operations
> in reverse, first starting my X-forwarded SSH session, then logging in
> via console, it will almost certainly not do what just about anyone
> wants.

Yeah, and similarly with the scenario Oleg describes.

Toggle quote (3 lines)
> This does presume the Shepherd can be started multiple times for a given
> user,

No no, but it assumes simple scenarios: when you first login locally, X
is not running yet, but you eventually start it and that’s the display
you want your services to use. Anything beyond that won’t work, as you
point out.

A simple improvement would be to stop the service when the relevant
/tmp/.X11-unix socket disappears.

As for which display to use when several are available (the SSH example
above), I don’t know. Apparently elogind doesn’t know which display
corresponds to a “seat”. Maybe we shouldn’t try to guess and instead
let users specify it, for instance with ‘herd start x11-display :42’?

Now, without this service the situation is even worse: shepherd and its
sub-processes inherit whatever ‘DISPLAY’ value was in its environment,
if any, and that’s it. This service is a hack, but might still do more
good than harm?

Ludo’.
O
O
Oleg Pykhalov wrote on 15 Sep 2023 00:39
(name . Ludovic Courtès)(address . ludo@gnu.org)
87il8c2xdd.fsf@gmail.com
Ludovic Courtès <ludo@gnu.org> writes:

[…]

Toggle quote (5 lines)
> Now, without this service the situation is even worse: shepherd and its
> sub-processes inherit whatever ‘DISPLAY’ value was in its environment,
> if any, and that’s it. This service is a hack, but might still do more
> good than harm?

Inheriting environment variable is under the user's controll. Finding a
readable file by the user is less (requires to start x11 sessions in a
specific order). By more controll I mean the user could stop Shepherd on
a DISPLAY :0 and start it on DISPLAY :1 without stopping x11 sessions.

In any case, current patch with or without specification of files order
(or ‘herd start x11-display :42’) will change current behaviour. So, I
think a small entry in ‘news.scm’ could save somebody a day.


Regards,
Oleg.
-----BEGIN PGP SIGNATURE-----

iQJIBAEBCgAyFiEEcjhxI46s62NFSFhXFn+OpQAa+pwFAmUDi44UHGdvLndpZ3Vz
dEBnbWFpbC5jb20ACgkQFn+OpQAa+pwnGg/7BLWmjPt8h3iVa3PMCaRFNwzx3fT9
W9RG8ww3ilfcbIYHPrvyWtZyBYdEQqblylpXs3IJCPbD0hz8X5Vr3FLOOnFg9kLA
RuW+yKMj9akEro/GHQXni0jadP6hIt8LPArz/p2sw8timsPJ37y3kGITX67bSh4D
g502n+WDqSPPyU5YMWQgzCtItd3iWCfCZqkTaa558wwZxABTCBEsbFxblUwikK6d
Bqxve/IWqvU75n8xksjLgFHe8ImQamP1zsWjEgICOsycNN1CKp5GP0jhoAJ9vHca
X/LgxS00LJBOBHHjP8cqva035ai+gSOqmVkpmW49JspR/+xRF2OCx3QwSg6w6gyZ
OSR29nYDuuNuV+Gk7P6vLUJeFfwQyetkrUV65Z0w2967M2kyBla/7w8wJp4shSk2
ciI9XjEb7LmhKEv2Na+wl+dFvy7RrC0JybAPaM8D7hHnTKoZVfc0urP8O0Pwzyo2
XaugdOHjq8DHioVz3Xa9crK08k0l7jcnr4omvigCvfZeFVHn48n/JmvFssjQ3caR
52K1kn1xwRn7IWt8TI1d5uHWKcfpQ3NHT3NaA8Rj8EhLo3re8vflT1AFlRGRgCRX
VD0X73Bs9tfrrO8mCS3xt9Pnj0PuS3bqWRH7tVr5UxQgm0sdYHHmdnRqIcuDPGfq
uRmr3mzKsxdtqpU=
=WdRH
-----END PGP SIGNATURE-----

L
L
Ludovic Courtès wrote on 20 Oct 2023 23:09
[PATCH v2] home: services: Add 'x11-display' service.
(address . 65343@debbugs.gnu.org)
c423ece09843ab3ac580c2182e9628f067902f03.1697835966.git.ludo@gnu.org
* 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’.

Toggle diff (173 lines)
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

base-commit: 6b0a32196982a0a2f4dbb59d35e55833a5545ac6
--
2.41.0
O
O
Oleg Pykhalov wrote on 3 Nov 2023 17:58
(name . Ludovic Courtès)(address . ludo@gnu.org)
87lebekc3u.fsf@gmail.com
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:

Toggle quote (21 lines)
> * 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’:
Toggle snippet (15 lines)
(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))))

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.
-----BEGIN PGP SIGNATURE-----

iQJIBAEBCgAyFiEEcjhxI46s62NFSFhXFn+OpQAa+pwFAmVFJrYUHGdvLndpZ3Vz
dEBnbWFpbC5jb20ACgkQFn+OpQAa+pz3pxAAqmUNlpqPUxbA7v7Uxe/Z2flWfjRA
HKesq8kCTQEP3XHAo6QTh/9W3FhWKOsk5Rs7votNLCsaB5ILylHXU2TpsQEB7f/L
vMsm4DOZ5tK1lXqAW4hG8hMSE8b2Sax2aZE+mldNwyZ6g3AV53ydryQGiZbj4K2Z
4/Ph7BkjTqzWSXI1MKskAvHG4aGdQQ0QLcq91ML7laLuecFOi3Q59SStuI/NoAMG
fMs1H6xKSzaCy2mnsIOjmlDgiJMZ6UACXXY+k4rvtlJsPUxU30ZoFRaIPoilVdnq
N+fFPaCt1v6QMALqqMkuqUIJOJC+/soWevyK0Fbg2p/UJTiG48y34tGpRAv8NJvw
/+vB55dfIMCtJ6Tc5P7l2hiLD6lfKZwAi6ufO/lT4EsK6jlV09azYHn74nU4hRIq
N2SgvR1EYyhWFqUBf+lAvS9Mrvt3OKlueFZzG1PxqmRfyyUb1ANBpcWAMc3oEC7o
1Fl9xtWQrwVIkgsTXreDfCcreUamRL0Rad83xm7IXZ5m1+jOic/gfL8V1K3ibyQF
yx0tDcT9YwETfrU3/smb3oKJo4hauRwzG6Xrxo4elfiJxV2BNcNROSsntLFzWgwl
pMVIqI5WaQ8cn3dkSJ/RA3j3uNzV3cfnw72dY+LXe8PlaupjctSSGY7c49azS0MK
/tJdGgHlfAaol8o=
=bLOA
-----END PGP SIGNATURE-----

L
L
Ludovic Courtès wrote on 5 Nov 2023 23:30
Re: [bug#65343] [PATCH v2] home: services: Add 'x11-display' service.
(name . Oleg Pykhalov)(address . go.wigust@gmail.com)
87wmuvygs9.fsf@gnu.org
Hi,

Oleg Pykhalov <go.wigust@gmail.com> skribis:

Toggle quote (15 lines)
> 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’.

Indeed, good catch! There was a bug: it’s not enough to call ‘setenv’
because ‘make-forkexec-constructor’ & co. have an explicit
#:environment-variables parameter, which defaults to the environment of
‘shepherd’ when it was started. (On top of that, setting the
‘default-environment-variables’ SRFI-39 parameter from the ‘start’
method of ‘x11-display’ wouldn’t work because each fiber has its own
dynamic environment…)

I had to make the change below. Result pushed as commit
08d94fe20eca47b69678b3eced8749dd02c700a4.

Thank you for reviewing and testing!

Ludo’.
Toggle diff (39 lines)
diff --git a/gnu/home/services/desktop.scm b/gnu/home/services/desktop.scm
index 45a319c0f8..91465bf168 100644
--- a/gnu/home/services/desktop.scm
+++ b/gnu/home/services/desktop.scm
@@ -92,6 +92,11 @@ (define (x11-shepherd-service delay)
(let ((display (or display (find-display #$delay))))
(when display
+ ;; Note: 'make-forkexec-constructor' calls take their
+ ;; default #:environment-variables value before this service
+ ;; is started and are thus unaffected by the 'setenv' call
+ ;; below. Users of this service have to explicitly query
+ ;; its value.
(setenv "DISPLAY" display))
display)))
(stop #~(lambda (_)
@@ -244,9 +249,20 @@ (define (redshift-shepherd-service config)
;; 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)))
+ (modules '((srfi srfi-1)
+ (srfi srfi-26)))
+ (start #~(lambda _
+ (fork+exec-command
+ (list #$(file-append
+ (home-redshift-configuration-redshift config)
+ "/bin/redshift")
+ "-c" #$config-file)
+
+ ;; Inherit the 'DISPLAY' variable set by 'x11-display'.
+ #:environment-variables
+ (cons (string-append "DISPLAY=" (getenv "DISPLAY"))
+ (remove (cut string-prefix? "DISPLAY=" <>)
+ (default-environment-variables))))))
(stop #~(make-kill-destructor))
(actions (list (shepherd-configuration-action config-file))))))
L
L
Ludovic Courtès wrote on 5 Nov 2023 23:30
control message for bug #65343
(address . control@debbugs.gnu.org)
87v8afygrt.fsf@gnu.org
close 65343
quit
?
Your comment

This issue is archived.

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

To respond to this issue using the mumi CLI, first switch to it
mumi current 65343
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