From 772c98984a1a6f3b7e438c144936a8f88c65ee86 Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Mon, 13 Jun 2022 16:52:19 -0400 Subject: [PATCH] Add package_as_prefix_forced_prefix generation option. This is a generation option that provides serves the same purpose as the existing GPB_OBJC_USE_PACKAGE_AS_PREFIX_PREFIX environment variable; just providing a different way to set/enable it. --- objectivec/README.md | 25 ++++++++++++------- .../objectivec/objectivec_generator.cc | 4 +++ .../compiler/objectivec/objectivec_helpers.cc | 11 ++++++-- .../compiler/objectivec/objectivec_helpers.h | 4 +++ 4 files changed, 33 insertions(+), 11 deletions(-) diff --git a/objectivec/README.md b/objectivec/README.md index a0fcd19307..f2c2a846a7 100644 --- a/objectivec/README.md +++ b/objectivec/README.md @@ -200,12 +200,13 @@ supported keys are: entry can be made as "no_package:PATH=prefix", where PATH is the path for the .proto file. - * `use_package_as_prefix` and `proto_package_prefix_exceptions_path`: The - `value` for `use_package_as_prefix` can be `yes` or `no`, and indicates - if a prefix should be derived from the proto package for all the symbols - for files that don't have the `objc_class_prefix` file option (mentioned - above). This helps ensure the symbols are more unique and means there is - less chance of ObjC class name collisions. + * `use_package_as_prefix`, `package_as_prefix_forced_prefix` and + `proto_package_prefix_exceptions_path`: The `value` for + `use_package_as_prefix` can be `yes` or `no`, and indicates if a prefix + should be derived from the proto package for all the symbols for files that + don't have the `objc_class_prefix` file option (mentioned above). This helps + ensure the symbols are more unique and means there is less chance of ObjC + class name collisions. To help in migrating code to using this support, `proto_package_prefix_exceptions_path` can be used to provide the path @@ -213,10 +214,16 @@ supported keys are: if prefixed with `#`). These package won't get the derived prefix, allowing migrations to the behavior one proto package at a time across a code base. + `package_as_prefix_forced_prefix` can be used to provide a value that will + be used before all prefixes derived from the packages to help group all of + these types with a common prefix. Thus it only makes sense to use it when + `use_package_as_prefix` is also enabled. For example, setting this to + "XYZ\_" and generating a file with the package "something" defining + "MyMessage", would have Objective-C class be `XYZ_Something_MyMessage`. + `use_package_as_prefix` currently defaults to `no` (existing behavior), but - in the future (as a breaking change), that is likely to change since it - helps prepare folks before they end up using a lot of protos and getting a - lot of collisions. + that could change in the future as it helps avoid collisions when more + protos get added to the build. Note that this would be a breaking change. * `headers_use_forward_declarations`: The `value` for this can be `yes` or `no`, and indicates if the generated headers use forward declarations for diff --git a/src/google/protobuf/compiler/objectivec/objectivec_generator.cc b/src/google/protobuf/compiler/objectivec/objectivec_generator.cc index 9dccf149a6..d8e4cb5277 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_generator.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_generator.cc @@ -230,6 +230,10 @@ bool ObjectiveCGenerator::GenerateAll( // - A comment can go on a line after a expected package/prefix pair. // (i.e. - "some.proto.package # comment") SetProtoPackagePrefixExceptionList(options[i].second); + } else if (options[i].first == "package_as_prefix_forced_prefix") { + // String to use as the prefix when deriving a prefix from the package + // name. So this only applies when use_package_as_prefix is also used. + SetForcedPackagePrefix(options[i].second); } else if (options[i].first == "headers_use_forward_declarations") { if (!StringToBool(options[i].second, &generation_options.headers_use_forward_declarations)) { diff --git a/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc b/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc index a4ce84c558..88208b5914 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc @@ -134,6 +134,7 @@ class PrefixModeStorage { // When using a proto package as the prefix, this should be added as the // prefix in front of it. const std::string& forced_package_prefix() const { return forced_prefix_; } + void set_forced_package_prefix(const std::string& prefix) { forced_prefix_ = prefix; } private: bool use_package_name_; @@ -155,8 +156,6 @@ PrefixModeStorage::PrefixModeStorage() { exception_path_ = exception_path; } - // This one is a not expected to be common, so it doesn't get a generation - // option, just the env var. const char* prefix = getenv("GPB_OBJC_USE_PACKAGE_AS_PREFIX_PREFIX"); if (prefix) { forced_prefix_ = prefix; @@ -254,6 +253,14 @@ void SetProtoPackagePrefixExceptionList(const std::string& file_path) { g_prefix_mode.set_exception_path(file_path); } +std::string GetForcedPackagePrefix() { + return g_prefix_mode.forced_package_prefix(); +} + +void SetForcedPackagePrefix(const std::string& prefix) { + g_prefix_mode.set_forced_package_prefix(prefix); +} + Options::Options() { // While there are generator options, also support env variables to help with // build systems where it isn't as easy to hook in for add the generation diff --git a/src/google/protobuf/compiler/objectivec/objectivec_helpers.h b/src/google/protobuf/compiler/objectivec/objectivec_helpers.h index d21fed215a..a1bcc4ba4a 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_helpers.h +++ b/src/google/protobuf/compiler/objectivec/objectivec_helpers.h @@ -63,6 +63,10 @@ void PROTOC_EXPORT SetUseProtoPackageAsDefaultPrefix(bool on_or_off); std::string PROTOC_EXPORT GetProtoPackagePrefixExceptionList(); void PROTOC_EXPORT SetProtoPackagePrefixExceptionList( const std::string& file_path); +// Get/Set a prefix to add before the prefix generated from the package name. +// This is only used when UseProtoPackageAsDefaultPrefix() is True. +std::string PROTOC_EXPORT GetForcedPackagePrefix(); +void PROTOC_EXPORT SetForcedPackagePrefix(const std::string& prefix); // Generator Prefix Validation Options (see objectivec_generator.cc for a // description of each):