Skip to content

Xsessions rework #102322

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
poscat0x04 wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Xsessions rework #102322

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b4eea4b
nixos/{desktop,display,window}-managers: rework xsessions generation
Nov 1, 2020
bdeaec9
nixos/window-managers/none: remove
Nov 1, 2020
7c5f54b
nixos/desktop-managers/{none,plasma5}: allow to use external window m…
Nov 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion nixos/modules/module-list.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -965,7 +965,6 @@
./services/x11/window-managers/icewm.nix
./services/x11/window-managers/bspwm.nix
./services/x11/window-managers/metacity.nix
./services/x11/window-managers/none.nix
./services/x11/window-managers/twm.nix
./services/x11/window-managers/windowlab.nix
./services/x11/window-managers/wmii.nix
Expand Down
28 changes: 18 additions & 10 deletions nixos/modules/services/x11/desktop-managers/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,15 +69,23 @@ in
scripts before forwarding the value to the
<varname>displayManager</varname>.
'';
apply = map (d: d // {
manage = "desktop";
start = d.start
+ optionalString (needBGCond d) ''
if [ -e $HOME/.background-image ]; then
${pkgs.feh}/bin/feh --bg-${cfg.wallpaper.mode} ${optionalString cfg.wallpaper.combineScreens "--no-xinerama"} $HOME/.background-image
fi
'';
});
apply = map (d:
let
dm = { supportExternalWM = false; } // d;
str = optionalString (needBGCond d) ''
if [ -e $HOME/.background-image ]; then
${pkgs.feh}/bin/feh --bg-${cfg.wallpaper.mode} ${optionalString cfg.wallpaper.combineScreens "--no-xinerama"} $HOME/.background-image
fi
'';
in
dm //(
if dm.supportExternalWM
then {
genStart = wm: dm.genStart wm + str;
}
else {
start = d.start + str;
}));
};

default = mkOption {
Expand All @@ -95,5 +103,5 @@ in

};

config.services.xserver.displayManager.session = cfg.session;
config.services.xserver.displayManager.dmSessions = cfg.session;
}
5 changes: 4 additions & 1 deletion nixos/modules/services/x11/desktop-managers/none.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
{
services.xserver.desktopManager.session =
[ { name = "none";
start = "";
supportExternalWM = true;
genStart = wm: ''
${wm}
'';
}
];
}
10 changes: 9 additions & 1 deletion nixos/modules/services/x11/desktop-managers/plasma5.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,15 @@ in
services.xserver.desktopManager.session = singleton {
name = "plasma5";
bgSupport = true;
start = startplasma;
genStart = wm:
if pkg == null
then startplasma
else ''
export KDEWM=${wm}

${startplasma}
'';
supportExternalWM = true;
};

security.wrappers = {
Expand Down
136 changes: 73 additions & 63 deletions nixos/modules/services/x11/display-managers/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,32 +210,26 @@ in
'';
};

session = mkOption {
dmSessions = mkOption {
default = [];
description = ''
List of enabled desktop manager sessions. Each session can
either support or not support the use of external window
managers. If external window are supported, it must provide
a function <varname>genStart</varname> that takes a package
(the window manager's startup script) and returns a string
(the desktop manager's startup script), otherwise it should
provide a string <varname>start</varname> that represents its
startup script.
'';
};

wmSessions = mkOption {
default = [];
example = literalExample
''
[ { manage = "desktop";
name = "xterm";
start = '''
''${pkgs.xterm}/bin/xterm -ls &
waitPID=$!
''';
}
]
'';
description = ''
List of sessions supported with the command used to start each
session. Each session script can set the
<varname>waitPID</varname> shell variable to make this script
wait until the end of the user session. Each script is used
to define either a window manager or a desktop manager. These
can be differentiated by setting the attribute
<varname>manage</varname> either to <literal>"window"</literal>
or <literal>"desktop"</literal>.

The list of desktop manager and window manager should appear
inside the display manager with the desktop manager name
followed by the window manager name.
List of enabled window manager sessions. All sessions should
provide a string <varname>start</varname> that represents its
startup script
'';
};

Expand Down Expand Up @@ -414,22 +408,17 @@ in
# that do not have upstream session files (those defined using services.{display,desktop,window}Manager.session options).
services.xserver.displayManager.sessionPackages =
let
dms = filter (s: s.manage == "desktop") cfg.displayManager.session;
wms = filter (s: s.manage == "window") cfg.displayManager.session;
dms = cfg.displayManager.dmSessions;
wms = cfg.displayManager.wmSessions;

# Script responsible for starting the window manager and the desktop manager.
xsession = dm: wm: pkgs.writeScript "xsession" ''
#! ${pkgs.bash}/bin/bash

xsession = startup: pkgs.writeShellScript "xsession" ''
# Legacy session script used to construct .desktop files from
# `services.xserver.displayManager.session` entries. Called from
# `sessionWrapper`.

# Start the window manager.
${wm.start}

# Start the desktop manager.
${dm.start}
# Run the startup script.
${startup}

${optionalString cfg.updateDbusEnvironment ''
${lib.getBin pkgs.dbus}/bin/dbus-update-activation-environment --systemd --all
Expand All @@ -441,39 +430,60 @@ in

exit 0
'';
writeSessionFile = {sessionName, script, desktopNames}:
pkgs.writeTextFile {
name = "${sessionName}-xsession";
destination = "/share/xsessions/${sessionName}.desktop";
# Desktop Entry Specification:
# - https://standards.freedesktop.org/desktop-entry-spec/latest/
# - https://standards.freedesktop.org/desktop-entry-spec/latest/ar01s06.html
text = ''
[Desktop Entry]
Version=1.0
Type=XSession
TryExec=${script}
Exec=${script}
Name=${sessionName}
DesktopNames=${desktopNames}
'';
} // {
providedSessions = [ sessionName ];
};
in
# We will generate every possible pair of WM and DM.
concatLists (
builtins.map
({dm, wm}: let
sessionName = "${dm.name}${optionalString (wm.name != "none") ("+" + wm.name)}";
script = xsession dm wm;
desktopNames = if dm ? desktopNames
then concatStringsSep ";" dm.desktopNames
else sessionName;
in
optional (dm.name != "none" || wm.name != "none")
(pkgs.writeTextFile {
name = "${sessionName}-xsession";
destination = "/share/xsessions/${sessionName}.desktop";
# Desktop Entry Specification:
# - https://standards.freedesktop.org/desktop-entry-spec/latest/
# - https://standards.freedesktop.org/desktop-entry-spec/latest/ar01s06.html
text = ''
[Desktop Entry]
Version=1.0
Type=XSession
TryExec=${script}
Exec=${script}
Name=${sessionName}
DesktopNames=${desktopNames}
'';
} // {
providedSessions = [ sessionName ];
})
)
builtins.map
({dm, wm}: optional dm.supportExternalWM
(let
sessionName = "${dm.name}+${wm.name}";
startup = dm.genStart wm.bin;
Copy link
Contributor

@xaverdh xaverdh Feb 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be wm.start instead of wm.bin?

Copy link
Contributor

@xaverdh xaverdh Feb 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my system fails to start from this pr (rebased on master) because the waitPID thing fails (the additional indirection introduced when calling the resulting script, hides the variable from the caller). If I replace this with wm.start as it used to be in the old code, the wm startup script gets included verbatim and it works again.

Copy link
Contributor

@taku0 taku0 Feb 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xaverdh could you provide a reproducing configuration?

Copy link
Contributor

@xaverdh xaverdh Feb 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should do (tested in vm)

{ pkgs, ... }:
{
  users.users.bob.isNormalUser = true;
  services.xserver = {
    enable = true;
    windowManager.xmonad.enable = true;
    displayManager = {
      autoLogin = {
        enable = true;
        user = "bob";
      };
      lightdm = {
        enable = true;
        greeter.enable = false;
      };
    };
  };
}

You can leave out the auto login stuff and log in manually as bob if you want.

Copy link
Contributor

@taku0 taku0 Feb 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The configuration doesn't work on master (without this PR) on QEMU:

rm -f nixos.qcow2 && nixos-rebuild build-vm -I nixos-config=./configuration.nix -I nixpkgs=. && ./result/bin/run-nixos-vm

Copy link
Contributor

@xaverdh xaverdh Feb 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works for me from master. What error do you get / in what way does it fail?
Note: If you just get a black screen, then everything is working (xmonad is a rather minimal window manager).

Copy link
Contributor

@taku0 taku0 Feb 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got a black screen. OK, I proceed to testing the PR.

Copy link
Contributor

@taku0 taku0 Feb 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current implementation generates the following script:

#! /nix/store/cwnwyy82wrq53820z6yg7869z8dl5s7g-bash-4.4-p23/bin/bash

# Legacy session script used to construct .desktop files from
# `services.xserver.displayManager.session` entries. Called from
# `sessionWrapper`.

# Start the window manager.
systemd-cat -t xmonad -- /nix/store/4nl2kcgi927xckk4p8lkxpxi4sxd65wy-xmonad-with-packages-8.10.4/bin/xmonad  &
waitPID=$!


# Start the desktop manager.
if [ -e $HOME/.background-image ]; then
  /nix/store/zv8gfabp44k5f1zxl3dkq7pvx6kqvpls-feh-3.6.3/bin/feh --bg-scale  $HOME/.background-image
fi




test -n "$waitPID" && wait "$waitPID"

/run/current-system/systemd/bin/systemctl --user stop graphical-session.target

exit 0

The new implementation generates the following script:

#!/nix/store/cwnwyy82wrq53820z6yg7869z8dl5s7g-bash-4.4-p23/bin/bash
# Legacy session script used to construct .desktop files from
# `services.xserver.displayManager.session` entries. Called from
# `sessionWrapper`.

# Run the startup script.
/nix/store/71ql8j8sdcnc5dpnn35syvy1h4gjwpv4-run-xmonad
if [ -e $HOME/.background-image ]; then
  /nix/store/zv8gfabp44k5f1zxl3dkq7pvx6kqvpls-feh-3.6.3/bin/feh --bg-scale  $HOME/.background-image
fi




test -n "$waitPID" && wait "$waitPID"

/run/current-system/systemd/bin/systemctl --user stop graphical-session.target

exit 0

where /nix/store/71ql8j8sdcnc5dpnn35syvy1h4gjwpv4-run-xmonad is the following.

#!/nix/store/cwnwyy82wrq53820z6yg7869z8dl5s7g-bash-4.4-p23/bin/bash
systemd-cat -t xmonad -- /nix/store/4nl2kcgi927xckk4p8lkxpxi4sxd65wy-xmonad-with-packages-8.10.4/bin/xmonad  &
waitPID=$!

The script sets waitPID but it is not propagated to the parent script.

So, using wm.start (raw script text) instead of wm.bin (script file name), as @xaverdh states, will fix this.

@poscat0x04 could you fix this?

script = xsession startup;
desktopNames = if dm ? desktopNames
then concatStringsSep ";" dm.desktopNames
else sessionName;
in
writeSessionFile {
inherit sessionName script desktopNames;
}
))
(cartesianProductOfSets { dm = dms; wm = wms; })
);
) ++ map (dm:
let
sessionName = dm.name;
startup =
if dm.supportExternalWM
then dm.genStart null
else dm.start;
script = xsession startup;
desktopNames =
if dm ? desktopNames
then concatStringsSep ";" dm.desktopNames
else sessionName;
in
writeSessionFile {
inherit sessionName script desktopNames;
}
) (filter (dm: dm.name != "none") dms);

# Make xsessions and wayland sessions available in XDG_DATA_DIRS
# as some programs have behavior that depends on them being present
Expand Down
13 changes: 7 additions & 6 deletions nixos/modules/services/x11/window-managers/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{ config, lib, ... }:
{ config, lib, pkgs, ... }:

with lib;

Expand Down Expand Up @@ -41,7 +41,7 @@ in
./xmonad.nix
./yeahwm.nix
./qtile.nix
./none.nix ];
];

options = {

Expand All @@ -59,9 +59,10 @@ in
scripts before forwarding the value to the
<varname>displayManager</varname>.
'';
apply = map (d: d // {
manage = "window";
});
apply = map (w:
w // {
bin = pkgs.writeShellScript "run-${w.name}" w.start;
});
};

default = mkOption {
Expand All @@ -80,6 +81,6 @@ in
};

config = {
services.xserver.displayManager.session = cfg.session;
services.xserver.displayManager.wmSessions = cfg.session;
};
}
12 changes: 0 additions & 12 deletions nixos/modules/services/x11/window-managers/none.nix
View file
Open in desktop

This file was deleted.