qtile: Fix QTile restart environment #299843

Rutherther · 2024-03-28T21:22:40Z

Description of changes

Issue

I stumbled across a bug in the QTile module where if qtile is restarted via their restart mechanism (https://github.com/qtile/qtile/blob/master/libqtile/core/lifecycle.py#L35), and there are extra packages configured in the module, the Python modules won't be in the new environment, and thus lead to ModuleNotFoundError if the modules are used in config.py.

Notes

The cause of this is that the executable that is being used for restart is the unwrapped package itself, not the python environment that contains the extra packages.

I would welcome a discussion about possible solutions. My solution is sort of an ad-hoc solution, that works, but doesn't seem ideal to me. It seems to me these are basically the options we have:

Change restart behavior inside of QTile to call the correct executable
Make sure the argv[0] for QTile is the python environment wrapper file instead of the QTile unwrapped package
pass packages to the unwrapped package to make module available inside of QTile unwrapped package (what I did)

The other issue is good implementation of one of these options.

I am open to suggestions and willing to implement them myself.

Current solution

Currently I implemented a solution where the package going to the module is overriden, and extraPackages are passed into the propagatedBuildInputs of the unwrapped package. This seems to be working fine. I have also removed the wrapping python environment since it seems to have become obsolete by the change I introduced.

Things done

Add a 👍 reaction to pull requests you find important.

arjan-s · 2024-05-30T10:53:09Z

This PR is obsolete since #307011 was merged with mostly the same code.

Rutherther · 2024-05-30T15:22:37Z

I disagree it's obsolete. This PR solves different issue. Yes, including with it also finalPackage that is now merged as well, so part of this PR should be adapted. The underlying issue is still present even with #307011, since it still uses wrapper environment instead of overriding the inputs of qtile itself.

Rutherther · 2024-05-30T15:46:02Z

Adapted the PR to new #307011 changes.

arjan-s · 2024-05-31T10:38:24Z

Interesting, thanks. And apologies for my too definitive word choice. I couldn't test this myself because I'm not running on xorg but on wayland (where restarting is not possible).

Anyway, I'm looking to change this code anyway in order to support actual session desktop files, so please hold on a bit longer. I hope the changes I want to make will help this case too.

Rutherther · 2024-05-31T12:13:11Z

TL;DR of the problem is that qtile sys.executable is set to one coming from pkgs.qtile instead of the finalPackage (final environment) that has the packages such as qtile-extras.

Sigmanificient · 2024-05-31T12:13:33Z

Qtile xorg user here.

Current my config is running on nixpkgs unstable at this commit (2024-05-21).
Using pkill -f qtile -SIGUSR2 to restart, seems to work fine

...
2024-05-31 13:55:42,431 WARNING libqtile lifecycle.py:_atexit():L33 Restarting Qtile with os.execv(...)

However, I updated my flake inputs to this commit (2024-05-31) and the same manipulation yield the following error:

...
2024-05-31 13:55:42,431 WARNING libqtile lifecycle.py:_atexit():L33 Restarting Qtile with os.execv(...)
2024-05-31 14:07:14,632 WARNING libqtile lifecycle.py:_atexit():L33 Restarting Qtile with os.execv(...)
2024-05-31 14:07:16,705 WARNING libqtile lifecycle.py:_atexit():L33 Restarting Qtile with os.execv(...)

(accidentally restart it twice)

With

 windowManager.qtile = {
   enable = true;
   backend = "x11";
+ extraPackages = ps: [ ps.qtile-extras ];
 };

it seems to work fine too

...
2024-05-31 14:07:16,705 WARNING libqtile lifecycle.py:_atexit():L33 Restarting Qtile with os.execv(...)
2024-05-31 14:12:13,115 WARNING libqtile lifecycle.py:_atexit():L33 Restarting Qtile with os.execv(...)

Am i missing anything?

Rutherther · 2024-05-31T12:15:27Z

Have you imported something from qtile-extras in your config.py?

Sigmanificient · 2024-05-31T12:21:08Z

Oh you right, added AnalogueClock from qtile_extras.widgets

2024-05-31 14:19:54,244 ERROR libqtile manager.py:restart():L250 Preventing restart because of a configuration error:
Traceback (most recent call last):
  File "/nix/store/jn8bd651xg70rpv9d8b5w5a17ayy4058-python3.11-qtile-0.25.0/lib/python3.11/site-packages/libqtile/core/manager.py", line 248, in restart
    self.config.load()
  File "/nix/store/jn8bd651xg70rpv9d8b5w5a17ayy4058-python3.11-qtile-0.25.0/lib/python3.11/site-packages/libqtile/confreader.py", line 129, in load
    self._reload_config_submodules(path)
  File "/nix/store/jn8bd651xg70rpv9d8b5w5a17ayy4058-python3.11-qtile-0.25.0/lib/python3.11/site-packages/libqtile/confreader.py", line 118, in _reload_config_submodules
    importlib.reload(module)
  File "/nix/store/7hnr99nxrd2aw6lghybqdmkckq60j6l9-python3-3.11.9/lib/python3.11/importlib/__init__.py", line 169, in reload
    _bootstrap._exec(spec, module)
  File "<frozen importlib._bootstrap>", line 621, in _exec
  File "<frozen importlib._bootstrap_external>", line 940, in exec_module
  File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
  File "/home/sigmanificient/.config/qtile/core/bar.py", line 1, in <module>
    from qtile_extras import widget
ModuleNotFoundError: No module named 'qtile_extras'

Sigmanificient · 2024-05-31T12:34:52Z

It's a bit hard to test the fix as is for me (without modifying some stuff on my config), i'll write a nixos wm test for it

Sigmanificient · 2024-05-31T13:27:41Z

I am working on a patch on the qtile nixos vm test to check it, not yet working atm:

move qtile.nix -> qtile/default.nix
reflect qtile.nix -> qtile/default.nix in all-tests.nix

nixos/tests/qtile.nix:

add the default config with a small patch featuring qtile_extras
add a test running pkiill -f qtile -SIGUSR2 to force qtile restart

qtile/add-widget.patch:

a small patch file to add the analogue clock from qtile-extras in the bar:

--- qtile.nix	2024-05-31 14:58:12.676212784 +0200
+++ qtile/default.nix	2024-05-31 16:47:08.694610848 +0200
@@ -1,15 +1,46 @@
-import ./make-test-python.nix ({ lib, ...} : {
+import ../make-test-python.nix ({ lib, ...} : {
   name = "qtile";
 
   meta = {
     maintainers = with lib.maintainers; [ sigmanificient ];
   };
 
-  nodes.machine = { pkgs, lib, ... }: {
-    imports = [ ./common/x11.nix ./common/user-account.nix ];
+  nodes.machine = { pkgs, lib, ... }: let
+    qtile-config = pkgs.callPackage
+      ({ stdenvNoCC, fetchurl, tree }:
+      stdenvNoCC.mkDerivation {
+      name = "qtile-config";
+      version = "0.0.1";
+
+      src = fetchurl {
+        url = "https://raw.githubusercontent.com/qtile/qtile/master/libqtile/resources/default_config.py";
+        hash = "sha256-Y5W277CWVNSi4BdgEW/f7Px/MMjnN9W9TDqdOncVwPc=";
+      };
+
+      prePatch = ''
+        cp $src config.py
+      '';
+
+      patches = [ ./add-widget.patch ];
+
+      dontUnpack = true;
+      dontBuild = true;
+
+      installPhase = ''
+        mkdir -p $out
+        cp config.py $out/config.py
+      '';
+    }) {};
+  in {
+    imports = [ ../common/x11.nix ../common/user-account.nix ];
     test-support.displayManager.auto.user = "alice";
 
-    services.xserver.windowManager.qtile.enable = true;
+    services.xserver.windowManager.qtile = {
+      enable = true;
+      configFile = "${qtile-config}/config.py";
+      extraPackages = ps: [ ps.qtile-extras ];
+    };
+
     services.displayManager.defaultSession = lib.mkForce "none+qtile";
 
     environment.systemPackages = [ pkgs.kitty ];
@@ -30,5 +61,8 @@
         machine.wait_for_window(r"alice.*?machine")
         machine.sleep(2)
         machine.screenshot("terminal")
+
+    with subtest("ensure we can restart qtile properly"):
+        ...
   '';
 })

--- /dev/null	2024-05-29 08:25:18.003968520 +0200
+++ qtile/add-widget.patch	2024-05-31 14:55:47.225636465 +0200
@@ -0,0 +1,19 @@
+--- a/config.py	2024-05-31 14:49:23.852287845 +0200
++++ b/config.py	2024-05-31 14:51:00.935182266 +0200
+@@ -29,6 +29,8 @@
+ from libqtile.lazy import lazy
+ from libqtile.utils import guess_terminal
+ 
++from qtile_extras import widget
++
+ mod = "mod4"
+ terminal = guess_terminal()
+ 
+@@ -162,6 +164,7 @@
+                 # NB Systray is incompatible with Wayland, consider using StatusNotifier instead
+                 # widget.StatusNotifier(),
+                 widget.Systray(),
++                widget.AnalogueClock(),
+                 widget.Clock(format="%Y-%m-%d %a %I:%M %p"),
+                 widget.QuickExit(),
+             ],

--- all-tests.nix	2024-05-31 15:03:10.720117695 +0200
+++ all-tests.nix	2024-05-31 15:02:53.903519615 +0200
@@ -783,7 +783,7 @@
   qgis = handleTest ./qgis.nix { qgisPackage = pkgs.qgis; };
   qgis-ltr = handleTest ./qgis.nix { qgisPackage = pkgs.qgis-ltr; };
   qownnotes = handleTest ./qownnotes.nix {};
-  qtile = handleTest ./qtile.nix {};
+  qtile = handleTest ./qtile {};
   quake3 = handleTest ./quake3.nix {};
   quicktun = handleTest ./quicktun.nix {};
   quorum = handleTest ./quorum.nix {};

Rutherther · 2024-05-31T14:35:50Z

Great, thanks for the test. I am not sure what's going on though, your test outputs an error that's completely unrelated to this issue, which can be seen from the stacktrace - for missing qtile-extras there would be stacktrace to config.py where import is. The error happens both with and without this PR applied if I am testing properly, so nothing new introduced by this.

Sigmanificient · 2024-05-31T14:37:05Z

Great, thanks for the test. I am not sure what's going on though, your test outputs an error that's completely unrelated to this issue, which can be seen from the stacktrace - for missing qtile-extras there would be stacktrace to config.py where import is. The error happens both with and without this PR applied if I am testing properly, so nothing new introduced by this.

Any idea how to fix this test, so that it checks properly Qtile restart with qtile-extras to work as expected

Rutherther · 2024-05-31T14:39:23Z

This is not related to qtile-extras, the issue can be reproduced even without that config patch

Sigmanificient · 2024-05-31T14:41:22Z

This is not related to qtile-extras, the issue can be reproduced even without that config patch

I think the command check at the end is wrong, but i dont know what to replace it with.

The test i proposed is not working, but it provide a test to start from (moving to qtile folder, setting config with patch). I would be nice if we made it work to ensure qtile can properly restart (with extra packages too)

Poor wording made it seem finished, my bad 😅
I Edited the comment and patch to reflect it's wip

Of course it's non-blocking btw

arjan-s · 2024-05-31T14:59:28Z

I can probably take a look at the test in the weekend.
Also, like I mentioned, I'm trying to make the qtile package provide proper desktop session files (for login managers), but this probably requires changes to the wrapping, potentially making work on this PR obsolete (in the best case) or incompatible.

Sigmanificient · 2024-05-31T15:04:06Z

Btw when nixos/tests/qtile: Run only on linux (#315749) will be merged, i think i'll pr a test with qtile-extras. Not restart check, but just a test to ensure it works in first place

arjan-s · 2024-06-07T20:58:24Z

FYI: the session files PR should now also fix the underlying issue that this PR addresses

Rutherther · 2024-06-08T06:37:08Z

Nice, yes, that includes basically the same code (and more of course), so it should fix this, thanks. Closing this PR.

github-actions bot added 6.topic: nixos 8.has: module (update) labels Mar 28, 2024

Rutherther changed the title ~~Fix QTile restart environment~~ qtile: Fix QTile restart environment Mar 28, 2024

ofborg bot added 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 labels Mar 28, 2024

wegank added the 2.status: merge conflict label May 22, 2024

Fix QTile restart environment

c8d6fa6

Rutherther force-pushed the qtile-environment-fix branch from b0f1c70 to c8d6fa6 Compare May 30, 2024 15:42

ofborg bot removed the 2.status: merge conflict label May 30, 2024

Sigmanificient mentioned this pull request May 31, 2024

nixos/tests/qtile: test extraPackages and qtile-extras #316208

Open

13 tasks

arjan-s mentioned this pull request Jun 7, 2024

qtile: install proper session .desktop files for both xorg and wayland #316239

Open

13 tasks

Rutherther closed this Jun 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

qtile: Fix QTile restart environment #299843

qtile: Fix QTile restart environment #299843

Rutherther commented Mar 28, 2024

arjan-s commented May 30, 2024

Rutherther commented May 30, 2024

Rutherther commented May 30, 2024

arjan-s commented May 31, 2024

Rutherther commented May 31, 2024

Sigmanificient commented May 31, 2024

Rutherther commented May 31, 2024

Sigmanificient commented May 31, 2024 •

edited

Sigmanificient commented May 31, 2024

Sigmanificient commented May 31, 2024 •

edited

Rutherther commented May 31, 2024

Sigmanificient commented May 31, 2024 •

edited

Rutherther commented May 31, 2024

Sigmanificient commented May 31, 2024 •

edited

arjan-s commented May 31, 2024

Sigmanificient commented May 31, 2024

arjan-s commented Jun 7, 2024

Rutherther commented Jun 8, 2024

qtile: Fix QTile restart environment #299843

qtile: Fix QTile restart environment #299843

Conversation

Rutherther commented Mar 28, 2024

Description of changes

Issue

Notes

Current solution

Things done

arjan-s commented May 30, 2024

Rutherther commented May 30, 2024

Rutherther commented May 30, 2024

arjan-s commented May 31, 2024

Rutherther commented May 31, 2024

Sigmanificient commented May 31, 2024

Rutherther commented May 31, 2024

Sigmanificient commented May 31, 2024 • edited

Sigmanificient commented May 31, 2024

Sigmanificient commented May 31, 2024 • edited

Rutherther commented May 31, 2024

Sigmanificient commented May 31, 2024 • edited

Rutherther commented May 31, 2024

Sigmanificient commented May 31, 2024 • edited

arjan-s commented May 31, 2024

Sigmanificient commented May 31, 2024

arjan-s commented Jun 7, 2024

Rutherther commented Jun 8, 2024

Sigmanificient commented May 31, 2024 •

edited

Sigmanificient commented May 31, 2024 •

edited

Sigmanificient commented May 31, 2024 •

edited

Sigmanificient commented May 31, 2024 •

edited