Skip to content
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

Open
wants to merge 5 commits into
base: trunk
Choose a base branch
from
Open

PHP 8.4 support #2038

wants to merge 5 commits into from

Conversation

adamziel
Copy link
Collaborator

@adamziel adamziel commented Nov 28, 2024

What is this PR doing?

Adds PHP 8.4 to Playground.

Also updates the dependencies:

  • Upgrades libz to 1.2.13 as PHP 8.4 was no longer compatible with 1.2.5 we've used before.
  • Adds libopenssl 1.1.1 required by PHP 8.4. We still keep 1.1.0h in the repo as I had issues building PHP 7.0 with 1.1.1.

Testing Instructions

  1. Go to http://localhost:5400/website-server/ and switch the PHP version to 8.4.
  2. Confirm that WordPress is functional and you can update the site in the site editor
  3. Go to /phpinfo.php, confirm the php version it 8.4
  4. Update settings to run with additional PHP extensions and networking support
  5. Confirm that WordPress is functional, you can update the site in the site editor, and the plugin directory shows plugins and you can install them
  6. Go to /phpinfo.php, confirm the php version it 8.4
  7. In CLI, run PHP=8.4 nx run php-wasm-cli:start -- -i and confirm the PHP version presented on the screen is 8.4

@adamziel
Copy link
Collaborator Author

adamziel commented Dec 4, 2024

Oh no, new PHP release = a bunch of new functions to add to the asyncify list 😢

@brandonpayton
Copy link
Member

brandonpayton commented Dec 17, 2024

I ran the tests with longer stack traces, and a big thing I see missing in the asyncify declarations is php_execute_script_ex which was added within the last year:
php/php-src@c149b4f#diff-2978fe1c2c45b4eca89dc476376ddc7193bc4e5e7fff0c7d1c465f057b35a5e6R2434

🤞 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.

@brandonpayton
Copy link
Member

I had trouble getting the builds to run with errors like:

#8 1.435 Err:5 http://ports.ubuntu.com/ubuntu-ports lunar Release             
#8 1.435   404  Not Found [IP: 185.125.190.39 80]                        
#8 1.663 Err:6 http://ports.ubuntu.com/ubuntu-ports lunar-updates Release
#8 1.663   404  Not Found [IP: 185.125.190.39 80]  
#8 1.885 Err:7 http://ports.ubuntu.com/ubuntu-ports lunar-backports Release
#8 1.885   404  Not Found [IP: 185.125.190.39 80]                            
#8 2.113 Err:8 http://ports.ubuntu.com/ubuntu-ports lunar-security Release    
#8 2.113   404  Not Found [IP: 185.125.190.39 80]             
#8 2.116 Reading package lists...
#8 2.120 E: The repository 'http://ports.ubuntu.com/ubuntu-ports lunar Release' does not have a Release file.                                                
#8 2.120 E: The repository 'http://ports.ubuntu.com/ubuntu-ports lunar-updates Release' does not have a Release file.                                        
#8 2.120 E: The repository 'http://ports.ubuntu.com/ubuntu-ports lunar-backports Release' does not have a Release file.                                      
#8 2.120 E: The repository 'http://ports.ubuntu.com/ubuntu-ports lunar-security Release' does not have a Release file.

and

#10 [ 6/12] RUN git clone https://github.com/emscripten-core/emsdk.git &&
./emsdk/emsdk install 3.1.61 &&     
/root/emsdk/emsdk activate 3.1.61          #10 0.110 Cloning into 'emsdk'...                                                                                                                            
#10 11.34 error: RPC failed; HTTP 503 curl 22 The requested URL returned error: 503                                                                          
#10 11.34 fatal: expected flush after ref listing
#10 ERROR:  process "/bin/bash -c git clone https://github.com/emscripten-core/emsdk.git &&
./emsdk/emsdk install 3.1.61 &&     
/root/emsdk/emsdk activate 3.1.61" did not complete successfully: exit code: 128  

To fix them, I had to:

  • update the ubuntu distribution to the newer "noble" release
  • update the emsdk version to 3.1.74

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:
/root/replace.sh $'s/if\s*\(\s*["\']string["\']\s*===\s*typeof Module\[["\']websocket["\']\]\[["\']url["\']\]\s*\)/if("function"===typeof Module["websocket"]["url"]) {\nurl = Module["websocket"]["url"](...arguments);\n}else if ("string" === typeof Module["websocket"]["url"])/g' \ /root/output/php.js; \

The code must have changed. Planning to look at that.

@brandonpayton
Copy link
Member

Now there is a remaining build error but much later in the process. This line fails:
/root/replace.sh $'s/if\s*(\s*["']string["']\s*===\stypeof Module[["']websocket["']][["']url["']]\s)/if("function"===typeof Module["websocket"]["url"]) {\nurl = Module["websocket"]"url";\n}else if ("string" === typeof Module["websocket"]["url"])/g' \ /root/output/php.js; \

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

2 participants