Skip to content

add xdebug dynamic extension #673

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.

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

Merged
merged 38 commits into from
Apr 19, 2025
Merged

add xdebug dynamic extension #673

merged 38 commits into from
Apr 19, 2025

Conversation

henderkes
Copy link
Collaborator

@henderkes henderkes commented Mar 23, 2025

What does this PR do?

see #672

Checklist before merging

If your PR involves the changes mentioned below and completed the action, please tick the corresponding option.
If a modification is not involved, please skip it directly.

  • If you modified *.php, run composer cs-fix at local machine.
  • If it's an extension or dependency update, make sure adding related extensions in src/global/test-extensions.php.
  • If you changed the behavior of static-php-cli, update docs in ./docs/.
  • If you updated config/xxx.json content, run bin/spc dev:sort-config xxx.

@henderkes henderkes requested a review from crazywhalecc March 23, 2025 08:41
@henderkes
Copy link
Collaborator Author

This requires some work from your side. Technically these extensions don't have libraries but should have a build function themselves, rather than adding a library with the same name. I only went this route to get a proof of concept working quickly.

There's also no functionality to test these extensions yet. It could be done by adding a extension=$name to the php.ini, running make install in the $name source directory and then launching buildroot/bin/php (requires cli compilation) to check if it loads the extension.

@crazywhalecc crazywhalecc added the new feature New feature or request label Mar 24, 2025
@crazywhalecc
Copy link
Owner

crazywhalecc commented Mar 24, 2025

Interesting. If it is confirmed to work for glibc and macOS, maybe it would be better to directly support this feature in Extension.php?

For example, when I build PHP, if I pass --with-shared-extensions=xdebug,XXX,YYY, Builder will call the buildShared() method in Extension.php after building the cli and verifying it. But we may also need to make additional adjustments to the doc structure, or only support one or a few extensions such as xdebug, and make special notes. What do you think?

Also, add "target": ["static", "shared"] for ext.json?

@henderkes
Copy link
Collaborator Author

henderkes commented Mar 24, 2025

Interesting. If it is confirmed to work for glibc and macOS, maybe it would be better to directly support this feature in Extension.php?

I cannot confirm macOS other than through github actions. I think we either use a different attribute (DynamicExt) because that way, we can distinguish which libraries/extensions need to be built before or after php. We could also support putting both attributes on an extension, that way it would be statically baked into php, but would also produce a .so extension to copy out.

For example, when I build PHP, if I pass --with-shared-extensions=xdebug,XXX,YYY, Builder will call the buildShared() method in Extension.php after building the cli and verifying it. But we may also need to make additional adjustments to the doc structure, or only support one or a few extensions such as xdebug, and make special notes. What do you think?

I think it would be nice for a few extensions. Treating it as a general possibility is better than hacking something together as I did for the proof of concept. This is your project and your decision about the way it's implemented.

My choice would be two different attributes and if an extension has #[DynamicExt] it'll be given a builder and a method will be called after. But we should reuse as much code as we can from the current dependency resolving system because dynamic extensions may also have library requirements.

Also, add "target": ["static", "shared"] for ext.json?

Yes, great idea. bin spc build "intl,pgsql" --with-shared="pgsql,xdebug" --build-embed --build-cli would bake pgsql into the php-cli/embed sapi, but also produce a pqsql.so that could be loaded dynamically.

Which... thinking about it... doesn't make all that much sense, I guess. But it doesn't hurt either, so why not.

@henderkes henderkes force-pushed the feat/xdebug-dynamic branch from 90b0d70 to 0c6dd7a Compare March 30, 2025 16:50
@crazywhalecc
Copy link
Owner

crazywhalecc commented Apr 18, 2025

It seems that some changes broke the dependency manager and I can't compile extension that contains ext-depends.

@crazywhalecc
Copy link
Owner

Everything looks good to me now, but I haven't fully tested all we added features.

@crazywhalecc crazywhalecc added need test This PR has not been tested yet, cannot merge now mixed PR This PR contains multiple updates labels Apr 18, 2025
@crazywhalecc crazywhalecc merged commit f0e634a into main Apr 19, 2025
32 checks passed
@crazywhalecc crazywhalecc deleted the feat/xdebug-dynamic branch April 19, 2025 08:01
@henderkes
Copy link
Collaborator Author

🥇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mixed PR This PR contains multiple updates need test This PR has not been tested yet, cannot merge now new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants