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

Build and publish Pyodide wheels #161

Merged
merged 9 commits into from
Sep 12, 2024

Conversation

agriyakhetarpal
Copy link
Contributor

Description

This PR implements a few things across the workflow(s):

  1. Adds configuration via standard CIBW_-prefixed environment variables for building Pyodide wheels. This requires a bump to the cibuildwheel version, which has been updated to version 2.20.0, the latest available stable version. Support for building and publishing Pyodide wheels was introduced in version 2.19.0.
  2. Switches cibuildwheel from being pip-installed to its action as provided in the GitHub Actions marketplace
  3. Adds a minimal Dependabot configuration file with weekly updates to bump the version for said action (and others). It can be triggered monthly, if needed.

Note

I didn't run the test command for the Pyodide wheel step as of now, because of two reasons:

  1. Pyodide support is now tested in out-of-tree PR builds in the main repository, as linked in the PR below. Of course, this is not a valid enough reason because this is a separate environment altogether. However, there's still the problem of a Pyodide fatal error coming from a missing pow_dd symbol when running the test suite (which I'll return in a while to investigate). If I am requested to enable the testing by the maintainers here, I can bring the alternative Node.js-based test runner file where the symbol visibility bug doesn't post a problem and use it to test the wheel here. Alternatively, the statsmodels submodule in this repository can be updated, and the file will appear. Please let me know if you would like me to do that.
  2. The Pyodide test suite is slower than other jobs because threading/multiprocessing is disabled in a WebAssembly-based environment, and pytest-xdist is not usable. Hence, testing the wheels takes much longer (~30 minutes). It will still be faster than the "ubuntu-latest, Python cp3X" jobs, however1.

This is a follow-up PR after statsmodels/statsmodels#9270, which describes the rationale for this change. Please let me know if points two and three are better placed in a separate PR. Thank you!

Footnotes

  1. Those jobs run for more than an hour, would it be alright if I parallelise them across the trio of manylinux-x86_64, musllinux-x86_64, and manylinux-aarch64? Doing this will lead to a 43% speedup – if yes, I could do so in a separate PR to keep the diff cleaner.

Copy link
Contributor

@bashtage bashtage left a comment

Choose a reason for hiding this comment

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

One small thing

.github/workflows/build-wheels.yml Outdated Show resolved Hide resolved
@agriyakhetarpal
Copy link
Contributor Author

Additionally, I removed the setup-python step in a690858, because the cibuildwheel action is a composite one and embeds it for you: https://github.com/pypa/cibuildwheel/blob/bd033a44476646b606efccdd5eed92d5ea1d77ad/action.yml#L27-L32, and when running, it downloads its own Python distributions from official sources, NuGet, or runs a (many/musl)linux container anyway.

@bashtage bashtage merged commit 1d50a6c into MacPython:main Sep 12, 2024
13 checks passed
@bashtage
Copy link
Contributor

Thanks.

@agriyakhetarpal agriyakhetarpal deleted the chore-emscripten-ci-updates branch September 13, 2024 12:27
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.

2 participants