brpc
brpc copied to clipboard
apache
Chore: rework Bazel build system
Totally rewrite the bazel build support.
-
Support latest version of bazel.
-
Build all external dependencies (except for mesalink) with bazel.
-
Support Protobuf 3.19.1 (breaking changes here: https://github.com/protocolbuffers/protobuf/commit/624d29d83306f3ce2c7e092dd44a891e04215e67)
-
Support generate
compile_commands.jsonfor bazel project. -
Rework the options flags & move all of them into
//bazel/config
Leave building with mesalink for future. I think maybe all dependencies should link against mesalink instead of openssl.
Nice job!
Only one question: Will the changes break the compilation of users with old versions of bazel, e.g. 0.25
@chenzhangyi In short it would break building with bazel 0.25. Many dependencies relying on a higher version of bazel. I didn't verify each of them. However, rules_foreign_cc 0.7.0 requires bazel v4.0.0+ (See https://github.com/bazelbuild/rules_foreign_cc/blob/0.7.0/README.md). We build several external dependencies (openssl, libevent, thrift, ...) with it.
The protobuf arena changes are still breaking. I'll fix it with GOOGLE_PROTOBUF_VERSION.
Fixed protobuf compatibility & separate it into another commit.
The failing unit test seems to be unrelated with this change. The only failing unittest is
Got it. After installed curl, it passed.
Hi @hcoona , Thank you so much for your contribution! But I have a little concern, some projects may depend on bazel 0.2x, like TensorFlow 1.x, and we have seen some teams using bRPC in TensorFlow. So, maybe we should consider the compatibility issues. cc @chenzhangyi @wwbmmm
@hcoona Can you extract the changes related to “Support Protobuf 3.19.1” into a separate PR so that this part can be merged faster？
@wwbmmm Sure, split the protobuf support to https://github.com/apache/incubator-brpc/pull/1679
@lorinlee @chenzhangyi In bazel philosophy, the final project owner need to take responsibility for all of its dependencies, AKA. specify how to download & build them in WORKSPACE file. For this change, there's nothing special for the BUILD files, so it should works fine building a project depends on brpc with an earlier version of Bazel if the project owner could provide necessary indirect dependencies for bRPC. I'll verify it offline, and ping you guys later few days.
Do my best to make it compiles with bazel 0.28.0 (0.27.1 failed). See example/build_with_old_bazel. @lorinlee Is this fine enough?
It means that if a user used bRPC in their project, it can compile with bazel 0.28.0 and higher.
@chenzhangyi Would you help to take a look at it? The user project depending on bRPC would still compile if they are using bazel v0.28.0 and higher. example/build_with_old_bazel verified it.
@hcoona Hi, Could you upload the newest version if anything has changed since last time?
@zyearn @wwbmmm please review this pull request and check whether it can be merged.
@zyearn @wwbmmm please review this pull request and check whether it can be merged.
LGTM @zyearn please see if there is any problem to merge it.
It's a great contribution, and could you please share a related blog or doc for that pr? I think it will be a good learning material and we will post it to our official website, please consider this, thanks a lot.
@zyearn PTAL. I removed bazel/third_party/glog/0001-improvement-copts.patch & rename bazel/third_party/glog/0002-mark-override.patch to a more meaningful name 0001-mark-override-resolve-warning.patch. If you think the example of building with old version of bazel for user is not necessary, I can remove it.
The CI failure cannot be reproduced on my dev environment with GCC 9.4.0 & bazel 4.2.2 by running CC=gcc CXX=g++ PURPOSE=compile-with-bazel-all-options sh build_in_travis_ci.sh. I looked the source code, it shouldn't be such error. Please help to rerun the failed jobs if possible.
@Huixxi I didn't realize there was any special work. Please look the Bazel official document & some existing repo driven by Bazel. Please raise to me if there was truly something special with details, I can help to find some document about them.
If you think the example of building with old version of bazel for user is not necessary, I can remove it.
I think the example of building with old version of bazel is useful. No need to remove it.