-
Notifications
You must be signed in to change notification settings - Fork 9
Move to grpc-swift-2 #49
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
Conversation
Motivation: To support incremental migration, v2 has moved to the 'grpc-swift-2' package. Modifications: - Update dependencies - Update generated code - Re-baseline availability Result: Easier to migrate from gRPC Swift 1 to 2
@@ -36,10 +36,10 @@ protoc_generate_grpc_swift="$bin_path/protoc-gen-grpc-swift" | |||
# - $4 onwards: options to forward to the plugin | |||
function generate_grpc { | |||
local proto=$1 | |||
local args=("--plugin=$protoc_generate_grpc_swift" "--proto_path=${2}" "--grpc-swift_out=${3}") | |||
local args=("--plugin=$protoc_generate_grpc_swift" "--proto_path=${2}" "--grpc-swift-2_out=${3}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a real change or a find/replace error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't see a matching change in grpc-swift-protobuf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a real change. The _out
and _opt
options to protoc plugins are derived from the name of the protoc plugin, so protoc-gen-grpc-swift-2
has --grpc-swift-2_out
and --grpc-swift-2-_opt
.
The matching change in grpc-swift-protobuf is here: https://github.com/grpc/grpc-swift-protobuf/blob/df605cde2957657a078eb754ed24f3a0e05d3f54/dev/protos/generate.sh#L29-L42
IIRC you can actually alter the name by doing --plugin=foo=/path/to/protoc-gen-grpc-swift-2
which would allow you to use --foo_opt
/--foo_out
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see. That's unfortunate. I think we may have missed some other changes in grpc-swift-protobuf then
https://github.com/grpc/grpc-swift-protobuf/blob/df605cde2957657a078eb754ed24f3a0e05d3f54/Plugins/PluginsShared/PluginUtils.swift#L95
https://github.com/grpc/grpc-swift-protobuf/blob/df605cde2957657a078eb754ed24f3a0e05d3f54/Sources/GRPCProtobuf/Documentation.docc/Articles/Generating-stubs.md?plain=1#L104
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see. That's unfortunate. I think we may have missed some other changes in grpc-swift-protobuf then https://github.com/grpc/grpc-swift-protobuf/blob/df605cde2957657a078eb754ed24f3a0e05d3f54/Plugins/PluginsShared/PluginUtils.swift#L95
This one's okay, it uses the --plugin=name=...
trick I mentioned above (also the CI would've caught the failure)
Blast. Will fix this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
for option in "${@:4}"; do | ||
args+=("--grpc-swift_opt=$option") | ||
args+=("--grpc-swift-2_opt=$option") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
Motivation:
To support incremental migration, v2 has moved to the 'grpc-swift-2' package.
Modifications:
Result:
Easier to migrate from gRPC Swift 1 to 2