feat(docker): add linux/riscv64 image to Docker release#1
feat(docker): add linux/riscv64 image to Docker release#1
Conversation
Summary of ChangesHello @gounthar, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request extends the Docker release process to include support for the Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Important Review skippedToo many files! This PR contains 298 files, which is 148 over the limit of 150. You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds linux/riscv64 to the multi-platform Docker build via a new QEMU-based Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions Runner
participant QEMU as QEMU (user-mode)
participant Buildx as docker buildx
participant Registry as Container Registry
participant Manifest as create-manifest / update-manifest
GH->>QEMU: setup QEMU binfmt & qemu-user-static
GH->>Buildx: setup buildx builder
GH->>Buildx: run buildx build for linux/riscv64 (emulated)
Buildx->>Registry: push riscv64 image
Registry-->>Buildx: return riscv64 digest
GH->>Manifest: create-manifest with amd64, arm64 digests
alt riscv64 digest available
GH->>Manifest: update-manifest with riscv64 digest
Manifest->>Registry: update multi-arch manifest to include riscv64
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://waps.l3s.uni-hannover.de/live/im_/https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for linux/riscv64 Docker images. The change in the Dockerfile correctly makes the installation of Bun conditional, skipping it on the riscv64 architecture where it's not available. My review includes a suggestion to make the shell command in the Dockerfile more concise. The changes are well-aligned with the goal of the pull request.
Dockerfile
Outdated
| RUN if [ "$(dpkg --print-architecture)" != "riscv64" ]; then \ | ||
| curl -fsSL https://bun.sh/install | bash; \ | ||
| fi |
There was a problem hiding this comment.
For better readability and conciseness, you can combine the conditional check and the command into a single line using &&. This is a common shell scripting idiom for conditional execution and achieves the same result as the multi-line if statement.
RUN [ "$(dpkg --print-architecture)" != "riscv64" ] && curl -fsSL https://bun.sh/install | bash
There was a problem hiding this comment.
The if/fi form is more readable and matches the existing conditional pattern used later in the same Dockerfile (the OPENCLAW_DOCKER_APT_PACKAGES block). Keeping the multi-line form for consistency.
![coderabbitai[bot]](https://waps.l3s.uni-hannover.de/live/im_/https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/docker-release.yml:
- Around line 143-147: The riscv64 job's tags block currently includes
unsuffixed tag types ("type=ref,event=branch" and
"type=semver,pattern={{version}}") which can overwrite shared tags; remove those
two unsuffixed entries from the riscv64 job's tags so it only emits
architecture-specific tags (e.g., keep only
"type=semver,pattern={{version}},suffix=-riscv64" and
"type=ref,event=branch,suffix=-riscv64"); update the tags list in the riscv64
job (the block containing those four tag lines) to exclude unsuffixed tag lines
to ensure only suffixed image tags are pushed.
🧹 Nitpick comments (1)
.github/workflows/docker-release.yml (1)
43-47: Consider removing unsuffixed tags from per-arch jobs entirely.Both
build-amd64andbuild-arm64also generate unsuffixed tags (Lines 43–44, 91–92), which means whichever finishes last temporarily "wins" the shared tag with a single-arch image until the manifest job overwrites it. Since the manifest job creates the canonical multi-arch tags, the per-arch jobs only need their suffixed variants.This is a pre-existing pattern and outside the scope of this PR, but it would be cleaner and safer to address it holistically.
Also applies to: 91-94
There was a problem hiding this comment.
Pull request overview
Adds Docker image support for linux/riscv64 by making Bun installation architecture-conditional and extending the Docker release workflow to build/publish riscv64 (via QEMU) and optionally include it in the multi-arch manifest.
Changes:
- Update
Dockerfileto skip Bun installation on riscv64. - Add a QEMU-emulated
build-riscv64job and conditionally include its digest in the manifest. - Fix per-arch tag suffix generation so each job only emits its own arch-suffixed tags.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| Dockerfile | Skip Bun install on riscv64 while keeping the existing build flow intact. |
| .github/workflows/docker-release.yml | Add riscv64 build job, update manifest creation to include riscv64 on success, and fix tag suffix generation per architecture. |
| # Build riscv64 image (QEMU-emulated, non-blocking) | ||
| build-riscv64: | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 120 | ||
| permissions: | ||
| packages: write | ||
| contents: read | ||
| outputs: |
There was a problem hiding this comment.
build-riscv64 is described as non-blocking, but without continue-on-error: true this job failing will still mark the entire workflow run as failed (which can block releases/branch protection even though amd64/arm64 images + manifest are produced). Consider adding continue-on-error: true to this job (or otherwise ensuring a riscv64 failure doesn’t fail the workflow).
There was a problem hiding this comment.
Agreed — added continue-on-error: true to the riscv64 job so it won't fail the overall workflow. Fixed in e10c2cb.
.github/workflows/docker-release.yml
Outdated
| needs: [build-amd64, build-arm64, build-riscv64] | ||
| if: always() && needs.build-amd64.result == 'success' && needs.build-arm64.result == 'success' |
There was a problem hiding this comment.
create-manifest now needs: [build-amd64, build-arm64, build-riscv64], which means publishing the multi-arch tags waits for the QEMU riscv64 build to finish (up to 120 minutes) even when you only need amd64+arm64. If the intent is that amd64+arm64 releases proceed promptly, consider publishing the amd64+arm64 manifest without waiting for riscv64, and only adding riscv64 later on success (this also requires ensuring the riscv64 build job does not push the unsuffixed tags that the manifest job owns).
There was a problem hiding this comment.
Good point. Restructured into two manifest jobs: create-manifest runs immediately after amd64+arm64 (no waiting), and update-manifest runs after riscv64 completes to add it to the manifest. This way releases ship promptly. Fixed in e10c2cb.
| curl -fsSL https://bun.sh/install | bash; \ | ||
| fi | ||
| ENV PATH="/root/.bun/bin:${PATH}" |
There was a problem hiding this comment.
On riscv64, Bun is intentionally not installed, but PATH is still unconditionally prefixed with /root/.bun/bin. Since the image later switches to USER node, that path entry is typically inaccessible/nonexistent at runtime and can make debugging harder if any command lookup hits it. Consider either installing/linking Bun into a non-root location (e.g., /usr/local/bin) when present, or only adding the Bun directory to PATH when Bun is installed (or removing it before switching users).
| curl -fsSL https://bun.sh/install | bash; \ | |
| fi | |
| ENV PATH="/root/.bun/bin:${PATH}" | |
| curl -fsSL https://bun.sh/install | bash && \ | |
| ln -s /root/.bun/bin/bun /usr/local/bin/bun; \ | |
| fi |
There was a problem hiding this comment.
The stale PATH entry is harmless (empty dir lookup is a no-op) and keeps the Dockerfile diff minimal. The existing comment on line 28 already documents that Bun may not be present on all architectures. Leaving as-is to keep the change focused.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/docker-release.yml:
- Around line 200-201: The current conditional for the update-manifest job uses
needs.build-riscv64.result which will be 'success' when the riscv64 job had
continue-on-error: true even if it actually failed; change the guard to verify
the riscv64 digest output is non-empty instead (e.g. replace the if:
needs.build-riscv64.result == 'success' check with an output existence check
like if: needs.build-riscv64.outputs.riscv_digest != ''), and ensure the
build-riscv64 job publishes that digest as an output (name the output e.g.
riscv_digest) so update-manifest only runs when a valid digest is present.
🧹 Nitpick comments (1)
.github/workflows/docker-release.yml (1)
43-47: Consider removing unsuffixed tags from per-arch jobs to avoid a transient race.Both
build-amd64andbuild-arm64push unsuffixed tags (type=ref,event=branchandtype=semver,pattern={{version}}). Whichever job finishes last briefly overwrites those shared tags with a single-architecture image untilcreate-manifestruns. This is pre-existing and the window is small for these two fast native builds, but it could be cleaned up so only the manifest job owns the unsuffixed tags.
|
Addressed the remaining feedback in 9f44ce8:
|
gounthar
force-pushed
the
feat/riscv64-docker
branch
from
9f44ce8 to
363d894
Compare
Skip Bun installation on riscv64 (no binary available) and add a QEMU-emulated build job for linux/riscv64 to the Docker release. Key design decisions: - riscv64 is non-blocking: continue-on-error prevents workflow failure - Per-arch jobs only push suffixed tags (-amd64, -arm64, -riscv64); only the manifest jobs own unsuffixed shared tags - create-manifest publishes amd64+arm64 immediately without waiting - update-manifest adds riscv64 to the manifest after it completes, guarded by digest output (not job result, which is unreliable with continue-on-error) - 120-minute timeout for QEMU-emulated Node.js builds Ref: openclaw#11977 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
gounthar
force-pushed
the
feat/riscv64-docker
branch
from
ff51976 to
54d20bd
Compare
1 participant
Summary
build-riscv64job to the Docker release workflowRef: openclaw#11977
Details
Dockerfile: Bun has no riscv64 binary, but it is not used during Docker builds (all build scripts use pnpm, and
OPENCLAW_PREFER_PNPM=1is already set). The install is wrapped in an arch check so it still installs on amd64/arm64.Workflow: The new
build-riscv64job follows the same pattern as the existing jobs but runs under QEMU onubuntu-latestwith a 120-minute timeout. Thecreate-manifestjob usesalways()with explicit success checks so that a riscv64 failure never blocks releases.Tag fix: The existing amd64 job was generating
-arm64suffix tags and vice versa. Each job now only generates its own arch suffix.Test plan
node openclaw.mjs --versionon QEMU or hardwareSummary by CodeRabbit