fix: make sure pyodide works without setup

pypa / cibuildwheel

🎡 Build Python wheels for all the platforms with minimal configuration.

https://cibuildwheel.pypa.io

Other

1.84k stars 235 forks source link

fix: make sure pyodide works without setup #1868

Closed henryiii closed 3 months ago

henryiii commented 3 months ago

Currently we are not always getting a version of Python that supports the pyodide platform in the action. This makes sure we do that.

Edit: This is not actually correct, the problem with or without this is cibuildwheel is looking for and can't find 3.12, even though it is running on 3.12. Running a setup-python (that updates the environment) before this fixes the problem. Maybe our discovery needs to be widened to include sys.executable?

mayeut commented 3 months ago

We could:

widen the search to sys.executable (might be applicable to other CI easily)
parse GHA toolcache (GHA only)
pull https://github.com/indygreg/python-build-standalone (I think uv & pipx can do that)
use a docker image instead of running on the host

henryiii commented 3 months ago

I like a) for now, and then maybe work on c) longer term. Possibly d), but I like the much higher performance of not pulling a docker image if it's not needed.

henryiii commented 3 months ago

Fixes the issue here: https://github.com/scikit-hep/boost-histogram/pull/935

mayeut commented 3 months ago

I like a) for now, and then maybe work on c) longer term. Possibly d), but I like the much higher performance of not pulling a docker image if it's not needed.

a) seems good enough for now.

Personally, I'd rather see d) than c) because it would allow local builds. I'm not worried at all about pulling an image (it should have a much much lower footprint than manylinux/musllinux images). I'm more worried about the image provenance/maintenance and the overhead by cibuildwheel copying into the container at the start (and maybe things like build cache).

henryiii commented 3 months ago

If the image already had emscripten, etc, then it probably would be fine performance wise, and would work better on macOS. I wonder if we could fix the pytest warning (which becomes an error if you have warnings as errors) about the source directory not being writable for the cache folder if we used a container?

henryiii commented 3 months ago

I'd like to make a release soon with this fix, so that users setting up the new Pyodide support don't have to add the extra setup. Anything else that needs to go in?

mayeut commented 3 months ago

Anything else that needs to go in?

I think we're good. Just saw a doc fix pop-up that might need to go in.

mayeut commented 3 months ago

On second thought, maybe we should wait for pip 24.1 ? I think the pip team wanted to get the release by June 17th. There can still be another release of cibuildwheel once it lands of course.

henryiii commented 3 months ago

Looks like b2 is coming soon, maybe we could at least wait for that. Patch releases should be pretty cheap, but this isn't too horribly inconvenient either, just one extra action is required.