Skip to content

fix: PHP readonly legacy files for nested messages#10320

Merged
fowles merged 1 commit intoprotocolbuffers:mainfrom
bshaffer:fix-readonly-with-nested-messages
Aug 2, 2022
Merged

fix: PHP readonly legacy files for nested messages#10320
fowles merged 1 commit intoprotocolbuffers:mainfrom
bshaffer:fix-readonly-with-nested-messages

Conversation

@bshaffer
Copy link
Copy Markdown
Contributor

@bshaffer bshaffer commented Jul 27, 2022

In #10041 we added escaping for PHP's ReadOnly keyword, but this misses writing the backwards compatibility file when the message is nested. For example, the file Nested\ReadOnly.php is not written for the proto message nested { message readonly {} }.

This fortunately should not cause any issues with existing users, as this file is only for compatibility, and without it, the previous message file remains (and works as expected). But as the previous ReadOnly.php throw a PHP fatal error on PHP 8.1, the result is that anyone upgrading to PHP 8.1 and still using the ReadOnly keyword will still receive a PHP fatal error.

This is an edge case, but still one we should fix.

@haberman
Copy link
Copy Markdown
Member

haberman commented Aug 2, 2022

Should this be cherry-picked into 21.x?

@fowles fowles merged commit 449e21a into protocolbuffers:main Aug 2, 2022
@fowles
Copy link
Copy Markdown
Contributor

fowles commented Aug 2, 2022

seems likely. Do you mind sending me a PR for that?

@bshaffer bshaffer deleted the fix-readonly-with-nested-messages branch August 2, 2022 22:29
bithium pushed a commit to bithium/protobuf that referenced this pull request Sep 4, 2023
…ith-nested-messages

fix: PHP readonly legacy files for nested messages
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants