Hmm... I don't think it should have to be so much extra stuff. You should be able to reuse qtbase.patch and do something like:

app_libpaths->append(#QTBASEDIR "/lib"));

Here: https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/libraries/qt-5/5.11/qtbase.patch#L893

and a definition for QTBASEDIR here:

https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/libraries/qt-5/modules/qtbase.nix#L186

On closure size, I actually don't think this will have a huge effect. IIRC qtbase is leaking into it anyway so I wouldn't worry about it much:

$ nix why-depends /nix/store/b9pqh1m7a5rdfl6khn6zb38w2m9qc58a-wireshark-qt-2.4.7 /nix/store/jlci4w1ssyspjv0waz0z7lzbgiy9svhp-qtbase-5.10.1-dev
/nix/store/b9pqh1m7a5rdfl6khn6zb38w2m9qc58a-wireshark-qt-2.4.7
╚═══bin/wireshark: …ber in DLT sentence../nix/store/jlci4w1ssyspjv0waz0z7lzbgiy9svhp-qtbase-5.10.1-dev/include/QtCor…
    => /nix/store/jlci4w1ssyspjv0waz0z7lzbgiy9svhp-qtbase-5.10.1-dev

Hmm... I don't think it should have to be so much extra stuff. You should be able to reuse qtbase.patch and do something like [adding QTBASEDIR to app_libpaths].

^{Shortened quote with what I understand of it.}

That would work fine for qtbase, but that won't add other plugin paths into the app's app_libpaths. There are ~150 packages (give or take for per-qt duplicates) packages providing plugins as per what Qt understands.

 $ nix-locate -1r qt-5.*plugins/.*so | grep -v '^(' | sort -u | wc -l
149

This does not only provide the platforms (like libqxcb), but also provides many parts for Qt applications to compose themselves with

 $ nix-locate -r qt-5.*plugins/.*so | grep -v '^(' | cut -b59- | cut -d'/' -f7- | sort | uniq | wc -l
1009

And as for "You should be able to reuse qtbase.patch", this is already noted as things to do (Merge with other qtbase patches? If not, at least rename the patch.) As for right now, keeping the patch separate makes it easier for me to adjust as needed, and makes it easier to review (I hope).

Had a quick ask-around on IRC today and @acowley replied with stuff I could try.

tribler

This PR "doesn't work for tribler". This points towards needing a bit of work for PyQt things. Though, since the new behaviour is additional to the previous behaviour, this means that it isn't inherently broken, it works exactly the way it works now.

What's happening is that it's searching relative to the python interpreter (understandably).

11998 lstat("/nix/store/d3aa78jf7pm9lk6ad67x4f21a068dxn6-python-2.7.15/bin/python2.7", {st_mode=S_IFREG|0555, st_size=7904, ...}) = 0
11998 access("/nix/store/d3aa78jf7pm9lk6ad67x4f21a068dxn6-python-2.7.15/bin/nix-support/qt-plugin-paths", F_OK) = -1 ENOENT (No such file or directory)
11998 access("/nix/store/d3aa78jf7pm9lk6ad67x4f21a068dxn6-python-2.7.15/nix-support/qt-plugin-paths", F_OK) = -1 ENOENT (No such file or directory)

Something would need to be done to nudge the patch into looking relative to tribler... Except that tribler itself doesn't have a qt-plugin-paths entry; it is python27Packages.pyqt5 that has it.

 $ ls -l /nix/store/fd21rmcwil26334bhcl69cqd0qz94l3d-python2.7-PyQt-5.10.1-dev/nix-support/qt-plugin-paths
-r--r--r-- 1 root root 511 Dec 31  1969 /nix/store/fd21rmcwil26334bhcl69cqd0qz94l3d-python2.7-PyQt-5.10.1-dev/nix-support/qt-plugin-paths

Thus, it may be required for the additional nudging to be done at that level, and not in the applications using PyQt. (Maybe with collaboration with an environment variable forcing a qt-plugin-paths file instead of detecting it?)

These are implementation details to implement if the overall idea is right; for now, it still seems to be right to me.

With a KDE desktop

18:13 <acowley> samueldr: My loose understanding of the problem was that integrating applications with a Plasma desktop was one of the breaking points. If the app insists on the plugins it was built with, and someone updates their desktop bringing in new plugin versions, then the two won't coexist happily. If the app relies on the environment for the plugins, then updating the desktop updates everything.
...
18:16 <acowley> samueldr: Right, that's the frustrating bit. But it seems that the alternative caused worse breakage for KDE users than those of us who just got a missing plugin message.

Uh, this is going to be hard to test with entire certainty. Though, I think I can do those test cases without too much pain (compiling all of KDE)

• build-vm a nixos-18.03 graphical desktop using kde
  • execute the full path without nix-enving it
  • nix-env Qt Apps and run them
  • Somehow add it to the KDE menu (?)

Where Qt Apps are the two tribler and quaternion. First, using nixos-unstable clean, then with my patch.

If nothing breaks, I may not be trying hard enough. Another thing to try is to downgrade the Qt minor version (qt 5.11.0) to try to force a version mismatch. (So, in addition to this current patch, use 5.11.0 for testing purposes.)

Progress!

Looks like that with the patch applied, using build-vm with services.xserver.desktopManager.plasma5.enable = true;, there is still an issue when using minor-release mismatched Qt.

Though, this may be solvable. Playing around with the env, I was able to reduce it to two environment variables (both need to be unset).

$ /nix/store/.../bin/$app
...
Cannot mix incompatible Qt library (version 0x50b01) with this library (version 0x50b00)
Aborted
$ ( unset KDE_FULL_SESSION XDG_CURRENT_DESKTOP; /nix/store/.../bin/$app )
# [ runs! ]
$ # re-running it here would fail due to using a subshell previously.

This means that for Qt things, tests inside and outside Plasma will need to be done. (Good to know!)

For the record, I have built a 5.11.0 equivalent without the patch to compare

Running without touching the environment results in:

qt.qpa.plugin: Could not load the Qt platform plugin "xcb" in "" even though it was found.

As it tried using the libqxcb.so from qtbase-5.11.1 from the environment.

As expected, unsetting both previous values doesn't change a thing.

This means that at the current stage: there is improvement, and no regression as far as I tested.

Exploring the variables

• https://github.com/qt/qtbase/blob/cd542b0094d485e5127794f32e77ea81d9092b05/src/platformsupport/services/genericunix/qgenericunixservices.cpp#L80-L89

They both are only used in detectDesktopEnvironment, which is in turn used in QGenericUnixServices::desktopEnvironment()

This probably means that it is rooted into one of the 11 files using this function. A chunk of those are platform definitions, which can be discarded. Left are three files, one using it for indicator support under unity (not an issue here), another using it for font configuration under unity and gnome (not an issue here).

Looks like qgenericunixthemes.cpp is our culprit.

It is Qt's integration for KDE theming that causes issues. unsetting KDE_FULL_SESSION, setting XDG_CURRENT_DESKTOP to KDE causes the same issue. Doing it again, but with XDG_CURRENT_DESKTOP=FOO doesn't cause issues, and using one of the values from the gtk-based environments doesn't seem to cause issue either. (Though this may be because of entirely missing plugin platforms.)

Further investigation is needed, but this looks like it could be solvable if qtPluginPrefix contained the minor version number too (as suggested on IRC).

Sorry I misunderstood what Qt plugins were for. It looks like you're doing great work here! I would definitely recommend getting this reviewed from @ttuegel who should be able to provide feedback on some of the internals of Qt stuff.

Maybe you know, whether or not github's CODEOWNER notifications are noisy enough that @ttuegel would have seen it without mentioning?

Thanks for the words, it's not initially obvious that so much is built as plugins in Qt.

Success!

^{In this screenshot, we see how the search paths go from a 5.11.1 folder, go up a couple, go down a couple searching for 5.11.0.}

I have to, first, do archaeological searches through the git log to see if there is any reason not to set the minor version in qtCompatVersion, but the comment says: "3. Update qtCompatVersion below if the minor version number changes.". (EDIT: Realised that in X.Y.Z Y is minor). Though it looks promising; adding the patch version there makes it so no environment manipulation is needed at all.

@ttuegel any particular reason for using the major.minor only? Qt seems to dislike mis-matched libraries, at least going from higher to lower patch releases.

For the record, here's the VM I'm using for testing, with glorious hardcoded paths in the build.sh script.

Is there currently a workaround to use the affected programs ?

any particular reason for using the major.minor only? Qt seems to dislike mis-matched libraries, at least going from higher to lower patch releases.

Qt claims that there is forward and backward binary compatibility along Z, if the version number is X.Y.Z. If you are seeing something different, I will defer to you; I learned long ago that everything they have to say about binary compatibility is an outright lie.

I have not had time to review this in depth, but the general principle seems sound to me. Thanks for tackling this!

Extending @timokau's comment a bit:
Especially when looking at the history and how often packages somehow broken, because wrapGAppsHook wasn't there, I'd also favour fixing this in our version of qt itself, rather than in a wrapper.

There might be users not very deep into packaging for NixOS (so for sure don't know about the wrapper) trying to get binaries linked with qt to work - it would be good if qt just did the right thing (tm).

I still need to test an assumption before speaking (and I hate to comment before testing it), but I think the hook might not solve the most damaging bug (#47552) where installing through nix-env can break the whole plasma session. (This is 100% an assumption and not verified, I have not even checked the wrapper's implementation and may be 100% wrong and kinda hope to be wrong here.) I also need to re-check whether this PR (#44047) fixes it, I think I verified and it did, but I need to re-check. The main issue here is that I need to go back to a 5.11 Qt world since it needs to happen between two minor releases (and maybe other factors?).

@samueldr

I think the hook might not solve the most damaging bug (#47552) where installing through nix-env can break the whole plasma session.

That bug is caused by having plugins installed into profile paths, which get picked up by Plasma packages. I did not see anything in this pull request to address that. Could you explain how this works to solve that problem?

The hook would solve #47552 once it is included in the Plasma packages because the QT_PLUGIN_PATH is prefixed with the build inputs' plugins. There would still be some corner cases, e.g.: if the user installs a Qt theme using a new Qt version and directs Plasma to load that theme, bad things would happen--but I don't see any way to solve that right now without breaking themes entirely.

Actually, this should already be dealt with by having a different $qtPluginPrefix for each minor version--that is why I added that around the time of Qt 5.9. 😕

@flokli

Especially when looking at the history and how often packages somehow broken, because wrapGAppsHook wasn't there, I'd also favour fixing this in our version of qt itself, rather than in a wrapper.

My experience has been that some packages always fall outside our attempts to automate. That is also true of the hook I wrote for #54525. The burden of debugging corner cases always falls on individual maintainers; in that case, the wrapper is easier to debug than a patched Qt. I want to automate packaging as much as possible, but not at the expense of giving packagers better tools when things go wrong.

After some compilation time, yes, #47552 is handled by this PR.

That bug is caused by having plugins installed into profile paths, which get picked up by Plasma packages. I did not see anything in this pull request to address that. Could you explain how this works to solve that problem?

I think you're right, re-reading the implementation, and thinking about how it works, nothing specifically addresses the issue, but the way the dependencies are resolved helps. Instead of going through the PATH elements, which may end up providing unmatched plugins, it will instead first look at what it was built with, providing working plugins.

The implementation basically bundles a QT_PLUGIN_PATH internally inside the built derivation, in nix-support, which the patched Qt knows to look for first. It might be possible to get unmatched plugins, I'm not a Qt expert so bear with me, if e.g. a specialized "image" plugin was installed in the profile, while only the basic image plugins were provided in the built derivation.

Actually, this should already be dealt with by having a different $qtPluginPrefix for each minor version--that is why I added that around the time of Qt 5.9. 😕

I might have misspoke and meant patch level. The easiest way to trigger the issue is with patch levels, assuming MAJOR.minor.patch, a mis-matched patch as in #47552 will cause breakage. The qtCompatVersion only handles MAJOR.minor. Going from memory, no reproduction noted, adding patch to qtCompatVersion wasn't "enough", I would have loved to have a reproduction and notes as to why I remember that.

The implementation basically bundles a QT_PLUGIN_PATH internally inside the built derivation, in nix-support, which the patched Qt knows to look for first.

That was my thought, too. #54525 addresses this the same way: paths registered at build-time are searched first, then we go through PATH.

The easiest way to trigger the issue is with patch levels, assuming MAJOR.minor.patch, a mis-matched patch as in #47552 will cause breakage. The qtCompatVersion only handles MAJOR.minor.

So, Qt-upstream claims that the patch-level versions are all ABI-compatible. I, uh, have found reason to mistrust them in the past. Particularly with "platform plugins", which is what causes the crashes. (Qt applications crash if they cannot load a platform plugin, but they usually handle other misversioned plugins correctly by not loading them.) I'm not going to merge anything until I'm satisfied that I can reproduce and fix #47552, but I think it will be enough not to put platform plugins into propagatedUserEnvPkgs.

I updated nix package manager and all my installed apps. I am getting the same error for kolourpaint (kde app). :/

Closing since #54525 handles this issue.

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

https://discourse.nixos.org/t/announcing-nomia-a-general-resource-manager-inspired-by-nix/12591/26