Skip to content
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

Add building wheels using cibuildwheel #1155

Merged
merged 235 commits into from
Mar 6, 2024
Merged

Add building wheels using cibuildwheel #1155

merged 235 commits into from
Mar 6, 2024

Conversation

JCGoran
Copy link
Contributor

@JCGoran JCGoran commented Feb 15, 2024

This is a follow-up of #1147 to greatly simplify and speed-up the whole build CI pipeline.

Overview of changes

  • building a (redistributable!) wheel for a platform is now as simple as pip install cibuildwheel && cibuildwheel. The resulting wheel will be available in the wheelhouse subdirectory. Some notes about this:
    • by default, it builds wheels for all Python versions supported by NMODL (currently 3.8-3.12); for quick testing and development, set the env variable CIBW_BUILD to something like cp311* (for Python 3.11) to only build a wheel for a specific Python version
    • the wheel built is native to the platform (on Linux, or if specifying --platform linux in the above, Docker is needed; if you have podman instead, set CIBW_CONTAINER_ENGINE=podman)
    • on MacOS, you need the installer from python.org (the Homebrew-installed one won't cut it)
    • on MacOS, we also run some tests once the wheel is built
    • on Linux, we only run tests in the CI (why? Because the manylinux Docker image used for building the container does not come with the Python shared library); if you want to test the built wheel locally on Linux, run bash packaging/test_wheel.bash $(command -v python3) WHEEL, where WHEEL is the wheel you just built
  • as a result of all of the above, removed packaging/build_wheels.bash
  • simplified CI to use cibuildwheel everywhere. This comes with a big reduction in the CI build time (see for instance old vs. new, 16 minutes per Python version vs. 7 minutes). By default we build wheels for all Python versions, which is probably not optimal, and I don't have strong opinions on this, so whatever the popular vote decides is fine by me (build all/build specific version/build oldest/build newest/build oldest and newest)
  • so far ARM64 builds work without issues, it's just a matter of setting up the proper CI pipeline (Azure doesn't have ARM64 MacOS runners, and GitHub doesn't have ARM64 Linux runners; for the former we can use GitHub, and for the latter we can use CircleCI), which, following some internal discussions, I am leaving out of this PR
  • update docs

What works

  • manylinux on x86_64 and ARM
  • MacOS on x86_64 and ARM

What needs work

  • after a bunch of Windows-specific fixes, jinja on Windows still has some issues with discovery of files, see https://github.com/BlueBrain/nmodl/actions/runs/7915828954/job/21608472715 EDIT: leave Windows support for later
  • build time on Linux per Python version can take a while (5 minutes) on x86_64, and, due to the need for QEMU, is even longer on aarch64. If we only want x86_64 builds though, we should be fine (EDIT: added CircleCI to build aarch64 Linux since it has a native runner) EDIT: it now builds wheels much faster than master
  • similar to the above, but with Python 3.8 on ARM on MacOS (however, there is a workaround for that one available) EDIT: ARM64 wheels are left for later
  • testing of the wheels is not implemented yet (EDIT: done)
  • we should update and/or build our own custom Docker images that have flex and bison to speed-up the building process; the current custom x86_64 latest image is 2 years old
  • apparently on manylinux images, the libpython.so library is not available (as described here and in Fails to detect Python on cibuildwheel scikit-build/scikit-build-core#173) so some workarounds may be needed for testing the nmodl binary in a portable way. The Python module tests work like a charm though!

PEP 621 does not allow `project.name` to be dynamic, and requires any
build backend to fail if one declares it as such. Since we only need to
change the name of the package when creating a nightly release, this
adds a `change_name.py` script, which only runs in the CI, that can
change the `project.name` entry in-place.
Note that, since `tomllib` has only become a part of the standard Python
library in version 3.11, we use the `tomli` (for reading) and `tomli-w`
(for writing) packages for parsing the `pyproject.toml` file.
On MacOS there seems to be an issue with `scikit-build-core` when
`build-dir` is unset, so now we explicitly set it when building the
wheel.
@bbpbuildbot

This comment has been minimized.

@bbpbuildbot

This comment has been minimized.

@bbpbuildbot

This comment has been minimized.

@JCGoran JCGoran mentioned this pull request Mar 4, 2024
14 tasks
@bbpbuildbot

This comment has been minimized.

@bbpbuildbot

This comment has been minimized.

@bbpbuildbot

This comment has been minimized.

Base automatically changed from jelic/pyproject to master March 6, 2024 08:18
@JCGoran JCGoran marked this pull request as ready for review March 6, 2024 08:43
@JCGoran JCGoran requested review from ohm314 and 1uc March 6, 2024 08:45
@bbpbuildbot
Copy link
Collaborator

Logfiles from GitLab pipeline #198641 (:white_check_mark:) have been uploaded here!

Status and direct links:

@JCGoran JCGoran merged commit 347f786 into master Mar 6, 2024
20 checks passed
@JCGoran JCGoran deleted the jelic/cibw branch March 6, 2024 11:58
ohm314 pushed a commit that referenced this pull request May 21, 2024
* use cibuildwheel for creating redistributable wheels
* simplify CI pipeline
* update packaging docs

---------

Co-authored-by: Nicolas Cornu <[email protected]>
Co-authored-by: Luc Grosheintz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants