Skip to content

build Python wheels with cibuildwheel and add to build workflow#406

Open
zacharyburnett wants to merge 3 commits intogoogle:masterfrom
zacharyburnett:python_wheels
Open

build Python wheels with cibuildwheel and add to build workflow#406
zacharyburnett wants to merge 3 commits intogoogle:masterfrom
zacharyburnett:python_wheels

Conversation

@zacharyburnett
Copy link
Copy Markdown
Contributor

@zacharyburnett zacharyburnett commented Jan 26, 2025

closes #389, blocked by #390 [fixed]

@zacharyburnett zacharyburnett changed the title build Python wheels with cibuildwheel build Python wheels with cibuildwheel and test with pytest Jan 29, 2025
@zacharyburnett zacharyburnett changed the title build Python wheels with cibuildwheel and test with pytest automatically build Python wheels with cibuildwheel and test with pytest Jan 29, 2025
@zacharyburnett
Copy link
Copy Markdown
Contributor Author

zacharyburnett commented Jan 29, 2025

The current blocking issue with building is that repairwheel can't find the C extension in the built wheel. Perhaps it is not included for some reason, maybe an issue with the setuptools config:

Repairing wheel...

    + sh -c 'auditwheel repair -w /tmp/cibuildwheel/repaired_wheel /tmp/cibuildwheel/built_wheel/s2geometry-0.0.0-cp37-cp37m-linux_x86_64.whl'
INFO:auditwheel.main_repair:Repairing s2geometry-0.0.0-cp37-cp37m-linux_x86_64.whl
INFO:auditwheel.main_repair:This does not look like a platform wheel, no ELF executable or shared library file (including compiled Python C extension) found in the wheel archive

@zacharyburnett zacharyburnett force-pushed the python_wheels branch 2 times, most recently from 7408992 to dceaa00 Compare June 16, 2025 17:30
Comment thread pyproject.toml
@zacharyburnett zacharyburnett changed the title automatically build Python wheels with cibuildwheel and test with pytest build Python wheels with scikit-build and add to build workflow with cibuildwheel Apr 10, 2026
Comment thread CMakeLists.txt Outdated
Comment on lines +78 to +81
find_package(PythonExtensions REQUIRED)
add_library(SwigBindings MODULE src/python)
python_extension_module(SwigBindings)
install(TARGETS SwigBindings LIBRARY DESTINATION s2geometry)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure exactly what to put here; this just modifies the example from https://scikit-build.readthedocs.io/en/latest/usage.html#basic-usage

@zacharyburnett zacharyburnett force-pushed the python_wheels branch 7 times, most recently from 9a6d267 to 79a1be3 Compare April 10, 2026 14:45
@zacharyburnett zacharyburnett changed the title build Python wheels with scikit-build and add to build workflow with cibuildwheel build Python wheels with scikit-build and add to build workflow Apr 10, 2026
@zacharyburnett zacharyburnett force-pushed the python_wheels branch 7 times, most recently from ce3b399 to 344ea65 Compare April 10, 2026 15:44
@zacharyburnett
Copy link
Copy Markdown
Contributor Author

zacharyburnett commented Apr 10, 2026

unfortunately, using scikit-build still fails to provide the s2geometry module:

Run source .venv/bin/activate && python -c "import s2geometry"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
    import s2geometry
ModuleNotFoundError: No module named 's2geometry'

@zacharyburnett zacharyburnett force-pushed the python_wheels branch 3 times, most recently from 713b7aa to 07817f3 Compare April 17, 2026 14:51
@zacharyburnett zacharyburnett force-pushed the python_wheels branch 2 times, most recently from 2e641d4 to e271937 Compare April 17, 2026 15:04
@zacharyburnett zacharyburnett changed the title build Python wheels with scikit-build and add to build workflow build Python wheels with cibuildwheel and add to build workflow Apr 17, 2026
@zacharyburnett zacharyburnett force-pushed the python_wheels branch 3 times, most recently from 7d322e8 to dfc5b91 Compare April 17, 2026 15:45
@zacharyburnett
Copy link
Copy Markdown
Contributor Author

zacharyburnett commented Apr 17, 2026

building the Python 3.8 wheels for macos-latest and macos-15-intel fails with the following error:
https://github.com/google/s2geometry/actions/runs/24573839604/job/71853883913#step:3:612

error: aligned allocation function of type 'void *(std::size_t, std::align_val_t)' is only available on macOS 10.13 or newer
error: aligned deallocation function of type 'void (void *, std::align_val_t) noexcept' is only available on macOS 10.13 or newer

@zacharyburnett zacharyburnett marked this pull request as ready for review April 17, 2026 16:20
@jmr
Copy link
Copy Markdown
Member

jmr commented Apr 18, 2026

error: aligned allocation function of type 'void *(std::size_t, std::align_val_t)' is only available on macOS 10.13 or newer

Since the error is in absl, this is an absl bug, although I think other places in S2 will have this problem.

https://github.com/abseil/abseil-cpp#support

Support for 10.13 will be dropped in the summer, so I wouldn't worry too much about this one.

https://github.com/google/oss-policies-info/blob/main/foundational-cxx-support-matrix.md#macos

Why is this only a problem for this PR?

We can add

set(CMAKE_OSX_DEPLOYMENT_TARGET "10.13" CACHE STRING "Minimum macOS version")

before project().

@zacharyburnett
Copy link
Copy Markdown
Contributor Author

zacharyburnett commented Apr 20, 2026

Progress! the repair wheel step now fails on macOS with

delocate.libsana.DelocationError: Failed to find any binary with the required architecture: 'arm64'
delocate.libsana.DelocationError: Failed to find any binary with the required architecture: 'x86_64'

@zacharyburnett
Copy link
Copy Markdown
Contributor Author

It looks like skipping wheel delocation for macOS lets cibuildwheel proceed, and the import tests pass with the built wheel; I'll test the uploaded artifacts on my arm64 machine when the workflow finishes

@zacharyburnett
Copy link
Copy Markdown
Contributor Author

zacharyburnett commented Apr 20, 2026

testing these wheels locally on macOS ARM, the tests at src/python/*_test.py that imported s2geometry passed, but the ones that tried to import s2geometry_pybind failed (I assume because those need Bazel in order to be included in the wheel?). I think this PR is ready to merge for the SWIG bindings.

In order to publish to PyPI, @figroc just needs to configure the PyPI project https://pypi.org/project/s2geometry/ to add this repository (https://github.com/google/s2geometry) and build workflow (build.yml) to the list of Trusted Publishers. Then, on the next github release, the wheels will automatically upload to PyPI with the tagged version

@zacharyburnett zacharyburnett marked this pull request as ready for review April 20, 2026 19:09
@zacharyburnett
Copy link
Copy Markdown
Contributor Author

zacharyburnett commented Apr 20, 2026

I don't know if C++ SWIG can make use of the "Limited API / Stable ABI" (https://docs.cython.org/en/latest/src/userguide/limited_api.html) but if so, that would make it so that only a single wheel would have to be built and published, instead of having to build a wheel for each Python version. But that would probably require changes in src/python/ that are out of scope of this PR

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.

Use cibuildwheel for Python CI

3 participants