Remove --disable-all configuration option in PHP compile process

[mho22]

What is this PR doing?

Based on #1127. This PR enables FileInfo and Posix extensions in PHP previously disabled by --disable-all.

What problem is it solving?

It allows the use of new finfo(). Previously, the following error occurred :

node node_modules/.bin/cli test.php

> Fatal error: Uncaught Error: Class "finfo" not found in /mho22/works/php-wasm-issue/test.php:3
> Stack trace:
> #0 {main}
>   thrown in /mho22/works/php-wasm-issue/test.php on line 3

Testing Instructions

1. mkdir php-wasm-issue && cd php-wasm-issue

2. npm install @php-wasm/cli

3. touch test.php :

<?php

$finfo = new finfo( FILEINFO_MIME );

echo json_encode( $finfo );

4. node node_modules/.bin/cli test.php

[adamziel]

packages/php-wasm/web/public/light/8_3_0/php_8_3.wasm is 5.6M on trunk but 6.9M on this branch. It seems like a few more PHP extensions should be disabled to get the original bundle + finfo.

[adamziel]

re: failing test – WP-CLI seems to work on the web:

In the CLI the output I get is

#!/usr/bin/env php
Success: Created post 4.

Which is the same as on trunk and should match /Success: Created post/. Perhaps that test needs to be adjusted.

[mho22]

@adamziel It seems the error with the wp-cli test is linked with the POSIX extension. When I add --disable-posix and recompile php-wasm/node@8.0, every tests become green.

[adamziel]

@mho22 good find! Let's keep it disabled, then. I don't think it's enabled right now and determining the rationale and consequences of enabling it might require an entire discussion.

[mho22]

@adamziel Last test, Here are the differences when I run this from my local environment and from playground.wordpress.net :

await playground.run({ code: `<?php echo json_encode( get_loaded_extensions() ); ?>` }).text

localhost :

'["Core","date","pcre","sqlite3","zlib","bcmath","calendar","ctype","`fileinfo`","filter","hash","json","SPL","PDO","pdo_sqlite","session","`posix`","Reflection","standard","Phar","tokenizer","zip"]'

playground :

'["Core","date","pcre","sqlite3","zlib","bcmath","calendar","ctype","filter","hash","json","SPL","PDO","pdo_sqlite","session","Reflection","standard","Phar","tokenizer","zip"]'

The only difference from localhost is fileinfo and posix. So I added --disable-posix based on your latest comment and decided to recompile light version to check for the file size : npm run recompile:web:light:8.0.

Unfortunately, the size increased from 5.6 MB to 6.7 MB. What should I do?

[adamziel]

@mho22 interesting! Are there any other significant differences in phpinfo() that would explain it? Note you can navigate to /phpinfo.php in Playground "browser UI"

Or perhaps enabling fileinfo brings in an additional library? This could be confirmed by consulting the build script in php-src repo.

Another idea would be to build PHP with -g2 to preserve the symbol names (the Node.js build does that by default) and list all the symbols with one of the tools listed in https://github.com/vshymanskyy/awesome-wasm-tools/blob/main/README.md. In theory, there should be a significant delta between the previous and new wasm binary.

[mho22]

@adamziel Here are the main differences between the two PHP versions listed above [ localhost and playground.wordpress.net ] :

I suppose what is taking that much size is the libmagic library.

I added the -g2 option, and php_8_0.wasm recompiled to 7.8 MB. This was expected. I'll likely need further instructions to proceed. Whenever you have time.

P.S. : I removed --disable-iconv since I don't think there's big improvement in it.

[adamziel]

@mho22 good investigation, that makes sense! I'd recommend only shipping fileinfo and libmagic in the kitchen sink bundle to keep the "light" bundle small. It is a bit counter-intuitive as fileinfo is indeed a default PHP extension, but I'd rather keep the initial download size small.

An ultimate solution would be to dynamically load specific extensions and libraries – the XDebug explorations will enable just that. Eventually, these may even be automatically lazy-loaded.

[mho22]

@adamziel What are your thoughts on introducing a new ARG WITH_FINFO option that would exclude --disable-all for kitchen-sink and node ? This would be a temporary measure before potentially exploring Xdebugand implementing automatic lazy loading, because of the uncertainty about the timeline for these developments.

[adamziel]

Could we get rid of --disable-all entirely and disable finfo specifically? If yes, let's do that. If not, let's do what you proposed.

[mho22]

@adamziel The only way i see in the actual configuration is to add a WITH_FILEINFO only for light :

packages/php-wasm/compile/Dockerfile:

# Remove fileinfo if needed
RUN if [ "$WITH_FILEINFO" = "no" ]; \
    then \
        # light bundle should compile without fileinfo and libmagic
        echo -n ' --disable-fileinfo' >> /root/.php-configure-flags; \
    fi;

and in the web-light defaults :

packages/php-wasm/compile/build.js:

['web-light']: {
        WITH_FILEINFO: 'no'
},

This could be correct but it is weird to see only one WITH_ on web-light and only one condition where if *** = "no".

I wanted to have your opinion on those two options and wait for your decision. Before implementing it.

[adamziel]

WITH_FINFO could default to "no" and then kitchen sink and Node would flip it to yes, otherwise this looks great 👍

[mho22]

@adamziel Finally, the pull request is ready for review. I understand that this one may not be as ambitious as expected, but the next ones involving libcurl and dynamic extension loading will be more substantial I hope.

[adamziel]

Thank you! The light build still seems 700kb-1.2mb heavier so perhaps getting rid of —disable-all isn’t possible after all. Thank you for exploring that! I think your original idea with still using —disable-all plus a comment to document the context would work better for that. I know these wasm rebuilds are annoying, thank you for persevering!

[mho22]

@adamziel I made a mistake in the previous commit. It didn't add --disable-fileinfo on light. Everything is sorted out now, with a php_8_0.wasm file size of 5.6 MB, just like on the trunk branch. It turns out that the libmagic extension was indeed responsible for that space. Ready for review! Again!

Note: You may want to rerun the tests due to a timeout issue, which I don't believe is related to this pull request.

[adamziel]

This is fantastic @mho22, thank you for all your work here!