-
Notifications
You must be signed in to change notification settings - Fork 361
PHP 8.4 support #2038
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
PHP 8.4 support #2038
Conversation
|
Oh no, new PHP release = a bunch of new functions to add to the asyncify list 😢 |
|
I ran the tests with longer stack traces, and a big thing I see missing in the asyncify declarations is 🤞 hopefully we won't need to add too many more. I'm fixing my docker setup and planning to rebuild PHP 8.4 with this function added. |
|
I had trouble getting the builds to run with errors like: and To fix them, I had to:
I'll create another PR for those changes. It seems fine to upgrade the versions we use for build. Now there is a remaining build error but much later in the process. This line fails: The code must have changed. Planning to look at that. |
I have a local patch that seems good for this, but now there is an issue due to a change in the Emscripten-generated JS. With Emscripten 3.1.74, our ExitStatus patch conflicts because the output changed from declaring ExitStatus as a function to declaring it as a class. Functions declared anywhere in a scope are available to all statements within a scope, even if the statements precede the function declaration. The same is not true for classes, so I started running into errors that were something like "ExitStatus cannot be referenced before initialized". I think we just need to pinpoint the location of the class declaration in the JS file and insert our overriding definition immediately after that. This is quite a lot of yak-shaving by now, but I'm planning to continue with these upgrades under a separate PR because they'll need to happen sometime and I'm unable to build php-wasm without them. |
|
@adamziel, I pushed what I think may be a sufficient asyncify addition for PHP 8.4 to work but am personally unable to rebuild PHP 8.4 until #2116 lands due to the missing version errors mentioned above. If php-wasm builds are still working for you, please feel free to rebuild and test PHP 8.4 asyncify again in the meantime. Otherwise, I am happy to rebuild after #2116 lands. |
|
@brandonpayton I've rebuilt but it keeps failing. I've added a few more functions and am still seeing those failures. We might need better debugging facilities to make progress here. Alternatively, there's a brute-force approach where we'd list all C functions added in 8.4 and, if it works, bisect to find the right one: |
|
I'm wrong. I rebuilt the web version and tested the node version. Your fix works great @brandonpayton. I'll push the rebuilt files soon |
|
Thanks for handling this and all the time rebuilding and testing, @adamziel! |
Note: This work is continued from a PR started under the WordPress GH org. There are a couple of reasons for this upgrade: 1. I am having trouble rebuilding php-wasm on my current system. References to older ubuntu images and emscripten versions [has given me trouble](#2038 (comment)). 2. We want [this Emscripten fix](emscripten-core/emscripten#22932) from @jeroenpf and @bgrgicak which was first included in Emscripten 3.1.73. This PR: - Bumps the ubuntu docker image version due to build errors related to missing image. Referencing a newer ubuntu image seemed to fix the "missing image" errors. - Bumps Emscripten version - Adjusts Playground-specific patches to Emscripten output since the latest breaks that patching. - CI (once we can run GH actions on A8C's GHE instance) --------- Co-authored-by: Brandon Payton <[email protected]>
## Motivation for the change, related issues [Emscripten merged a fix for `statfs`](emscripten-core/emscripten#23381) in `4.0.1`, so this PR updates Emscripten to allow us to run `statfs` in PHP functions like `disk_total_space`. ## Implementation details This PR updates the version of Emscripten to `4.0.5`, rebuilds all PHP-wasm libraries, and recompiles PHP-wasm. While rebuilding PHP-wasm libraries there was a bug in the `libcurl` build script because it expected `libopenssl` to be located in `libopenssl/asyncify/dist`, but in reality `libopenssl` was located in `libopenssl/asyncify/dist/1.1.0h` (similarly for JSPI). We have two versions of `libopenssl` because it [was requires for PHP 7.0](#2038). This PR drops `libopenssl 1.1.0h` support because [Playground doesn't support PHP 7.0 anymore.](Automattic/wordpress-playground-private#74) ## Testing Instructions (or ideally a Blueprint) - CI
What is this PR doing?
Adds PHP 8.4 to Playground.
Also updates the dependencies:
Testing Instructions
/phpinfo.php, confirm the php version it 8.4/phpinfo.php, confirm the php version it 8.4PHP=8.4 nx run php-wasm-cli:start -- -iand confirm the PHP version presented on the screen is 8.4