-
Notifications
You must be signed in to change notification settings - Fork 10.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Migrate bazel proto building rules to stackb/rules_proto #18317
Conversation
Thank you for your pull request. Before we can look at your contribution, we need to ensure all contributors are covered by a Contributor License Agreement. After the following items are addressed, please respond with a new comment here, and the automated system will re-verify.
Regards, |
Associated my github account with @thelinuxfoundation |
I think these are new test failures from this PR: https://source.cloud.google.com/results/invocations/023cddaf-ff1f-4fbe-ace3-720645c3dacb/targets/grpc%2Fcore%2Fpull_request%2Flinux%2Fgrpc_python_bazel_test/log
|
Working on this. Having trouble wrangling the Python tests. |
The remaining problem seems to be associated with namespace shadowing related to the code generated using the python_proto_library/python_grpc_library rules: $ bazel build src/proto/grpc/testing:py_empty_proto $ bazel build src/proto/grpc/reflection/v1alpha:py_reflection_proto $ bazel test src/python/grpcio_tests/tests/reflection:all Looking at the python_grpc_library rule:
So PYTHONPATH is being augmented and one shadows the other. I believe this is a known python2 issue related to the init.py files (which bazel generates automatically). |
@jdwestbro I'm not convinced that migrating over to https://github.com/stackb/rules_proto is what we want - what are the benefits? Also this PR is somewhat misleadingly called "fixed bazel build", but it seems to do a lot more that that. |
This issue/PR has been automatically marked as stale because it has not had any update (including commits, comments, labels, milestones, etc) for 180 days. It will be closed automatically if no further update occurs in 1 day. Thank you for your contributions! |
CC @gnossen in case he wants to reopen. |
We ended up going in a different direction. I think this issue is rightly closed. |
This patch migrates the Bazel build to use https://github.com/stackb/rules_proto
This fixes #18256