nixos/plasma: leave displayManager.setupCommands alone

[Yarny0]

Description of changes

Fix #187963 . I bisected the problem down to commit 4832352 from #100057 .

That commit replaced the plasma5 session startup script with its upstream version from the pkgs.libsForQt5.plasma5.plasma-workspace package. The old script performs some basic initializations in the user's home directory: It adds files/directories like .config and .gtkrc-2.0. Besides, it calls the activationScript of the plasma module which rebuilds the "kbuildsyscoca5 cache" (plasma start menu) in .cache.

However, the old session startup script wasn't removed; it is now used as display manager setup script, i.e., it was added to the option displayManager.setupCommands. I am not sure if this happened intentionally and if it has any advantage.

Most likely, this means all the initialization efforts described above

• are no longer applied to the user session that is opened by the display manager at login,
• are now applied when the display manager itself initializes, with HOME=/. So the display manager will create some dot files in the root filesystem.

I suppose this is not the desired outcome.

The pull-request at hand completely removes the old session startup script. I guess this is the right thing to do, but I am not sure. The script wasn't applied to any user session since #100057 got merged in Oct 2021, so apparently it is not needed. All plasma-related tests (plasma5, plasma5-systemd-start, plasma-bigscreen and even maestral) still pass with the pull request at hand applied. On the other hand, running the session startup script when the display manager starts up might have some positive effect that I am overlooking right now.

Further notes

• notifying the sole KDE desktop maintainer team (qt-kde) member @ttuegel
• notifying the author of the previous commit @pasqui23

Note that the effect of this pull request can easily be applied to a NixOS system -- without merging anything -- by just setting services.xserver.displayManager.setupCommands = lib.mkForce ""; in configuration.nix. This is what I am using on my desktop PC (NixOS 22.05 with plasma5 and lightdm) right now, without any issues. But it should probably be tested by some else as well, e.g. with sddm as display manager.

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 22.11 Release Notes (or backporting 22.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
  • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
• Fits CONTRIBUTING.md.

Result of nixpkgs-review run on x86_64-linux 1

1 package blacklisted:

• nixos-install-tools

[azahi]

I didn't apply your patch directly but setting services.xserver.displayManager.setupCommands = lib.mkForce ""; does fix that issue. I've tested this both on my current Plasma setup and on a fresh one in a VM. It would be great if we could get this PR into 22.11.

[Yarny0]

Thanks for your feedback, @azahi ! Given #187963 (comment) , I suppose you tested with SDDM display manager.

I turned it into a proper (non-draft) PR now. I would be fine with this getting merged, but some feedback from @ttuegel or @pasqui23 would still be reassuring.

[inclyc]

Not a serious review, but this patch is really helpful for me.

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/1792

[samueldr]

See second edit. Sorry for the noise.

Goose chase enclosed

I just started my investigation, but this PR broke Plasma Mobile.

I'm mentioning it so it is known, and I'm now looking into why, and how to fix.

---

EDIT: I expected that the intent behind this change wasn't the real problem, but that this was broken by some other thing relying on some implicit behaviour here.

With this PR applied, so with the breakage, the only thing required to fix is to ensure XDG_CONFIG_HOME is set.

diff --git a/nixos/modules/services/x11/desktop-managers/plasma5.nix b/nixos/modules/services/x11/desktop-managers/plasma5.nix
index eb0373c686c..63013035220 100644
--- a/nixos/modules/services/x11/desktop-managers/plasma5.nix
+++ b/nixos/modules/services/x11/desktop-managers/plasma5.nix
@@ -78,6 +78,10 @@ let
     XDG_CONFIG_HOME=''${XDG_CONFIG_HOME:-$HOME/.config}
   '';
 
+  startplasma = ''
+    ${set_XDG_CONFIG_HOME}
+  '';
+
 in
 
 {
@@ -402,6 +406,7 @@ in
 
       # Update the start menu for each user that is currently logged in
       system.userActivationScripts.plasmaSetup = activationScript;
+      services.xserver.displayManager.setupCommands = startplasma;
 
       nixpkgs.config.firefox.enablePlasmaBrowserIntegration = true;
     })

Will be looking at a more correct fix, but this is interesting. On a hunch I tried an empty setupCommands and that doesn't work.

---

EDIT ♯2: alright, ignore all that, it was masking an error in my config by the presence of setupCommands. An undocumented interaction of setupCommands with lightdm, is that if the script in display-setup-script= fails, it will not attempt to launch. Each fragments added to setupCommands can be ordered in any order, and if a fragment ends-up being the last, and the last command fails, that last command will stop lightdm from working. So the presence of any commands that wouldn't fail here was accidentally happening later than my "broken" setup.