-
Notifications
You must be signed in to change notification settings - Fork 28
Build wheels for more platform/arch combinations #475
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
Conversation
Actually, we do build wheels for Apple Silicon and Python versions 3.9 through 3.13, as you can see at https://pypi.org/project/ffsim/#files. If you have an issue with installation, please open an issue describing it with instructions to reproduce it. |
pyproject.toml
Outdated
# By default, cibuildwheel builds for all supported Python versions available on | ||
# the CI runner. To build for a specific set of versions, uncomment the | ||
# "build" line below and edit the list. | ||
# build = "cp39-* cp310-* cp311-* cp312-* cp313-*" |
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 one wheel that is compatible with all Python versions. Please delete this comment.
pyproject.toml
Outdated
test-requires = "pytest" | ||
test-command = "pytest {project}/tests" | ||
|
||
[tool.cibuildwheel.macos] |
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 don't use cibuildwheel for macos. Please delete these lines.
Signed-off-by: Nigel Jones <[email protected]>
Closing until a working solution is tested as much as possible on a fork. |
The current build is using manylinux2014 -- this is based on centos7 (hence the patch for yum in the builds). Unfortunately the required tools (rust etc) don't appear in the repos for arm. Trying to create a config which does not affect any platform except aarch64 (and uses manylinux_2_28). Currently paused on runner availability for testing. |
Title:
build: Enable future-proof builds for all architectures and Python versions
Body:
This PR updates the
cibuildwheel
configuration inpyproject.toml
to provide pre-compiled binary wheels for a wide range of platforms and to ensure support for future Python versions.Problem
Previously, the project's wheel-building configuration was restrictive, causing two main issues:
arm64
architecture on macOS, forcing users on Apple Silicon Macs to build the library from source. This resulted in a slow installation process that required a local Rust toolchain.Solution
The
[tool.cibuildwheel]
section inpyproject.toml
has been updated to:build
list has been commented out.cibuildwheel
will now default to its automatic behavior, building for all available Python versions on the CI runner that are compatible with the project'srequires-python = ">=3.9"
setting. This makes the process future-proof.archs
option has been added for both macOS and Linux to ensure wheels are built for all intended platforms.pyproject.toml
to clearly explain the build configuration and how to switch back to a pinned list of versions if desired.Enabled Builds
This change enables the following builds:
Python Versions:
Architectures:
x86_64
(Intel) andarm64
(Apple Silicon).x86_64
andaarch64
.Python Wheel Compatibility
It's important to note that Python wheels are specific to the Python version they were compiled for. A wheel's filename contains a "Python tag" (e.g.,
cp39
for CPython 3.9). When a user on Python 3.12 runspip install
,pip
will only look for wheels with a compatiblecp312
tag. It will never install an incompatiblecp39
wheel on a Python 3.12 environment, as the underlying C-API is different. This change ensures that a correctly tagged wheel will be available for each major Python version.Impact & Trade-offs
In Closing ....
This is an issue I noticed whilst using the library. It feels like a useful improvement, but I don't have experience with building or developing on this codebase, so apologies if I missed something or didn't consider other impacts!
Raising as draft initially to verify CI