Permalink
Browse files

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
  • Loading branch information...
primeos committed Apr 23, 2017
1 parent 5c25c33 commit 1273f414a784af87363ac440af2ce948b6a656b1
@@ -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;
@@ -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
@@ -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
{

4 comments on commit 1273f41

@vidbina

This comment has been minimized.

Show comment
Hide comment
@vidbina

vidbina Dec 2, 2017

Contributor

If you have a sec, please explain what if [ "''${1:0:1}" = "/" ]; does. On my tests the hard quotes are always part of the string as characters. I figured you're trying to check that the first character of arg $1 is a backslash with ${1:0:1} but I'm drawing blanks while figuring out the hard quotes. What am I missing? Thank you in advance for helping me understand 🙇

Contributor

vidbina replied Dec 2, 2017

If you have a sec, please explain what if [ "''${1:0:1}" = "/" ]; does. On my tests the hard quotes are always part of the string as characters. I figured you're trying to check that the first character of arg $1 is a backslash with ${1:0:1} but I'm drawing blanks while figuring out the hard quotes. What am I missing? Thank you in advance for helping me understand 🙇

@primeos

This comment has been minimized.

Show comment
Hide comment
@primeos

primeos Dec 2, 2017

Member

Hi @vidbina, yes that's actually a bit strange because it's a mix of Nix and Bash. The two single quotes are needed to escape the ${expr} syntax from the Nix expression language:

Since ${ and '' have special meaning in indented strings, you need a way to quote them. ${ can be escaped by prefixing it with '' (that is, two single quotes), i.e., ''${.

I.e. after the evaluation it will be if [ "${1:0:1}" = "/" ];.

Member

primeos replied Dec 2, 2017

Hi @vidbina, yes that's actually a bit strange because it's a mix of Nix and Bash. The two single quotes are needed to escape the ${expr} syntax from the Nix expression language:

Since ${ and '' have special meaning in indented strings, you need a way to quote them. ${ can be escaped by prefixing it with '' (that is, two single quotes), i.e., ''${.

I.e. after the evaluation it will be if [ "${1:0:1}" = "/" ];.

@vidbina

This comment has been minimized.

Show comment
Hide comment
@vidbina

vidbina Dec 2, 2017

Contributor

I wasn't even looking at this nix ❄️ side of things, but now I vaguely recall glancing over that escaping issue in the manual a while ago. Thank you so much for pointing this out, @primeos. 🙇

Contributor

vidbina replied Dec 2, 2017

I wasn't even looking at this nix ❄️ side of things, but now I vaguely recall glancing over that escaping issue in the manual a while ago. Thank you so much for pointing this out, @primeos. 🙇

@primeos

This comment has been minimized.

Show comment
Hide comment
@primeos

primeos Dec 3, 2017

Member

@vidbina No problem, you're welcome 😄.

Member

primeos replied Dec 3, 2017

@vidbina No problem, you're welcome 😄.

Please sign in to comment.