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
feat: build wheels using CIBuildWheel #103
Conversation
We need to work out how the architcture labels play between Github Workflow and CIBuildWheel, so that we only build Win32 wheels when we've built the corresponding C library.
matrix: | ||
os: | ||
- macos-10.15 | ||
#- macos-11 |
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.
Due to the changes we made previously to fix the OS X 11 bug, we ended up not needing a separate Big Sur build, correct?
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 believe so -- even the wheels built on Big Sur were segfaulting on Big Sur with Python 3.8 before that fix.
with: | ||
submodules: 'recursive' | ||
|
||
- name: Build C Library |
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.
Why is this needed as a separate github action step if it's also being built as a cibuildwheel prep step?
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.
We only build it as a prep step on Linux, where we need to build it using the same manylinux
image being used to build the wheels. Here, splitting it out can help optimize the build (via a cache -- see below where I added the cache for the Windows library build), as well as making the built libraries potentitally available as downloadable artifacts.
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.
Shall we change the PR name to feat: so that we get a version bump? I should have added one back in 1.1.3, but better late than never.
Makes sense -- the M1 builds are certainly a new feature. One question before we merge this PR: the workflow I have now makes more sense for the release-tag build than for a presubmit: it bulids and tests all the wheels we know how to build. Perhaps we could trim back the wheels we build and test for the presubmit? (No |
7adadfe
to
c07b78e
Compare
@andrewsg I think we can add handling a |
Hmm, that sounds a bit complex... we've got quite a bit going on in this release, do you think we should push that through as well or can we accept the longer presubmits for now? |
Instead, run nightly, as well as on creation of a release. Uploads to PyPI only occur during release runs.
PR #103 replaces them, but cannot be merged with the requirements still in place.
No description provided.