You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brpc.apache.org by GitBox <gi...@apache.org> on 2021/12/29 08:23:45 UTC
[GitHub] [incubator-brpc] hcoona opened a new pull request #1657: Chore: rework Bazel build system
hcoona opened a new pull request #1657:
URL: https://github.com/apache/incubator-brpc/pull/1657
Totally rewrite the bazel build support.
1. Support latest version of bazel.
2. Build all external dependencies (except for mesalink) with bazel.
3. Support Protobuf 3.19.1 (breaking changes here: https://github.com/protocolbuffers/protobuf/commit/624d29d83306f3ce2c7e092dd44a891e04215e67)
4. Support generate `compile_commands.json` for bazel project.
5. Rework the options flags
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] chenzhangyi commented on pull request #1657: Chore: rework Bazel build system
Posted by GitBox <gi...@apache.org>.
chenzhangyi commented on pull request #1657:
URL: https://github.com/apache/incubator-brpc/pull/1657#issuecomment-1002476725
Nice job!
Only one question: Will the changes break the compilation of users with old versions of bazel, e.g. 0.25
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] hcoona commented on pull request #1657: Chore: rework Bazel build system
Posted by GitBox <gi...@apache.org>.
hcoona commented on pull request #1657:
URL: https://github.com/apache/incubator-brpc/pull/1657#issuecomment-1017533604
@wwbmmm Sure, split the protobuf support to https://github.com/apache/incubator-brpc/pull/1679
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] hcoona commented on pull request #1657: Chore: rework Bazel build system
Posted by GitBox <gi...@apache.org>.
hcoona commented on pull request #1657:
URL: https://github.com/apache/incubator-brpc/pull/1657#issuecomment-1021887882
@chenzhangyi @lorinlee ping~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] wwbmmm commented on pull request #1657: Chore: rework Bazel build system
Posted by GitBox <gi...@apache.org>.
wwbmmm commented on pull request #1657:
URL: https://github.com/apache/incubator-brpc/pull/1657#issuecomment-1008517610
> @wwbmmm PTAL
LGTM
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] mapx commented on pull request #1657: Chore: rework Bazel build system
Posted by GitBox <gi...@apache.org>.
mapx commented on pull request #1657:
URL: https://github.com/apache/incubator-brpc/pull/1657#issuecomment-1051726870
Would you help merge it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] hcoona edited a comment on pull request #1657: Chore: rework Bazel build system
Posted by GitBox <gi...@apache.org>.
hcoona edited a comment on pull request #1657:
URL: https://github.com/apache/incubator-brpc/pull/1657#issuecomment-1002528872
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] hcoona edited a comment on pull request #1657: Chore: rework Bazel build system
Posted by GitBox <gi...@apache.org>.
hcoona edited a comment on pull request #1657:
URL: https://github.com/apache/incubator-brpc/pull/1657#issuecomment-1017641502
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] wwbmmm commented on pull request #1657: Chore: rework Bazel build system
Posted by GitBox <gi...@apache.org>.
wwbmmm commented on pull request #1657:
URL: https://github.com/apache/incubator-brpc/pull/1657#issuecomment-1014155643
@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？
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] hcoona commented on pull request #1657: Chore: rework Bazel build system
Posted by GitBox <gi...@apache.org>.
hcoona commented on pull request #1657:
URL: https://github.com/apache/incubator-brpc/pull/1657#issuecomment-1002528872
@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).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] hcoona commented on a change in pull request #1657: Chore: rework Bazel build system
Posted by GitBox <gi...@apache.org>.
hcoona commented on a change in pull request #1657:
URL: https://github.com/apache/incubator-brpc/pull/1657#discussion_r776259689
##########
File path: src/brpc/protocol.cpp
##########
@@ -203,7 +203,7 @@ BUTIL_FORCE_INLINE bool ParsePbFromZeroCopyStreamInlined(
// According to source code of pb, SetTotalBytesLimit is not a simple set,
// avoid calling the function when the limit is definitely unreached.
if (PB_TOTAL_BYETS_LIMITS < FLAGS_max_body_size) {
- decoder.SetTotalBytesLimit(INT_MAX, -1);
Review comment:
fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] hcoona commented on pull request #1657: Chore: rework Bazel build system
Posted by GitBox <gi...@apache.org>.
hcoona commented on pull request #1657:
URL: https://github.com/apache/incubator-brpc/pull/1657#issuecomment-1002529690
The protobuf arena changes is still breaking. I'll fix it with `GOOGLE_PROTOBUF_VERSION`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] hcoona edited a comment on pull request #1657: Chore: rework Bazel build system
Posted by GitBox <gi...@apache.org>.
hcoona edited a comment on pull request #1657:
URL: https://github.com/apache/incubator-brpc/pull/1657#issuecomment-1002608196
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] hcoona commented on pull request #1657: Chore: rework Bazel build system
Posted by GitBox <gi...@apache.org>.
hcoona commented on pull request #1657:
URL: https://github.com/apache/incubator-brpc/pull/1657#issuecomment-1017641502
Do my best to make it compiles with bazel 0.28.0 (0.27.1 failed). See `example/build_with_old_bazel`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] lorinlee commented on pull request #1657: Chore: rework Bazel build system
Posted by GitBox <gi...@apache.org>.
lorinlee commented on pull request #1657:
URL: https://github.com/apache/incubator-brpc/pull/1657#issuecomment-1012803307
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] hcoona commented on pull request #1657: Chore: rework Bazel build system
Posted by GitBox <gi...@apache.org>.
hcoona commented on pull request #1657:
URL: https://github.com/apache/incubator-brpc/pull/1657#issuecomment-1011693324
Is there something else to make it merged?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] hcoona edited a comment on pull request #1657: Chore: rework Bazel build system
Posted by GitBox <gi...@apache.org>.
hcoona edited a comment on pull request #1657:
URL: https://github.com/apache/incubator-brpc/pull/1657#issuecomment-1017641502
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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] hcoona commented on pull request #1657: Chore: rework Bazel build system
Posted by GitBox <gi...@apache.org>.
hcoona commented on pull request #1657:
URL: https://github.com/apache/incubator-brpc/pull/1657#issuecomment-1057626592
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] wwbmmm commented on a change in pull request #1657: Chore: rework Bazel build system
Posted by GitBox <gi...@apache.org>.
wwbmmm commented on a change in pull request #1657:
URL: https://github.com/apache/incubator-brpc/pull/1657#discussion_r776231259
##########
File path: src/brpc/protocol.cpp
##########
@@ -203,7 +203,7 @@ BUTIL_FORCE_INLINE bool ParsePbFromZeroCopyStreamInlined(
// According to source code of pb, SetTotalBytesLimit is not a simple set,
// avoid calling the function when the limit is definitely unreached.
if (PB_TOTAL_BYETS_LIMITS < FLAGS_max_body_size) {
- decoder.SetTotalBytesLimit(INT_MAX, -1);
Review comment:
You can check protobuf version and keep compatibility with old version protobuf
#if GOOGLE_PROTOBUF_VERSION >= 3006000
decoder.SetTotalBytesLimit(INT_MAX);
#else
decoder.SetTotalBytesLimit(INT_MAX, -1);
#endif
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] hcoona edited a comment on pull request #1657: Chore: rework Bazel build system
Posted by GitBox <gi...@apache.org>.
hcoona edited a comment on pull request #1657:
URL: https://github.com/apache/incubator-brpc/pull/1657#issuecomment-1002529690
The protobuf arena changes are still breaking. I'll fix it with `GOOGLE_PROTOBUF_VERSION`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] hcoona edited a comment on pull request #1657: Chore: rework Bazel build system
Posted by GitBox <gi...@apache.org>.
hcoona edited a comment on pull request #1657:
URL: https://github.com/apache/incubator-brpc/pull/1657#issuecomment-1002608196
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] hcoona commented on pull request #1657: Chore: rework Bazel build system
Posted by GitBox <gi...@apache.org>.
hcoona commented on pull request #1657:
URL: https://github.com/apache/incubator-brpc/pull/1657#issuecomment-1002608196
Fixed protobuf compatibility & separate it into another commit.
The failing unit test seems to be unrelated with this change. The only failing unittest is
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] hcoona commented on pull request #1657: Chore: rework Bazel build system
Posted by GitBox <gi...@apache.org>.
hcoona commented on pull request #1657:
URL: https://github.com/apache/incubator-brpc/pull/1657#issuecomment-1017537329
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] hcoona commented on pull request #1657: Chore: rework Bazel build system
Posted by GitBox <gi...@apache.org>.
hcoona commented on pull request #1657:
URL: https://github.com/apache/incubator-brpc/pull/1657#issuecomment-1008293071
@wwbmmm PTAL
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org