display-managers: Fix the xsession parameters

The xsession script was called with inconsistent (depending on the display managers) and wrong parameters. The main reason for this where the spaces the parameter syntax. In order to fix this the old syntax: $1 = '<desktop-manager> + <window-manager>' Will be replaced with a new syntax: $1 = "<desktop-manager>+<window-manager>" This assumes that neither "<desktop-manager>" nor "<window-manager>" contain the "+" character but this shouldn't be a problem. This patch also fixes the quoting by using double quotes (") instead of single quotes (') [0]. Last but not least this'll add some comments for the better understanding of the script. [0]: https://specifications.freedesktop.org/desktop-entry-spec/latest/ar01s06.html
NixOS · Apr 28, 2017 · 1273f41 · 1273f41 · vidbina · Dec 2, 2017
1 parent 5c25c33
commit 1273f41
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 10 deletions.
diff --git a/nixos/modules/services/x11/display-managers/default.nix b/nixos/modules/services/x11/display-managers/default.nix
@@ -32,8 +32,32 @@ let
     ''
       #! ${pkgs.bash}/bin/bash
 
-      # Handle being called by SDDM.
-      if test "''${1:0:1}" = / ; then eval exec "$1" "$2" ; fi
+      # Expected parameters:
+      #   $1 = <desktop-manager>+<window-manager>
+
+      # Actual parameters (FIXME):
+      # SDDM is calling this script like the following:
+      #   $1 = /nix/store/xxx-xsession (= $0)
+      #   $2 = <desktop-manager>+<window-manager>
+      # SLiM is using the following parameter:
+      #   $1 = /nix/store/xxx-xsession <desktop-manager>+<window-manager>
+      # LightDM keeps the double quotes:
+      #   $1 = /nix/store/xxx-xsession "<desktop-manager>+<window-manager>"
+      # The fake/auto display manager doesn't use any parameters and GDM is
+      # broken.
+      # If you want to "debug" this script don't print the parameters to stdout
+      # or stderr because this script will be executed multiple times and the
+      # output won't be visible in the log when the script is executed for the
+      # first time (e.g. append them to a file instead)!
+
+      # All of the above cases are handled by the following hack (FIXME).
+      # Since this line is *very important* for *all display managers* it is
+      # very important to test changes to the following line with all display
+      # managers:
+      if [ "''${1:0:1}" = "/" ]; then eval exec "$1" "$2" ; fi
+
+      # Now it should be safe to assume that the script was called with the
+      # expected parameters.
 
       ${optionalString cfg.displayManager.logToJournal ''
         if [ -z "$_DID_SYSTEMD_CAT" ]; then
@@ -107,11 +131,12 @@ let
           fi
       fi
 
-      # The session type is "<desktop-manager> + <window-manager>", so
-      # extract those.
-      windowManager="''${sessionType##* + }"
+      # The session type is "<desktop-manager>+<window-manager>", so
+      # extract those (see:
+      # http://wiki.bash-hackers.org/syntax/pe#substring_removal).
+      windowManager="''${sessionType##*+}"
       : ''${windowManager:=${cfg.windowManager.default}}
-      desktopManager="''${sessionType% + *}"
+      desktopManager="''${sessionType%%+*}"
       : ''${desktopManager:=${cfg.desktopManager.default}}
 
       # Start the window manager.
@@ -142,6 +167,9 @@ let
       exit 0
     '';
 
+  # Desktop Entry Specification:
+  # - https://standards.freedesktop.org/desktop-entry-spec/latest/
+  # - https://standards.freedesktop.org/desktop-entry-spec/latest/ar01s06.html
   mkDesktops = names: pkgs.runCommand "desktops"
     { # trivial derivation
       preferLocalBuild = true;
@@ -155,7 +183,7 @@ let
         Version=1.0
         Type=XSession
         TryExec=${cfg.displayManager.session.script}
-        Exec=${cfg.displayManager.session.script} '${n}'
+        Exec=${cfg.displayManager.session.script} "${n}"
         X-GDM-BypassXsession=true
         Name=${n}
         Comment=
@@ -238,7 +266,7 @@ in
           wm = filter (s: s.manage == "window") list;
           dm = filter (s: s.manage == "desktop") list;
           names = flip concatMap dm
-            (d: map (w: d.name + optionalString (w.name != "none") (" + " + w.name))
+            (d: map (w: d.name + optionalString (w.name != "none") ("+" + w.name))
               (filter (w: d.name != "none" || w.name != "none") wm));
           desktops = mkDesktops names;
           script = xsession wm dm;

diff --git a/nixos/modules/services/x11/display-managers/lightdm.nix b/nixos/modules/services/x11/display-managers/lightdm.nix
@@ -61,7 +61,7 @@ let
     let
       dm = xcfg.desktopManager.default;
       wm = xcfg.windowManager.default;
-    in dm + optionalString (wm != "none") (" + " + wm);
+    in dm + optionalString (wm != "none") ("+" + wm);
 in
 {
   # Note: the order in which lightdm greeter modules are imported

diff --git a/nixos/modules/services/x11/display-managers/sddm.nix b/nixos/modules/services/x11/display-managers/sddm.nix
@@ -69,7 +69,7 @@ let
     let
       dm = xcfg.desktopManager.default;
       wm = xcfg.windowManager.default;
-    in dm + optionalString (wm != "none") (" + " + wm);
+    in dm + optionalString (wm != "none") ("+" + wm);
 
 in
 {