From 2b220f8be7c80d4d51f9506f359608fce3b78ccb Mon Sep 17 00:00:00 2001 From: Makarand Dharmapurikar <makarandd@google.com> Date: Fri, 8 Jul 2016 10:47:57 -0700 Subject: [PATCH 1/5] fixes #7219 Modified objc plugin to add two types of import statements for well known protos. Deleted unnecessary empty.proto as we'll be using wellknown empty.proto Modified test to use the well known empty.proto. --- src/compiler/objective_c_plugin.cc | 20 ++++++++- src/objective-c/tests/InteropTests.m | 12 ++--- .../tests/RemoteTestClient/RemoteTest.podspec | 4 +- .../tests/RemoteTestClient/empty.proto | 44 ------------------- .../tests/RemoteTestClient/test.proto | 4 +- 5 files changed, 29 insertions(+), 55 deletions(-) delete mode 100644 src/objective-c/tests/RemoteTestClient/empty.proto diff --git a/src/compiler/objective_c_plugin.cc b/src/compiler/objective_c_plugin.cc index 3ccfd5b037c..43e4b3647d0 100644 --- a/src/compiler/objective_c_plugin.cc +++ b/src/compiler/objective_c_plugin.cc @@ -39,6 +39,8 @@ #include "src/compiler/objective_c_generator.h" #include "src/compiler/objective_c_generator_helpers.h" +#include <google/protobuf/compiler/objectivec/objectivec_helpers.h> + class ObjectiveCGrpcGenerator : public grpc::protobuf::compiler::CodeGenerator { public: ObjectiveCGrpcGenerator() {} @@ -72,7 +74,21 @@ class ObjectiveCGrpcGenerator : public grpc::protobuf::compiler::CodeGenerator { for (int i = 0; i < file->dependency_count(); i++) { ::grpc::string header = grpc_objective_c_generator::MessageHeaderName( file->dependency(i)); - proto_imports += ::grpc::string("#import \"") + header + "\"\n"; + const grpc::protobuf::FileDescriptor *dependency = file->dependency(i); + if (::google::protobuf::compiler::objectivec::IsProtobufLibraryBundledProtoFile(dependency)) { + ::grpc::string base_name = header; + grpc_generator::StripPrefix(&base_name, "google/protobuf/"); + proto_imports += + ::grpc::string("#if GPB_USE_PROTOBUF_FRAMEWORK_IMPORTS\n") + + ::grpc::string(" #import <") + + ::google::protobuf::compiler::objectivec::ProtobufLibraryFrameworkName + + ("/") + base_name + ">\n" + + ::grpc::string("#else\n") + + ::grpc::string(" #import \"") + header + "\"\n" + + ::grpc::string("#endif\n"); + } else { + proto_imports += ::grpc::string("#import \"") + header + "\"\n"; + } } ::grpc::string declarations; @@ -85,7 +101,7 @@ class ObjectiveCGrpcGenerator : public grpc::protobuf::compiler::CodeGenerator { static const ::grpc::string kNonNullEnd = "\nNS_ASSUME_NONNULL_END\n"; Write(context, file_name + ".pbrpc.h", - imports + '\n' + proto_imports + '\n' + kNonNullBegin + + imports + '\n' + proto_imports + '\n' + kNonNullBegin + declarations + kNonNullEnd); } diff --git a/src/objective-c/tests/InteropTests.m b/src/objective-c/tests/InteropTests.m index a503f020591..a2f63ac40ca 100644 --- a/src/objective-c/tests/InteropTests.m +++ b/src/objective-c/tests/InteropTests.m @@ -110,12 +110,12 @@ static cronet_engine *cronetEngine = NULL; XCTAssertNotNil(self.class.host); __weak XCTestExpectation *expectation = [self expectationWithDescription:@"EmptyUnary"]; - RMTEmpty *request = [RMTEmpty message]; + GPBEmpty *request = [GPBEmpty message]; - [_service emptyCallWithRequest:request handler:^(RMTEmpty *response, NSError *error) { + [_service emptyCallWithRequest:request handler:^(GPBEmpty *response, NSError *error) { XCTAssertNil(error, @"Finished with unexpected error: %@", error); - id expectedResponse = [RMTEmpty message]; + id expectedResponse = [GPBEmpty message]; XCTAssertEqualObjects(response, expectedResponse); [expectation fulfill]; @@ -343,9 +343,9 @@ static cronet_engine *cronetEngine = NULL; __weak XCTestExpectation *expectation = [self expectationWithDescription:@"RPC after closing connection"]; - RMTEmpty *request = [RMTEmpty message]; + GPBEmpty *request = [GPBEmpty message]; - [_service emptyCallWithRequest:request handler:^(RMTEmpty *response, NSError *error) { + [_service emptyCallWithRequest:request handler:^(GPBEmpty *response, NSError *error) { XCTAssertNil(error, @"First RPC finished with unexpected error: %@", error); #pragma clang diagnostic push @@ -353,7 +353,7 @@ static cronet_engine *cronetEngine = NULL; [GRPCCall closeOpenConnections]; #pragma clang diagnostic pop - [_service emptyCallWithRequest:request handler:^(RMTEmpty *response, NSError *error) { + [_service emptyCallWithRequest:request handler:^(GPBEmpty *response, NSError *error) { XCTAssertNil(error, @"Second RPC finished with unexpected error: %@", error); [expectation fulfill]; }]; diff --git a/src/objective-c/tests/RemoteTestClient/RemoteTest.podspec b/src/objective-c/tests/RemoteTestClient/RemoteTest.podspec index 887380eb55f..25c9c7f8418 100644 --- a/src/objective-c/tests/RemoteTestClient/RemoteTest.podspec +++ b/src/objective-c/tests/RemoteTestClient/RemoteTest.podspec @@ -15,7 +15,9 @@ Pod::Spec.new do |s| BINDIR=../../../../bins/$CONFIG PROTOC=$BINDIR/protobuf/protoc PLUGIN=$BINDIR/grpc_objective_c_plugin - $PROTOC --plugin=protoc-gen-grpc=$PLUGIN --objc_out=. --grpc_out=. *.proto + # we use this path to locate well-known proto files + PROTO_SRC=../../../../third_party/protobuf/src + $PROTOC --plugin=protoc-gen-grpc=$PLUGIN --objc_out=. --grpc_out=. *.proto -I $PROTO_SRC -I . CMD s.subspec "Messages" do |ms| diff --git a/src/objective-c/tests/RemoteTestClient/empty.proto b/src/objective-c/tests/RemoteTestClient/empty.proto deleted file mode 100644 index a678048289e..00000000000 --- a/src/objective-c/tests/RemoteTestClient/empty.proto +++ /dev/null @@ -1,44 +0,0 @@ -// Copyright 2015, Google Inc. -// All rights reserved. -// -// Redistribution and use in source and binary forms, with or without -// modification, are permitted provided that the following conditions are -// met: -// -// * Redistributions of source code must retain the above copyright -// notice, this list of conditions and the following disclaimer. -// * Redistributions in binary form must reproduce the above -// copyright notice, this list of conditions and the following disclaimer -// in the documentation and/or other materials provided with the -// distribution. -// * Neither the name of Google Inc. nor the names of its -// contributors may be used to endorse or promote products derived from -// this software without specific prior written permission. -// -// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR -// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT -// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - -syntax = "proto3"; - -package grpc.testing; - -option objc_class_prefix = "RMT"; - -// An empty message that you can re-use to avoid defining duplicated empty -// messages in your project. A typical example is to use it as argument or the -// return value of a service API. For instance: -// -// service Foo { -// rpc Bar (grpc.testing.Empty) returns (grpc.testing.Empty) { }; -// }; -// -message Empty {} diff --git a/src/objective-c/tests/RemoteTestClient/test.proto b/src/objective-c/tests/RemoteTestClient/test.proto index 514c3b80955..5c359c5c129 100644 --- a/src/objective-c/tests/RemoteTestClient/test.proto +++ b/src/objective-c/tests/RemoteTestClient/test.proto @@ -31,7 +31,7 @@ // of unary/streaming requests/responses. syntax = "proto3"; -import "empty.proto"; +import "google/protobuf/empty.proto"; import "messages.proto"; package grpc.testing; @@ -42,7 +42,7 @@ option objc_class_prefix = "RMT"; // performance with various types of payload. service TestService { // One empty request followed by one empty response. - rpc EmptyCall(grpc.testing.Empty) returns (grpc.testing.Empty); + rpc EmptyCall(google.protobuf.Empty) returns (google.protobuf.Empty); // One request followed by one response. rpc UnaryCall(SimpleRequest) returns (SimpleResponse); From 46cc9eebb17b2711515007d7af9108cd0d78788a Mon Sep 17 00:00:00 2001 From: Makarand Dharmapurikar <makarandd@google.com> Date: Fri, 8 Jul 2016 11:42:07 -0700 Subject: [PATCH 2/5] addressed feedback using 'using'. Removed unnecessary grpc::string. fixed indentation. --- src/compiler/objective_c_plugin.cc | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/compiler/objective_c_plugin.cc b/src/compiler/objective_c_plugin.cc index 43e4b3647d0..3878a332515 100644 --- a/src/compiler/objective_c_plugin.cc +++ b/src/compiler/objective_c_plugin.cc @@ -41,6 +41,9 @@ #include <google/protobuf/compiler/objectivec/objectivec_helpers.h> +using ::google::protobuf::compiler::objectivec::ProtobufLibraryFrameworkName; +using ::google::protobuf::compiler::objectivec::IsProtobufLibraryBundledProtoFile; + class ObjectiveCGrpcGenerator : public grpc::protobuf::compiler::CodeGenerator { public: ObjectiveCGrpcGenerator() {} @@ -75,17 +78,16 @@ class ObjectiveCGrpcGenerator : public grpc::protobuf::compiler::CodeGenerator { ::grpc::string header = grpc_objective_c_generator::MessageHeaderName( file->dependency(i)); const grpc::protobuf::FileDescriptor *dependency = file->dependency(i); - if (::google::protobuf::compiler::objectivec::IsProtobufLibraryBundledProtoFile(dependency)) { + if (IsProtobufLibraryBundledProtoFile(dependency)) { ::grpc::string base_name = header; grpc_generator::StripPrefix(&base_name, "google/protobuf/"); proto_imports += - ::grpc::string("#if GPB_USE_PROTOBUF_FRAMEWORK_IMPORTS\n") + - ::grpc::string(" #import <") + - ::google::protobuf::compiler::objectivec::ProtobufLibraryFrameworkName + - ("/") + base_name + ">\n" + - ::grpc::string("#else\n") + - ::grpc::string(" #import \"") + header + "\"\n" + - ::grpc::string("#endif\n"); + "#if GPB_USE_PROTOBUF_FRAMEWORK_IMPORTS\n" + " #import <" + ::grpc::string(ProtobufLibraryFrameworkName) + + "/" + base_name + ">\n" + "#else\n" + " #import \"" + header + "\"\n" + "#endif\n"; } else { proto_imports += ::grpc::string("#import \"") + header + "\"\n"; } From 86202529a81ce45a1da160ef1dcc58540785d4b2 Mon Sep 17 00:00:00 2001 From: Makarand Dharmapurikar <makarandd@google.com> Date: Fri, 8 Jul 2016 12:41:12 -0700 Subject: [PATCH 3/5] fixed minor indent and resubmit --- src/compiler/objective_c_plugin.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/compiler/objective_c_plugin.cc b/src/compiler/objective_c_plugin.cc index 3878a332515..5026088db1e 100644 --- a/src/compiler/objective_c_plugin.cc +++ b/src/compiler/objective_c_plugin.cc @@ -88,9 +88,9 @@ class ObjectiveCGrpcGenerator : public grpc::protobuf::compiler::CodeGenerator { "#else\n" " #import \"" + header + "\"\n" "#endif\n"; - } else { + } else { proto_imports += ::grpc::string("#import \"") + header + "\"\n"; - } + } } ::grpc::string declarations; From 77f8da22eefedbb49c13a414ab3ff748137e93d5 Mon Sep 17 00:00:00 2001 From: Makarand Dharmapurikar <makarandd@google.com> Date: Fri, 8 Jul 2016 15:03:20 -0700 Subject: [PATCH 4/5] added a comment just to retrigger tests --- src/compiler/objective_c_plugin.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/compiler/objective_c_plugin.cc b/src/compiler/objective_c_plugin.cc index 5026088db1e..be647764024 100644 --- a/src/compiler/objective_c_plugin.cc +++ b/src/compiler/objective_c_plugin.cc @@ -81,6 +81,7 @@ class ObjectiveCGrpcGenerator : public grpc::protobuf::compiler::CodeGenerator { if (IsProtobufLibraryBundledProtoFile(dependency)) { ::grpc::string base_name = header; grpc_generator::StripPrefix(&base_name, "google/protobuf/"); + // create the import code snippet proto_imports += "#if GPB_USE_PROTOBUF_FRAMEWORK_IMPORTS\n" " #import <" + ::grpc::string(ProtobufLibraryFrameworkName) + From 027ae0b4eba2e65d8b4a6e4ff0005d1ac8836689 Mon Sep 17 00:00:00 2001 From: Makarand Dharmapurikar <makarandd@google.com> Date: Fri, 8 Jul 2016 16:39:37 -0700 Subject: [PATCH 5/5] removed unneeded import. fixes travis build --- src/objective-c/tests/InteropTests.m | 1 - 1 file changed, 1 deletion(-) diff --git a/src/objective-c/tests/InteropTests.m b/src/objective-c/tests/InteropTests.m index a2f63ac40ca..494743d6041 100644 --- a/src/objective-c/tests/InteropTests.m +++ b/src/objective-c/tests/InteropTests.m @@ -40,7 +40,6 @@ #import <GRPCClient/GRPCCall+Tests.h> #import <GRPCClient/GRPCCall+Cronet.h> #import <ProtoRPC/ProtoRPC.h> -#import <RemoteTest/Empty.pbobjc.h> #import <RemoteTest/Messages.pbobjc.h> #import <RemoteTest/Test.pbobjc.h> #import <RemoteTest/Test.pbrpc.h>