Skip to content
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

pythonPackages: use latest qt version instead of 5.14 #99956

Merged
merged 45 commits into from Oct 8, 2020
Merged

Conversation

@FRidh
Copy link
Member

@FRidh FRidh commented Oct 7, 2020

Motivation for this change

In 3cbcc14 the Qt version used in Python packages was pinned to 5.14. This was done because certain applications were not compatible. In principle, we should try to have the same Qt version in the Python packages set as in the top-level set, and override applications in case they're not compatible.

There were some problems with how the pinning to 5.14 was done, which eventually led to #98197.

This PR now unpins the Qt version. Applications are going to need some fixing, as is done for Qutebrowser in #98197.

This fixes #99937 + fixes #99982 + fixes #99951 .

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Packages

Just to note anyone wishing to help, these are the affected packages, filtered from ofborg's eval:

  • aj-snapshot
  • anki
  • ankisyncd
  • appleseed
  • asymptote
  • audiality2
  • backintime
  • backintime-qt4
  • cadence
  • calibre
  • calibre-py2 - build and run with libsForQt5.callPackage (instead of libsForQt514.callPackage) - not sure if this needs to be committed, considering 22167ae .
  • calibre-py3 - build and run with libsForQt5.callPackage (instead of libsForQt514.callPackage) - not sure if this needs to be committed, considering 22167ae .
  • carla
  • cq-editor
  • cura - The gui seems to work, though it segfaulted when I quit the program, and the console showed lots of deprecation warning. If qt5.14 will be desired for it, it will need a big diff for an override.
  • curaengine
  • curaLulzbot - same as cura
  • dupeguru - fails to build from before this change (example).
  • electron-cash
  • electrum
  • electrum-ltc
  • faust2jack - only affected by sip patch, should be find if it builds.
  • faust2jaqt - only affected by sip patch, should be find if it builds.
  • fdroidserver
  • ffado - only affected by sip patch, should be find if it builds.
  • flatcam
  • flent
  • freecad
  • freecadStable
  • frescobaldi
  • friture - runs fine sort of, no qt related errors but does print opengl related errors.
  • giada
  • gitAndTools.git-annex-metadata-gui
  • gitAndTools.git-cola
  • git-cola
  • gmic-qt-krita
  • gns3-gui - should be OK, as it's only pyqt5 that was upgraded for it.
  • gnss-sdr - should be tested with #99685
  • gnuradio - should be tested with #99685
  • gnuradio-with-packages - should be unaffected with #99685
  • gqrx - should be unaffected with #99685
  • gr-ais - should be unaffected with #99685
  • gr-gsm - should be unaffected with #99685
  • gr-limesdr - should be unaffected with #99685
  • gr-nacl - should be unaffected with #99685
  • gr-osmosdr - should be unaffected with #99685
  • gr-rds - should be unaffected with #99685
  • hplip
  • hplip_3_16_11
  • hplip_3_18_5
  • hplipWithPlugin
  • hplipWithPlugin_3_16_11
  • hplipWithPlugin_3_18_5
  • ihaskell
  • inkcut
  • inspectrum - should be unaffected with #99685
  • jack1
  • jack2
  • jack2Full
  • jackmix_jack1
  • kcc
  • kmymoney
  • koboredux
  • koboredux-free
  • krita
  • krop
  • labelImg - - should be OK, as it's only pyqt5 that was upgraded for it.
  • leo-editor
  • libffado
  • littlegptracker
  • lsp-plugins
  • ltc-tools
  • luppp
  • maestral-gui (#99589)
  • manuskript
  • mnemosyne
  • mooSpace
  • multibootusb
  • nagstamon
  • onionshare
  • onionshare-gui
  • openshot-qt
  • persepolis
  • picard - should be OK
  • plover.dev
  • puddletag (qt.qpa.plugin: Could not load the Qt platform plugin "xcb" in "" even though it was found. )
  • pulseeffects
  • python27Packages.androguard
  • python27Packages.appleseed
  • python27Packages.binwalk-full
  • python27Packages.jupyter
  • python27Packages.pivy
  • python27Packages.poppler-qt5
  • python27Packages.pykdl
  • python27Packages.pyqt4
  • python27Packages.pyqt5
  • python27Packages.pyqt5_with_qtmultimedia
  • python27Packages.pyqt5_with_qtwebkit
  • python27Packages.pyqtgraph
  • python27Packages.pyqtwebengine
  • python27Packages.pyside2
  • python27Packages.pyside2-tools
  • python27Packages.qscintilla
  • python27Packages.qscintilla-qt4
  • python27Packages.qscintilla-qt5
  • python27Packages.qtconsole
  • python27Packages.shiboken2
  • python27Packages.sip
  • python27Packages.subdownloader
  • python3Packages.androguard
  • python3Packages.ansible-kernel
  • python3Packages.binwalk-full
  • python3Packages.enaml
  • python3Packages.enamlx
  • python3Packages.jupyter
  • python3Packages.libarcus
  • python3Packages.libsavitar
  • python3Packages.lightparam
  • python3Packages.mayavi
  • python3Packages.pivy
  • python3Packages.poppler-qt5
  • python3Packages.pykdl
  • python3Packages.pyqt4
  • python3Packages.pyqt5
  • python3Packages.pyqt5_with_qtmultimedia
  • python3Packages.pyqt5_with_qtwebkit
  • python3Packages.pyqtgraph
  • python3Packages.pyqtwebengine
  • python3Packages.pyside2
  • python3Packages.pyside2-tools
  • python3Packages.qimage2ndarray
  • python3Packages.qreactor
  • python3Packages.qscintilla-qt5
  • python3Packages.qtconsole
  • python3Packages.quamash
  • python3Packages.roboschool
  • python3Packages.shiboken2
  • python3Packages.sip
  • python3Packages.skein
  • python3Packages.spyder
  • python3Packages.spyder_3 (qt.qpa.plugin: Could not load the Qt platform plugin "xcb" in "" even though it was found., old version, probably best removed)
  • python3Packages.stytra
  • python3Packages.trezor_agent
  • python3Packages.uranium
  • python3Packages.vega
  • python3Packages.weboob - builds, but I couldn't find a single binary that should launch a gui there.
  • qarte
  • qgis (error: no matching function for call to ‘QsciAPIs::QsciAPIs()’)
  • qgis-unwrapped
  • qnotero
  • qpaeq
  • qradiolink - should be unaffected / tested along with #99685
  • qutebrowser - Should work, given that qt is not updated for it (commit).
  • rapid-photo-downloader
  • retext
  • scudcloud
  • seexpr
  • shotcut
  • sl1-to-photon
  • sonic-pi
  • soundtracker
  • spyder
  • sumorobot-manager
  • syncplay
  • tambura
  • tetraproc
  • tortoisehg
  • trezor_agent
  • tribler
  • tuxguitar
  • urh
  • virtscreen
  • vorta
  • webbrowser
  • webcamoid
  • webmacs
@FRidh FRidh requested a review from jonringer as a code owner Oct 7, 2020
@FRidh FRidh requested review from petabyteboy, doronbehar and jorsn Oct 7, 2020
@FRidh
Copy link
Member Author

@FRidh FRidh commented Oct 7, 2020

It's important here that we build and test at least some Python+Qt applications.

@doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Oct 7, 2020

I have a question regarding qutebrowser. Why isn't like this:

diff --git i/pkgs/top-level/all-packages.nix w/pkgs/top-level/all-packages.nix
index c13b60c5d80..73300080f66 100644
--- i/pkgs/top-level/all-packages.nix
+++ w/pkgs/top-level/all-packages.nix
@@ -22990,7 +22990,8 @@ in
     pkgs_ = pkgs.extend(_: prev: {
       pythonInterpreters = prev.pythonInterpreters.override(oldAttrs: {
         pkgs = oldAttrs.pkgs.extend(_: _: {
-          inherit (pkgs) qt5 libsForQt514;
+          qt5 = qt514;
+          libsForQt5 = libsForQt514;
         });
       });
     });
@FRidh
Copy link
Member Author

@FRidh FRidh commented Oct 7, 2020

I have a question regarding qutebrowser. Why isn't like this:

There are two parts:

  1. me (?) 3fafb02 being sloppy, using a versioned and unversioned attribute.
  2. pkgs was the one originally passed into the Python packages set. Since we're overriding, I am of the opinion we should aim at picking items from that set. In practice I suppose it does not matter much.
@doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Oct 7, 2020

According to qutebrowser latest release notes, qt5.15 is supported. I'm confused now - #97586 tried to make it use qt5.15, why won't it use 5.15 now?

@FRidh
Copy link
Member Author

@FRidh FRidh commented Oct 7, 2020

According to qutebrowser latest release notes, qt5.15 is supported. I'm confused now - #97586 tried to make it use qt5.15, why won't it use 5.15 now?

Indeed, see #97586. However.....see also 3fafb02. So the sloppiness wasn't mine here ;)

@FRidh FRidh force-pushed the FRidh:qt branch from 2667af4 to 07ef267 Oct 7, 2020
@FRidh FRidh mentioned this pull request Oct 7, 2020
5 of 10 tasks complete
@doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Oct 7, 2020

There are many libsForQt514.callPackage attributes in all-packages.nix . Some of which call expressions that import pythonPackages which then use pyqt5 and that pyqt5 is built with qt5.15.. Most of these pinnings were made in 22167ae , not sure where @ttuegel got the list of what broke with qt5.15.

In any case, If it'll turn out that we can't switch to libsForQt5.callPackage for these packages, I'm afraid we'll wind up with many more ugly qutebrowser like overrides.. Do you think @FRidh we could make that a bit prettier?

@FRidh
Copy link
Member Author

@FRidh FRidh commented Oct 7, 2020

In any case, If it'll turn out that we can't switch to libsForQt5.callPackage for these packages, I'm afraid we'll wind up with many more ugly qutebrowser like overrides.. Do you think @FRidh we could make that a bit prettier?

I suppose one could do

mypackage = (pkgs.extend(final: prev: {
  qt5 = prev.qt514;
  libsForQt5 = prev.libsForQt514;
})).libsForQt5.callPackage ... { };

which I think is cleanest. This probably takes more effort to evaluate though and thus probably should be avoided.

@SFrijters
Copy link
Member

@SFrijters SFrijters commented Oct 7, 2020

maestral-gui is fixed by this PR.

@doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Oct 7, 2020

maestral-gui is fixed by this PR.

@SFrijters it's a bit surprising to hear. It seems to me that we should have included the changes in #99589 here, or at least it should be safer to do so.

@ofborg ofborg bot requested a review from klntsky Oct 7, 2020
doronbehar added a commit to FRidh/nixpkgs that referenced this pull request Oct 8, 2020
Remove included upstream patches. Use qt5.15 (NixOS#99956). Spare double
wrapping by using `makeWrapperArgs+=()`.
@doronbehar doronbehar force-pushed the FRidh:qt branch from 686323b to cb59838 Oct 8, 2020
@ofborg ofborg bot requested a review from Lassulus Oct 8, 2020
@piegamesde
Copy link
Contributor

@piegamesde piegamesde commented Oct 13, 2020

Hm, this does not fix it for me. NIX_PATH=nixpkgs=https://github.com/nixos/nixpkgs/archive/master.tar.gz nix-shell -p 'electrum' --run electrum still has the same error. Kile fails as well, on the other side Git-cola works.

@jorsn
Copy link
Member

@jorsn jorsn commented Oct 13, 2020

NIX_PATH=nixpkgs=https://github.com/nixos/nixpkgs/archive/master.tar.gz nix-shell -p 'electrum' --run electrum runs just fine for me, even with a --pure nix shell.

@piegamesde
Copy link
Contributor

@piegamesde piegamesde commented Oct 13, 2020

I'm sorry, I misread the error message. electrum failed in an unrelated way and it indeed works now. But NIX_PATH=nixpkgs=https://github.com/nixos/nixpkgs/archive/master.tar.gz niGLIntel nix-shell -p 'kile' --run kile still fails:

qt.qpa.plugin: Could not load the Qt platform plugin "xcb" in "" even though it was found.
This application failed to start because no Qt platform plugin could be initialized. Reinstalling the application may fix this problem.

Available platform plugins are: wayland-egl, wayland, wayland-xcomposite-egl, wayland-xcomposite-glx, eglfs, linuxfb, minimal, minimalegl, offscreen, vnc, xcb.

Should I open a new issue then?

@doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Oct 13, 2020

Kile was not affected by this issue, since it does not use pyqt5. So it is unrelated.

@jorsn
Copy link
Member

@jorsn jorsn commented Oct 13, 2020

@doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Oct 13, 2020

The problem with kile, is similar to what was fixed here. kdeApplications uses qt514 by default, while all-packages.nix uses qt515:

kdeApplications =
let
mkApplications = import ../applications/kde;
attrs = {
libsForQt5 = libsForQt514;
inherit lib fetchurl;
inherit okteta;
};
in
recurseIntoAttrs (makeOverridable mkApplications attrs);

That's why kile ends up with qt514 packages in QT_PLUGIN_PATH and QML2_IMPORT_PATH tool in it's wrapper, while it is built with qt515.

@piegamesde you should open an issue similar to #99937 but refer to kdeApplications instead of pythonInterpreters - what @ttuegel did in a1f4287 was as severe as what @FRidh did in 3cbcc14 .

Working on this PR took me and @FRidh a full day, I hope it won't take you as long. Good luck.

@doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Oct 13, 2020

Specifically for kile, I'd also do this:

diff --git i/pkgs/applications/editors/kile/default.nix w/pkgs/applications/editors/kile/default.nix
index f928dbb6ad1..cebd59c98b9 100644
--- i/pkgs/applications/editors/kile/default.nix
+++ w/pkgs/applications/editors/kile/default.nix
@@ -30,14 +30,17 @@ mkDerivation rec {
 
   };
 
-  nativeBuildInputs = [ extra-cmake-modules wrapGAppsHook ];
+  nativeBuildInputs = [
+    extra-cmake-modules
+    wrapGAppsHook
+    kdoctools
+  ];
 
-  propagatedBuildInputs = [
+  buildInputs = [
     kconfig
     kcrash
     kdbusaddons
     kdelibs4support
-    kdoctools
     kguiaddons
     kiconthemes
     kinit
@@ -49,6 +52,10 @@ mkDerivation rec {
     poppler
     qtscript
   ];
+  dontWrapGApps = true;
+  preFixup = ''
+    makeWrapperArgs+=("''${gappsWrapperArgs[@]}")
+  '';
 
   propagatedUserEnvPkgs = [ konsole ];
 

FYI, konsole is the one that's built with qt514, due to the kdeApplications override, and that makes the environment of it part of the environment of kile, as it seems.

@jorsn
Copy link
Member

@jorsn jorsn commented Oct 13, 2020

I agree. electrum is also wrapped doubly.

@FRidh
Copy link
Member Author

@FRidh FRidh commented Oct 13, 2020

Working now on cherry-picking this to 20.09 #100428

FRidh added a commit to FRidh/nixpkgs that referenced this pull request Oct 13, 2020
Remove included upstream patches. Use qt5.15 (NixOS#99956). Spare double
wrapping by using `makeWrapperArgs+=()`.

(cherry picked from commit 2edd4ed)
FRidh added a commit to FRidh/nixpkgs that referenced this pull request Oct 13, 2020
The same as used by pyqt5 (NixOS#99956).

Also: Fix double wrapping.
(cherry picked from commit bc0113e)
FRidh added a commit to FRidh/nixpkgs that referenced this pull request Oct 13, 2020
Don't mix qt5.14 and pyqt5 which uses qt5.15 (NixOS#99956).

(cherry picked from commit 16c2b3c)
FRidh added a commit to FRidh/nixpkgs that referenced this pull request Oct 13, 2020
Bonus: Spare double wrapping.
(cherry picked from commit 3d8267e)
FRidh added a commit to FRidh/nixpkgs that referenced this pull request Oct 13, 2020
FRidh added a commit to FRidh/nixpkgs that referenced this pull request Oct 13, 2020
FRidh added a commit to FRidh/nixpkgs that referenced this pull request Oct 13, 2020
FRidh added a commit to FRidh/nixpkgs that referenced this pull request Oct 13, 2020
FRidh added a commit to FRidh/nixpkgs that referenced this pull request Oct 13, 2020
FRidh added a commit to FRidh/nixpkgs that referenced this pull request Oct 13, 2020
FRidh added a commit to FRidh/nixpkgs that referenced this pull request Oct 13, 2020
FRidh added a commit to FRidh/nixpkgs that referenced this pull request Oct 13, 2020
FRidh added a commit to FRidh/nixpkgs that referenced this pull request Oct 13, 2020
FRidh added a commit to FRidh/nixpkgs that referenced this pull request Oct 13, 2020
FRidh added a commit to FRidh/nixpkgs that referenced this pull request Oct 13, 2020
FRidh added a commit to FRidh/nixpkgs that referenced this pull request Oct 13, 2020
FRidh added a commit to FRidh/nixpkgs that referenced this pull request Oct 13, 2020
@zimbatm
Copy link
Member

@zimbatm zimbatm commented Nov 24, 2020

Please stop using pkgs.extend, it blows up the memory usage of 130MB on each instance!

@doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Nov 24, 2020

Please stop using pkgs.extend, it blows up the memory usage of 130MB on each instance!

Interesting metric. We should be able to get rid of it in #104474 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.