Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
✅ Files skipped from review due to trivial changes (7)
Walkthrough引入多架构跨编译支持：新增交叉编译工具链与脚本，扩展 Docker 多阶段镜像（含双 sysroot 的 Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(52,160,255,0.5)
participant CI as CI/CD
end
rect rgba(102,204,102,0.5)
participant Builder as Buildx
participant Registry as Registry
end
rect rgba(255,153,51,0.5)
participant Verifier as SmokeTest
end
CI->>Builder: 构建 rmcs-base (amd64/arm64)
Builder->>Registry: 推送并导出各架构 digest
Registry-->>CI: 返回 digest 文件
CI->>Builder: 使用 digest 构建 rmcs-develop / rmcs-runtime（按架构）
Builder->>Registry: 推送并导出 digest
CI->>Builder: 使用两端 sysroot digest 构建 rmcs-develop-full
Builder->>Registry: 推送并导出 digest
CI->>Verifier: 拉取按 digest 的镜像，运行交叉烟雾测试（build-rmcs-cross、readelf 校验）
Verifier-->>CI: 验证结果
CI->>Builder: 创建并推送 multi-arch manifest 列表 (:latest)
Builder->>Registry: 发布 manifest
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 分钟 Possibly related PRs
Suggested reviewers
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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.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: 1
🧹 Nitpick comments (3)
rmcs_ws/toolchain.cmake (1)
138-142: 考虑是否需要使用FORCE覆盖CMAKE_PREFIX_PATH。使用
FORCE会无条件覆盖用户可能通过命令行传入的CMAKE_PREFIX_PATH值。由于build-rmcs-cross已经通过环境变量设置了CMAKE_PREFIX_PATH（见 context snippet 1），这里的硬编码路径可能与环境变量中的设置产生冲突。建议：仅在变量未定义时设置默认值，或者移除
FORCE以允许用户覆盖。♻️ 可选的修改方案
-set(CMAKE_PREFIX_PATH - "/opt/ros/jazzy" - "/usr" - CACHE STRING "Target-side prefix path for cross-compilation" FORCE -) +if(NOT DEFINED CACHE{CMAKE_PREFIX_PATH}) + set(CMAKE_PREFIX_PATH + "/opt/ros/jazzy" + "/usr" + CACHE STRING "Target-side prefix path for cross-compilation" + ) +endif()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rmcs_ws/toolchain.cmake` around lines 138 - 142, The current set(CMAKE_PREFIX_PATH ... CACHE STRING ... FORCE) unconditionally overwrites any CMAKE_PREFIX_PATH a user or build script provided; change it so the toolchain only supplies defaults and does not clobber user input: either remove the FORCE flag or wrap the cache assignment in a conditional that only sets CMAKE_PREFIX_PATH when it is not already defined/has no value (i.e., ensure the code that sets CMAKE_PREFIX_PATH in toolchain.cmake only provides defaults and allows overrides rather than forcing them)..devcontainer/devcontainer.json (1)
5-5: 默认切换到latest-full镜像可能影响不需要交叉编译的开发者。
latest-full镜像包含对向架构的 sysroot 和交叉工具链，体积会显著增大。对于不需要交叉编译功能的开发者，这会增加镜像拉取时间和磁盘占用。建议：
- 在文档中说明两种镜像的区别和适用场景
- 或者保留
latest作为默认，让需要交叉编译的开发者手动切换到latest-full🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/devcontainer.json at line 5, Change the default devcontainer image from "qzhhhi/rmcs-develop:latest-full" to the lighter "qzhhhi/rmcs-develop:latest" by updating the "image" property, and add documentation (e.g., README or CONTRIBUTING) explaining the difference between the two tags and when to switch to "latest-full" for cross-compilation, including a short example or command showing how to override the "image" property to opt into the full image..script/build-rmcs-cross (1)
153-165:set -u模式下空数组展开可能导致问题当
colcon_args数组为空时，在 Bash 4.3 及更早版本中，"${colcon_args[@]}"会触发 "unbound variable" 错误（因为脚本启用了set -u）。虽然 Bash 4.4+ 已修复此行为，但为了更好的兼容性，建议使用防御性写法。🛡️ 建议的防御性写法
CLICOLOR_FORCE=1 NINJA_STATUS="" \ colcon \ --log-base "${log_base}" \ build \ --merge-install \ --build-base "${build_base}" \ --install-base "${install_base}" \ - "${colcon_args[@]}" \ + ${colcon_args[@]+"${colcon_args[@]}"} \ --cmake-args \ -DCMAKE_TOOLCHAIN_FILE="${toolchain_file}" \ -DRMCS_TARGET_ARCH="${RMCS_TARGET_ARCH}" \ -DRMCS_SYSROOT="${RMCS_SYSROOT}" \ -DRMCS_TARGET_TRIPLET="${RMCS_TARGET_TRIPLET}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.script/build-rmcs-cross around lines 153 - 165, The use of "${colcon_args[@]}" can trigger an "unbound variable" under set -u when the array is unset (Bash ≤4.3); update the expansion in the colcon invocation to a defensive form that yields an empty expansion when the array is not set (e.g., use a default-empty expansion like "${colcon_args[@]:-}" or equivalent) so the CLICOLOR_FORCE... colcon build command (where "${colcon_args[@]}" is referenced) will not fail on older Bash versions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/zh-cn/cross_build.md`:
- Around line 66-72: The self-check commands mismatch the build scope: the
script invocation ./.script/build-rmcs-cross --packages-up-to rmcs_core only
builds rmcs_core (and deps) but the checklist also calls readelf on
librmcs_executor.so; either remove the executor check or expand the build
target—update the snippet so the build command uses --packages-up-to
rmcs_executor if you intend to validate librmcs_executor.so, or delete the
readelf call for librmcs_executor.so if you only want to build/check rmcs_core;
refer to the ./.script/build-rmcs-cross invocation and the readelf checks for
librmcs_core.so and librmcs_executor.so when making the change.
---
Nitpick comments:
In @.devcontainer/devcontainer.json:
- Line 5: Change the default devcontainer image from
"qzhhhi/rmcs-develop:latest-full" to the lighter "qzhhhi/rmcs-develop:latest" by
updating the "image" property, and add documentation (e.g., README or
CONTRIBUTING) explaining the difference between the two tags and when to switch
to "latest-full" for cross-compilation, including a short example or command
showing how to override the "image" property to opt into the full image.
In @.script/build-rmcs-cross:
- Around line 153-165: The use of "${colcon_args[@]}" can trigger an "unbound
variable" under set -u when the array is unset (Bash ≤4.3); update the expansion
in the colcon invocation to a defensive form that yields an empty expansion when
the array is not set (e.g., use a default-empty expansion like
"${colcon_args[@]:-}" or equivalent) so the CLICOLOR_FORCE... colcon build
command (where "${colcon_args[@]}" is referenced) will not fail on older Bash
versions.
In `@rmcs_ws/toolchain.cmake`:
- Around line 138-142: The current set(CMAKE_PREFIX_PATH ... CACHE STRING ...
FORCE) unconditionally overwrites any CMAKE_PREFIX_PATH a user or build script
provided; change it so the toolchain only supplies defaults and does not clobber
user input: either remove the FORCE flag or wrap the cache assignment in a
conditional that only sets CMAKE_PREFIX_PATH when it is not already defined/has
no value (i.e., ensure the code that sets CMAKE_PREFIX_PATH in toolchain.cmake
only provides defaults and allows overrides rather than forcing them).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8a837972-2ea2-4403-a013-024884282c37
📒 Files selected for processing (15)
.devcontainer/devcontainer.json.github/workflows/update-image.yml.gitignore.gitmodules.script/build-rmcs-cross.script/complete/_build-rmcs-cross.script/host/rmcsDockerfileREADME.mddocs/zh-cn/build_docker_image.mddocs/zh-cn/cross_build.mdrmcs_ws/src/hikcamerarmcs_ws/src/rmcs_core/src/hardware/mecanum_hero.cpprmcs_ws/src/serialrmcs_ws/toolchain.cmake
💤 Files with no reviewable changes (4)
- rmcs_ws/src/hikcamera
- rmcs_ws/src/serial
- rmcs_ws/src/rmcs_core/src/hardware/mecanum_hero.cpp
- .gitmodules
Remove the local hikcamera/serial submodules and the unused mecanum_hero.cpp source file to shrink the workspace and speed up cross-builds.
qzhhhi
added a commit
that referenced
this pull request
Apr 28, 2026
- Add rmcs-develop:latest-full with opposite-architecture sysroots, cross toolchains, and a build-rmcs-cross entrypoint for local cross compilation. - Update the image publishing workflow to build per-arch digests, smoke test cross builds, and promote multi-arch manifests for base, develop, develop-full, and runtime images.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Dockerfile (1)
139-146:⚠️ Potential issue | 🔴 Critical不要把私钥烘焙进开发镜像。
Line 141-143 会把
.ssh/id_rsa复制进/home/ubuntu/.ssh，而rmcs-develop/rmcs-develop-full又会被推送到镜像仓库。这样任何能拉镜像的人都能拿到同一把私钥，等于直接泄露凭据。这里应改成运行时挂载或启动时生成，而不是在构建期写进镜像层。🔐 建议修改
-RUN --mount=type=bind,target=/tmp/.ssh,source=.ssh,readonly=false \ - cd /home/ubuntu && mkdir -p .ssh && \ - if [ ! -f "/tmp/.ssh/id_rsa" ]; then ssh-keygen -N "" -f "/tmp/.ssh/id_rsa"; fi && \ - cp -r /tmp/.ssh/* .ssh && \ - chown -R 1000:1000 .ssh && chmod 600 .ssh/id_rsa && \ - mkdir -p .unison && \ +RUN cd /home/ubuntu && \ + mkdir -p .ssh .unison && \ + chown -R 1000:1000 .ssh .unison && \ echo 'confirmbigdel = false' >> ".unison/default.prf" && \ chown -R 1000:1000 .unison🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 139 - 146, The Dockerfile RUN block is baking a private key into the image by generating/copying .ssh/id_rsa into /home/ubuntu/.ssh (the lines using ssh-keygen, cp -r /tmp/.ssh/* .ssh and chmod 600 .ssh/id_rsa); remove those steps so private keys are not written into the build layer and instead expect keys to be provided at runtime via a mounted volume or create them at container start in an entrypoint script. Update the Dockerfile to omit the ssh-keygen/cp/chmod for .ssh/id_rsa and keep only non-sensitive setup (mkdir -p .ssh, chown -R 1000:1000 .ssh if needed), and add/modify the container entrypoint or startup logic (e.g., an entrypoint script referenced by the image) that checks for a mounted /home/ubuntu/.ssh or generates keys on first run, sets secure permissions (chmod 600) and chown there; this ensures images like rmcs-develop / rmcs-develop-full do not contain baked private keys.
🧹 Nitpick comments (1)
.github/workflows/update-image.yml (1)
267-267: 优先改用${CROSS_TRIPLET}-readelf做 ELF 校验。Line 267 现在依赖未显式保证存在的通用
readelf。rmcs-develop-full在DockerfileLine 205-209 只明确安装了交叉binutils-*，这里直接调用带 triplet 的工具会更稳，避免把镜像里的宿主binutils差异误判成 smoke test 失败。🛠️ 建议修改
- readelf -h "${lib_path}" | grep -q "Machine:.*${EXPECTED_MACHINE}" + "${CROSS_TRIPLET}-readelf" -h "${lib_path}" | grep -q "Machine:.*${EXPECTED_MACHINE}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/update-image.yml at line 267, Replace the plain readelf invocation with the triplet-prefixed tool: use "${CROSS_TRIPLET}-readelf -h \"${lib_path}\" | grep -q \"Machine:.*${EXPECTED_MACHINE}\"" so the workflow calls the cross-compiled binutils binary explicitly; if desired, add a pre-check that ${CROSS_TRIPLET}-readelf exists and fail fast with a clear message if not present to avoid falling back to a host readelf.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Dockerfile`:
- Around line 193-244: The rmcs-develop-full stage installs toolchains and
sysroots but does not include the entry script .script/build-rmcs-cross or put
it on PATH; add a step in the rmcs-develop-full stage to COPY (or --mount) the
repository script .script/build-rmcs-cross into the image (e.g.
/usr/local/bin/build-rmcs-cross), make it executable (chmod +x), and ensure its
directory is on PATH (or symlink into /usr/local/bin) so the build-rmcs-cross
command is available at runtime and for users pulling latest-full.
In `@rmcs_ws/toolchain.cmake`:
- Around line 138-156: 在交叉编译的搜索路径中缺少 /usr/local 及其 pkgconfig 子目录：在
toolchain.cmake 中将 "/usr/local" 加入 CMAKE_PREFIX_PATH，并在 _rmcs_pkg_config_libdirs
列表中加入对应的
"${RMCS_SYSROOT}/usr/local/lib/${RMCS_TARGET_TRIPLET}/pkgconfig"、"${RMCS_SYSROOT}/usr/local/lib/pkgconfig"
和 "${RMCS_SYSROOT}/usr/local/share/pkgconfig"（以确保 ENV{PKG_CONFIG_LIBDIR}
包含这些项），同时同步更新构建脚本 build-rmcs-cross 中设置的相关环境变量以反映这些新增路径（注意使用相同的 RMCS_SYSROOT/
RMCS_TARGET_TRIPLET 变量形式）。
---
Outside diff comments:
In `@Dockerfile`:
- Around line 139-146: The Dockerfile RUN block is baking a private key into the
image by generating/copying .ssh/id_rsa into /home/ubuntu/.ssh (the lines using
ssh-keygen, cp -r /tmp/.ssh/* .ssh and chmod 600 .ssh/id_rsa); remove those
steps so private keys are not written into the build layer and instead expect
keys to be provided at runtime via a mounted volume or create them at container
start in an entrypoint script. Update the Dockerfile to omit the
ssh-keygen/cp/chmod for .ssh/id_rsa and keep only non-sensitive setup (mkdir -p
.ssh, chown -R 1000:1000 .ssh if needed), and add/modify the container
entrypoint or startup logic (e.g., an entrypoint script referenced by the image)
that checks for a mounted /home/ubuntu/.ssh or generates keys on first run, sets
secure permissions (chmod 600) and chown there; this ensures images like
rmcs-develop / rmcs-develop-full do not contain baked private keys.
---
Nitpick comments:
In @.github/workflows/update-image.yml:
- Line 267: Replace the plain readelf invocation with the triplet-prefixed tool:
use "${CROSS_TRIPLET}-readelf -h \"${lib_path}\" | grep -q
\"Machine:.*${EXPECTED_MACHINE}\"" so the workflow calls the cross-compiled
binutils binary explicitly; if desired, add a pre-check that
${CROSS_TRIPLET}-readelf exists and fail fast with a clear message if not
present to avoid falling back to a host readelf.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1d8a8276-3430-4b4b-8872-f68eb76e53f0
📒 Files selected for processing (15)
.devcontainer/devcontainer.json.github/workflows/update-image.yml.gitignore.gitmodules.script/build-rmcs-cross.script/complete/_build-rmcs-cross.script/host/rmcsDockerfileREADME.mddocs/zh-cn/build_docker_image.mddocs/zh-cn/cross_build.mdrmcs_ws/src/hikcamerarmcs_ws/src/rmcs_core/src/hardware/mecanum_hero.cpprmcs_ws/src/serialrmcs_ws/toolchain.cmake
💤 Files with no reviewable changes (4)
- rmcs_ws/src/hikcamera
- rmcs_ws/src/serial
- .gitmodules
- rmcs_ws/src/rmcs_core/src/hardware/mecanum_hero.cpp
✅ Files skipped from review due to trivial changes (6)
- .gitignore
- .script/complete/_build-rmcs-cross
- .devcontainer/devcontainer.json
- README.md
- .script/host/rmcs
- docs/zh-cn/cross_build.md
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/zh-cn/build_docker_image.md
- Add rmcs-develop:latest-full with opposite-architecture sysroots, cross toolchains, and a build-rmcs-cross entrypoint for local cross compilation. - Update the image publishing workflow to build per-arch digests, smoke test cross builds, and promote multi-arch manifests for base, develop, develop-full, and runtime images.
ZGZ713912
pushed a commit
that referenced
this pull request
May 2, 2026
- Add rmcs-develop:latest-full with opposite-architecture sysroots, cross toolchains, and a build-rmcs-cross entrypoint for local cross compilation. - Update the image publishing workflow to build per-arch digests, smoke test cross builds, and promote multi-arch manifests for base, develop, develop-full, and runtime images. (cherry picked from commit 23d3469)
This was referenced May 3, 2026
Closed
This was referenced May 10, 2026
noskillzheng
pushed a commit
that referenced
this pull request
May 11, 2026
- Add rmcs-develop:latest-full with opposite-architecture sysroots, cross toolchains, and a build-rmcs-cross entrypoint for local cross compilation. - Update the image publishing workflow to build per-arch digests, smoke test cross builds, and promote multi-arch manifests for base, develop, develop-full, and runtime images.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR 摘要：多架构交叉构建开发镜像（feat(cross): Add multi-arch cross-build development image）
核心功能
Dockerfile 与镜像构建
CI/CD / 工作流
文档与配置
破坏性/重要更改
变更规模与审查建议