Skip to content

Use bazel-build protoc and plugins to regenerate protos for some languages.#22703

Merged
jtattermusch merged 8 commits intogrpc:masterfrom
jtattermusch:bazel_binaries_for_proto_regen
Apr 20, 2020
Merged

Use bazel-build protoc and plugins to regenerate protos for some languages.#22703
jtattermusch merged 8 commits intogrpc:masterfrom
jtattermusch:bazel_binaries_for_proto_regen

Conversation

@jtattermusch
Copy link
Copy Markdown
Contributor

@jtattermusch jtattermusch commented Apr 17, 2020

  • with bazel protoc and grpc_*_plugin are much easier to build
  • we want to use make as little as possible

This PR also regenerates the out-of-date generated files.

@jtattermusch jtattermusch added lang/all wrapped languages release notes: no Indicates if PR should not be in release notes labels Apr 17, 2020
muxi
muxi reviewed Apr 17, 2020
View reviewed changes
src/objective-c/tests/run_plugin_tests.sh Outdated
BAZEL_EXEC_ROOT=$ROOT_DIR/bazel-out/darwin-fastbuild/bin
PROTOC=$BAZEL_EXEC_ROOT/external/com_google_protobuf/protoc
PLUGIN=$BAZEL_EXEC_ROOT/src/compiler/grpc_objective_c_plugin
PROTOC=bazel-bin/external/com_google_protobuf/protoc
Copy link
Copy Markdown
Contributor

@muxi muxi Apr 17, 2020

Choose a reason for hiding this comment

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

You probably still need $ROOT_DIR here

Copy link
Copy Markdown
Contributor Author

@jtattermusch jtattermusch Apr 17, 2020

Choose a reason for hiding this comment

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

Good catch! Fixed.

muxi
muxi approved these changes Apr 17, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@muxi muxi left a comment

Choose a reason for hiding this comment

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

objc script LGTM

@jtattermusch
Copy link
Copy Markdown
Contributor Author

jtattermusch commented Apr 18, 2020 •
edited
Loading

There's a potential problem with php:
https://source.cloud.google.com/results/invocations/f92bf93c-a053-4ec5-b831-a25b51d5cae6/targets/github%2Fgrpc%2Finterop_test/tests;group=tests;test=cloud_to_cloud:php7:ruby_server:large_unary:tls;row=232

Update:
PHP interop test build actually runs the generate script here:

grpc/tools/dockerfile/interoptest/grpc_interop_php/build_interop.sh

Line 48 in 7cb9d43

./bin/generate_proto_php.sh
(and it's not clear why).

Update2:

It's odd that build_docker_php passes, while the generate_proto_php.sh has failed.
Explanation: Looks like all the generate_proto_*.sh files use "set +e", which can actually eat errors.

@jtattermusch jtattermusch force-pushed the bazel_binaries_for_proto_regen branch from abf0f4b to 5156db5 Compare April 18, 2020 17:04
@jtattermusch
Copy link
Copy Markdown
Contributor Author

jtattermusch commented Apr 20, 2020

Known failures: #21839

@jtattermusch jtattermusch merged commit d512a18 into grpc:master Apr 20, 2020
@jtattermusch
Copy link
Copy Markdown
Contributor Author

jtattermusch commented Apr 20, 2020

I might try to do some followup for php.

@jtattermusch jtattermusch mentioned this pull request Apr 20, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@apolcyn apolcyn apolcyn approved these changes

@veblush veblush Awaiting requested review from veblush

@stanley-cheung stanley-cheung Awaiting requested review from stanley-cheung

+1 more reviewer

@muxi muxi muxi approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

lang/all wrapped languages release notes: no Indicates if PR should not be in release notes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jtattermusch @muxi @apolcyn