upgrade to latest rules_proto 7.x with legacy protoc#3
Conversation
79b4987 to
707c92d
Compare
| @@ -0,0 +1,70 @@ | |||
| #!/usr/bin/env python3 | |||
There was a problem hiding this comment.
Done. Moved the filtering logic into main.py and removed protoc_wrapper.py. Commit: 5012485
| filtered_argv = [sys.argv[0]] + filter_unsupported_flags(sys.argv[1:]) | ||
|
|
||
| # Replace sys.argv with filtered arguments | ||
| sys.argv = filtered_argv |
There was a problem hiding this comment.
Check whether protoc version is below 32 before performing the filter.
There was a problem hiding this comment.
Done. Added get_protoc_version() function that detects the protoc version. The filter only runs if the version is below 32. If the version is 32 or above, or cannot be determined, all flags are passed through unchanged. Commit: 5012485
thirdparty/grpcio_tools/main.py
Outdated
| # Try to get version by running protoc --version | ||
| # We need to temporarily capture the output | ||
| result = subprocess.run( | ||
| [sys.executable, '-m', 'grpc_tools.protoc', '--version'], |
There was a problem hiding this comment.
Don't do a process call. Use the modules' version instead, from its package metadata.
There was a problem hiding this comment.
Done. Changed to use package metadata via importlib.metadata (with pkg_resources fallback). The function now gets the protobuf version from package metadata and converts it to protoc version (e.g., protobuf 5.28.3 → protoc 28.3). No subprocess calls. Commit: e63816a
|
@copilot rebase on main and merge conflicts then check bazel build output and see if there are any errors. |
I checked the repository state and attempted to build with bazel. Findings: Rebase Status:
Build Check:
The code changes are ready and syntactically correct. The bazel build would need to be tested in an environment with network access to releases.bazel.build. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Co-authored-by: chrisirhc <132584+chrisirhc@users.noreply.github.com>
Co-authored-by: chrisirhc <132584+chrisirhc@users.noreply.github.com>
…o 7.1.0 Co-authored-by: chrisirhc <132584+chrisirhc@users.noreply.github.com>
Co-authored-by: chrisirhc <132584+chrisirhc@users.noreply.github.com>
Co-authored-by: chrisirhc <132584+chrisirhc@users.noreply.github.com>
Co-authored-by: chrisirhc <132584+chrisirhc@users.noreply.github.com>
159bc94 to
ddee3d5
Compare
thirdparty/grpcio_tools/main.py
Outdated
| # Try importlib.metadata first (Python 3.8+) | ||
| try: | ||
| from importlib.metadata import version | ||
| protobuf_version = version('protobuf') |
There was a problem hiding this comment.
Instead of using protobuf package version, it should get the protoc version from the grpc_version in the grpcio package. Source https://github.com/grpc/grpc/blob/master/tools/distrib/python/grpcio_tools/grpc_version.py
Add a unit test for checking this behavior.
There was a problem hiding this comment.
Done. Changed to use grpc_version.VERSION from the grpcio_tools package instead of protobuf package metadata. Added comprehensive unit tests in main_test.py with 14 test cases covering version parsing and filtering behavior. All tests pass. Commit: a016ade
thirdparty/grpcio_tools/main.py
Outdated
| # Convert protobuf version to protoc version | ||
| # protobuf 5.x.y -> protoc 2x.y (e.g., 5.28.3 -> 28.3) | ||
| # protobuf 32.x.y -> protoc 32.x.y (future versions) | ||
| if major == 5: |
There was a problem hiding this comment.
Rather than checking the major number, discard the first number if there are 3 components to the version. Because major can be 6 and we'll still use the 2nd and 3rd components.
There was a problem hiding this comment.
Done. Changed version parsing logic to discard the first number if there are 3 components to the version, regardless of what the major number is. For example:
- "5.28.3" → (28, 3, 0)
- "6.30.1" → (30, 1, 0)
- "28.3" → (28, 3, 0)
- "32.0" → (32, 0, 0)
Commit: a016ade
Co-authored-by: chrisirhc <132584+chrisirhc@users.noreply.github.com>
rules_proto 7.x emits
--option_dependenciesand--option_dependencies_violation_msgflags requiring protoc 32.0+. grpcio-tools bundles protoc ~28.x which errors on these flags.Changes
Enhanced
main.py: Added intelligent filtering logic that checks protoc version before filteringgrpc_version.VERSIONin grpcio_tools package--flag valueand--flag=valueformatsAdded comprehensive unit tests: Created
main_test.pywith 14 test casesUpgraded to latest versions:
Example
The wrapper intelligently transforms protoc invocations based on version:
This maintains the ~29X build speedup from prebuilt protoc while using latest Bazel rules, and automatically adapts when grpcio-tools is upgraded.
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.