[bug#29447] [PATCH 0/2] Make Xorg modules configurable, remove WindowMaker dependency

[bug#29447] [PATCH 0/2] Make Xorg modules configurable, remove WindowMaker dependency

[Ludovic Courtès]

Hi,

The first patch here cleans up ‘xorg-configuration-file’ et al. such
that users can pass a list of modules to use (until now the list of
modules was hard-coded in ‘xorg-configuration-file’.)

The second one removes WindowMaker as the default fallback session.
Instead, the default is to look for a .desktop file at run time in the
user and system profiles.

The downside is that if no package provides a .dekstop file, then login
fails.  I think that’s acceptable though, because people expect to
specify a list of window managers in the ‘packages’ field anyway, as
show in the lightweight-desktop example.

Thoughts?

Ludo’.

Ludovic Courtès (2):
  services: xorg: Allow users to specify a list of modules.
  services: xorg: Remove WindowMaker as a default fallback.

 doc/guix.texi         |  26 ++++--
 gnu/services/xorg.scm | 244 +++++++++++++++++++++++++++++++++++---------------
 2 files changed, 189 insertions(+), 81 deletions(-)

-- 
2.15.0

[bug#29447] [PATCH 1/2] services: xorg: Allow users to specify a list of modules.

[Ludovic Courtès]

* gnu/services/xorg.scm (%default-xorg-fonts): New variable.
(xorg-configuration-file): Add #:modules and #:fonts.  Rewrite to return
a 'computed-file' that honors MODULES and FONTS.
(xorg-wrapper): Pass #:modules to 'xorg-configuration-file'.
(xorg-start-command): Add #:fonts.  Pass #:fonts and #:modules to
'xorg-configuration-file'.
* doc/guix.texi (X Window): Adjust documentation of 'xorg-start-command'
and 'xorg-configuration-file'.
---
 doc/guix.texi         |  26 +++++---
 gnu/services/xorg.scm | 181 ++++++++++++++++++++++++++++++++------------------
 2 files changed, 134 insertions(+), 73 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index f8188fbb1..f54a07c8d 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -11142,31 +11142,41 @@ The G-Expression denoting the default SLiM theme and its name.
 @end defvr
 
 @deffn {Scheme Procedure} xorg-start-command [#:guile] @
-  [#:configuration-file #f] [#:xorg-server @var{xorg-server}]
-Return a derivation that builds a @var{guile} script to start the X server
-from @var{xorg-server}.  @var{configuration-file} is the server configuration
-file or a derivation that builds it; when omitted, the result of
-@code{xorg-configuration-file} is used.
+  [#:modules %default-xorg-modules] @
+  [#:fonts %default-xorg-fonts] @
+  [#:configuration-file (xorg-configuration-file @dots{})] @
+  [#:xorg-server @var{xorg-server}]
+Return a @code{startx} script in which @var{modules}, a list of X module
+packages, and @var{fonts}, a list of X font directories, are available.  See
+@code{xorg-wrapper} for more details on the arguments.  The result should be
+used in place of @code{startx}.
 
 Usually the X server is started by a login manager.
 @end deffn
 
 @deffn {Scheme Procedure} xorg-configuration-file @
+  [#:modules %default-xorg-modules] @
+  [#:fonts %default-xorg-fonts] @
   [#:drivers '()] [#:resolutions '()] [#:extra-config '()]
 Return a configuration file for the Xorg server containing search paths for
 all the common drivers.
 
+@var{modules} must be a list of @dfn{module packages} loaded by the Xorg
+server---e.g., @code{xf86-video-vesa}, @code{xf86-input-keyboard}, and so on.
+@var{fonts} must be a list of font directories to add to the server's
+@dfn{font path}.
+
 @var{drivers} must be either the empty list, in which case Xorg chooses a
 graphics driver automatically, or a list of driver names that will be tried in
-this order---e.g., @code{(\"modesetting\" \"vesa\")}.
+this order---e.g., @code{("modesetting" "vesa")}.
 
 Likewise, when @var{resolutions} is the empty list, Xorg chooses an
 appropriate screen resolution; otherwise, it must be a list of
 resolutions---e.g., @code{((1024 768) (640 480))}.
 
 Last, @var{extra-config} is a list of strings or objects appended to the
-@code{text-file*} argument list.  It is used to pass extra text to be added
-verbatim to the configuration file.
+configuration file.  It is used to pass extra text to be
+added verbatim to the configuration file.
 @end deffn
 
 @deffn {Scheme Procedure} screen-locker-service @var{package} [@var{name}]
diff --git a/gnu/services/xorg.scm b/gnu/services/xorg.scm
index c5a1a0d42..c71279387 100644
--- a/gnu/services/xorg.scm
+++ b/gnu/services/xorg.scm
@@ -1,6 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2017 Andy Wingo <wingo@igalia.com>
-;;; Copyright © 2013, 2014, 2015, 2016 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2013, 2014, 2015, 2016, 2017 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2015 Sou Bunnbu <iyzsong@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
@@ -45,6 +45,7 @@
   #:use-module (ice-9 match)
   #:export (xorg-configuration-file
             %default-xorg-modules
+            %default-xorg-fonts
             xorg-wrapper
             xorg-start-command
             xinitrc
@@ -70,11 +71,50 @@
 ;;;
 ;;; Code:
 
-(define* (xorg-configuration-file #:key (drivers '()) (resolutions '())
+(define %default-xorg-modules
+  ;; Default list of modules loaded by the server.  Note that the order
+  ;; matters since it determines which driver is going to be used when there's
+  ;; a choice.
+  (list xf86-video-vesa
+        xf86-video-fbdev
+        xf86-video-ati
+        xf86-video-cirrus
+        xf86-video-intel
+        xf86-video-mach64
+        xf86-video-nouveau
+        xf86-video-nv
+        xf86-video-sis
+
+        ;; Libinput is the new thing and is recommended over evdev/synaptics:
+        ;; <http://who-t.blogspot.fr/2015/01/xf86-input-libinput-compatibility-with.html>.
+        xf86-input-libinput
+
+        xf86-input-evdev
+        xf86-input-keyboard
+        xf86-input-mouse
+        xf86-input-synaptics))
+
+(define %default-xorg-fonts
+  ;; Default list of fonts available to the X server.
+  (list (file-append font-alias "/share/fonts/X11/75dpi")
+        (file-append font-alias "/share/fonts/X11/100dpi")
+        (file-append font-alias "/share/fonts/X11/misc")
+        (file-append font-alias "/share/fonts/X11/cyrillic")
+        (file-append font-adobe75dpi "/share/fonts/X11/75dpi")))
+
+(define* (xorg-configuration-file #:key
+                                  (modules %default-xorg-modules)
+                                  (fonts %default-xorg-fonts)
+                                  (drivers '()) (resolutions '())
                                   (extra-config '()))
   "Return a configuration file for the Xorg server containing search paths for
 all the common drivers.
 
+@var{modules} must be a list of @dfn{module packages} loaded by the Xorg
+server---e.g., @code{xf86-video-vesa}, @code{xf86-input-keyboard}, and so on.
+@var{fonts} must be a list of font directories to add to the server's
+@dfn{font path}.
+
 @var{drivers} must be either the empty list, in which case Xorg chooses a
 graphics driver automatically, or a list of driver names that will be tried in
 this order---e.g., @code{(\"modesetting\" \"vesa\")}.
@@ -84,17 +124,32 @@ appropriate screen resolution; otherwise, it must be a list of
 resolutions---e.g., @code{((1024 768) (640 480))}.
 
 Last, @var{extra-config} is a list of strings or objects appended to the
-@code{mixed-text-file} argument list.  It is used to pass extra text to be
+configuration file.  It is used to pass extra text to be
 added verbatim to the configuration file."
-  (define (device-section driver)
-    (string-append "
+  (define all-modules
+    ;; 'xorg-server' provides 'fbdevhw.so' etc.
+    (append modules (list xorg-server)))
+
+  (define build
+    #~(begin
+        (use-modules (ice-9 match)
+                     (srfi srfi-1)
+                     (srfi srfi-26))
+
+        (call-with-output-file #$output
+          (lambda (port)
+            (define drivers
+              '#$drivers)
+
+            (define (device-section driver)
+              (string-append "
 Section \"Device\"
   Identifier \"device-" driver "\"
   Driver \"" driver "\"
 EndSection"))
 
-  (define (screen-section driver resolutions)
-    (string-append "
+            (define (screen-section driver resolutions)
+              (string-append "
 Section \"Screen\"
   Identifier \"screen-" driver "\"
   Device \"device-" driver "\"
@@ -108,65 +163,56 @@ Section \"Screen\"
   EndSubSection
 EndSection"))
 
-  (apply mixed-text-file "xserver.conf" "
-Section \"Files\"
-  FontPath \"" font-alias "/share/fonts/X11/75dpi\"
-  FontPath \"" font-alias "/share/fonts/X11/100dpi\"
-  FontPath \"" font-alias "/share/fonts/X11/misc\"
-  FontPath \"" font-alias "/share/fonts/X11/cyrillic\"
-  FontPath \"" font-adobe75dpi "/share/fonts/X11/75dpi\"
-  ModulePath \"" xf86-video-vesa "/lib/xorg/modules/drivers\"
-  ModulePath \"" xf86-video-fbdev "/lib/xorg/modules/drivers\"
-  ModulePath \"" xf86-video-ati "/lib/xorg/modules/drivers\"
-  ModulePath \"" xf86-video-cirrus "/lib/xorg/modules/drivers\"
-  ModulePath \"" xf86-video-intel "/lib/xorg/modules/drivers\"
-  ModulePath \"" xf86-video-mach64 "/lib/xorg/modules/drivers\"
-  ModulePath \"" xf86-video-nouveau "/lib/xorg/modules/drivers\"
-  ModulePath \"" xf86-video-nv "/lib/xorg/modules/drivers\"
-  ModulePath \"" xf86-video-sis "/lib/xorg/modules/drivers\"
+            (define (expand modules)
+              ;; Append to MODULES the relevant /lib/xorg/modules
+              ;; sub-directories.
+              (append-map (lambda (module)
+                            (filter-map (lambda (directory)
+                                          (let ((full (string-append module
+                                                                     directory)))
+                                            (and (file-exists? full)
+                                                 full)))
+                                        '("/lib/xorg/modules/drivers"
+                                          "/lib/xorg/modules/input"
+                                          "/lib/xorg/modules/multimedia"
+                                          "/lib/xorg/modules/extensions")))
+                          modules))
 
-  # Libinput is the new thing and is recommended over evdev/synaptics
-  # by those who know:
-  # <http://who-t.blogspot.fr/2015/01/xf86-input-libinput-compatibility-with.html>.
-  ModulePath \"" xf86-input-libinput "/lib/xorg/modules/input\"
-
-  ModulePath \"" xf86-input-evdev "/lib/xorg/modules/input\"
-  ModulePath \"" xf86-input-keyboard "/lib/xorg/modules/input\"
-  ModulePath \"" xf86-input-mouse "/lib/xorg/modules/input\"
-  ModulePath \"" xf86-input-synaptics "/lib/xorg/modules/input\"
-  ModulePath \"" xorg-server "/lib/xorg/modules\"
-  ModulePath \"" xorg-server "/lib/xorg/modules/drivers\"
-  ModulePath \"" xorg-server "/lib/xorg/modules/extensions\"
-  ModulePath \"" xorg-server "/lib/xorg/modules/multimedia\"
-EndSection
+            (display "Section \"Files\"\n" port)
+            (for-each (lambda (font)
+                        (format port "  FontPath \"~a\"~%" font))
+                      '#$fonts)
+            (for-each (lambda (module)
+                        (format port
+                                "  ModulePath \"~a\"~%"
+                                module))
+                      (append (expand '#$all-modules)
 
+                              ;; For fbdevhw.so and so on.
+                              (list #$(file-append xorg-server
+                                                   "/lib/xorg/modules"))))
+            (display "EndSection\n" port)
+            (display "
 Section \"ServerFlags\"
   Option \"AllowMouseOpenFail\" \"on\"
-EndSection
-"
-  (string-join (map device-section drivers) "\n") "\n"
-  (string-join (map (cut screen-section <> resolutions)
-                    drivers)
-               "\n")
+EndSection\n" port)
 
-  "\n"
-  extra-config))
+            (display (string-join (map device-section drivers) "\n")
+                     port)
+            (newline port)
+            (display (string-join
+                      (map (cut screen-section <> '#$resolutions)
+                           drivers)
+                      "\n")
+                     port)
+            (newline port)
+
+            (for-each (lambda (config)
+                        (display config port))
+                      '#$extra-config)))))
+
+  (computed-file "xserver.conf" build))
 
-(define %default-xorg-modules
-  (list xf86-video-vesa
-        xf86-video-fbdev
-        xf86-video-ati
-        xf86-video-cirrus
-        xf86-video-intel
-        xf86-video-mach64
-        xf86-video-nouveau
-        xf86-video-nv
-        xf86-video-sis
-        xf86-input-libinput
-        xf86-input-evdev
-        xf86-input-keyboard
-        xf86-input-mouse
-        xf86-input-synaptics))
 
 (define (xorg-configuration-directory modules)
   "Return a directory that contains the @code{.conf} files for X.org that
@@ -196,8 +242,9 @@ in @var{modules}."
 
 (define* (xorg-wrapper #:key
                        (guile (canonical-package guile-2.0))
-                       (configuration-file (xorg-configuration-file))
                        (modules %default-xorg-modules)
+                       (configuration-file (xorg-configuration-file
+                                            #:modules modules))
                        (xorg-server xorg-server))
   "Return a derivation that builds a @var{guile} script to start the X server
 from @var{xorg-server}.  @var{configuration-file} is the server configuration
@@ -221,12 +268,16 @@ in place of @code{/usr/bin/X}."
 
 (define* (xorg-start-command #:key
                              (guile (canonical-package guile-2.0))
-                             (configuration-file (xorg-configuration-file))
                              (modules %default-xorg-modules)
+                             (fonts %default-xorg-fonts)
+                             (configuration-file
+                              (xorg-configuration-file #:modules modules
+                                                       #:fonts fonts))
                              (xorg-server xorg-server))
-  "Return a derivation that builds a @code{startx} script in which a number of
-X modules are available.  See @code{xorg-wrapper} for more details on the
-arguments.  The result should be used in place of @code{startx}."
+  "Return a @code{startx} script in which @var{modules}, a list of X module
+packages, and @var{fonts}, a list of X font directories, are available.  See
+@code{xorg-wrapper} for more details on the arguments.  The result should be
+used in place of @code{startx}."
   (define X
     (xorg-wrapper #:guile guile
                   #:configuration-file configuration-file
-- 
2.15.0

[bug#29447] [PATCH 2/2] services: xorg: Remove WindowMaker as a default fallback.

[Ludovic Courtès]

* gnu/services/xorg.scm (xinitrc)[builder](system-profile)
(user-profile): New variables.
(xsession-command, find-session): New procedures.
When FALLBACK-SESSION is #f, find a valid session at run time.
---
 gnu/services/xorg.scm | 63 ++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 55 insertions(+), 8 deletions(-)

diff --git a/gnu/services/xorg.scm b/gnu/services/xorg.scm
index c71279387..c13d6bf21 100644
--- a/gnu/services/xorg.scm
+++ b/gnu/services/xorg.scm
@@ -296,10 +296,15 @@ used in place of @code{startx}."
                   fallback-session)
   "Return a system-wide xinitrc script that starts the specified X session,
 which should be passed to this script as the first argument.  If not, the
-@var{fallback-session} will be used."
+@var{fallback-session} will be used or, if @var{fallback-session} is false, a
+desktop session from the system or user profile will be used."
   (define builder
     #~(begin
-        (use-modules (ice-9 match))
+        (use-modules (ice-9 match)
+                     (ice-9 regex)
+                     (ice-9 ftw)
+                     (srfi srfi-1)
+                     (srfi srfi-26))
 
         (define (close-all-fdes)
           ;; Close all the open file descriptors except 0 to 2.
@@ -323,16 +328,59 @@ which should be passed to this script as the first argument.  If not, the
             (execl shell shell "--login" "-c"
                    (string-join (cons command args)))))
 
+        (define system-profile
+          "/run/current-system/profile")
+
+        (define user-profile
+          (and=> (getpw (getuid))
+                 (lambda (pw)
+                   (string-append (passwd:dir pw) "/.guix-profile"))))
+
+        (define (xsession-command desktop-file)
+          ;; Read from DESKTOP-FILE its X session command and return it as a
+          ;; list.
+          (define exec-regexp
+            (make-regexp "^[[:blank:]]*Exec=(.*)$"))
+
+          (call-with-input-file desktop-file
+            (lambda (port)
+              (let loop ()
+                (match (read-line port)
+                  ((? eof-object?) #f)
+                  ((= (cut regexp-exec exec-regexp <>) result)
+                   (if result
+                       (string-tokenize (match:substring result 1))
+                       (loop))))))))
+
+        (define (find-session profile)
+          ;; Return an X session command from PROFILE or #f if none was found.
+          (let ((directory (string-append profile "/share/xsessions")))
+            (match (scandir directory
+                            (cut string-suffix? ".desktop" <>))
+              ((or () #f)
+               #f)
+              ((sessions ...)
+               (any xsession-command
+                    (map (cut string-append directory "/" <>)
+                         sessions))))))
+
         (let* ((home          (getenv "HOME"))
                (xsession-file (string-append home "/.xsession"))
                (session       (match (command-line)
-                                ((_)       (list #$fallback-session))
-                                ((_ x ..1) x))))
+                                ((_)
+                                 #$(if fallback-session
+                                       #~(list #$fallback-session)
+                                       #f))
+                                ((_ x ..1)
+                                 x))))
           (if (file-exists? xsession-file)
               ;; Run ~/.xsession when it exists.
               (apply exec-from-login-shell xsession-file session)
-              ;; Otherwise, start the specified session.
-              (apply exec-from-login-shell session)))))
+              ;; Otherwise, start the specified session or a fallback.
+              (apply exec-from-login-shell
+                     (or session
+                         (find-session user-profile)
+                         (find-session system-profile)))))))
 
   (program-file "xinitrc" builder))
 
@@ -450,8 +498,7 @@ reboot_cmd " shepherd "/sbin/reboot\n"
                        (theme %default-slim-theme)
                        (theme-name %default-slim-theme-name)
                        (xauth xauth) (shepherd shepherd) (bash bash)
-                       (auto-login-session (file-append windowmaker
-                                                        "/bin/wmaker"))
+                       (auto-login-session #f)
                        (startx (xorg-start-command)))
   "Return a service that spawns the SLiM graphical login manager, which in
 turn starts the X display server with @var{startx}, a command as returned by
-- 
2.15.0

[bug#29447] [PATCH 0/2] Make Xorg modules configurable, remove WindowMaker dependency

[ng0]

[-- Attachment #1: Type: text/plain, Size: 1517 bytes --]

Ludovic Courtès transcribed 1.0K bytes:
> Hi,
> 
> The first patch here cleans up ‘xorg-configuration-file’ et al. such
> that users can pass a list of modules to use (until now the list of
> modules was hard-coded in ‘xorg-configuration-file’.)
> 
> The second one removes WindowMaker as the default fallback session.
> Instead, the default is to look for a .desktop file at run time in the
> user and system profiles.
> 
> The downside is that if no package provides a .dekstop file, then login
> fails.  I think that’s acceptable though, because people expect to
> specify a list of window managers in the ‘packages’ field anyway, as
> show in the lightweight-desktop example.
> 
> Thoughts?

Overall, good.
What I think is lacking is that we add in the Documentation that
adding a WM/DM is necessary to get started.
It might sound like a little too much of a Cpt. Obvious, but
we can't assume everyone understand the implications of not
defining a WM/DM in the config.

> Ludo’.
> 
> Ludovic Courtès (2):
>   services: xorg: Allow users to specify a list of modules.
>   services: xorg: Remove WindowMaker as a default fallback.
> 
>  doc/guix.texi         |  26 ++++--
>  gnu/services/xorg.scm | 244 +++++++++++++++++++++++++++++++++++---------------
>  2 files changed, 189 insertions(+), 81 deletions(-)
> 
> -- 
> 2.15.0
> 
> 
> 
> 
> 

-- 
GnuPG: A88C8ADD129828D7EAC02E52E22F9BBFEE348588
GnuPG: https://d.n0.is/dist/keys/
  WWW: https://n0.is

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[bug#29447] [PATCH 0/2] Make Xorg modules configurable, remove WindowMaker dependency

[Adonay Felipe Nogueira]

+1

2017-11-25T22:39:25+0000 ng0 wrote:
>
> Overall, good.
> What I think is lacking is that we add in the Documentation that
> adding a WM/DM is necessary to get started.
> It might sound like a little too much of a Cpt. Obvious, but
> we can't assume everyone understand the implications of not
> defining a WM/DM in the config.
>

-- 
- https://libreplanet.org/wiki/User:Adfeno
- Palestrante e consultor sobre /software/ livre (não confundir com
  gratis).
- "WhatsApp"? Ele não é livre. Por favor, veja formas de se comunicar
  instantaneamente comigo no endereço abaixo.
- Contato: https://libreplanet.org/wiki/User:Adfeno#vCard
- Arquivos comuns aceitos (apenas sem DRM): Corel Draw, Microsoft
  Office, MP3, MP4, WMA, WMV.
- Arquivos comuns aceitos e enviados: CSV, GNU Dia, GNU Emacs Org, GNU
  GIMP, Inkscape SVG, JPG, LibreOffice (padrão ODF), OGG, OPUS, PDF
  (apenas sem DRM), PNG, TXT, WEBM.

bug#29447: [PATCH 0/2] Make Xorg modules configurable, remove WindowMaker dependency

[Ludovic Courtès]

Hi,

Adonay Felipe Nogueira <adfeno@hyperbola.info> skribis:

> +1
>
> 2017-11-25T22:39:25+0000 ng0 wrote:
>>
>> Overall, good.
>> What I think is lacking is that we add in the Documentation that
>> adding a WM/DM is necessary to get started.
>> It might sound like a little too much of a Cpt. Obvious, but
>> we can't assume everyone understand the implications of not
>> defining a WM/DM in the config.
>>

That makes sense.  I’ve cleaned up the SLiM documentation a bit and then
adjusted it as you suggest:

  https://git.savannah.gnu.org/cgit/guix.git/commit/?id=65c0f43649b455db94f1e8b0a244a889cb961b25

Thanks for your feedback!

Ludo’.