Move some qt5 style packages to qt5-packages.nix #102594
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ofborg
bot
added
6.topic: nixos
6.topic: pantheon
7 tasks
FRidh
reviewed
Nov 3, 2020
nixos/modules/config/qt5.nix
Outdated
| @@ -9,8 +9,18 @@ let | |||
| isQGnome = cfg.platformTheme == "gnome" && builtins.elem cfg.style ["adwaita" "adwaita-dark"]; | |||
| isQtStyle = cfg.platformTheme == "gtk2" && !(builtins.elem cfg.style ["adwaita" "adwaita-dark"]); | |||
|
|
|||
| packages = if isQGnome then [ pkgs.qgnomeplatform pkgs.adwaita-qt ] | |||
| else if isQtStyle then [ pkgs.libsForQt5.qtstyleplugins ] | |||
| packages = if isQGnome then [ | |||
There was a problem hiding this comment.
I don't know what this config does, but this seems a bit like using a hammer?
There was a problem hiding this comment.
TBH I don't use it as well. I think it just sets some env vars and adds packages to the system path, not sure how exactly that does what it should, but @worldofpeace is the author of it so they should know better.
ttuegel
requested changes
Nov 3, 2020
|
I have the same question, this is like a big hammer. Currently 👎 |
OK I don't know if this change even helps the situation, but I think it is self contradictory to pin packages to libsForQt514, while not adding the qt514 versions of these themes to people's closures. If we had a qt-applications.nix, and somehow magically we'd be able to track down in this module what qt514 packages are there in someone's But even without a qt-applications.nix, the least we should do is to track down to the bit why some packages are pinned to qt514, and if we'd find out many can use qt515, we maybe could consider that putting there only the qt515 versions of the themes is sensible, while we know that qt514 applications would want the qt514 version of the themes. Then we could maybe document in the modules' options' descriptions that one should add more qt versions of the themse to their We could as a start, indeed not add all of these versions of the themes and use only the qt515 versions of them. Then, we could introduce a new option that would be an array of qt versions one wishes to add the versions of the themes to their closure. But, when I consider the scheme of this course of resolution, and thinking about what default value to give to that qt versions array, I think that we have to at least track down why packages are pinned to qt514, and find out whether it's justified or not. After that, considering how many packages are pinned righteously to qt514 we'd be able to sensibly decide whether that array of qt5 versions for the themes should include 514 or not. |
|
It seems this doesn’t fix #98009 (am I testing it correctly?): nix-shell -E 'let pkgs=import (fetchTarball {url="https://github.com/NixOS/nixpkgs/archive/d06a89efc918154353af5fe74d1be5b43d48f79a.tar.gz";}) {}; in pkgs.mkShell {buildInputs = [pkgs.vlc];}' --run vlc
|
I'm afraid not. The nixos module that I changed here is installing some qt libraries to your system, and this change ensures other qt libraries are installed as well. So, you'd need to rebuild your system with this patch - i.e perhaps: Note the change in the hash. |
|
@doronbehar Rebuild failed with (see the bottom part about incompatible types, actually it seems that it was |
|
@doronbehar Nope, it was something different: |
|
OK, sorry to put you into the effort before testing myself. Indeed qgnomeplatform 0.6.1 doesn't build with qt5.15, without an upstream patch I included only now. I tested all new packages and now all of them build. |
|
The command to build a system with this PR is: sudo nixos-rebuild switch -I nixpkgs=https://github.com/doronbehar/nixpkgs/archive/f54a8fb491a3f7914d24e1f605218f8081de7bfb.tar.gz |
|
It’s still rebuilding... Takes a lot of time. |
|
@gnidorah Nope |
|
Same for audacious |
@unclechu I don't know if you finished building your system with this PR, but you could also try this, and it should cost you less rebuilds: |
|
@doronbehar It’s still rebuilding, I don’t want to interrupt it since it spent already so much time, though I have no idea is it close to the end or far behind it. By the way, thanks for the MR as a patch download link example. I didn’t know this is possible, very handy. |
|
@doronbehar Do you have any ideas why this takes so much time to rebuild? Why couldn’t it take most of the derivations from binary cache? And why do you think that solution with applying a patch would take less rebuilding time? |
|
@doronbehar @gnidorah |
|
@doronbehar Your latest example works for |
|
Now in the |
What do you mean? This PR can be used to do it? Would you like to understand why this PR works? |
|
I marked this as stale due to inactivity. → More info |
stale
bot
added
the
2.status: stale
|
@doronbehar sorry, I forgot to answer to your question back then. Anyway, as of now on NixOS 21.05 there are no problems with dark theme inheritance by Qt applications. This MR became obsolete and can be closed, the problem is already solved. I have a permission to close the MR but I neither wasn’t the author nor a reviewer. So I leave this to the author @doronbehar or to the reviewers (@FRidh @ttuegel). |
stale
bot
removed
the
2.status: stale
Also make pantheon module use the attribute without relying on the alias.
doronbehar
force-pushed
the
pkg/qt515/postFix
branch
from
f54a8fb
to
00508b2
Compare
ofborg
bot
requested review from
amaxine,
dasj19,
hedning and
jtojnar
and removed request for
worldofpeace
ofborg
bot
added
10.rebuild-darwin: 1-10
10.rebuild-linux: 11-100
and removed
10.rebuild-darwin: 0
10.rebuild-linux: 1-10
labels
My suspicion is that there are less qt514 packages on NixOS 21.05 and that's why you don't experience the issues you experienced in the past. Due to that, I think that now, it'd be appropriate to not add all qt versions of the packages in the |
Refresh gsettings schemas path patch and remove other hooks.
doronbehar
changed the title
|
Updated qgnomeplatform to 0.8.0 as well. |
|
Most of the changes here were done in #136071 . |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation for this change
#98009
Things done
Didn't test a thing.
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)