-
-
Notifications
You must be signed in to change notification settings - Fork 15.4k
Fix cairocffi build #56839
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
Fix cairocffi build #56839
Conversation
@GrahamcOfBorg python2.pkgs.cairocffi python3.pkgs.cairocffi |
On Tue, Mar 05, 2019 at 12:29:50AM -0800, Frederik Rietdijk wrote:
@GrahamcOfBorg python2.pkgs.cairocffi python3.pkgs.cairocffi
FYI cairocffi is 3.5+ library now.
Probably I should add assert as well.
|
@GrahamcOfBorg build python3.pkgs.cairocffi |
On Tue, Mar 05, 2019 at 01:10:25AM -0800, Frederik Rietdijk wrote:
Added ``disabled = pythonOlder "3.5"`` stanza
|
@FRidh @avnik I'd propose to add an Also wondering if the Python update script should contain some functionality to actually test the package before bumping it (and pushing this to master). |
@Ma27 Other option -- add some backporing patch to this package, I am ok with both solutions. |
Ack. Didn't look at the package's version when writing this 😅
Can you please be more precise in what you mean? Not sure if I understand correctly what you meant :) |
On Thu, Mar 07, 2019 at 08:37:44PM +0000, Maximilian Bosch wrote:
> Other option -- add some backporing patch to this package, I am ok with both solutions.
Can you please be more precise in what you mean? Not sure if I understand correctly what you meant :)
I mean, if you take commit, which removing 2.x support, make reversing
patch, and will support it ;)
If you want overtake cairoffi, you are welcomed -- I care about it,
because it transitive dependency of qtile.
|
I doubt that it'll be that easy. Python 2.x and 3.x diverge pretty much. But I'll have a look at this and see how to proceed here :) |
On Sat, Mar 09, 2019 at 06:07:44AM -0800, Maximilian Bosch wrote:
> I mean, if you take commit, which removing 2.x support, make reversing patch, and will support it ;)
I doubt that it'll be that easy. Python 2.x and 3.x diverge pretty much.
I know. I am in middle of porting mid size project from 2.x and gevent
to 3.x and aio. Lot of pain and sorrows ;)
|
Just a short update: I created a |
I pushed three commits on top of your branch:
@FRidh @avnik it would be great if you could review this as well :) |
pkgs/top-level/python-packages.nix
Outdated
@@ -1311,7 +1311,7 @@ in { | |||
|
|||
canmatrix = callPackage ../development/python-modules/canmatrix {}; | |||
|
|||
cairocffi = callPackage ../development/python-modules/cairocffi {}; | |||
inherit (callPackage ../development/python-modules/cairocffi {}) cairocffi cairocffi_0_9; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about
inherit (callPackage ../development/python-modules/cairocffi {}) cairocffi cairocffi_0_9; | |
cairocffi = if isPy3k then | |
(callPackage ../development/python-modules/cairocffi {}).cairocffi | |
else | |
(callPackage ../development/python-modules/cairocffi {}).cairocffi_0_9; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right! That made the fix for thumbor
far easier. I also exposed cairocffi_1_0
and cairocffi_0_9
for now as it seems to be a common pattern to add all versioned attributes to the package set and alias one of those to the default package name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer not to expose them because pythonPackages
should only contain a single version of each package. Btw, I think the prettiest way to do this is by using let inherit (calPackage ...) cairocffi cairocffi_0_9; in if isPy3k ...
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The commit messages for cairocffi should say pythonPackages.cairocffi: ...
.
According to `setup.py` in 0.8 older Python wouldn't work.
This patch ensures that the currently broken `thumbor`[1] package builds and works again. The following problems were fixed: * Rather than placing required packages (like `gifsicle` or `exiftool`) into the build input list, we reference them explicitly where needed to ensure that the package works after the build without further installs. * Skip the `test_redeye_applied` test case which is broken for a while now. [1] https://hydra.nixos.org/build/90290998
This patch ensures that Python2 can be used if cairocffi is used. Version 1.0 dropped Python 2 support.
dff320d
to
e878fd6
Compare
@avnik is there a way to use it with overridden nixpkgs/pkgs/applications/window-managers/qtile/default.nix Lines 3 to 6 in 6004e5d
I can't make it work after this change. |
@kamilchm Sorry for messing that up. An easy way to make it work would be to use three files: |
Motivation for this change
cairocffi doesn't build, preventing qtile from build.
(patch doesn't applying correctly)
Patch was refreshed, as well as more packages required for tests added.
/cc @FRidh
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)