ci: add riscv64 native build on RISE runners - #1
Conversation
Signed-off-by: Bruno Verachten <gounthar@gmail.com>
|
Note Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported. |
📝 WalkthroughWalkthroughA new GitHub Actions workflow file is added to enable automated building and testing of the mosquitto project on RISC-V64 architecture. The workflow executes on the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/alt-arch-riscv64.yml (1)
31-31: Consider a follow-up job with tests on riscv64 once dependencies are stable.Line 31 disables tests (
-DWITH_TESTS=OFF), which is fine for bootstrap, but this workflow currently only validates compilation + binary invocation. A latercteststage would improve regression coverage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/alt-arch-riscv64.yml at line 31, Current workflow disables tests via the cmake invocation flag "-DWITH_TESTS=OFF"; add a follow-up job (e.g., "riscv64-tests") that, once riscv64 dependencies are stable, builds with "-DWITH_TESTS=ON" (or toggles that flag in the new job) and runs "ctest" after the build to exercise the test suite; ensure the job runs on the same riscv64 runner and includes any extra dependency setup steps required before invoking the cmake/build/ctest sequence so tests can pass reliably.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/alt-arch-riscv64.yml:
- Line 16: The workflow currently sets the job-level YAML flag
continue-on-error: true which masks CI failures; remove that key (or set it to
false) from the job definition in .github/workflows/alt-arch-riscv64.yml so the
job will fail the workflow on errors, and if this job is flaky instead configure
a finer-grained strategy (e.g., a separate non-blocking workflow or retry logic)
rather than using continue-on-error.
- Around line 41-42: Add strict shell flags so pipeline failures aren't masked:
at the start of the run block that executes "./build/client/mosquitto_pub --help
| head -3" and "./build/client/mosquitto_sub --help | head -3", set "set -euo
pipefail" (or equivalent) so any failure in the commands
(mosquitto_pub/mosquitto_sub) causes the job to fail instead of the final pipe
stage (head) returning success.
---
Nitpick comments:
In @.github/workflows/alt-arch-riscv64.yml:
- Line 31: Current workflow disables tests via the cmake invocation flag
"-DWITH_TESTS=OFF"; add a follow-up job (e.g., "riscv64-tests") that, once
riscv64 dependencies are stable, builds with "-DWITH_TESTS=ON" (or toggles that
flag in the new job) and runs "ctest" after the build to exercise the test
suite; ensure the job runs on the same riscv64 runner and includes any extra
dependency setup steps required before invoking the cmake/build/ctest sequence
so tests can pass reliably.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e0e0f72d-9071-4600-b5f5-7b1324b2e539
📒 Files selected for processing (1)
.github/workflows/alt-arch-riscv64.yml
| build: | ||
| name: Build Mosquitto (riscv64) | ||
| runs-on: ubuntu-24.04-riscv | ||
| continue-on-error: true |
There was a problem hiding this comment.
Do not mask CI failures at job level.
Line 16 sets continue-on-error: true, which can report success even when build/verify fails. That weakens PR protection for this new architecture path.
🔧 Suggested fix
- continue-on-error: true📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| continue-on-error: true |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/alt-arch-riscv64.yml at line 16, The workflow currently
sets the job-level YAML flag continue-on-error: true which masks CI failures;
remove that key (or set it to false) from the job definition in
.github/workflows/alt-arch-riscv64.yml so the job will fail the workflow on
errors, and if this job is flaky instead configure a finer-grained strategy
(e.g., a separate non-blocking workflow or retry logic) rather than using
continue-on-error.
| ./build/client/mosquitto_pub --help 2>&1 | head -3 | ||
| ./build/client/mosquitto_sub --help 2>&1 | head -3 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/alt-arch-riscv64.yml | head -60Repository: gounthar/mosquitto
Length of output: 1384
🏁 Script executed:
sed -n '35,50p' .github/workflows/alt-arch-riscv64.ymlRepository: gounthar/mosquitto
Length of output: 349
Add set -euo pipefail to ensure verification failures are not masked by piping to head.
Lines 41–42 pipe command output to head, which will exit successfully even if the binary invocation fails. This masks verification failures since the pipeline exit code becomes that of head rather than the actual command. Add strict shell flags at the start of the run block:
Suggested fix
- name: Verify
run: |
+ set -euo pipefail
./build/src/mosquitto --version
./build/client/mosquitto_pub --help 2>&1 | head -3
./build/client/mosquitto_sub --help 2>&1 | head -3
file ./build/src/mosquitto🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/alt-arch-riscv64.yml around lines 41 - 42, Add strict
shell flags so pipeline failures aren't masked: at the start of the run block
that executes "./build/client/mosquitto_pub --help | head -3" and
"./build/client/mosquitto_sub --help | head -3", set "set -euo pipefail" (or
equivalent) so any failure in the commands (mosquitto_pub/mosquitto_sub) causes
the job to fail instead of the final pipe stage (head) returning success.
|
Closing this test PR. It successfully validated the riscv64 build on RISE native runners: The upstream PR with ECA-signed commit has been submitted: eclipse-mosquitto#3546 |
1 participant
Test PR to validate Mosquitto riscv64 build on RISE native runners.
Build proven on BananaPi F3 (SpacemiT K1, rv64gc):
Summary by CodeRabbit