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

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

[Bruno Victal]

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

[bug#60756] [PATCH 1/2] services: Add x11-socket-directory-service-type.

[Bruno Victal]

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(-)

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))
+
 \f
 ;;;
 ;;; 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

[bug#60756] [PATCH 2/2] Revert "tests: Add gdm tests."

[Bruno Victal]

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

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

[bug#60756] (no subject)

[Bruno Victal]

cc Ludovic, Grigory

[bug#60756] [PATCH v2 1/2] services: Add x11-socket-directory-service-type.

[Bruno Victal]

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(-)

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))
+
 \f
 ;;;
 ;;; 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

[bug#60756] [PATCH v2 2/2] tests: gdm: Remove tmpfs related tests.

[Bruno Victal]

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(-)

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

[bug#60756] [PATCH v3 1/2] services: Add x11-socket-directory-service-type.

[Bruno Victal]

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(-)

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))
+
 \f
 ;;;
 ;;; 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

[bug#60756] [PATCH v3 2/2] tests: gdm: Remove tmpfs related tests.

[Bruno Victal]

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.

[1]: <https://issues.guix.gnu.org/57589>

* 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(-)

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

[bug#60756] [PATCH v2 1/2] services: Add x11-socket-directory-service-type.

[Josselin Poiret via Guix-patches via]

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:

> 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.

> 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

bug#60756: [PATCH 0/2] Add x11-socket-directory-service-type.

[Maxim Cournoyer]

Hi,

Bruno Victal <mirai@makinata.eu> writes:

> 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:

--8<---------------cut here---------------start------------->8---
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)
--8<---------------cut here---------------end--------------->8---

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