Skip to content

fix(host): Export correct symbols for shared library builds - #29

Merged
qzhhhi merged 4 commits into
mainfrom
dev/fix-shared-lib
Mar 4, 2026
Merged

fix(host): Export correct symbols for shared library builds#29
qzhhhi merged 4 commits into
mainfrom
dev/fix-shared-lib

Conversation

@qzhhhi

@qzhhhiqzhhhi commented Mar 3, 2026

Copy link
Copy Markdown
Member

The shared library was built with -fvisibility=hidden but had no export annotations, resulting in zero exported symbols. Introduce LIBRMCS_API visibility macro for Handler and PacketBuilder, enabling correct symbol export in shared library builds. Default build is changed to static; release Dockerfile explicitly opts into shared.

  • Add host/include/librmcs/export.hpp with platform-aware export macro
  • Mark Handler and PacketBuilder with LIBRMCS_API
  • Add -fvisibility-inlines-hidden for GCC/Clang
  • Change BUILD_STATIC_LIBRMCS default to ON
  • Set -DBUILD_STATIC_LIBRMCS=OFF in Dockerfile.build_sdk for release

修复共享库构建的符号导出

该 PR 通过引入平台感知的导出宏并调整构建配置,修复了在开启 -fvisibility=hidden 时共享库没有导出任何符号的问题。

主要变更

新增导出机制(host/include/librmcs/export.hpp)

  • 增加 LIBRMCS_API 宏以控制符号导出/导入。
  • MSVC:构建库时使用 __declspec(dllexport),使用库时使用 __declspec(dllimport);静态链接/不适用时为空。
  • GCC/Clang:构建库时使用 attribute((visibility("default")));否则为空。
  • 为不支持的编译器提供空定义回退。

标注公共接口(host/include/librmcs/protocol/handler.hpp)

  • 为 librmcs::host::protocol::Handler 及其嵌套类 PacketBuilder 添加 LIBRMCS_API,确保这些类型在共享库构建时被正确导出。
  • 添加对 librmcs/export.hpp 的包含。

编译器选项与构建模式调整(host/CMakeLists.txt)

  • 保留 -fvisibility=hidden,并在非 MSVC 路径下为 C++ 增加 -fvisibility-inlines-hidden(更严格控制内联符号可见性)。
  • 将 CMake 选项 BUILD_STATIC_LIBRMCS 的默认值改为 ON,默认生成静态库;需要构建共享库时需显式将其置为 OFF。

Docker 构建配置(Dockerfile.build_sdk)

  • 简化/合并构建流程;在 release 构建流程中通过 -DBUILD_STATIC_LIBRMCS=OFF 明确选择共享库构建并生成 DEB 包(构建产物移动与清理流程也做了相应整理)。

影响

  • 修复了因 -fvisibility=hidden 导致共享库无导出符号的问题,使需要导出的类型在共享库构建下正确导出。
  • 默认改为静态构建以保持兼容性;若需要共享库则在 CI/Docker或本地构建命令中显式设置 BUILD_STATIC_LIBRMCS=OFF。

The shared library was built with -fvisibility=hidden but had no export annotations, resulting in zero exported symbols.
Introduce LIBRMCS_API visibility macro for Handler and PacketBuilder, enabling correct symbol export in shared library builds.
Default build is changed to static; release Dockerfile explicitly opts into shared.
- Add host/include/librmcs/export.hpp with platform-aware export macro
- Mark Handler and PacketBuilder with LIBRMCS_API
- Add -fvisibility-inlines-hidden for GCC/Clang
- Change BUILD_STATIC_LIBRMCS default to ON
- Set -DBUILD_STATIC_LIBRMCS=OFF in Dockerfile.build_sdk for release
@coderabbitai

coderabbitaiBot commented Mar 3, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3bda9f1 and 5751d07.

📒 Files selected for processing (4)
  • Dockerfile.build_sdk
  • host/CMakeLists.txt
  • host/include/librmcs/export.hpp
  • host/include/librmcs/protocol/handler.hpp

Walkthrough

更改构建流程:将 CMake 中 BUILD_STATIC_LIBRMCS 默认改为 ON,但在 Docker 构建中显式传入 -DBUILD_STATIC_LIBRMCS=OFF;新增跨平台导出宏 LIBRMCS_API 并在 Handler 与其嵌套类上使用该宏以控制符号可见性。

Changes

Cohort / File(s)Summary
构建配置
Dockerfile.build_sdk, host/CMakeLists.txt
将 CMake 选项 BUILD_STATIC_LIBRMCS 默认值改为 ON;在 Docker 构建命令中用 -DBUILD_STATIC_LIBRMCS=OFF 覆盖;为非 MSVC 路径添加 -fvisibility-inlines-hidden 编译选项。
API 导出与头改动
host/include/librmcs/export.hpp, host/include/librmcs/protocol/handler.hpp
新增 LIBRMCS_API 宏以实现 MSVC/GCC/Clang 的导出/导入/可见性控制;在 handler.hpp 中包含该头并将 Handler 及其嵌套 PacketBuilder 标记为可导出。

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

诗歌

🐰 我在构建草地上轻跳,开关一翻风声笑,
符号披上小外衣,跨编译器也不怕,
打包与发布忙不迭,库和头文件握手跑。

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ Passed标题准确总结了PR的主要变更——为共享库构建添加正确的符号导出,直接反映了解决共享库缺少导出注解的核心问题。

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch dev/fix-shared-lib

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
host/CMakeLists.txt (1)

37-37: 建议把 -fvisibility-inlines-hidden 限定到 C++ 编译单元,提高代码一致性。

虽然当前项目中没有 C 源文件,但 -fvisibility-inlines-hidden 是 C++ 专用选项。为了与 firmware/c_board/CMakeLists.txt 中的做法保持一致,建议用 COMPILE_LANGUAGE:CXX 条件分离,便于未来维护。

🔧 参考修复
- add_compile_options(-g -Wall -Wextra -Wpedantic -fvisibility=hidden -fvisibility-inlines-hidden)+ add_compile_options(-g -Wall -Wextra -Wpedantic -fvisibility=hidden)+ add_compile_options($<$<COMPILE_LANGUAGE:CXX>:-fvisibility-inlines-hidden>)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@host/CMakeLists.txt` at line 37, The add_compile_options invocation adds the
C++-only flag -fvisibility-inlines-hidden globally; restrict this flag to C++
compilations by using the generator expression form so only CXX files get
-fvisibility-inlines-hidden (leave the common flags like -g -Wall -Wextra
-Wpedantic -fvisibility=hidden applied to all languages). Update the
add_compile_options call (the one using add_compile_options with -g -Wall
-Wextra -Wpedantic -fvisibility=hidden -fvisibility-inlines-hidden) to apply
-fvisibility-inlines-hidden with a $<COMPILE_LANGUAGE:CXX> generator expression
so the option is only used for C++ translation units.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@host/include/librmcs/export.hpp`:
- Around line 3-18: The preprocessor macro block defining LIBRMCS_API has
indentation/formatting that violates clang-format rules and causes CI lint
failures; run clang-format (or apply the project's formatting rules) to reflow
the conditional blocks around LIBRMCS_API so `#if/`#elif/#else/#endif and the
inner lines have consistent indentation and spacing, ensure the GCC/Clang and
MSVC branches and the final else are formatted uniformly, remove any trailing
whitespace, and commit the formatted export.hpp changes.
---
Nitpick comments:
In `@host/CMakeLists.txt`:
- Line 37: The add_compile_options invocation adds the C++-only flag
-fvisibility-inlines-hidden globally; restrict this flag to C++ compilations by
using the generator expression form so only CXX files get
-fvisibility-inlines-hidden (leave the common flags like -g -Wall -Wextra
-Wpedantic -fvisibility=hidden applied to all languages). Update the
add_compile_options call (the one using add_compile_options with -g -Wall
-Wextra -Wpedantic -fvisibility=hidden -fvisibility-inlines-hidden) to apply
-fvisibility-inlines-hidden with a $<COMPILE_LANGUAGE:CXX> generator expression
so the option is only used for C++ translation units.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3bda9f1 and 58abf2e.

📒 Files selected for processing (4)
  • Dockerfile.build_sdk
  • host/CMakeLists.txt
  • host/include/librmcs/export.hpp
  • host/include/librmcs/protocol/handler.hpp

Comment threadhost/include/librmcs/export.hpp Outdated
@qzhhhi

Copy link
Copy Markdown
MemberAuthor

@coderabbitai full review

@coderabbitai

Copy link
Copy Markdown
✅ Actions performed

Full review triggered.

@qzhhhi
qzhhhi merged commit 63101d0 into mainMar 4, 2026
5 checks passed
@github-project-automationgithub-project-automationBot moved this from Todo to Done in RMCS Slave SDKMar 4, 2026
@qzhhhi
qzhhhi deleted the dev/fix-shared-lib branch March 4, 2026 09:37
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant

@qzhhhi