Skip to content

fix(php): Remove public and weak dependencies from generated PHP.#26598

Open
alowde wants to merge 1 commit intoprotocolbuffers:mainfrom
alowde:php-protobuf-public-imports-fix
Open

fix(php): Remove public and weak dependencies from generated PHP.#26598
alowde wants to merge 1 commit intoprotocolbuffers:mainfrom
alowde:php-protobuf-public-imports-fix

Conversation

@alowde
Copy link
Copy Markdown

@alowde alowde commented Mar 27, 2026

In issue #11146 people reported problems with the PHP package version 3.20.0RC3 and above crashing when the native extension was used with protobufs that had transitive dependencies. This comment noted that it looked like an issue with the dependency list being modified.

This PR resolves the issue by also removing public and weak dependency references in the emitted PHP metadata classes.
I've added a unit test showing that it compiles OK, but the unit tests don't exercise the PHP side so I've taken the liberty of extending the reproducer case kindly provided by @tforesti and pushed a version that shows the fix to github.com/alowde/php-protobuf-ext-issue. The Dockerfile builds a copy of protoc from my branch and shows that the patched version doesn't cause a crash on import when the image is launced.

An LLM was used to investigate the issue and write the unit test, with human review. The fix itself and the fix demonstration were written by @alowde. The fix demo builds on the failure case Dockerfile by @tforesti to provide a clear demonstration of the fix.

Please let me know if any further information or changes would be useful. Thanks!

@alowde alowde requested a review from a team as a code owner March 27, 2026 00:31
@alowde alowde requested review from bshaffer and removed request for a team March 27, 2026 00:31
@google-cla
Copy link
Copy Markdown

google-cla bot commented Mar 27, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@alowde
Copy link
Copy Markdown
Author

alowde commented Mar 29, 2026

CLA has been signed and I'm keeping this rebased on main as mentioned in CONTRIBUTING.md.

@alowde alowde force-pushed the php-protobuf-public-imports-fix branch 3 times, most recently from fef2083 to 76682e9 Compare April 1, 2026 00:58
@esrauchg esrauchg added the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Apr 6, 2026
@github-actions github-actions bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Apr 6, 2026
@alowde alowde force-pushed the php-protobuf-public-imports-fix branch from 76682e9 to f3d47e8 Compare April 8, 2026 00:42
@alowde
Copy link
Copy Markdown
Author

alowde commented Apr 9, 2026

Looks like it did pass tests before I rebased to origin 😅. I'll rebase again now but avoid it in future if I see the tests have run - apologies for the hassle @esrauchg.

@alowde alowde force-pushed the php-protobuf-public-imports-fix branch from f3d47e8 to 1885219 Compare April 9, 2026 01:32
Fixes issue protocolbuffers#11146 caused by transitive dependencies referencing
removed dependencies in the emitted PHP metadata classes.
@alowde alowde force-pushed the php-protobuf-public-imports-fix branch from 1885219 to 263d4ab Compare April 12, 2026 02:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants