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
253 stars 160 forks source link

Fix quoting of descriptor_set_in for protoc on windows. #221

Closed kruno-exponent closed 1 year ago

kruno-exponent commented 2 years ago

There is a bug with protoc on windows and dotnet.

build_protoc_args appends in --descriptor_set_in multiple descriptors. On windows separator is ";". Combining that with bash that means that without proper escaping second descriptor is being interpreted as command and bash tries to execute the descriptor itself!

I don't know why ctx.actions.run_shell doesn't escape the arguments, but it doesn't. Sorry, I don't have the time to add proper test for this since it's 1 AM. If you can create a patch release for this, that would be great :)

STR

load("@rules_proto//proto:defs.bzl", "proto_library")
load("@rules_proto_grpc//csharp:defs.bzl", "csharp_grpc_library")
load("@io_bazel_rules_dotnet//dotnet:defs.bzl", "csharp_library")

package(default_visibility = ["//visibility:public"])

proto_library(
  name = "xx_protos",
  srcs = glob(["**/*.proto"]),
  deps = ["@com_google_protobuf//:any_proto"] # <<-- The second descriptor added here.
)

csharp_grpc_library(
  name = "xx.dll",
  protos = [":xx_protos"],
  srcs = glob(["**/*.cs"])
)
# blaze build --host_platform=@io_bazel_rules_dotnet//dotnet/toolchain:darwin_arm64_6.0.101 --platforms=@io_bazel_rules_dotnet//dotnet/toolchain:darwin_arm64_6.0.101  //:xx.dll
aaliddell commented 1 year ago

An alternate similar fix was merged from #228. Are you able to test if that has fixed your issue, since that fix is substantially simpler and allows keeping the Args object?

aaliddell commented 1 year ago

Closing as per above comment about alternate fix. Thanks for the PR nonetheless