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 musllinux wheels #2126

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ben9923
Copy link
Contributor

@ben9923 ben9923 commented Jul 30, 2022

Summary

  • OS: Linux (musl)
  • Bug fix: no
  • Type: wheels

Description

Added musllinux wheel support to CI. Installing fully-featured variants of common commands, compared to their busybox variants (namely ps, df) was the only thing necessary for tests to pass.
Should be useful for Alpine Linux users, which probably make a not-too-little percentage of the Linux users (Was ~1% back in 2017, I expect it to be much higher now).

It's especially useful to have wheels for musl-based distributions, as they tend to have smaller footprint installs than their glibc-counterparts. Those musl distros usually have no build tools (among other utilities) available by default to be so llightweight, which is suitable for cloud workloads.
For example, the 48.7MB python:alpine base Docker image grows to 170MB (installing the minimal gcc musl-dev linux-headers psutil build requirements), making it a much less atractive base image than it could be (compared to the glibc-based 126MB python:slim).

No link for an issue as apparently there is none for musl/Alpine wheels 😛

BTW - In case cibuildwheel configuration becomes toml-based, an 'override' for musllinux may be used instead of the hacky command -v check here. (EDIT: Done!)

@ben9923 ben9923 force-pushed the ci/musllinux-wheels branch 2 times, most recently from 4b9ccb7 to 29dde78 Compare September 9, 2022 11:35
@ben9923
Copy link
Contributor Author

ben9923 commented Sep 9, 2022

@giampaolo Rebased to show changes as part of 5.9.3.
Shouldn't #2037 appear there too? It currently shows up as part of 5.9.1.

Apparently psutil.tests.test_linux.TestSystemNetIfStats.test_flags is failing on master so checks are not passing, but you can see the previous run succeeded :)

@ben9923
Copy link
Contributor Author

ben9923 commented Sep 15, 2022

@giampaolo Added a fix for master Python 2 fails (which also affect Alpine) + (EDIT: Already fixed in master)
Now those ifconfig tests run for Python 3 (previously skipped) :)

BTW, net-tools ifconfig statistics are somewhat broken under musl... They use non-standard format strings that break /proc/net/dev parsing with musl. The alpine package also doesn't seem to override the busybox variant.
I just gave up on installing it under Alpine and let it behave like the old version in the Python 2 image.

freebsd appears to fail on something completely unrelated, I guess there was some sort of remote package update that makes it fail. (EDIT: Fixed in master)

@ben9923
Copy link
Contributor Author

ben9923 commented Oct 21, 2022

@giampaolo Adapted for pyproject.toml-based changes.

Please consider merging those changes :)
I don't think they make the package harder to maintain/release, and pretty much align with every other major Python package that now ships musllinux wheels.
Thanks for the effort in merging macOS arm64 and abi3!

@MrMino
Copy link

MrMino commented Apr 17, 2023

@giampaolo is there anything else needed for this to go through? This would speed up lots of CI runs and let us remove psutil build dependencies from our containers.

@Sodki
Copy link

Sodki commented May 27, 2023

psutil is a dependency of many projects, not having a wheel for musllinux breaks a lot of pipelines unnecessarily.

@MrMino
Copy link

MrMino commented Jul 26, 2023

For the record, the musllinux build skip comes from #2024, namely this commit made by @giampaolo. It then got moved into pyproject.toml in 3f81c62.

It looks like the patch was made in order to fix build failures. Since @ben9923 changes seem to result in cibuildwheel correctly building the wheels, there doesn't seem to be any apparent reason not to accept this.

@giampaolo are we missing something? Sorry for spamming, but this is a serious pain point for me and many others, since we cannot test psutil-dependant code on Alpine without modifying our Docker images, and it looks like the fix is just one click away.

@MrMino
Copy link

MrMino commented Oct 16, 2023

@giampaolo kind nudge. This has little to no overhead and would greatly improve the usage of psutil in containers.

@sgrtye
Copy link

sgrtye commented Oct 24, 2023

@giampaolo This change is essential for Alpine-based images. A small image size is a top priority for Alpine users, and these improvements will make psutil more compatible and efficient in Python-Alpine environments. Let's streamline this process and bring the benefits of psutil to a wider audience in Alpine containers. 🐧✨

@MrMino
Copy link

MrMino commented Nov 30, 2023

I suppose the request would be more persuasive if we fixed the build failures. Green circles look better in the PR list.

@ben9923 can you confirm with any degree of certainty that these do not stem from your changes?

@ben9923
Copy link
Contributor Author

ben9923 commented Nov 30, 2023

I suppose the request would be more persuasive if we fixed the build failures. Green circles look better in the PR list.

@ben9923 can you confirm with any degree of certainty that these do not stem from your changes?

The same failing test fails on master as well, and it's on Windows. Nothing to do with this PR 😀

- Install required apk packages for Alpine tests.

Signed-off-by: Ben Raz <[email protected]>
Python 3 manylinux CI image does not have net-tools (hence ifconfig)
pre-installed, so tests using it were skippied altogether.

Signed-off-by: Ben Raz <[email protected]>
@ben9923
Copy link
Contributor Author

ben9923 commented Dec 22, 2023

#2326 contained the build tools installation instructions from this PR and was merged to master. Rebased to fix conflicts :)

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