Refactor Bazel build with custom proto rules and build all test targets#3313
Open
chenBright wants to merge 1 commit into
Open
Refactor Bazel build with custom proto rules and build all test targets#3313chenBright wants to merge 1 commit into
chenBright wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the Bazel build by introducing a custom brpc_proto_library macro (decoupled from the protobuf version) and reorganizing test/BUILD.bazel to compile and run all test targets via a generated cc_test per source file. It also threads a number of supporting changes (asan-aware tcmalloc selection, runfiles helper for cert files, CI workflow updates, libunwind overlay bump) and small unrelated source-level fixes in endpoint.cpp, percentile.h, symbolize.cc, task_tracer.cpp, and several unit tests.
Changes:
- New
bazel/tools/brpc_proto_library.bzl+proto_gen.bzlcustom rules; root/example/testBUILD files migrated offproto_library/cc_proto_library. - Test build rewritten:
generate_unittestsmacro,root_runfilesrule for cert/jsonout data, ASAN-aware tcmalloc dep, expanded.bazelrc(incl.--config=test,--strip=never, asan config), CI now runsbazel test //test/.... - Misc source/test fixes: symbolize PT_LOAD-based base address, epoll-to-poll modifier masking, percentile bucket pre-sizing, task_tracer formatting, and several test stability tweaks.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| bazel/tools/brpc_proto_library.bzl | New macro wrapping brpc_proto_gen + cc_library. |
| bazel/tools/proto_gen.bzl | New Starlark rule that runs protoc directly via ProtoInfo. |
| bazel/tools/BUILD.bazel | Marks bazel/tools as a package exporting the .bzl files. |
| BUILD.bazel | Switches to brpc_proto_library, splits COPTS/DEFINES, drops protoc_lib from :butil. |
| example/BUILD.bazel | Migrates example proto targets to brpc_proto_library. |
| test/BUILD.bazel | Uses brpc_proto_library, generate_unittests, root_runfiles, ASAN tcmalloc select. |
| test/generate_unittests.bzl | Helper that fans out per-source cc_tests + test_suite. |
| test/root_runfiles.bzl | Custom rule to symlink data files at the runfiles workspace root. |
| test/brpc_alpn_protocol_unittest.cpp | LOG(NOTICE)→LOG(INFO), EXPECT_EQ→ASSERT_EQ in SetUp. |
| test/brpc_http_rpc_protocol_unittest.cpp | Removes GOOGLE_CHECK_NOTNULL, uses TextFormat::PrintToString. |
| test/brpc_protobuf_json_unittest.cpp | Loosens recursion error message assertion. |
| test/brpc_streaming_rpc_unittest.cpp | Replaces fixed sleep with poll loop on _expected_next_value. |
| test/bthread_mutex_unittest.cpp | Removes bogus errno check; fixes thread function passed to pthread_create. |
| src/butil/endpoint.cpp | Masks out epoll-only flags before CHECK_EQ in epoll_to_poll_events. |
| src/butil/third_party/symbolize/symbolize.cc | Computes ELF base via PT_LOAD headers from /proc/self/mem. |
| src/bvar/detail/percentile.h | Pre-sizes concurrent sampler buckets in constructor. |
| src/bthread/task_tracer.cpp | Header reorder; fixed-width hex pointer formatting. |
| CMakeLists.txt | Updates absl dependency list (adds log_globals/random_*, drops optional). |
| cmake/CMakeLists.download_gtest.in | Bumps cmake_minimum_required to 2.8.12. |
| docs/cn/bthread_tracer.md | Removes backticks from section headings. |
| MODULE.bazel | Switches libunwind to brpc-no-unwind variant; adds gperftools dev_dep; adds brpc registry. |
| registry/modules/libunwind/1.8.3.brpc-no-unwind/source.json | Updates MODULE.bazel integrity hash. |
| .bazelrc | Adds JVM args, brpc registry, --strip=never, :test config, asan configs. |
| .github/workflows/ci-linux.yml | Replaces compile-only jobs with bazel test //test/...; adds protobuf-34 job. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
|
LGTM |
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
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.
What problem does this PR solve?
Issue Number: resolve #2904 , resolve #2850
Problem Summary:
Proto version coupling: It directly loads
@rules_proto//proto:defs.bzland usesproto_library/cc_proto_library, which creates tight coupling to specific protobuf/Bazel version combinations. Different protobuf versions (3.x vs 27.x+) expose incompatibleProtoInfoprovider fields, causing build failures when upgrading either Bazel or protobuf.Minimal test coverage: The old
test/BUILD.bazelonly defined a handful of hardcodedcc_testtargets (brpc_prometheus_test,brpc_auto_concurrency_limiter_test,brpc_redis_cluster_test), leaving the vast majority of brpc unit tests unbuilt and untested under Bazel.Duplicate
mainsymbol: Mostbrpc_*_unittest.cppfiles define their ownint main(). A naiveglob(["brpc_*_unittest.cpp"])into a singlecc_testwould link all sources together, causing duplicatemainsymbol errors and preventing any tests from building.Failed UT: Some UTs fail when using bazel.
What is changed and the side effects?
Changed:
Side effects:
Performance effects:
Breaking backward compatibility:
Check List: