NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
17.69k stars 13.83k forks source link

Night color is broken after updated to plasma 5.21 #117036

Closed poscat0x04 closed 3 years ago

poscat0x04 commented 3 years ago

Describe the bug Night color cannot be enabled and it shows an error message "Failed to connect to the Window Manager" on the settings panel.

To Reproduce Steps to reproduce the behavior:

  1. enable plasma and use kwin as the window manager (default)
  2. open settings -> Display and Monitor -> Night Color

Expected behavior Night color should work

Metadata

Maintainer information:

# a list of nixpkgs attributes affected by the problem
attribute:
# a list of nixos modules affected by the problem
module:
andrevmatos commented 3 years ago

This is caused by the lack of lcms2 on kwin's buildInputs in order for cmake to build the nightcolor plugin. The dependencies for the krunner plugin is also missing. I have this overlay on my system which solves both bugs:

final: prev: {
  plasma5Packages = prev.plasma5Packages // {
    plasma5 = prev.plasma5Packages.plasma5 // {
      kwin = prev.plasma5Packages.plasma5.kwin.overrideAttrs (
        old: {
          buildInputs = (old.buildInputs or [ ]) ++ [ prev.lcms2 prev.plasma5Packages.krunner ];
        }
      );
    };
  };
}
tim-hilt commented 3 years ago

Can confirm; the overlay works for me 👍

mdevlamynck commented 3 years ago

PR? :pray:

tim-hilt commented 3 years ago

In the process, but I don't know, how I could specify plasma5Packages.krunner as a dependency (First PR and Nix Newbie 😄 ).

When I search for krunner via search.nixos.org, I see krunner-pass and some libsForQt...krunner. Do I even need a prefix for krunner or can I just include it?

And as another question: Should I perform the PR directly on master or some other branch?

mdevlamynck commented 3 years ago

In most case you want to target master and maybe backport the change to the current release after (you can find some informations about that in https://github.com/NixOS/nixpkgs#contributing and https://github.com/NixOS/nixpkgs/blob/master/.github/CONTRIBUTING.md).

After a quick search some other package use krunner directly (like kdeplasma-addons and kdevelop) so something like that should work (at least it builds on my machine, I haven't been able to test it yet):

index 2008529a38b..ad5d4e3cfbf 100644
--- a/pkgs/desktops/plasma-5/kwin/default.nix
+++ b/pkgs/desktops/plasma-5/kwin/default.nix
@@ -12,7 +12,7 @@
   kcoreaddons, kcrash, kdeclarative, kdecoration, kglobalaccel, ki18n,
   kiconthemes, kidletime, kinit, kio, knewstuff, knotifications, kpackage,
   kscreenlocker, kservice, kwayland, kwayland-server, kwidgetsaddons, kwindowsystem, kxmlgui,
-  plasma-framework, libcap, libdrm, mesa, pipewire
+  plasma-framework, libcap, libdrm, mesa, pipewire, lcms2, krunner
 }:

 # TODO (ttuegel): investigate qmlplugindump failure
@@ -31,7 +31,7 @@ mkDerivation {
     kcoreaddons kcrash kdeclarative kdecoration kglobalaccel ki18n kiconthemes
     kidletime kinit kio knewstuff knotifications kpackage kscreenlocker kservice
     kwayland kwayland-server kwidgetsaddons kwindowsystem kxmlgui plasma-framework
-    libcap libdrm mesa pipewire
+    libcap libdrm mesa pipewire lcms2 krunner
   ];
   outputs = [ "dev" "out" ];
   patches = [

@andrevmatos What does the kwin plugin for krunner do? I can't find any information about it.

Edit: Maybe the krunner part should be added in a separate PR if it's not required to fix this issue.

tim-hilt commented 3 years ago

I'm currently not able to test the changes on my nixpkgs-fork. Would a PR still be a valid option or should I first test extensively?

mdevlamynck commented 3 years ago

Well making sure the PR actually fixes the issue is kind of needed. I'll make the PR if I can test it. Feel free to do it if you can test it first :)

tim-hilt commented 3 years ago

Thanks for all the information you've given! I think I'll contribute a bit later, when I have more time to get into all this stuff.

mdevlamynck commented 3 years ago

Seems to be already fixed on staging.

solson commented 3 years ago

Is there an easy way to check (or get a notification) for when staging next gets merged into master?

mdevlamynck commented 3 years ago

I think it's part of https://github.com/NixOS/nixpkgs/pull/123279.

andrevmatos commented 3 years ago

krunner plugin for kwin is what allows krunner to search for open windows and select them. If kwin isn't compiled with krunner, this funcitonality doesn't work.

solson commented 3 years ago

This is fixed for me in the latest nixos-unstable.

andrevmatos commented 3 years ago

I can confirm it's fixed on current unstable. This issue can be closed.