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

Python packages depending on qt5, use qt514 while all-packages.nix defaults to qt515, causing incompatibilities. #99937

Closed
doronbehar opened this issue Oct 7, 2020 · 9 comments · Fixed by #99956

Comments

@doronbehar
Copy link
Contributor

doronbehar commented Oct 7, 2020

Describe the bug

Experienced here: #99465 (comment) and here: #99685 (comment)

How come pyqt5 winds up with qt-5.14 in it's inputs and in it's references? Considering:

qt5 = if stdenv.hostPlatform.isDarwin then qt512 else qt515;
libsForQt5 = if stdenv.hostPlatform.isDarwin then libsForQt512 else libsForQt515;

And:

pyqt5 = pkgs.libsForQt5.callPackage ../development/python-modules/pyqt/5.x.nix { pythonPackages = self; };
pyqt5_with_qtmultimedia = self.pyqt5.override { withMultimedia = true; };

To Reproduce

nix-store --query --references $(nix-build --no-out-link -A python3.pkgs.pyqt5) | grep 5.14

Expected behavior

No qt5.14 references.

Screenshots
If applicable, add screenshots to help explain your problem.

Additional context
Add any other context about the problem here.

Notify maintainers

cc @sander

Maintainer information:

# a list of nixpkgs attributes affected by the problem
attribute: python.pkgs.pyqt5
# a list of nixos modules affected by the problem
module:
@doronbehar
Copy link
Contributor Author

Updating pyqt5 in #99933 does not solve the issue.

@doronbehar doronbehar mentioned this issue Oct 7, 2020
10 tasks
@jorsn
Copy link
Member

jorsn commented Oct 7, 2020

qt5 is overridden for all python. I described what I know of the history here: #99458 (comment).
I know of one reason to use qt514: qtwebkit is broken in qt515. But maybe @ttuegel can explain 282aadb, 282aadb (why does pyqt5 need to have version 5.14)?

Anyway, I think packages shouldn't blindly expect any qt5 version in python. We should somehow add a test whether a package uses inconsistend qt5 versions, but in general, packages using pyqt5 should adapt to it or override it, since adapting is almost trivial. Unfortunately, overriding isn't: Look at qutebrowser here: #98197. Unfortunately, there is no check for inconsistent dependencies implemented, I think. Almost all python+qt5 packages that use inconsistent versions should fail.

@doronbehar
Copy link
Contributor Author

doronbehar commented Oct 7, 2020

Thanks for the references. To summarize, this explains the situation:

pythonInterpreters = callPackage ./../development/interpreters/python {
# Overrides that apply to all Python interpreters
pkgs = pkgs.extend (final: _: {
qt5 = final.qt514;
libsForQt5 = final.libsForQt514;
});
};

But why do that? This is not inline with all-packages.nix which defaults to 5.15. I suspect that more packages are affected and we don't know it yet. I also understand now how your picard PR fixes the issue.

I think we should remove that override, and override every python package to use libsForQt514 only if needed.

@doronbehar doronbehar changed the title pyqt5 is built with qt5.14 while most packages expect it to link to qt5.15 Python packages depending on qt5, use qt514 while all-packages.nix defaults to qt515, causing incompatibilities. Oct 7, 2020
@FRidh
Copy link
Member

FRidh commented Oct 7, 2020

Applications can be overridden to use a different Qt, libraries cannot.

I don't know which one we should be using here. There was quite some confusion regarding the Qt versions, along with some incorrect commits (including mine) setting Qt versions for Python packages.

If we think the libraries should be built with 5.15, and overridden for certain applications, then that is of course fine, and we should remove this override.

@jorsn
Copy link
Member

jorsn commented Oct 7, 2020

Applications can be overridden to use a different Qt, libraries cannot. – @FRidh

Why? Because of Nixpkgs or because of the libraries themselves?
I think, in Nixpkgs you can override the qt version python libraries use, but only cleanly by overriding python.

If we think the libraries should be built with 5.15, and overridden for certain applications, then that is of course fine, and we should remove this override.

If we think the libraries are compatible with 5.15. At least qtwebkit isn't, but this is a qt library anyway and it's probably best overridden for every application using it.

@ttuegel
Copy link
Member

ttuegel commented Oct 7, 2020

Applications can be overridden to use a different Qt, libraries cannot.

This is essentially correct, but let me expand on it:

  1. Restriction imposed by Qt: Any derivation that depends on Qt must have only one Qt version in its entire transitive closure.
  2. Applications can pick a particular version of Qt because nothing depends on them, so they can't violate restriction (1).
  3. As a consequence of (1) + (2), every library must be built with every version of Qt, so that applications have all their library dependencies available. Therefore, libraries don't get to pick a Qt version.

@FRidh
Copy link
Member

FRidh commented Oct 7, 2020

Thank you for the explanation. That is exactly how it is for Python packages as well.

I think that is also what makes it so tricky here, both Qt and Python having these type of restrictions. For any package, all its Qt libraries need to be compatible, all its Python packages need to be compatible. Finally, the bindings that are used need to be handle the versions.

@doronbehar
Copy link
Contributor Author

@ttuegel thanks for joining in. Could you please explain a bit more how did you pick the packages to use libsForQt514 in 22167ae ?

@ttuegel
Copy link
Member

ttuegel commented Oct 8, 2020

Could you please explain a bit more how did you pick the packages to use libsForQt514 in 22167ae ?

These packages didn't build with Qt 5.15, which we were about to make the default on master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants