[PATCH 0/2] Add x11-socket-directory-service-type.

  • Done
  • quality assurance status badge
Details
3 participants
  • Josselin Poiret
  • Maxim Cournoyer
  • Bruno Victal
Owner
unassigned
Submitted by
Bruno Victal
Severity
normal
B
B
Bruno Victal wrote on 12 Jan 2023 16:43
(address . guix-patches@gnu.org)(name . Bruno Victal)(address . mirai@makinata.eu)
cover.1673537696.git.mirai@makinata.eu
This replaces x11-socket-directory-service with a shepherd one-shot service
that takes file-system as a dependent target.


Fixes #57589.


Bruno Victal (2):
services: Add x11-socket-directory-service-type.
Revert "tests: Add gdm tests."

gnu/local.mk | 1 -
gnu/services/desktop.scm | 44 ++++++++++----
gnu/tests/gdm.scm | 127 ---------------------------------------
gnu/tests/lightdm.scm | 2 +-
4 files changed, 34 insertions(+), 140 deletions(-)
delete mode 100644 gnu/tests/gdm.scm


base-commit: ef0613a81dca73602e702cb5f5444ee94566f983
--
2.38.1
B
B
Bruno Victal wrote on 12 Jan 2023 16:46
[PATCH 1/2] services: Add x11-socket-directory-service-type.
(address . 60756@debbugs.gnu.org)(name . Bruno Victal)(address . mirai@makinata.eu)
6d06ecb7463a09830b87395200849037decd4596.1673537696.git.mirai@makinata.eu
The x11-socket-directory-service misuses activation-service-type
to create directories. This kind of usage is incorrect since
activation-service-type does not depend of file-systems and incompatible
with user defined /tmp mount.

This commit turns x11-socket-directory-service into a shepherd one-shot
service by defining a new x11-socket-directory-service-type.

* gnu/services/desktop.scm (x11-socket-directory-service-type): New variable.
(x11-socket-directory-service): Deprecate variable.
(desktop-services-for-system): Use new service-type.
* gnu/tests/lightdm.scm: Use new service-type.
---
gnu/services/desktop.scm | 44 ++++++++++++++++++++++++++++++----------
gnu/tests/lightdm.scm | 2 +-
2 files changed, 34 insertions(+), 12 deletions(-)

Toggle diff (94 lines)
diff --git a/gnu/services/desktop.scm b/gnu/services/desktop.scm
index fe1f0fd20a..b2983667b8 100644
--- a/gnu/services/desktop.scm
+++ b/gnu/services/desktop.scm
@@ -14,6 +14,7 @@
;;; Copyright © 2020 Reza Alizadeh Majd <r.majd@pantherx.org>
;;; Copyright © 2021 Brice Waegeneire <brice@waegenei.re>
;;; Copyright © 2021, 2022 muradm <mail@muradm.net>
+;;; Copyright © 2023 Bruno Victal <mirai@makinata.eu>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -148,7 +149,8 @@ (define-module (gnu services desktop)
xfce-desktop-service
xfce-desktop-service-type
- x11-socket-directory-service
+ x11-socket-directory-service ;deprecated
+ x11-socket-directory-service-type
enlightenment-desktop-configuration
enlightenment-desktop-configuration?
@@ -1496,18 +1498,38 @@ (define lxqt-desktop-service-type
;;; X11 socket directory service
;;;
-(define x11-socket-directory-service
+(define x11-socket-directory-service-type
+ (let ((x11-socket-directory-shepherd-service
+ (shepherd-service
+ (documentation "Create /tmp/.X11-unix for XWayland.")
+ (requirement '(file-systems))
+ (provision '(x11-socket-directory))
+ (one-shot? #t)
+ (start #~(lambda _
+ (let ((directory "/tmp/.X11-unix"))
+ (mkdir-p directory)
+ (chmod directory #o1777)))))))
+ (service-type
+ (name 'x11-socket-directory-service)
+ (extensions
+ (list
+ (service-extension shepherd-root-service-type
+ (compose
+ list
+ (const x11-socket-directory-shepherd-service)))))
+ (default-value #f) ; no default value required
+ (description
+ "Create @file{/tmp/.X11-unix} for XWayland. When using X11, libxcb
+takes care of creating that directory however, when using XWayland, we
+need to create it beforehand."))))
+
+(define-deprecated x11-socket-directory-service
+ x11-socket-directory-service-type
;; Return a service that creates /tmp/.X11-unix. When using X11, libxcb
;; takes care of creating that directory. However, when using XWayland, we
;; need to create beforehand. Thus, create it unconditionally here.
- (simple-service 'x11-socket-directory
- activation-service-type
- (with-imported-modules '((guix build utils))
- #~(begin
- (use-modules (guix build utils))
- (let ((directory "/tmp/.X11-unix"))
- (mkdir-p directory)
- (chmod directory #o1777))))))
+ (service x11-socket-directory-service-type))
+
;;;
;;; Enlightenment desktop service.
@@ -1808,7 +1830,7 @@ (define* (desktop-services-for-system #:optional
(service ntp-service-type)
- x11-socket-directory-service
+ (service x11-socket-directory-service-type)
(service pulseaudio-service-type)
(service alsa-service-type)
diff --git a/gnu/tests/lightdm.scm b/gnu/tests/lightdm.scm
index 57d029a75a..d260d844d6 100644
--- a/gnu/tests/lightdm.scm
+++ b/gnu/tests/lightdm.scm
@@ -50,7 +50,7 @@ (define minimal-desktop-services
(service polkit-service-type)
(elogind-service)
(dbus-service)
- x11-socket-directory-service))
+ (service x11-socket-directory-service-type)))
(define %lightdm-os
(operating-system
--
2.38.1
B
B
Bruno Victal wrote on 12 Jan 2023 16:46
[PATCH 2/2] Revert "tests: Add gdm tests."
(address . 60756@debbugs.gnu.org)(name . Bruno Victal)(address . mirai@makinata.eu)
779306f1aa618bc36ddd6d36716aaf67f820cff4.1673537696.git.mirai@makinata.eu
This reverts commit b2a848d23d37f31496e1ff664f1dcf6abcdcc388.

No longer required with the introduction of x11-socket-directory-service-type.

These tests never managed to reveal the problem described in #57589 because
from gnu/system/vm.scm it is seen that "/tmp" is mounted with (needed-for-boot? #t)
and that the virtualized-operating-system procedure strips our custom defined "/tmp"
filesystem entries.
---
gnu/local.mk | 1 -
gnu/tests/gdm.scm | 127 ----------------------------------------------
2 files changed, 128 deletions(-)
delete mode 100644 gnu/tests/gdm.scm

Toggle diff (147 lines)
diff --git a/gnu/local.mk b/gnu/local.mk
index 184f43e753..e0841c8dbb 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -765,7 +765,6 @@ GNU_SYSTEM_MODULES = \
%D%/tests/docker.scm \
%D%/tests/file-sharing.scm \
%D%/tests/ganeti.scm \
- %D%/tests/gdm.scm \
%D%/tests/guix.scm \
%D%/tests/monitoring.scm \
%D%/tests/nfs.scm \
diff --git a/gnu/tests/gdm.scm b/gnu/tests/gdm.scm
deleted file mode 100644
index 70a86b9065..0000000000
--- a/gnu/tests/gdm.scm
+++ /dev/null
@@ -1,127 +0,0 @@
-;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2022 Bruno Victal <mirai@makinata.eu>.
-;;;
-;;; This file is part of GNU Guix.
-;;;
-;;; GNU Guix is free software; you can redistribute it and/or modify it
-;;; under the terms of the GNU General Public License as published by
-;;; the Free Software Foundation; either version 3 of the License, or (at
-;;; your option) any later version.
-;;;
-;;; GNU Guix is distributed in the hope that it will be useful, but
-;;; WITHOUT ANY WARRANTY; without even the implied warranty of
-;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-;;; GNU General Public License for more details.
-;;;
-;;; You should have received a copy of the GNU General Public License
-;;; along with GNU Guix. If not, see <http://www.gnu.org/licenses/>.
-
-(define-module (gnu tests gdm)
- #:use-module (gnu tests)
- #:use-module (gnu packages freedesktop)
- #:use-module (gnu services)
- #:use-module (gnu services desktop)
- #:use-module (gnu services xorg)
- #:use-module (gnu system)
- #:use-module (gnu system file-systems)
- #:use-module (gnu system vm)
- #:use-module (guix gexp)
- #:use-module (ice-9 format)
- #:export (%test-gdm-x11
- %test-gdm-wayland
- %test-gdm-wayland-tmpfs))
-
-(define* (make-os #:key wayland? tmp-tmpfs?)
- (operating-system
- (inherit %simple-os)
- (services
- (modify-services %desktop-services
- (gdm-service-type config => (gdm-configuration
- (inherit config)
- (wayland? wayland?)))))
- (file-systems (if tmp-tmpfs? (cons (file-system
- (mount-point "/tmp")
- (device "none")
- (type "tmpfs")
- (flags '(no-dev no-suid))
- (check? #f))
- %base-file-systems)
- %base-file-systems))))
-
-(define* (run-gdm-test #:key wayland? tmp-tmpfs?)
- "Run tests in a vm which has gdm running."
- (define os
- (marionette-operating-system
- (make-os #:wayland? wayland? #:tmp-tmpfs? tmp-tmpfs?)
- #:imported-modules '((gnu services herd))))
-
- (define vm
- (virtual-machine
- (operating-system os)
- (memory-size 1024)))
-
- (define name (format #f "gdm-~:[x11~;wayland~]~:[~;-tmpfs~]" wayland? tmp-tmpfs?))
-
- (define test
- (with-imported-modules '((gnu build marionette))
- #~(begin
- (use-modules (gnu build marionette)
- (ice-9 format)
- (srfi srfi-64))
-
- (let* ((marionette (make-marionette (list #$vm)))
- (expected-session-type #$(if wayland? "wayland" "x11")))
-
- (test-runner-current (system-test-runner #$output))
- (test-begin #$name)
-
- ;; service for gdm is called xorg-server
- (test-assert "service is running"
- (marionette-eval
- '(begin
- (use-modules (gnu services herd))
- (start-service 'xorg-server))
- marionette))
-
- (test-assert "gdm ready"
- (wait-for-file "/var/run/gdm/gdm.pid" marionette))
-
- (test-equal (string-append "session-type is " expected-session-type)
- expected-session-type
- (marionette-eval
- '(begin
- (use-modules (ice-9 popen)
- (ice-9 rdelim))
-
- (let* ((loginctl #$(file-append elogind "/bin/loginctl"))
- (get-session-cmd (string-join `(,loginctl "show-user" "gdm"
- "--property Display" "--value")))
- (session (call-with-port (open-input-pipe get-session-cmd) read-line))
- (get-type-cmd (string-join `(,loginctl "show-session" ,session
- "--property Type" "--value")))
- (type (call-with-port (open-input-pipe get-type-cmd) read-line)))
- type))
- marionette))
-
- (test-end)))))
-
- (gexp->derivation (string-append name "-test") test))
-
-(define %test-gdm-x11
- (system-test
- (name "gdm-x11")
- (description "Basic tests for the GDM service. (X11)")
- (value (run-gdm-test))))
-
-(define %test-gdm-wayland
- (system-test
- (name "gdm-wayland")
- (description "Basic tests for the GDM service. (Wayland)")
- (value (run-gdm-test #:wayland? #t))))
-
-(define %test-gdm-wayland-tmpfs
- (system-test
- ;; See <https://issues.guix.gnu.org/57589>.
- (name "gdm-wayland-tmpfs")
- (description "Basic tests for the GDM service. (Wayland, /tmp as tmpfs)")
- (value (run-gdm-test #:wayland? #t #:tmp-tmpfs? #t))))
--
2.38.1
B
B
Bruno Victal wrote on 12 Jan 2023 16:49
(no subject)
(address . 60756@debbugs.gnu.org)
e2350b6f-1763-0897-2ee9-360d0450bda9@makinata.eu
cc Ludovic, Grigory
B
B
Bruno Victal wrote on 18 Feb 2023 16:19
[PATCH v2 1/2] services: Add x11-socket-directory-service-type.
(address . 60756@debbugs.gnu.org)
156c8abd42d67c8174f274ca933bdc495efc1962.1676733543.git.mirai@makinata.eu
The x11-socket-directory-service misuses activation-service-type
to create directories. This kind of usage is incorrect since
activation-service-type does not depend of file-systems and incompatible
with user defined /tmp mount.

This commit turns x11-socket-directory-service into a shepherd one-shot
service by defining a new x11-socket-directory-service-type.

* gnu/services/desktop.scm (x11-socket-directory-service-type): New variable.
(x11-socket-directory-service): Deprecate variable.
(desktop-services-for-system): Use new service-type.
* gnu/tests/lightdm.scm: Use new service-type.
---
gnu/services/desktop.scm | 44 ++++++++++++++++++++++++++++++----------
gnu/tests/lightdm.scm | 2 +-
2 files changed, 34 insertions(+), 12 deletions(-)

Toggle diff (94 lines)
diff --git a/gnu/services/desktop.scm b/gnu/services/desktop.scm
index fe1f0fd20a..b2983667b8 100644
--- a/gnu/services/desktop.scm
+++ b/gnu/services/desktop.scm
@@ -14,6 +14,7 @@
;;; Copyright © 2020 Reza Alizadeh Majd <r.majd@pantherx.org>
;;; Copyright © 2021 Brice Waegeneire <brice@waegenei.re>
;;; Copyright © 2021, 2022 muradm <mail@muradm.net>
+;;; Copyright © 2023 Bruno Victal <mirai@makinata.eu>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -148,7 +149,8 @@ (define-module (gnu services desktop)
xfce-desktop-service
xfce-desktop-service-type
- x11-socket-directory-service
+ x11-socket-directory-service ;deprecated
+ x11-socket-directory-service-type
enlightenment-desktop-configuration
enlightenment-desktop-configuration?
@@ -1496,18 +1498,38 @@ (define lxqt-desktop-service-type
;;; X11 socket directory service
;;;
-(define x11-socket-directory-service
+(define x11-socket-directory-service-type
+ (let ((x11-socket-directory-shepherd-service
+ (shepherd-service
+ (documentation "Create /tmp/.X11-unix for XWayland.")
+ (requirement '(file-systems))
+ (provision '(x11-socket-directory))
+ (one-shot? #t)
+ (start #~(lambda _
+ (let ((directory "/tmp/.X11-unix"))
+ (mkdir-p directory)
+ (chmod directory #o1777)))))))
+ (service-type
+ (name 'x11-socket-directory-service)
+ (extensions
+ (list
+ (service-extension shepherd-root-service-type
+ (compose
+ list
+ (const x11-socket-directory-shepherd-service)))))
+ (default-value #f) ; no default value required
+ (description
+ "Create @file{/tmp/.X11-unix} for XWayland. When using X11, libxcb
+takes care of creating that directory however, when using XWayland, we
+need to create it beforehand."))))
+
+(define-deprecated x11-socket-directory-service
+ x11-socket-directory-service-type
;; Return a service that creates /tmp/.X11-unix. When using X11, libxcb
;; takes care of creating that directory. However, when using XWayland, we
;; need to create beforehand. Thus, create it unconditionally here.
- (simple-service 'x11-socket-directory
- activation-service-type
- (with-imported-modules '((guix build utils))
- #~(begin
- (use-modules (guix build utils))
- (let ((directory "/tmp/.X11-unix"))
- (mkdir-p directory)
- (chmod directory #o1777))))))
+ (service x11-socket-directory-service-type))
+
;;;
;;; Enlightenment desktop service.
@@ -1808,7 +1830,7 @@ (define* (desktop-services-for-system #:optional
(service ntp-service-type)
- x11-socket-directory-service
+ (service x11-socket-directory-service-type)
(service pulseaudio-service-type)
(service alsa-service-type)
diff --git a/gnu/tests/lightdm.scm b/gnu/tests/lightdm.scm
index 57d029a75a..d260d844d6 100644
--- a/gnu/tests/lightdm.scm
+++ b/gnu/tests/lightdm.scm
@@ -50,7 +50,7 @@ (define minimal-desktop-services
(service polkit-service-type)
(elogind-service)
(dbus-service)
- x11-socket-directory-service))
+ (service x11-socket-directory-service-type)))
(define %lightdm-os
(operating-system
--
2.39.1
B
B
Bruno Victal wrote on 18 Feb 2023 16:19
[PATCH v2 2/2] tests: gdm: Remove tmpfs related tests.
(address . 60756@debbugs.gnu.org)
a920209ef893b81a0cdae90dbe9c62b91c0fb0c1.1676733543.git.mirai@makinata.eu
No longer required with the introduction of x11-socket-directory-service-type.

This test never managed to reveal the problem described in #57589 because
from gnu/system/vm.scm it is seen that our "/tmp" mount is filtered out and replaced
with a "/tmp" file-system that is mounted with (needed-for-boot? #t). This last bit
is crucial as the problem was caused by the user specified "/tmp" file-system lacking this part
which caused "/tmp" being mounted after x11-socket-directory-service has run,
effectively shadowing the directory.

* gnu/tests/gdm.scm (%test-gdm-wayland-tmpfs): Delete variable.
(make-os): Remove tmpfs? argument.
(run-gdm-test): Remove tmpfs? argument. Add a small delay since
waiting for gdm.pid is not enough causing the tests to fail sporadically.
---
gnu/tests/gdm.scm | 36 +++++++++++-------------------------
1 file changed, 11 insertions(+), 25 deletions(-)

Toggle diff (86 lines)
diff --git a/gnu/tests/gdm.scm b/gnu/tests/gdm.scm
index 70a86b9065..70affb3ee6 100644
--- a/gnu/tests/gdm.scm
+++ b/gnu/tests/gdm.scm
@@ -1,5 +1,5 @@
;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2022 Bruno Victal <mirai@makinata.eu>.
+;;; Copyright © 2022?–?2023 Bruno Victal <mirai@makinata.eu>.
;;;
;;; This file is part of GNU Guix.
;;;
@@ -23,36 +23,26 @@ (define-module (gnu tests gdm)
#:use-module (gnu services desktop)
#:use-module (gnu services xorg)
#:use-module (gnu system)
- #:use-module (gnu system file-systems)
#:use-module (gnu system vm)
#:use-module (guix gexp)
#:use-module (ice-9 format)
#:export (%test-gdm-x11
- %test-gdm-wayland
- %test-gdm-wayland-tmpfs))
+ %test-gdm-wayland))
-(define* (make-os #:key wayland? tmp-tmpfs?)
+(define* (make-os #:key wayland?)
(operating-system
(inherit %simple-os)
(services
(modify-services %desktop-services
(gdm-service-type config => (gdm-configuration
(inherit config)
- (wayland? wayland?)))))
- (file-systems (if tmp-tmpfs? (cons (file-system
- (mount-point "/tmp")
- (device "none")
- (type "tmpfs")
- (flags '(no-dev no-suid))
- (check? #f))
- %base-file-systems)
- %base-file-systems))))
-
-(define* (run-gdm-test #:key wayland? tmp-tmpfs?)
+ (wayland? wayland?)))))))
+
+(define* (run-gdm-test #:key wayland?)
"Run tests in a vm which has gdm running."
(define os
(marionette-operating-system
- (make-os #:wayland? wayland? #:tmp-tmpfs? tmp-tmpfs?)
+ (make-os #:wayland? wayland?)
#:imported-modules '((gnu services herd))))
(define vm
@@ -60,7 +50,7 @@ (define* (run-gdm-test #:key wayland? tmp-tmpfs?)
(operating-system os)
(memory-size 1024)))
- (define name (format #f "gdm-~:[x11~;wayland~]~:[~;-tmpfs~]" wayland? tmp-tmpfs?))
+ (define name (format #f "gdm-~:[x11~;wayland~]" wayland?))
(define test
(with-imported-modules '((gnu build marionette))
@@ -86,6 +76,9 @@ (define* (run-gdm-test #:key wayland? tmp-tmpfs?)
(test-assert "gdm ready"
(wait-for-file "/var/run/gdm/gdm.pid" marionette))
+ ;; waiting for gdm.pid is not enough, tests may still sporadically fail.
+ (sleep 1)
+
(test-equal (string-append "session-type is " expected-session-type)
expected-session-type
(marionette-eval
@@ -118,10 +111,3 @@ (define %test-gdm-wayland
(name "gdm-wayland")
(description "Basic tests for the GDM service. (Wayland)")
(value (run-gdm-test #:wayland? #t))))
-
-(define %test-gdm-wayland-tmpfs
- (system-test
- ;; See <https://issues.guix.gnu.org/57589>.
- (name "gdm-wayland-tmpfs")
- (description "Basic tests for the GDM service. (Wayland, /tmp as tmpfs)")
- (value (run-gdm-test #:wayland? #t #:tmp-tmpfs? #t))))
--
2.39.1
B
B
Bruno Victal wrote on 6 Mar 2023 13:35
[PATCH v3 1/2] services: Add x11-socket-directory-service-type.
(address . 60756@debbugs.gnu.org)
4eab1cc425f89bf321477b432ba407a71285e974.1678105938.git.mirai@makinata.eu
The x11-socket-directory-service misuses activation-service-type
to create directories. This kind of usage is incorrect since
activation-service-type does not depend on file-systems, hence incompatible
with user defined /tmp mount.

This commit turns x11-socket-directory-service into a shepherd one-shot
service by defining a new x11-socket-directory-service-type.

* gnu/services/desktop.scm (x11-socket-directory-service-type): New variable.
(x11-socket-directory-service): Deprecate procedure.
(desktop-services-for-system): Use new service-type.
* gnu/tests/lightdm.scm: Ditto.
---

Changes since v2:
* Tweaked commit message.
* Resolved merge conflict.

gnu/services/desktop.scm | 44 ++++++++++++++++++++++++++++++----------
gnu/tests/lightdm.scm | 2 +-
2 files changed, 34 insertions(+), 12 deletions(-)

Toggle diff (94 lines)
diff --git a/gnu/services/desktop.scm b/gnu/services/desktop.scm
index aa9f93997d..59f325b24b 100644
--- a/gnu/services/desktop.scm
+++ b/gnu/services/desktop.scm
@@ -14,6 +14,7 @@
;;; Copyright © 2020 Reza Alizadeh Majd <r.majd@pantherx.org>
;;; Copyright © 2021 Brice Waegeneire <brice@waegenei.re>
;;; Copyright © 2021, 2022 muradm <mail@muradm.net>
+;;; Copyright © 2023 Bruno Victal <mirai@makinata.eu>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -154,7 +155,8 @@ (define-module (gnu services desktop)
xfce-desktop-service
xfce-desktop-service-type
- x11-socket-directory-service
+ x11-socket-directory-service ;deprecated
+ x11-socket-directory-service-type
enlightenment-desktop-configuration
enlightenment-desktop-configuration?
@@ -1573,18 +1575,38 @@ (define sugar-desktop-service-type
;;; X11 socket directory service
;;;
-(define x11-socket-directory-service
+(define x11-socket-directory-service-type
+ (let ((x11-socket-directory-shepherd-service
+ (shepherd-service
+ (documentation "Create /tmp/.X11-unix for XWayland.")
+ (requirement '(file-systems))
+ (provision '(x11-socket-directory))
+ (one-shot? #t)
+ (start #~(lambda _
+ (let ((directory "/tmp/.X11-unix"))
+ (mkdir-p directory)
+ (chmod directory #o1777)))))))
+ (service-type
+ (name 'x11-socket-directory-service)
+ (extensions
+ (list
+ (service-extension shepherd-root-service-type
+ (compose
+ list
+ (const x11-socket-directory-shepherd-service)))))
+ (default-value #f) ; no default value required
+ (description
+ "Create @file{/tmp/.X11-unix} for XWayland. When using X11, libxcb
+takes care of creating that directory however, when using XWayland, we
+need to create it beforehand."))))
+
+(define-deprecated x11-socket-directory-service
+ x11-socket-directory-service-type
;; Return a service that creates /tmp/.X11-unix. When using X11, libxcb
;; takes care of creating that directory. However, when using XWayland, we
;; need to create beforehand. Thus, create it unconditionally here.
- (simple-service 'x11-socket-directory
- activation-service-type
- (with-imported-modules '((guix build utils))
- #~(begin
- (use-modules (guix build utils))
- (let ((directory "/tmp/.X11-unix"))
- (mkdir-p directory)
- (chmod directory #o1777))))))
+ (service x11-socket-directory-service-type))
+
;;;
;;; Enlightenment desktop service.
@@ -1885,7 +1907,7 @@ (define* (desktop-services-for-system #:optional
(service ntp-service-type)
- x11-socket-directory-service
+ (service x11-socket-directory-service-type)
(service pulseaudio-service-type)
(service alsa-service-type)
diff --git a/gnu/tests/lightdm.scm b/gnu/tests/lightdm.scm
index dda472bd74..6011d2c515 100644
--- a/gnu/tests/lightdm.scm
+++ b/gnu/tests/lightdm.scm
@@ -50,7 +50,7 @@ (define minimal-desktop-services
(service polkit-service-type)
(service elogind-service-type)
(service dbus-root-service-type)
- x11-socket-directory-service))
+ (service x11-socket-directory-service-type)))
(define %lightdm-os
(operating-system
--
2.39.1
B
B
Bruno Victal wrote on 6 Mar 2023 13:35
[PATCH v3 2/2] tests: gdm: Remove tmpfs related tests.
(address . 60756@debbugs.gnu.org)
56f1f2fbd4a0b9b442a53ebb41137a1d982d4bbe.1678105938.git.mirai@makinata.eu
This test never managed to reveal the problem described in [1] because
from gnu/system/vm.scm it is seen that our "/tmp" mount is filtered out and
replaced with a "/tmp" file-system that is mounted with (needed-for-boot? #t).
This last bit is crucial as the problem was caused by the user specified "/tmp"
file-system lacking this part which caused "/tmp" being mounted after
x11-socket-directory-service has run, effectively shadowing the directory.


* gnu/tests/gdm.scm (%test-gdm-wayland-tmpfs): Delete variable.
(make-os): Remove tmpfs? argument.
(run-gdm-test): Remove tmpfs? argument. Add a small delay since
waiting for gdm.pid is not enough, causing the tests to fail sporadically.
---

Changes since v2:
* Tweaked commit message.
* substitute let* with let

gnu/tests/gdm.scm | 40 +++++++++++++---------------------------
1 file changed, 13 insertions(+), 27 deletions(-)

Toggle diff (97 lines)
diff --git a/gnu/tests/gdm.scm b/gnu/tests/gdm.scm
index 70a86b9065..ec1df4b797 100644
--- a/gnu/tests/gdm.scm
+++ b/gnu/tests/gdm.scm
@@ -1,5 +1,5 @@
;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2022 Bruno Victal <mirai@makinata.eu>.
+;;; Copyright © 2022?–?2023 Bruno Victal <mirai@makinata.eu>.
;;;
;;; This file is part of GNU Guix.
;;;
@@ -23,36 +23,26 @@ (define-module (gnu tests gdm)
#:use-module (gnu services desktop)
#:use-module (gnu services xorg)
#:use-module (gnu system)
- #:use-module (gnu system file-systems)
#:use-module (gnu system vm)
#:use-module (guix gexp)
#:use-module (ice-9 format)
#:export (%test-gdm-x11
- %test-gdm-wayland
- %test-gdm-wayland-tmpfs))
+ %test-gdm-wayland))
-(define* (make-os #:key wayland? tmp-tmpfs?)
+(define* (make-os #:key wayland?)
(operating-system
(inherit %simple-os)
(services
(modify-services %desktop-services
(gdm-service-type config => (gdm-configuration
(inherit config)
- (wayland? wayland?)))))
- (file-systems (if tmp-tmpfs? (cons (file-system
- (mount-point "/tmp")
- (device "none")
- (type "tmpfs")
- (flags '(no-dev no-suid))
- (check? #f))
- %base-file-systems)
- %base-file-systems))))
-
-(define* (run-gdm-test #:key wayland? tmp-tmpfs?)
+ (wayland? wayland?)))))))
+
+(define* (run-gdm-test #:key wayland?)
"Run tests in a vm which has gdm running."
(define os
(marionette-operating-system
- (make-os #:wayland? wayland? #:tmp-tmpfs? tmp-tmpfs?)
+ (make-os #:wayland? wayland?)
#:imported-modules '((gnu services herd))))
(define vm
@@ -60,7 +50,7 @@ (define* (run-gdm-test #:key wayland? tmp-tmpfs?)
(operating-system os)
(memory-size 1024)))
- (define name (format #f "gdm-~:[x11~;wayland~]~:[~;-tmpfs~]" wayland? tmp-tmpfs?))
+ (define name (format #f "gdm-~:[x11~;wayland~]" wayland?))
(define test
(with-imported-modules '((gnu build marionette))
@@ -69,8 +59,8 @@ (define* (run-gdm-test #:key wayland? tmp-tmpfs?)
(ice-9 format)
(srfi srfi-64))
- (let* ((marionette (make-marionette (list #$vm)))
- (expected-session-type #$(if wayland? "wayland" "x11")))
+ (let ((marionette (make-marionette (list #$vm)))
+ (expected-session-type #$(if wayland? "wayland" "x11")))
(test-runner-current (system-test-runner #$output))
(test-begin #$name)
@@ -86,6 +76,9 @@ (define* (run-gdm-test #:key wayland? tmp-tmpfs?)
(test-assert "gdm ready"
(wait-for-file "/var/run/gdm/gdm.pid" marionette))
+ ;; waiting for gdm.pid is not enough, tests may still sporadically fail.
+ (sleep 1)
+
(test-equal (string-append "session-type is " expected-session-type)
expected-session-type
(marionette-eval
@@ -118,10 +111,3 @@ (define %test-gdm-wayland
(name "gdm-wayland")
(description "Basic tests for the GDM service. (Wayland)")
(value (run-gdm-test #:wayland? #t))))
-
-(define %test-gdm-wayland-tmpfs
- (system-test
- ;; See <https://issues.guix.gnu.org/57589>.
- (name "gdm-wayland-tmpfs")
- (description "Basic tests for the GDM service. (Wayland, /tmp as tmpfs)")
- (value (run-gdm-test #:wayland? #t #:tmp-tmpfs? #t))))
--
2.39.1
J
J
Josselin Poiret wrote on 6 Mar 2023 14:16
Re: [bug#60756] [PATCH v2 1/2] services: Add x11-socket-directory-service-type.
(address . 60756@debbugs.gnu.org)
877cvuqau2.fsf@jpoiret.xyz
This message wasn't cced to the ML, so here it is again (sorry)
FTR. This was in response to v2.



Hi Bruno,

Bruno Victal <mirai@makinata.eu> writes:

Toggle quote (4 lines)
> The x11-socket-directory-service misuses activation-service-type
> to create directories. This kind of usage is incorrect since
> activation-service-type does not depend of file-systems and incompatible

Small typo: s/depend of/depend on/, that can be fixed by the committer.

Toggle quote (10 lines)
> with user defined /tmp mount.
>
> This commit turns x11-socket-directory-service into a shepherd one-shot
> service by defining a new x11-socket-directory-service-type.
>
> * gnu/services/desktop.scm (x11-socket-directory-service-type): New variable.
> (x11-socket-directory-service): Deprecate variable.
> (desktop-services-for-system): Use new service-type.
> * gnu/tests/lightdm.scm: Use new service-type.

Looks good to me, tested it myself (note to self: don't forget
`-enable-kvm`). Removing the tmpfs-specifc test is a good call here as
well.

Noting here that for the same reason as the test being useless, you
can't test this patchset properly with `guix system vm`, since the
file-systems get overridden. I tested it with `guix system image`
instead, which only overrides the root and esp file systems if present.

Best,
--
Josselin Poiret
M
M
Maxim Cournoyer wrote on 21 Mar 2023 21:50
Re: bug#60756: [PATCH 0/2] Add x11-socket-directory-service-type.
(name . Bruno Victal)(address . mirai@makinata.eu)
87y1nplth4.fsf_-_@gmail.com
Hi,

Bruno Victal <mirai@makinata.eu> writes:

Toggle quote (13 lines)
> The x11-socket-directory-service misuses activation-service-type
> to create directories. This kind of usage is incorrect since
> activation-service-type does not depend on file-systems, hence incompatible
> with user defined /tmp mount.
>
> This commit turns x11-socket-directory-service into a shepherd one-shot
> service by defining a new x11-socket-directory-service-type.
>
> * gnu/services/desktop.scm (x11-socket-directory-service-type): New variable.
> (x11-socket-directory-service): Deprecate procedure.
> (desktop-services-for-system): Use new service-type.
> * gnu/tests/lightdm.scm: Ditto.

I've applied this series, with the small change:

Toggle snippet (12 lines)
modified gnu/services/desktop.scm
@@ -1578,7 +1578,7 @@ (define sugar-desktop-service-type
(define x11-socket-directory-service-type
(let ((x11-socket-directory-shepherd-service
(shepherd-service
- (documentation "Create /tmp/.X11-unix for XWayland.")
+ (documentation "Create @file{/tmp/.X11-unix} for XWayland.")
(requirement '(file-systems))
(provision '(x11-socket-directory))
(one-shot? #t)

Thanks for the contribution and to Josselin for the review, which made
me much more confident to install it (along with the QA badge).

--
Thanks,
Maxim
Closed
?