Skip to content

Add optional standalone CMake component libraries - #3596

Open
wasphin wants to merge 4 commits into
apache:masterfrom
wasphin:build-cmake-component-libraries
Open

wasphin wants to merge 4 commits into
apache:masterfrom
wasphin:build-cmake-component-libraries

Conversation

@wasphin

@wasphin wasphin commented Oct 10, 2026 •

Copy link
Copy Markdown
Member

What problem does this PR solve?

Issue Number: N/A

Problem Summary:

CMake currently packages the production modules into the brpc bundle. Applications
that only need butil, bvar, bthread, json2pb or mcpack2pb cannot consume an installed
component target independently, which also prevents packaging these modules for
standalone use through vcpkg.

What is changed and the side effects?

Changed:

Merge preference: Please use Rebase and merge rather than Squash and merge
to preserve the four focused commits and the step-by-step migration history.

The four commits separate directory ownership, target ownership, component
packaging and mcpack IDL ownership so each stage can be reviewed independently:

  1. Move production source definitions into their module directories.
  2. Define module-owned object targets, scope feature/dependency handling to the
    owning modules and reuse those targets for the legacy bundles and tests.
  3. Add optional component libraries and exported CMake package targets.
  4. Make mcpack2pb own the IDL option schema, generated public header and symbols,
    updating CMake, Make and Bazel rules without changing public import names.
  • BRPC_BUILD_COMPONENT_LIBS is OFF by default. Enabling it builds all six
    components alongside the existing bundle; BUILD_SHARED_LIBS controls the
    standalone libraries' linkage.
  • Export brpc::butil, brpc::bvar, brpc::bthread, brpc::json2pb,
    brpc::mcpack2pb and brpc::brpc with their transitive dependencies.
    The RPC-only library is named libbrpc-rpc to avoid conflicting with the
    existing libbrpc bundle.
  • Install headers directly from module source/build directories and use target
    build/install interfaces instead of staging copies in output/include.
  • Keep generated IDL definitions in mcpack2pb and obtain them transitively from
    the RPC component. Preserve import "idl_options.proto" and
    #include "idl_options.pb.h".
  • Do not distribute producer-side custom Find modules in the installed package.
  • Add six standalone consumer checks, including custom-option consumption and a
    complete RPC client/server exchange using an ephemeral server port.
  • Export UMDK headers for URMA mock consumers without requiring liburma;
    exercise static and shared mock packages in existing all-options CI jobs.
  • Configure CMake examples through an explicit installation prefix instead of
    searching for the removed output/include staging directory.
  • Update both English and Chinese getting-started documentation, including an
    independent vcpkg consumer example. The documented components feature is
    provided by an overlay port, not the current registry port.

Side effects:

  • Performance effects: Production objects are compiled once and reused by the
    bundle and standalone libraries. Enabling components adds library linking and
    packaging work; it does not duplicate production source compilation.

  • Breaking backward compatibility: Existing bundle names and linkage remain
    unchanged, and standalone components are opt-in. CMake builds no longer stage
    headers in output/include; users relying on that directory should run
    cmake --install build --prefix build/output. Do not link the bundle and
    standalone components into the same application because they share symbols.


Check List:

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Shared component objects lack required PIC settings, URMA mock packages omit public headers, and consumer checks are not automated.

3 open findings
What changed in this PR

Adds opt-in standalone CMake libraries for bRPC modules while preserving the legacy bundle.

Changes:

  • Introduces six exported component targets with transitive dependencies.
  • Moves CMake ownership into module directories and relocates mcpack IDL ownership.
  • Adds consumer checks, packaging support, and updated documentation.
File Description
CMakeLists.txt Adds component option and delegates module builds.
Makefile Updates mcpack IDL generation and installation.
BUILD.bazel Relocates the mcpack IDL schema.
cmake/​bRPCComponents.cmake Defines component and export helpers.
cmake/​bRPCHeaders.cmake Installs module-owned headers.
cmake/​brpc-config.cmake.in Configures component discovery and dependencies.
cmake/​CompileProto.cmake Removes staged protobuf-header copies.
cmake/​SetupGtest.cmake Uses module-relative template lookup.
src/​CMakeLists.txt Aggregates module objects into legacy bundles.
src/​brpc/​CMakeLists.txt Defines RPC objects, dependencies, and component.
src/​bthread/​CMakeLists.txt Defines the bthread component.
src/​butil/​CMakeLists.txt Defines butil sources and dependencies.
src/​butil/​config.h.in Adds generated butil feature configuration.
src/​bvar/​CMakeLists.txt Defines the bvar component.
src/​json2pb/​CMakeLists.txt Defines the json2pb component.
src/​mcpack2pb/​CMakeLists.txt Defines mcpack2pb and IDL targets.
src/​mcpack2pb/​idl_options.proto Moves the public IDL option schema.
test/​CMakeLists.txt Adapts tests to module-owned targets.
test/​Makefile Adds the relocated schema import path.
test/​cmake_components/​CMakeLists.txt Defines standalone consumer checks.
test/​cmake_components/​consumer.cpp Exercises all component targets.
test/​cmake_components/​echo.proto Supplies the RPC consumer schema.
test/​cmake_components/​mcpack.proto Exercises custom mcpack options.
tools/​rpc_press/​CMakeLists.txt Uses directory-local source discovery.
tools/​rpc_replay/​CMakeLists.txt Uses directory-local source discovery.
example/​cmake/​BrpcExample.cmake Makes example discovery directory-relative.
example/​dynamic_partition_echo_c++/​CMakeLists.txt Fixes local resource copying.
example/​http_c++/​CMakeLists.txt Fixes certificate copying.
example/​multi_threaded_echo_c++/​CMakeLists.txt Fixes certificate copying.
example/​partition_echo_c++/​CMakeLists.txt Fixes server-list copying.
example/​ubring_performance/​CMakeLists.txt Makes output discovery directory-relative.
docs/​en/​getting_started.md Documents components and vcpkg consumption.
docs/​cn/​getting_started.md Adds equivalent Chinese documentation.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/brpc/CMakeLists.txt
Comment thread src/mcpack2pb/CMakeLists.txt
Comment thread test/cmake_components/CMakeLists.txt
Keep source selection and bundled artifacts unchanged while moving each
module source list and RPC protocol generation out of the root project.
Retain the existing test source inputs during this first migration step.
@wasphin
wasphin force-pushed the build-cmake-component-libraries branch from bba0672 to bf7d21b Compare October 10, 2026 16:12
@wasphin
wasphin requested a balanced review from Copilot October 10, 2026 16:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Optional-feature and macOS component package paths lack consumer-level CI coverage.

2 open findings
2 resolved since last review

🧠 Review effort: Balanced

Comment thread .github/workflows/ci-linux.yml Outdated
@wasphin
wasphin force-pushed the build-cmake-component-libraries branch from bf7d21b to 7ac6986 Compare October 11, 2026 03:44
@wasphin
wasphin requested a balanced review from Copilot October 11, 2026 03:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The Zvbc CMake scoping omits several RVV callers and implementations, disabling paths and potentially leaving missing symbols.

2 open findings
2 resolved since last review

🧠 Review effort: Balanced

Comment thread src/butil/CMakeLists.txt Outdated
Comment on lines +95 to +99
if(WITH_RISCV_ZVBC)
list(APPEND butil_crc32c_options -march=rv64gcv_zbc_zvbc)
# Preserve the existing RVV base64 implementation as well.
set_property(SOURCE ${CMAKE_CURRENT_SOURCE_DIR}/third_party/modp_b64/modp_b64_rvv.cc
APPEND PROPERTY COMPILE_OPTIONS -march=rv64gcv_zbc_zvbc)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The fix restores the previous target-wide ISA setting for GNU RISC-V builds. With WITH_RISCV_ZVBC=ON, brpc_common_config supplies -march=rv64gcv_zbc_zvbc to the internal module build configurations, covering the CRC32C and base64 implementations as well as modp_b64.cc, iobuf.cpp, string_piece.cc, and string_compare_rvv.cc. This setting is not conditional on the build type.

I checked the generated compile commands for the Zvbc, Zbc-only, and baseline configurations, including the test debug-object targets. This was configure-only validation, not a RISC-V compile or runtime test. I am leaving this thread open pending a real GNU RISC-V build with WITH_RISCV_ZVBC=ON, including Debug/test objects, followed by the affected tests.

Comment thread test/CMakeLists.txt Outdated
Comment on lines +216 to +219
if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND
CMAKE_SYSTEM_PROCESSOR MATCHES "riscv64" AND WITH_RISCV_ZVBC)
set_property(SOURCE ${CMAKE_CURRENT_SOURCE_DIR}/../src/butil/third_party/modp_b64/modp_b64_rvv.cc
APPEND PROPERTY COMPILE_OPTIONS -march=rv64gcv_zbc_zvbc)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The test debug-object targets now inherit the same ISA options as production through brpc_test_config -> brpc_build_config -> the module configurations -> brpc_common_config. The partial per-source RISC-V assignments have been removed, so BUTIL_DEBUG_LIB gets the architecture setting for the complete source list, including the RVV callers and implementations.

Generated compile-command checks confirmed the option reaches these debug objects. However, I have not performed a real RISC-V build, and those checks were not a completed Debug build. I am leaving this thread open; the remaining validation is an actual GNU RISC-V Debug build with WITH_RISCV_ZVBC=ON and the affected tests.

@wasphin
wasphin force-pushed the build-cmake-component-libraries branch from 7ac6986 to 068b52b Compare October 11, 2026 04:10
@wasphin
wasphin requested a balanced review from Copilot October 11, 2026 04:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The build-tree mcpack2pb export does not expose the relocated public IDL schema.

3 open findings

🧠 Review effort: Balanced

Comment thread src/mcpack2pb/CMakeLists.txt
@wasphin
wasphin force-pushed the build-cmake-component-libraries branch from 068b52b to 8e14def Compare October 11, 2026 04:31
@wasphin
wasphin requested a balanced review from Copilot October 11, 2026 04:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The generated butil configuration header is shared through the source tree, allowing different build directories to overwrite each other’s configuration.

3 open findings
1 resolved since last review

🧠 Review effort: Balanced

Comment thread src/butil/CMakeLists.txt Outdated
@wasphin
wasphin force-pushed the build-cmake-component-libraries branch from 8e14def to 90ad128 Compare October 11, 2026 05:31
@wasphin
wasphin requested a balanced review from Copilot October 11, 2026 05:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@wasphin
wasphin force-pushed the build-cmake-component-libraries branch from 90ad128 to 21d896f Compare October 11, 2026 06:03
@chenBright
chenBright requested a lite review from Copilot October 11, 2026 13:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Cross-build-system packaging needs human validation, and URMA header exports and example builds remain unresolved.

4 open findings
1 resolved since last review

🧠 Review effort: Lite

Comment thread src/brpc/CMakeLists.txt Outdated
Comment thread example/cmake/BrpcExample.cmake Outdated
@wasphin
wasphin force-pushed the build-cmake-component-libraries branch from 21d896f to a379cb6 Compare October 11, 2026 15:20
Let each module own its production object library and move the mcpack
protoc generator into its module. Aggregate the module objects into the
existing brpc bundle without compiling production sources twice.

Move butil configuration-header generation, feature-option handling and
dependency discovery into the owning modules. Keep RPC-only headers out
of lower-library compile settings and assemble legacy bundle dependencies
after module setup.

Apply CRC32C instruction flags only to the relevant sources, including
debug builds. Preserve the RISC-V vector base64 options and the existing
AArch64 libcontext workaround.

Query module targets for the existing unit-test source sets instead of
passing source lists through parent directory scopes. Preserve the test
compile definitions and bundled debug library linkage.

Install source and generated headers directly from their owning modules.
Keep build-tree include paths separate from installed target interfaces
instead of copying headers into an output staging tree.

Use Protobuf's imported targets to preserve static transitive dependencies
and Debug/Release library selection, and handle config-mode import paths.
@wasphin
wasphin force-pushed the build-cmake-component-libraries branch from a379cb6 to bd7a5e1 Compare October 11, 2026 15:32

@wwbmmm wwbmmm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[replied by brpc-oncall robot] The latest revision resolves the examples and URMA findings and looks functionally sound; the remaining concern is the hand-maintained optional-dependency discovery in the installed config, which will keep drifting from the module CMakeLists.

endif()
endif()

foreach(_brpc_dep glog mesalink unwind unwind_x86_64 thrift rdma snappy ub)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[replied by brpc-oncall robot] brpc-config.cmake.in still hand-reimplements discovery for gflags/leveldb/OpenSSL/BoringSSL/glog/thrift/rdma/snappy/urma/unwind with bespoke find_library/find_path blocks, duplicating the logic that now lives in the per-module CMakeLists (src/butil, src/brpc, src/bthread). Each optional dependency must be kept in sync across the module files, this template and the pkg-config strings, and the URMA mock/real split shows how quickly this grows. Consider deriving the exported dependency list from the component targets, or at least documenting the sync points next to these loops, so a new WITH_* feature cannot silently be handled in only some of them.

Let each module define its standalone library when
BRPC_BUILD_COMPONENT_LIBS is enabled. Reuse production objects with the
unchanged bundle and follow BUILD_SHARED_LIBS for component linkage.

Expose six brpc:: component targets through build-tree and relocatable
install-tree packages. Name the RPC-only artifact libbrpc-rpc so it can
link lower components without embedding their objects or using the bundle.

Add standalone consumer checks for all six libraries, including a complete
client/server RPC exchange and selective, repeated component imports.
Export URMA headers in mock mode without requiring liburma, and exercise
that dependency closure in existing static and shared CI configurations.

Discover consumer dependencies without distributing the producer's Find
modules. Use an explicit installation prefix for CMake examples rather
than searching for a removed build-tree header staging directory.
Document standalone CMake/vcpkg consumption in both language guides,
including the required overlay for unpublished component support.
Generate IDL option objects in the mcpack module and include them in its
standalone library. Keep one copy in each legacy bundle and debug bundle;
the RPC-only component obtains the definitions through mcpack2pb.

Declare idl_options.pb.h as a public header before configuring installation
and install the schema alongside it. Prefer generated build-tree headers
and add explicit generation dependencies for RPC, the generator and tests.

Move the source schema into src/mcpack2pb and update CMake, Make and Bazel
rules while retaining the public idl_options.proto import and header names.

Check standalone custom-option consumption without producer include paths
and exercise the IDL-enabled service through the client/server smoke test.
@wasphin
wasphin force-pushed the build-cmake-component-libraries branch from bd7a5e1 to 637249f Compare October 12, 2026 05:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants