anki: use wrapQtAppsHook to fix execution #66796

oxij · 2019-08-17T20:45:37Z

What?

At the moment running anki fails with

qt: Could not find the Qt platform plugin "xcb" in ""
qt: This application failed to start because no Qt platform plugin could be initialized. Reinstalling the application may fix this problem.

This PR fixes it, properly, by inheriting the hook from the pyqtwebengine so that QT versions match.

Why?

The use of wrapQtAppsHook is now required for Qt apps. See #65399 for more info.

`git log`

pyqtwebengine: passthru wrapQtAppsHook
anki: use wrapQtAppsHook from pyqtwebengine
anki: add myself as a maintainer

(Adding to the front of the list because it was I who added this expression to
nixpkgs in e00c031 7 years ago. :])

`nix-instantiate` environment

Host OS: Linux 4.9, SLNOS 19.09
Nix: nix-env (Nix) 2.2.2
Multi-user: yes
Sandbox: yes
NIXPKGS_CONFIG:

{
  checkMeta = true;
  doCheckByDefault = true;
}

`nix-env -qaP` diffs

On x86_64-linux:
- Updated (1):
  - anki
On aarch64-linux: noop
On x86_64-darwin:
- Updated (1):
  - anki

/cc @Profpatsch @asymmetric @ttuegel

(Adding to the front of the list because it was I who added this expression to nixpkgs in e00c031 7 years ago. :])

pkgs/development/python-modules/pyqtwebengine/default.nix

oxij · 2019-08-17T21:11:04Z

can't we just use `wrapQtAppsHook` directly in the `anki` derivation?

The OP post says:

... by inheriting the hook from the `pyqtwebengine` so that QT versions match.

If we do it directly, we would need add asserts to keep QT versions used in `anki` and `pyqtwebengine` synchronized, or something similarly pointless.

pkgs/games/anki/default.nix

oxij · 2019-08-17T21:31:18Z

Can we sort these plz? :)

Well, AFAICS the usual etiquette is to keep maintainers in the order of their appearance. Not that I insist doing it this way, but I feel like this would be a question deserving an RFC. :)

Unrelated, but what's the advantage of passing `lib` as an input to the derivation?

Because `lib`, semantically speaking, is not really a part of `stdenv`. It is also shorter. So, since `lib` was already in the arguments it seemed prudent to change this.

flokli · 2019-08-17T23:18:48Z

On 19-08-17 14:11:05, Jan Malakhovski wrote: > can't we just use `wrapQtAppsHook` directly in the `anki` derivation? The OP post says: > ... by inheriting the hook from the `pyqtwebengine` so that QT versions match. If we do it directly, we would need add asserts to keep QT versions used in `anki` and `pyqtwebengine` synchronized, or something similarly pointless.

You're right - I missed that, and it makes a lot of sense. Thanks for the clarification!

worldofpeace · 2019-08-18T01:59:19Z

pkgs/games/anki/default.nix

@@ -158,6 +161,9 @@ buildPythonApplication rec {
      cp -rv anki aqt web $pp/

      wrapPythonPrograms
+      for program in $out/bin/*; do


Why do we have to do wrapPythonPrograms in buildPythonApplication? I thought that'd happen automatically.

Also by doing this we get two wrappers, maybe give makeWrapperArgs "\${qtWrapperArgs[@]}"?

asymmetric · 2019-08-18T12:56:07Z

From a quick run, this works as expected.

symphorien · 2019-08-18T13:15:15Z

I think "keeping the version of qt in qtwebengine and anki in sync" is the job of libsForQt5.callPackage.

@worldofpeace

As suggested by @worldofpeace.

oxij · 2019-08-18T17:46:20Z

@worldofpeace These are good suggestions, applied both. Thanks! @symphorien

I think "keeping the version of qt in qtwebengine and anki" is the job of libsForQt5.callPackage.

Normally, yes, but it is `pyqtwebengine`, not `qtwebengine`, i.e. it is a package from `pythonPackages`, not from `qtPackages`.

asymmetric · 2019-08-18T21:34:15Z

pkgs/games/anki/default.nix

      # copy the manual into $doc
      cp -r ${manual}/share/doc/anki/html $doc/share/doc/anki
    '';

+    dontWrapQtApps = true;
+    makeWrapperArgs = [
+        ''--prefix PATH ':' "${lame}/bin:${mplayer}/bin"''


Should we use the same string type for both array elements?

Suggested change

''--prefix PATH ':' "${lame}/bin:${mplayer}/bin"''

"--prefix PATH ':' ${lame}/bin:${mplayer}/bin"

worldofpeace · 2019-08-18T21:44:55Z

@oxij How about calling anki with libsForQt5.callPackage instead of python3Packages?
You'd then have to use python3Packages directly in the expression, but you wouldn't have to use a passthru attribute in pyqtwebengine

oxij · 2019-08-19T22:34:16Z

@asymmetric

Should we use the same string type for both array elements?

IMHO quotes around paths there would not hurt. (I would rather use more quotes than less in case Nix would eventually allow spaces in paths.) Also, IMHO, a discussion of potentially superfluous quotation marks feels uncomfortably bikeshedding-ish. @worldofpeace

@oxij How about calling `anki` with `libsForQt5.callPackage` instead of `python3Packages`? You'd then have to use `python3Packages` directly in the expression, but you wouldn't have to use a passthru attribute in `pyqtwebengine`

Let's say `pyqtwebengine` needs some specific version of Qt (like, say, 5.6 or something) different from the default one (which it doesn't need now, but could need in the future). `anki` needs Qt environment matching that of `pyqtwebengine` it depends on. Then, the simplest consistent way to satisfy this is to `passthru` the thing that produces the correct environment of `pyqtwebengine` from `pyqtwebengine` so that `anki` would just use it. Which I did here. Another consistent way would be to call `anki` with `libsForQt5.callPackage` and then locally `.override` `pyqtwebengine` inside of that `anki` expression with the corresponding Qt attributes. (Which sounds similar to the thing you propose.) But this would just add another version of `pyqtwebengine` we will have to maintain and compile (since it could differ from the default one) without any practical benefit I can see. Finally, another consistent way would be to `passthru` `libsForQt5.callPackage` used by `pyqtwebengine` instead of just `wrapQtAppsHook` and use that internally in `anki` expression to extract `wrapQtAppsHook`, but that would uber-ugly. I would rather not do that either until `anki` needs a lot, A LOT, of that `passthru` stuff. So, can we merge this yet?

ttuegel · 2019-08-20T14:31:22Z

I would rather not do that either until anki needs a lot, A LOT, of that passthru stuff.

I agree that the passthru attribute is probably the least-bad way to handle a one-off problem like this. My only reservation is that someone finds this example first and thinks it's the preferred solution, and then it proliferates. @oxij would you add a comment where the passthru is added that says something like, "this is a one-off solution for anki", so that it makes sense to someone looking at it out of context? Then I am happy to merge.

flokli · 2019-08-20T22:33:34Z

@ttuegel isn't that the preferred/only solution for all python applications using some qt bindings in one way or another? In that case, it probably should be added to the documentation…

ttuegel · 2019-08-22T18:17:14Z

@flokli If every Python package is going to start using this, then no: it is not the preferred solution and we need to actually solve the problem.

oxij · 2019-08-23T10:16:57Z

If every Python package is going to start using this, then no: it is not the preferred solution and we need to actually solve the problem.

Okay, but how exactly? I suppose doing this in a way consistent with the documented instructions means moving `pyqtwebengine` and similar pythonic bindings into Qt attrset so that `libsForQt5.callPackage` could resolve them. But then, `pyqtwebengine` and similar bindings are not actually a part of Qt, so I expect they would get out of sync with the rest of Qt (and between each other) and require specific older versions of Qt at least some of the time. That is to say, in the general case, I don't see how this can be done in any other way except the way this PR does it or with the aforementioned worse alternatives. I would happy to be proven wrong, though. Adding `passthru`s everywhere would be really ugly, I agree.

timokau · 2019-08-25T15:44:12Z

Can we merge this now? I have no opinion on the meta discussion, but for now it would be nice to have anki working on unstable again.

timokau · 2019-08-26T10:10:57Z

Given that it has been a few days without new discussion and this PR is a definite improvement for now, I'll merge. Thanks @oxij for taking care of this!

Profpatsch · 2019-08-27T11:14:10Z

Great, thanks @oxij

oxij added 3 commits August 17, 2019 20:31

pyqtwebengine: passthru wrapQtAppsHook

eb8e973

anki: use wrapQtAppsHook from pyqtwebengine

2326319

anki: add myself as a maintainer

1a11eb3

(Adding to the front of the list because it was I who added this expression to nixpkgs in e00c031 7 years ago. :])

oxij requested a review from FRidh as a code owner August 17, 2019 20:45

oxij mentioned this pull request Aug 17, 2019

Tracking issue for wrapQtAppsHook #65399

Closed

ofborg bot added the 6.topic: python label Aug 17, 2019

flokli reviewed Aug 17, 2019

View reviewed changes

pkgs/development/python-modules/pyqtwebengine/default.nix Show resolved Hide resolved

ofborg bot requested review from the-kenny and Profpatsch August 17, 2019 20:58

ofborg bot added 11.by: package-maintainer 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Aug 17, 2019

asymmetric reviewed Aug 17, 2019

View reviewed changes

pkgs/games/anki/default.nix Show resolved Hide resolved

asymmetric reviewed Aug 17, 2019

View reviewed changes

pkgs/games/anki/default.nix Show resolved Hide resolved

worldofpeace reviewed Aug 18, 2019

View reviewed changes

anki: simplify the expression, elide superfluous wappers

39028ee

As suggested by @worldofpeace.

asymmetric reviewed Aug 18, 2019

View reviewed changes

timokau mentioned this pull request Aug 23, 2019

Anki fails to launch with "No module named 'PyQt5.QtWebEngineWidgets'" #65623

Closed

timokau merged commit 09cc908 into NixOS:master Aug 26, 2019

puzzlewolf mentioned this pull request Jan 3, 2020

frescobaldi: 3.0.0 -> 3.1 #76889

Merged

10 tasks

oxij deleted the pkgs/fix-anki branch August 12, 2023 09:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

anki: use wrapQtAppsHook to fix execution #66796

anki: use wrapQtAppsHook to fix execution #66796

oxij commented Aug 17, 2019

oxij commented Aug 17, 2019 via email

oxij commented Aug 17, 2019 via email

flokli commented Aug 17, 2019 via email

worldofpeace Aug 18, 2019

asymmetric commented Aug 18, 2019

symphorien commented Aug 18, 2019 •

edited

oxij commented Aug 18, 2019 via email

asymmetric Aug 18, 2019

worldofpeace commented Aug 18, 2019

oxij commented Aug 19, 2019 via email

ttuegel commented Aug 20, 2019

flokli commented Aug 20, 2019

ttuegel commented Aug 22, 2019

oxij commented Aug 23, 2019 via email

timokau commented Aug 25, 2019

timokau commented Aug 26, 2019

Profpatsch commented Aug 27, 2019

	''--prefix PATH ':' "${lame}/bin:${mplayer}/bin"''
	"--prefix PATH ':' ${lame}/bin:${mplayer}/bin"

anki: use wrapQtAppsHook to fix execution #66796

anki: use wrapQtAppsHook to fix execution #66796

Conversation

oxij commented Aug 17, 2019

What?

Why?

git log

nix-instantiate environment

nix-env -qaP diffs

oxij commented Aug 17, 2019 via email

oxij commented Aug 17, 2019 via email

flokli commented Aug 17, 2019 via email

worldofpeace Aug 18, 2019

Choose a reason for hiding this comment

asymmetric commented Aug 18, 2019

symphorien commented Aug 18, 2019 • edited

oxij commented Aug 18, 2019 via email

asymmetric Aug 18, 2019

Choose a reason for hiding this comment

worldofpeace commented Aug 18, 2019

oxij commented Aug 19, 2019 via email

ttuegel commented Aug 20, 2019

flokli commented Aug 20, 2019

ttuegel commented Aug 22, 2019

oxij commented Aug 23, 2019 via email

timokau commented Aug 25, 2019

timokau commented Aug 26, 2019

Profpatsch commented Aug 27, 2019

`git log`

`nix-instantiate` environment

`nix-env -qaP` diffs

symphorien commented Aug 18, 2019 •

edited