Xsessions rework #102322
Xsessions rework #102322
Conversation
ofborg
bot
added
6.topic: nixos
|
Code-wise LGTM. Please rebase on |
poscat0x04
force-pushed
the
xsession-rework
branch
from
3cf51c3 to
9655a9b
Compare
The previous xsessions generation method generates one xession file for every pair of window managers and desktop managers. The desktops managers are also agnostic with regards to window manager it is using. This rework allows finer contol over which desktop managers are allowed to generate xsession files.
poscat0x04
force-pushed
the
xsession-rework
branch
from
9655a9b to
7c5f54b
Compare
|
rebased. |
|
We probably need some more testing. It worked when it was introduced, but I'm not sure if it still works now. |
| ({dm, wm}: optional dm.supportExternalWM | ||
| (let | ||
| sessionName = "${dm.name}+${wm.name}"; | ||
| startup = dm.genStart wm.bin; |
There was a problem hiding this comment.
shouldn't this be wm.start instead of wm.bin?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@xaverdh could you provide a reproducing configuration?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
I got a black screen. OK, I proceed to testing the PR.
There was a problem hiding this comment.
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?
|
Apart from the issue mentioned above, this works fine for me. |
|
@poscat0x04, what's the status on this? Could it be made ready for the next release? |
|
I marked this as stale due to inactivity. → More info |
stale
bot
added
the
2.status: stale
Artturin
added
the
12.approvals: 1
stale
bot
removed
the
2.status: stale
stale
bot
added
the
2.status: stale
wegank
added
the
2.status: merge conflict
stale
bot
removed
the
2.status: stale
wegank
added
the
2.status: stale
Motivation for this change
Note:
xcfg := config.services.xserverA summary of how xsession files are currently generated and used:
xcfg.desktopManager.sessionandxcfg.windowManager.sessionresepctively.xcfg.displayManager.sessionPackages.xcfg.displayManager.sessionData) which can be consumed by different display managers (sddm, gdm, etc.)This generation method is less than ideal because:
KDEWMto be set. This also means that the window manager will not have its environment varibles set by the desktop manager, which causes issues like plasma5 + i3 ignore qt theme settings #94482.Summary of this change
An option
supportExternalWMwhich defaults to false is added for all desktop managers. Desktop managers that support external window managers should set this option to true and provide a functiongenStartthat accepts a package (the binary/startup script of the WM) or null (when the DM is used without an external WM) and returns a string (the final startup script for the DM). The generation method is also changed to adapt to this option.Note that #100057 will have to change if this pr is merged.
This pr will close #82074, close #94482
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)