Add riscv64 build, make Linux wheel build matrix more explicit - #288
Conversation
|
Hey @threexc thank you for the contribution! General approach looks good -- would you mind merging or rebasing on the new changes in main? |
3d21af0 to
124c4eb
Compare
Should be OK now. Please let me know if there are any other issues. |
|
Ran the workflow on my fork and hit some issues. I was thinking that we can just skip platform:
- runner: ubuntu-latest
target: x86_64-unknown-linux-gnu
arch: x86_64
zig: true
- runner: ubuntu-latest
target: riscv64gc-unknown-linux-gnu
arch: riscv64
zig: falseand updating the args line: args: --release --out dist ${{ matrix.python-version != '3.14t' && matrix.platform.zig && '--zig' || '' }}I tried this out, but I ran into another issue Might have to make a different choice than |
124c4eb to
be4ce10
Compare
Thanks for catching that. I've modified the patch to set |
Make the Linux matrix more explicit by distinguishing between the 'target' and the 'arch', which are not always the same. This pattern is better suited to architectures where they don't match. Also explicitly set a boolean flag for 'zig', since not all architectures may support it. Signed-off-by: Trevor Gamblin <tgamblin@baylibre.com>
Skip zig to avoid this error: |error: unable to find or provide libc for target 'riscv64-linux.4.19...6.5.7-gnu.2.17' |info: zig can provide libc for related target riscv64-linux-gnu |info: zig can provide libc for related target riscv64-linux-musl Signed-off-by: Trevor Gamblin <tgamblin@baylibre.com>
be4ce10 to
3bc6837
Compare
|
@hudson-ai I think I misread your reply the first time around. I've fixed things up now. Notably, I've added Testing here: threexc#2 |
|
@threexc it's still not working due to some linker issues. Please see the workflow run here: |
|
Annoyingly, it's not trivial to run the right workflow from a fork; you may have to add a workflow-dispatch trigger (please remove before we actually merge...) and possibly even run it from the command-line, like |
|
@threexc I got it working with the following configuration. Note the before-script that explicitly installs and sets the linker. I also made the matrix a lot more explicit. linux:
runs-on: ${{ matrix.platform.runner }}
needs: [sdist]
strategy:
matrix:
platform:
- runner: ubuntu-latest
target: x86_64-unknown-linux-gnu
arch: x86_64
zig: true
manylinux: auto
- runner: ubuntu-latest
target: i686-unknown-linux-gnu
arch: x86
zig: true
manylinux: auto
- runner: ubuntu-latest
target: aarch64-unknown-linux-gnu
arch: aarch64
zig: true
manylinux: auto
- runner: ubuntu-latest
target: riscv64gc-unknown-linux-gnu
arch: riscv64
zig: false
manylinux: 2_39
before-script: 'sudo apt-get install -y gcc-riscv64-linux-gnu && mkdir -p ~/.cargo && printf "[target.riscv64gc-unknown-linux-gnu]\nlinker = \"riscv64-linux-gnu-gcc\"\n" >> ~/.cargo/config.toml'
python:
- version: '3.12'
zig: true
- version: '3.14t'
zig: false
exclude:
- platform:
arch: riscv64
python:
version: '3.14t'
steps:
- uses: actions/checkout@v6
- uses: actions/setup-python@v6
with:
python-version: ${{ matrix.python.version }}
- name: Build wheels
uses: PyO3/maturin-action@v1
with:
target: ${{ matrix.platform.target }}
args: --release --out dist ${{ matrix.python.zig && matrix.platform.zig && '--zig' || '' }}
sccache: 'true'
manylinux: ${{ matrix.platform.manylinux }}
before-script-linux: ${{ matrix.platform.before-script || '' }}
- name: Upload wheels
uses: actions/upload-artifact@v6
with:
name: wheels-linux-${{ matrix.platform.arch }}-${{ matrix.python.version }}
path: dist |
|
Note that I excluded the (3.14t, riscv64) pair, which failed seemingly due to some PyO3 weirdness: https://github.com/hudson-ai/llguidance/actions/runs/22318409028/job/64569802785 @ngoldbaum maybe you know what's going on here and can chime in? |
|
My concern with this is that we don't actually have RISC-V runners to validate on. If we merge, we should make it very clear that this arch is provided as-is. |
Looks like a maturin bug, you shouldn't be getting an abi3 wheel. I opened PyO3/maturin#3034. |
Got it. Thanks a bunch! |
Thank you! I hadn't gotten around to trying your suggestions yet. Do you want to make an extra commit to fix and squash the changes, or do you want me to submit that? |
|
@threexc I'm happy to push a commit and squash. Sorry if I stepped on your toes here. Just eager to merge in before doing a release :) |
…exclude riscv64 x python3.14
|
Workflow is green: https://github.com/hudson-ai/llguidance/actions/runs/22322576909 @riedgar-ms you ok with me merging if I note the untested riscv support in the changelog upon the next release? |
riedgar-ms
left a comment
There was a problem hiding this comment.
OK with the 'untested' note
|
@hudson-ai no problem! I'm just happy to see RISC-V support :) Thank you everyone. |
maturin supports riscv64 cross build, but the arch string and the target toolchain for rustc don't exactly match up (this is also the case for other architectures like ppc64le and x86). To make the riscv64 addition consistent with the others, change the platform list so that arch and target are distinct fields.
Note that this is being done on behalf of the RISE Project to improve Python ecosystem support on riscv64 platforms.