search

rules-proto-grpc / rules_proto_grpc

Bazel rules for building Protobuf and gRPC code and libraries from proto_library targets
https://rules-proto-grpc.com
Apache License 2.0
256 stars 159 forks source link

Fix grpclib_compile not working with paths with dashes #320

Closed chrisirhc closed 4 months ago

chrisirhc commented 5 months ago

There was a missing |python conversion for grpclib. Modified the default thing example to demonstrate the failure in https://github.com/rules-proto-grpc/rules_proto_grpc/pull/320/commits/8685ee8671b261fe6fa3f236d6b80f25b3ddbbfb . However, I do see that a bunch of tests depend on the examples.proto path so I reverted it in my PR. Would like some opinions on whether I should make eamples.proto path contain a dash or leave the test change out of this PR. I'm concerned that this regression would occur again in the future.

I verified in https://github.com/vmagamedov/grpclib/blob/5916cba69fc1f8a25fe8e749e3c4427897b3d228/grpclib/plugin/main.py#L174 and https://github.com/vmagamedov/grpclib/blob/5916cba69fc1f8a25fe8e749e3c4427897b3d228/grpclib/plugin/main.py#L251 that |python is appropriate.

Fixes #321

aaliddell commented 4 months ago

Excellent thanks. Regarding testing, there is a test workspace specifically for Python with dashes in paths, just grpclib was missed there: https://github.com/rules-proto-grpc/rules_proto_grpc/tree/master/test_workspaces/python_dashes

I'll update that test workspace to encompass grpclib too