Repository navigation
Conversation
There was a problem hiding this comment.
🟡 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.
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.
bba0672 to
bf7d21b
Compare
bf7d21b to
7ac6986
Compare
| 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) |
There was a problem hiding this comment.
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.
| 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) |
There was a problem hiding this comment.
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.
7ac6986 to
068b52b
Compare
068b52b to
8e14def
Compare
8e14def to
90ad128
Compare
90ad128 to
21d896f
Compare
21d896f to
a379cb6
Compare
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.
a379cb6 to
bd7a5e1
Compare
wwbmmm
left a comment
There was a problem hiding this comment.
[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) |
There was a problem hiding this comment.
[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.
bd7a5e1 to
637249f
Compare



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:
owning modules and reuse those targets for the legacy bundles and tests.
updating CMake, Make and Bazel rules without changing public import names.
BRPC_BUILD_COMPONENT_LIBSis OFF by default. Enabling it builds all sixcomponents alongside the existing bundle;
BUILD_SHARED_LIBScontrols thestandalone libraries' linkage.
brpc::butil,brpc::bvar,brpc::bthread,brpc::json2pb,brpc::mcpack2pbandbrpc::brpcwith their transitive dependencies.The RPC-only library is named
libbrpc-rpcto avoid conflicting with theexisting
libbrpcbundle.build/install interfaces instead of staging copies in
output/include.the RPC component. Preserve
import "idl_options.proto"and#include "idl_options.pb.h".complete RPC client/server exchange using an ephemeral server port.
exercise static and shared mock packages in existing all-options CI jobs.
searching for the removed output/include staging directory.
independent vcpkg consumer example. The documented
componentsfeature isprovided 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 runcmake --install build --prefix build/output. Do not link the bundle andstandalone components into the same application because they share symbols.
Check List: