Skip to content

pythonPackages.pyqt5: fix sip dependency#52613

Merged
timokau merged 2 commits intoNixOS:masterfrom
nyanloutre:pyqt5-fix
Dec 21, 2018
Merged

pythonPackages.pyqt5: fix sip dependency#52613
timokau merged 2 commits intoNixOS:masterfrom
nyanloutre:pyqt5-fix

Conversation

@nyanloutre
Copy link
Member

@nyanloutre nyanloutre commented Dec 21, 2018 •
edited
Loading

Motivation for this change

Related to the discution in #49400

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@nyanloutre nyanloutre requested a review from FRidh as a code owner December 21, 2018 10:24
@GrahamcOfBorg GrahamcOfBorg added the 6.topic: python Python is a high-level, general-purpose programming language. label Dec 21, 2018
FRidh
FRidh previously requested changes Dec 21, 2018
View reviewed changes
pkgs/development/python-modules/pyqt/5.x.nix Outdated
Copy link
Member

@FRidh FRidh Dec 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace .so with ${stdenv.hostPlatform.extensions.sharedLibrary}

Copy link
Member Author

@nyanloutre nyanloutre Dec 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done 👍

@GrahamcOfBorg GrahamcOfBorg added 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Dec 21, 2018
@FRidh FRidh mentioned this pull request Dec 21, 2018
Closed
@nyanloutre nyanloutre force-pushed the pyqt5-fix branch 2 times, most recently from 093a322 to e5f164e Compare December 21, 2018 10:42
@timokau
Copy link
Member

timokau commented Dec 21, 2018

@GrahamcOfBorg eval

1 similar comment
@timokau
Copy link
Member

timokau commented Dec 21, 2018

@GrahamcOfBorg eval

@timokau
Copy link
Member

timokau commented Dec 21, 2018

@GrahamcOfBorg build python2.pkgs.pyqt5 python3.pkgs.pyqt5

@nyanloutre
Copy link
Member Author

nyanloutre commented Dec 21, 2018 •
edited
Loading

Sorry I had to rewrite it a bit. I first tried to skip the module rename by only linking to the library but this wasn't enough.

I didn't try to find side effects of this name changing though

@GrahamcOfBorg GrahamcOfBorg added 10.rebuild-linux: 101-500 This PR causes between 101 and 500 packages to rebuild on Linux. and removed 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Dec 21, 2018
@timokau
Copy link
Member

timokau commented Dec 21, 2018

@GrahamcOfBorg build python2.pkgs.pyqt5 python3.pkgs.pyqt5

@timokau
Copy link
Member

timokau commented Dec 21, 2018

@GrahamcOfBorg build calibre

@timokau
Copy link
Member

timokau commented Dec 21, 2018

The qtwebkit failures on darwin and aarch64 are unrelated and handled in #51846.

Thanks for the fix!

@timokau timokau merged commit bfca708 into NixOS:master Dec 21, 2018
@nyanloutre nyanloutre deleted the pyqt5-fix branch December 21, 2018 14:30
@veprbl
Copy link
Member

veprbl commented Dec 21, 2018

I wonder if --sip-module needs to be applied conditionally to preserve compatibility for those who use sip without pyqt5. Also, perhaps, we should now move sip out of pyqt5.propagatedBuildInputs to just pyqt5.buildInputs so that sip can't clash with pyqt5. Also sip and pyqt5 currently often go together in dependencies of other packages, we would probably need to eliminate those references to sip as well.

@orivej
Copy link
Contributor

orivej commented Dec 25, 2018

This broke tortoisehg which uses pyqt4 rather than pyqt5.

@orivej
Copy link
Contributor

orivej commented Dec 25, 2018

It seems that pyqt5 depends on https://pypi.org/project/PyQt5-sip/ rather than on https://pypi.org/project/SIP/.

@orivej orivej mentioned this pull request Dec 26, 2018
Closed
10 tasks
@orivej
Copy link
Contributor

orivej commented Dec 26, 2018

Possible fix: #52897

@veprbl veprbl mentioned this pull request Dec 27, 2018
Closed
10 tasks
orivej added a commit to orivej/nixpkgs that referenced this pull request Dec 27, 2018
@orivej
pythonPackages.sip: rename the module from PyQt5.sip back to sip
ce3080c 
PyQt5 5.11 wants a copy of sip in PyQt5.sip to ensure that sip-generated PyQt5
modules have the correct sip runtime version. This issue is not relevant to
Nixpkgs, therefore PyQt5 may be reverted to use the common sip runtime.

This is an alternative to NixOS#52613 that does not break packages that need to import sip.

Fixes the build of tortoisehg.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@FRidh FRidh FRidh left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: python Python is a high-level, general-purpose programming language. 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 101-500 This PR causes between 101 and 500 packages to rebuild on Linux.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@nyanloutre @timokau @veprbl @orivej @FRidh @GrahamcOfBorg