Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Objc update WKT generated import paths. #23228

Merged
merged 1 commit into from Jun 18, 2020
Merged

Objc update WKT generated import paths. #23228

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/compiler/objective_c_plugin.cc
View file
Open in desktop
Expand Up @@ -52,15 +52,16 @@ inline ::grpc::string ImportProtoHeaders(

::grpc::string base_name = header;
grpc_generator::StripPrefix(&base_name, "google/protobuf/");
Copy link
Member

@muxi muxi Jun 17, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am thinking that if protobuf moved things out of google/protobuf, users should probably not continue to use google/protobuf prefix in their #import statement? That sounds like a breaking change though so I'm not sure if that's true.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They only changed paths of .pbobjc.h/m files. For .proto files, their paths are still the same. The header variable is generated based on these .proto files, so we still need to strip google/protobuf prefix.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah that makes sense!

::grpc::string file_name = "GPB" + base_name;
Copy link
Member

@muxi muxi Jun 17, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ thomasvl - could you confirm that your change in protobuf follows the pattern of 1) moving WKT files from objectivec/google/protobuf/ to objectivec/ and 2) add GPB prefix to the headers?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've already check with Thomas, we just need to update paths from objectivec/google/protobuf/to objectivec/ and add GPB prefix to the headers.

He mentioned that we also need to set the min version of protobuf to the release(v3.12.0) where they shipped this. I've check !ProtoCompiler.podspec, the version is 3.12.2, looks like we don't need to change it.

// create the import code snippet
::grpc::string framework_header =
::grpc::string(ProtobufLibraryFrameworkName) + "/" + base_name;
::grpc::string(ProtobufLibraryFrameworkName) + "/" + file_name;

static const ::grpc::string kFrameworkImportsCondition =
"GPB_USE_PROTOBUF_FRAMEWORK_IMPORTS";
return PreprocIfElse(kFrameworkImportsCondition,
indent + SystemImport(framework_header),
indent + LocalImport(header));
indent + LocalImport(file_name));
}

} // namespace
Expand Down