-
-
Notifications
You must be signed in to change notification settings - Fork 14.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix GTK dependencies in wrappers #24133
Conversation
@abbradar, thanks for your PR! By analyzing the history of the files in this pull request, we identified @peterhoeg, @ttuegel and @dezgeg to be potential reviewers. |
It looks like the search path for plugins is empty. Could you post the contents of the wrapper? |
@@ -47,10 +47,11 @@ stdenv.mkDerivation { | |||
--argv0 '"$0"' \ | |||
--suffix PATH : "$env/bin" \ | |||
--prefix XDG_CONFIG_DIRS : "$env/etc/xdg" \ | |||
--prefix XDG_DATA_DIRS : "$env/share" \ | |||
--prefix XDG_DATA_DIRS : "$env/share:${gtk3}/share/gsettings-schemas/${gtk3.name}" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe only ${gtk3.out}/share
should be put into $XDG_DATA_DIRS
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't gtk3
= gtk3.out
(we don't have bin
for gtk3
)?
Notice that |
Agreed. I am trying to remove That wrapper, though. That is all kinds of messed up.
Something is seriously wrong with |
I think there's more to it:
That's some random Qt 5 application on master. It was unwrapped but when I wrapped it nothing has changed. Any idea what's going on (this is without the patch)? |
Yeah, actually I know exactly what's going on.
env = buildEnv {
inherit name meta;
paths = builtins.map lib.getBin (unwrapped ++ paths);
pathsToLink = [ "/bin" "/share" "/lib/qt5" "/etc/xdg" ];
}; The old |
@ttuegel Strange thing is -- it's not a
EDIT: maybe it's just broken -- any idea of another plain Qt 5 application? |
@abbradar As for antimicro, I can't get X to start in a VM on master, so I'm trying to reproduce this on unstable channel. But, everything works fine on unstable (for me). If you can't reproduce it on unstable either, it is probably related to the Qt 5.8 update. |
Let me retract this statement: there is no problem here. We did have this bug at one time, but it was fixed in a non-obvious way by propagating everything to the environment. |
@ttuegel I do this on unstable too and I can reproduce it perfectly. Anyway, I also can't start qutebrowser from master because of the same reason and other people on IRC can't reproduce that. So this is something with my system... Trying to investigate with strace. |
Interesting... if I build the package with Qt 5.7 on master it works. There may be another source of impurity like what you fixed some time ago. I have |
So, in the end it was |
It may be there. What package does |
With removed EDIT: looking at |
Yeah, I had a feeling about something like that. The disclaimer that should come with Qt-based applications on NixOS is like the standard FCC disclaimer "this device may not produce harmful impurities, but it must accept any impurities." I should really find a way to block off the system profile entirely... |
I've managed now to test those patches properly and fix several things. File dialogs now work for me, along with their settings. |
It's effectively required for GTK3 applications because various parts of the library use GIO to store settings. Also propagate GTK for clarity (it should be there anyway).
Hope it's okay (my limited tests are fine and luckily this is no mass rebuild so it's a bit easier to fix if something is wrong after all). I haven't cherry-picked |
I actually cannot reproduce the okular bug on 17.03, are you sure that happens there, too? 🤔 |
Hm, it should (because Qt 5.7 supports GTK3 and has it in |
Really not sure, it should matter -- maybe Qt 5.7 doesn't use file chooser dialogs from GTK so the problem doesn't trigger? Still, there are other places where GTK uses dconf and it could manifest itself somewhere. so let's be on the safe side (this shouldn't break anything, only grow closure size :D). |
This reverts commit 40db638. Despite testing this in QEMU on my own machine, it breaks SDDM for everyone else.
Qt 5.6 should be the default Qt version on the release branch. |
Motivation for this change
This is a minimal patchset to fix #23474 without introducing some new wrappers infrastructure (so that we can cherry-pick this into release too). It also should make GTK respect system configuration for native GTK packages (because of dconf). Given that
dconf
is now used much more extensively I've split it (gtk3
is also to go but it's harder).Things done
(nix.useSandbox on NixOS,
or option
build-use-sandbox
innix.conf
on non-NixOS)
nix-shell -p nox --run "nox-review wip"
./result/bin/
)This is not yet tested! I wanted to test with
okular
but it won't work because it needs Qt dependencies (they are usually added bymakeQtWrapper
, aren't they?). Given that it works on KDE I must mislook something:cc @ttuegel