-
Notifications
You must be signed in to change notification settings - Fork 88
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
Build wheels on github actions #224
Conversation
strategy: | ||
fail-fast: false | ||
matrix: | ||
os: [ubuntu-18.04, windows-latest, macos-latest] |
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.
Shouldn't this be CentOS 6 for manylinux2010
compatibility?
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.
No. ubuntu is only host for manylinux docker container (quay.io/pypa/manylinux2010_x86_64 and quay.io/pypa/manylinux2010_i686)
Hey. I'm just wondering the status of this PR. It seems like @Czaki addressed @jakirkham's question. |
This could use some reviews from others 🙂 cc @rabernat @sofroniewn (as we discussed this today) @jni (as this may be of interest) |
I rebase onto current master |
I am supportive of what this PR wants to achieve, but I have no experience building wheels, so I can't be of much help. |
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.
As @rabernat pointed out last night during the community, not having numcodecs wheels leads to time-consuming builds. I don't have experience with cibuildwheel
, but the action itself seems fine. I'd suggest that we get this merged, tag 0.6.4, gets wheels deployed and then iterate to 0.6.5 etc. as necessary.
My one heads up from our experience with manylinux is that some libraries (like openssl) are statically linked such that a re-release is necessary when upstream releases. This is likely less of an issue for Zarr but I wanted to at least raise the issue.
cc: @manics
@nbren12 Could you write more? I try this wheel on MacOS 10.14.16 and all test passed (Some xfail, but I'm not sure what is proper state). I also fix configuration and currently all test runs. I use @joshmoore Could you provide link to example wheel. For me it looks like wrong result of @jakirkham I do not know why AppVeyor build fail. |
@Czaki the problem occurs when openssl is statically linked into a linux wheel https://github.com/ome/zeroc-ice-py-manylinux/releases/tag/0.3.0 It will usually load, but you occasionally run into several downstream problems such as ciphers or certificates not being found. In addition you also take on the responsibility of having to rebuild the wheel when openssl is updated. For this reason we only ever used these manylinux wheels for CI testing of our other packages, it's not used in production. |
@manics So the problem is static linking ssl library. And it could create problem in many cases, ex. when ssl is upgraded in system. |
@Czaki Forget I said it doesn't work. I was trying to pip install the py3.6 wheel from python 3.7 🤦 . It works perfectly when I pip install the correct wheel. |
@jakirkham If I good check appveyor and OSX CI runs are to build wheel. So they could be removed? @manics If You use |
We've hit the lack of wheels for |
Can someone retry the appveyor builds? (2 of 3) |
I do not know why this PR stuck. There are any questions which I should response? From my point of view appveyor is obsolete because it is for building wheel which is covered by github actions from this PR, but @joshmoore I could force repush last commit. |
I agree with @Czaki. It would be great to see this merged to avoid having to rebuild numcodecs repeatedly. This PR didn't edit the appveyor configs so it's not clear why that is failing. |
As I look to log it fail because of network problem. |
I don't have access to the appveyor build so I can't restart, but I can ignore that failure and merge. The only caveat I can think of is that once the wheels are present, users on a broken platform (if there is one) will need to use pip install --no-binary until a subsequent PR can fix the issue. Alternatively, we could delete the wheels and revert this PR. My vote would be to move forward with this to start gathering feedback. Since this potentially breaks users space, I'd say this falls under [https://github.com/zarr-developers/governance/blob/master/GOVERNANCE.md#decision-making-process]("Changes to the APIs"). However, it's also been open for sometime so shall we say: 👉 Objections to merging this by the end of the week. If there are no thumbs down or negative comments below by the end of business Friday (CEST), I'll merge (unless someone beats me to it). I'd appreciate some extra eyes watching for community issues over the weekend and next week. Obviously, if someone has an addition that will improve the OS support, feel free to speak up. P.S. I note that on my 10.15.6 mac with a fresh Python 3.7 conda, the wheel does install for me. |
What type of error you expect? If I good understand macos do not have problem to work with code compiled against older os (backward compatibility). So why You expect some problems? |
Hi @Czaki. That was the only one issue I saw mentioned above, but testing it myself I didn't see anything. i.e. I don't know of any issues but this is asking if anyone else does. ;) |
@joshmoore But this mentioned problem comes from try install python 3.6 wheel to python 3.7
|
Do you know if it is possible to build wheels for PyPy? Is there infrastructure that we would be able to leverage for that? |
|
Build wheel on github actions using cibuildwheel. Cibuildwheel maintainers will fix most of problems with build system during development.
This PR contains only build wheel. Then it need to be manually uploaded to pypi.org. It is possible to ad such step to this.
Fixes #70
EDIT.
There is no such tools like dealocate or audithweel for windows, so maybe someone can check windows wheel on his machine.