ci: build riscv64#1948
Conversation
There was a problem hiding this comment.
Pull request overview
Adds CI coverage and build configuration for riscv64 (Linux) builds, including sysroot setup and workflow matrix expansion. It also adjusts sccache handling to avoid using a non-static riscv64 sccache binary by falling back to the host architecture binary.
Changes:
- Add GN args + sysroot installation for
riscv64targets inbuild.rs. - Extend GitHub Actions CI matrix to build/test
riscv64gc-unknown-linux-gnu(debug/release, with and without simdutf). - Add a riscv64 cross-compilation toolchain install step and adjust sccache platform selection for riscv64.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| build.rs | Adds riscv64 target CPU + sysroot install logic for V8 GN generation. |
| .github/workflows/ci.yml | Adds riscv64 CI jobs, installs riscv64 cross toolchain/QEMU, and maps riscv64 to host sccache binary. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| SCCACHE_IDLE_TIMEOUT: 0 | ||
| run: | | ||
| $version = "v0.8.2" | ||
| $version = "v0.14.0" |
There was a problem hiding this comment.
The sccache version is bumped to v0.14.0 for the main build job, but other jobs in this workflow still pin sccache to v0.8.2. If the version bump is intentional, consider updating the other sccache install steps (or centralizing the version into a single env/variable) to avoid inconsistent behavior and harder-to-debug CI issues across jobs.
bartlomieju
left a comment
bartlomieju
left a comment
There was a problem hiding this comment.
Thanks for adding riscv64 support! The build.rs changes look solid — the underlying V8/chromium infrastructure already supports riscv64 sysroots, so the plumbing is straightforward.
A few suggestions:
sccache version bump (v0.8.2 → v0.14.0): Could you split this into a separate PR? It's an unrelated change that bumps the version only in the build job while the other three jobs (build-asan, build-windows-arm64, build-windows-arm64-simdutf) still use v0.8.2. Separating it will make both PRs easier to review and merge independently.
| sudo ln -s /usr/riscv64-linux-gnu/lib/ld-linux-riscv64-lp64d.so.1 \ | ||
| /lib/ld-linux-riscv64-lp64d.so.1 |
There was a problem hiding this comment.
This will fail if the symlink already exists (e.g. on re-runs or different runner images). Use ln -sf for idempotency:
| sudo ln -s /usr/riscv64-linux-gnu/lib/ld-linux-riscv64-lp64d.so.1 \ | |
| /lib/ld-linux-riscv64-lp64d.so.1 | |
| sudo ln -sf /usr/riscv64-linux-gnu/lib/ld-linux-riscv64-lp64d.so.1 \ | |
| /lib/ld-linux-riscv64-lp64d.so.1 |
There was a problem hiding this comment.
Thanks for the review! Updated. (and also the aarch64 block)
| rustup target add riscv64gc-unknown-linux-gnu | ||
|
|
||
| sudo apt update | ||
| sudo apt install -yq --no-install-suggests --no-install-recommends \ |
There was a problem hiding this comment.
The aarch64 step doesn't install debootstrap. Is it actually needed for the riscv64 cross-compilation? If not, removing it would keep the install leaner.
There was a problem hiding this comment.
Oops. It was added when I did some experiments to run riscv64 sccache. Dropped.
| maybe_install_sysroot("arm"); | ||
| } |
There was a problem hiding this comment.
The aarch64 block only sets target_cpu but not v8_target_cpu. Is the extra v8_target_cpu needed for riscv64? If so, a brief comment explaining why would help.
There was a problem hiding this comment.
Updated. Cross-compiling needs to set v8_target_cpu. The arm block also sets this.
Co-authored-by: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= <biwanczuk@gmail.com>
Xeonacid
force-pushed
the
copilot/add-riscv64-cross-compilation-support
branch
from
b489404 to
83c4bf9
Compare
Thanks for the review! Reverted the changed and will send a PR later. |
|
Any chance to get this PR reviewed/merged? Thank you very much! |
Unlike aarch64, the riscv64 sccache binary is not static, so use the host architecture one. 1