feat: enable edition for php #16712
feat: enable edition for php #16712
Conversation
github-actions
bot
removed
the
🅰️ safe for tests
| @@ -37,7 +37,13 @@ class PROTOC_EXPORT Generator : public CodeGenerator { | |||
| std::string* error) const override; | |||
|
|
|||
| uint64_t GetSupportedFeatures() const override { | |||
| return FEATURE_PROTO3_OPTIONAL; | |||
| return Feature::FEATURE_PROTO3_OPTIONAL | Feature::FEATURE_SUPPORTS_EDITIONS; | |||
There was a problem hiding this comment.
We probably should hold off on advertising support until it's ready. Any protos under google/protobuf are allowlisted, and we could tweak php_proto_library to pass --experimental editions if that doesn't work
There was a problem hiding this comment.
Maybe I'm doing something wrong, but when I remove FEATURE_SUPPORT_EDITIONS, I get the following error:
test_messages_proto3_editions.proto: is an editions file, but code generator --php_out hasn't been updated to support editions yet. Please ask the owner of this code generator to add support or switch back to proto2/proto3.
There was a problem hiding this comment.
protoc will skip this check for any proto path under "google/protobuf/" and "upb/". We should probably add "editions/" to that list too
There was a problem hiding this comment.
Alternatively, you could modify internal_php_proto_library to accept an experimental_editions argument and have it set --experimental_editions on protoc
There was a problem hiding this comment.
I looked into both of these and, to do the first, editions/ does not bypass the check because the filename being passed in is test_messages_proto3_editions.proto (the editions/ path is not part of the filename, I am not sure why). As for the second, I cannot find a way to set custom flags on the protoc binary, so it seems like I'd have to modify _proto_gen, which I am not super excited about doing.
github-actions
bot
removed
the
🅰️ safe for tests
github-actions
bot
removed
the
🅰️ safe for tests
github-actions
bot
removed
the
🅰️ safe for tests
github-actions
bot
removed
the
🅰️ safe for tests
Expose editions for PHP - Skips `TextFormatOutput` tests, as text payload is not supported for PHP - Skips `TestAllTypesEdition2023` tests, as editions-specific features are not yet supported for PHP - Skips `TestAllTypesProto2` tests for Editions, as proto2 is not supported in PHP Additionally, refactors `conformance_php.php` by using `switch` statements for readability, and adding consistent whitespace formatting. Closes #16712 COPYBARA_INTEGRATE_REVIEW=#16712 from bshaffer:php-editions a1c41ad FUTURE_COPYBARA_INTEGRATE_REVIEW=#16712 from bshaffer:php-editions a1c41ad PiperOrigin-RevId: 631193005
Expose editions for PHP - Skips `TextFormatOutput` tests, as text payload is not supported for PHP - Skips `TestAllTypesEdition2023` tests, as editions-specific features are not yet supported for PHP - Skips `TestAllTypesProto2` tests for Editions, as proto2 is not supported in PHP Additionally, refactors `conformance_php.php` by using `switch` statements for readability, and adding consistent whitespace formatting. Closes #16712 COPYBARA_INTEGRATE_REVIEW=#16712 from bshaffer:php-editions a1c41ad FUTURE_COPYBARA_INTEGRATE_REVIEW=#16712 from bshaffer:php-editions a1c41ad PiperOrigin-RevId: 631193005
|
It looks like these changes were submitted in 01744cc#diff-5ebd764f133e72efb1415d478675b92c7b091b3bf007045c2256aa8742267c75, so I'm closing this PR now. |
COPYBARA_INTEGRATE_REVIEW=protocolbuffers#16712 from bshaffer:php-editions a1c41ad PiperOrigin-RevId: 631824623
Expose editions for PHP
TextFormatOutputtests, as text payload is not supported for PHPTestAllTypesEdition2023tests, as editions-specific features are not yet supported for PHPTestAllTypesProto2tests for Editions, as proto2 is not supported in PHPAdditionally, refactors
conformance_php.phpby usingswitchstatements for readability, and adding consistent whitespace formatting.