From: "Ludovic Courtès" <ludo@gnu.org>
To: Oleg Pykhalov <go.wigust@gmail.com>
Cc: 65343@debbugs.gnu.org, Brian Cully <bjc@spork.org>
Subject: [bug#65343] [PATCH v2] home: services: Add 'x11-display' service.
Date: Sun, 05 Nov 2023 23:30:30 +0100 [thread overview]
Message-ID: <87wmuvygs9.fsf@gnu.org> (raw)
In-Reply-To: <87lebekc3u.fsf@gmail.com> (Oleg Pykhalov's message of "Fri, 03 Nov 2023 19:58:29 +0300")
[-- Attachment #1: Type: text/plain, Size: 1434 bytes --]
Hi,
Oleg Pykhalov <go.wigust@gmail.com> skribis:
> 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’.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 1963 bytes --]
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))))))
prev parent reply other threads:[~2023-11-05 22:31 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-16 17:43 [bug#65343] [PATCH] home: services: Add 'x11-display' service Ludovic Courtès
2023-08-16 19:03 ` Oleg Pykhalov
2023-08-16 20:55 ` Brian Cully via Guix-patches via
2023-09-05 12:00 ` Andrew Tropin
2023-09-14 20:38 ` Ludovic Courtès
2023-09-14 22:39 ` Oleg Pykhalov
2023-10-20 21:09 ` [bug#65343] [PATCH v2] " Ludovic Courtès
2023-11-03 16:58 ` Oleg Pykhalov
2023-11-05 22:30 ` Ludovic Courtès [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87wmuvygs9.fsf@gnu.org \
--to=ludo@gnu.org \
--cc=65343@debbugs.gnu.org \
--cc=bjc@spork.org \
--cc=go.wigust@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.