GitHub Actions CI: explain Zig usage in Python wheel release#315
GitHub Actions CI: explain Zig usage in Python wheel release#315jayaddison wants to merge 3 commits into
Conversation
The Zig toolchain functionality enabled here originates from a build workflow[1] in the `uap-rust` repository; as far as I can tell, Zig is not required when building and releasing the Python wheels. My understanding is that the `maturin-action` that installs the Zig dependencies would proceed[2] even if they cannot be installed, so I don't think that their failure would block the release process here. Even so, I think that software build processes should usually try to limit their build requirements to only what is necessary, so I'm offering this as a cleanup. Resolves ua-parser#312. [1] - ua-parser/uap-rust#3 [2] - https://github.com/PyO3/maturin-action/blob/04ac600d27cdf7a9a280dadf7147097c42b757ad/dist/index.js#L47396-L47407
|
@masklinn hm. re-reading the It looks like the CI job assigns various native build runners from a matrix of options -- so I don't know whether we need the overhead (and extra surface area) of a Docker build. One idea would be to explicitly configure |
That was never intended, so doesn't matter.
The question is less whether we need the overhead and more whether we need to avoid it. The wheels workflow doesn't run normally, only when the PR is flagged to run it or when cutting a release, so it doesn't matter. |
|
Although it does look like the dockerised maturin is not happy: https://github.com/ua-parser/uap-python/actions/runs/25629265798/job/75229894764#step:5:262 |
|
Hm. OK. There is a note in the
...and some of that text originates from: PyO3/maturin#2457 I might ask there about how/why Zig helps here. One other thing I noticed is a couple of the warnings in the failed build:
Naively inspecting the code those led me to the Rust If Zig really is required for wheel builds, as per the README, then I expect we'd find a different form of build failure if we attempt to configure |
|
Self-quoting re: Zig requirement:
I was sagely pointed towards the documentation, and the answer I get from there is that
So, roughly speaking, this is a route to build Self-quoting re: Python detection:
Perhaps Edit: use permalink for code reference. |
|
@masklinn roughly speaking: I think I now understand that So: I think we can close this. If it would be worthwhile to get the Docker build back into a usable state as an alternative/comparison build (it wouldn't produce identical binaries, I'd guess, because the toolchain is different -- but having diverse available build options is good), I could try exploring this further, probably on the OpenCulinary fork of the repo. |
|
@jayaddison would you mind repurposing this PR just to add a comment to the zig option, in case someone else notices the same thing you did heads down the same rabbit hole? Something along the lines of
or something along those lines? I'd do it myself, but your branch is under an org repository and AFAIK github still does not support maintainer pushes to such. |
This reverts commit a18896d.
Co-authored-by: masklinn <github.com@masklinn.net>
The Zig toolchain functionality enabled here originates from a build workflow in the
uap-rustrepository;as far as I can tell, Zig is not required when building and releasing the Python wheels.My understanding is that the
maturin-actionthat installs the Zig dependencies would proceed even if they cannot be installed, so I don't think that their failure would block the release process here.Even so, I think that software build processes should usually try to limit their build requirements to only what is necessary, so I'm offering this as a cleanup.Update: Zig is used during the relevant GitHub Actions workflow to cross-build
manylinuxwheels; this PR has been repurposed to add an explanatory comment, instead of removing the flag to enable Zig as initial suggested.Resolves #312.