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

Curl extension for the Node.js build of PHP.wasm #1273

Merged
merged 10 commits into from
Apr 29, 2024
Merged

Curl extension for the Node.js build of PHP.wasm #1273

merged 10 commits into from
Apr 29, 2024

Conversation

mho22
Copy link
Contributor

@mho22 mho22 commented Apr 18, 2024

What is this PR doing?

This PR builds PHP with --with-libcurl option to support the curl extension.

It also changes two nuances in the overall PHP build process:

  • It replaces the select(2) function using -Wl,--wrap=select emcc option instead of patching PHP source code – this enables supporting asynchronous select(2) in curl without additional patches.
  • Brings the __wrap_select implementation more in line with select(2), add support for POLLERR.
  • Adds support for polling file descriptors that represent neither child processes nor streams in poll(2) – that's because libcurl polls /dev/urandom.

Builds on top of and supersedes #1133

Debugging Asyncify problems

The typical way of resolving Asyncify crashes didn't work during the work on this PR. Functions didn't come up in the error messages and even raw stack traces. The reasons are unclear.

The JSPI build of PHP was more helpful as it enabled logging the current stack trace in all the asynchronous calls, which quickly revealed all the missing ASYNCIFY_ONLY functions. This is the way to debug any future issues until we fully migrate to JSPI.

Testing Instructions

Confirm the CI checks pass. This PR ships a few new tests specifically targeting networking with curl.

Related resources

@mho22
Copy link
Contributor Author

mho22 commented Apr 18, 2024

I already tried some things but nothing was efficient enough. Here is what I did :

I explored the different methods called in the unreachable list given above. And I found out that some methods are called but not listed between ossl_connect_common such as Curl_infof or even Curl_debug that you can find in the screenshot.

So after a lot of trial and error, I added this new list in the Dockerfile between rand_status on line 501 and RAND_poll :

"_php_stream_set_option",\
"php_openssl_sockop_set_option",\

"Curl_infof",\
"Curl_debug",\
"ossl_connect_step1",\
"rand_enough",\

"RAND_get_rand_method",\
"RAND_OpenSSL",\
"RAND_get0_primary",\
"EVP_RAND_get_state",\
"SSL_CTX_new",\
"SSL_CTX_new_ex",\
"RAND_bytes",\
"rand_nopseudo_bytes",\
"rand_bytes",\

But it always stops at SSL_CTX_new. I am stuck here.

@adamziel
Copy link
Collaborator

I've spent some time tracing it down. These RAND_ functions come from libopenssl and I bet this is either because libopenssl isn't built with -Wl,--wrap=select, or libcurl is linked with libopenssl during the build (vs linking everything together in the final emcc call where the ASYNCIFY options are specified).

@adamziel
Copy link
Collaborator

Or maybe I'm wrong. I rebuilt both with the additional wrapping and it still didn't work. Also, the ASYNCIFY having effect on some Openssl functions tells me maybe the linking shouldn't matter after all. I'm going through the openssl source code to find even more functions to add to ASYNCIFY_ONLY.

@adamziel
Copy link
Collaborator

adamziel commented Apr 18, 2024

Aha, this got me to the next function call (at OPENSSL_init_ssl):

+"Curl_infof",\
+"Curl_debug",\
+"ossl_connect_step1",\
+"rand_enough",\
+"RAND_get_rand_method",\
+"RAND_OpenSSL",\
+"RAND_get0_primary",\
+"EVP_RAND_get_state",\
+"OPENSSL_init_ssl",\
+"lh_SSL_SESSION_new",\
+"CRYPTO_THREAD_lock_new",\
+"CRYPTO_NEW_REF",\
+"SSL_get_ex_data_X509_STORE_CTX_idx",\
+"OPENSSL_zalloc",\
+"OPENSSL_strdup",\
+"ssl_load_ciphers",\
+"X509_STORE_new",\
+"SSL_get_ex_data_X509_STORE_CTX_idx",\
+"ssl_load_groups",\
+"ssl_setup_sigalgs",\
+"ssl_cert_new",\
+"SSL_CTX_set_ciphersuites",\
+"sk_SSL_CIPHER_num",\
+"ssl_create_cipher_list",\
+"OSSL_default_cipher_list",\
+"OPENSSL_zalloc",\
+"X509_VERIFY_PARAM_new",\
+"CRYPTO_NEW_REF",\
+"ssl_evp_md_fetch",\
+"sk_X509_NAME_new_null",\
+"OPENSSL_free",\
+"CRYPTO_new_ex_data",\
+"CRYPTO_THREAD_lock_new",\
+"ERR_raise",\
+"OPENSSL_secure_zalloc",\
+"ossl_comp_has_alg",\
+"RAND_priv_bytes_ex",\
+"RAND_bytes_ex",\
+"SSL_CTX_set_client_cert_engine",\
+"ssl_ctx_srp_ctx_init_intern",\
+"SSL_CTX_new",\
+"SSL_CTX_new_ex",\
+"RAND_bytes",\
+"rand_nopseudo_bytes",\
+"rand_bytes",\

So it's going to be all about finding the right functions manually. Yikes! I wonder why aren't these functions showing up in the stack traces, it would've simplified a lot.

@mho22
Copy link
Contributor Author

mho22 commented Apr 18, 2024

@adamziel This looks like a job for me ! I found out the next step OPENSSL_init_crypto by adding :

+ "ERR_raise",\
+ "OPENSSL_init_crypto",\
+ "RUN_ONCE",\
+ "RUN_ONCE_ALT",\

@adamziel
Copy link
Collaborator

Yay! I couldn't crack the next one, tossing some printf() statements in there or stepping through the execution in chrome devtools might reveal where it crashes. I think @brandonpayton got that to work by building PHP with -g3 or -g4 to produce a separate DWARF file and then loading it in Chrome Canary devtools with the CPP/WASM extension installed. We've had a conversation about that somewhere in #1199 or #1128.

@mho22
Copy link
Contributor Author

mho22 commented Apr 18, 2024

@adamziel I tried to printf things in files from openssl by copying these files into the openssl lib before make libopenssl but nothing worked. It seems like nothing is printed.

It worked before in curl because I had a method called infof using a given context to print in console.
To stay on my current workspace and my separated node project I don't know how to print things in console.

Any idea ? Your suggestions above could work with kitchen-sink and web but not node right ?

@mho22
Copy link
Contributor Author

mho22 commented Apr 18, 2024

@adamziel I found out something extremely strange to me while trying things you suggested :

If I replace line 1015 in Dockerfile : emcc $OPTIMIZATION_FLAGS \

with -03 I get an unreachable error :

     * __wrap_select
      * RAND_poll
      * rand_status
      * RAND_status
      * Curl_ossl_seed
      * ossl_connect_common
      * Curl_ossl_connect_nonblocking
      * Curl_ssl_connect_nonblocking
      * https_connecting
      * Curl_http_connect
      * curl_multi_perform
      * zif_curl_exec
      * ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER
      * execute_ex
      * zend_execute
      * zend_execute_scripts
      * php_execute_script
      * do_cli
      * run_cli
      
RuntimeError: unreachable
    at OPENSSL_init_crypto (wasm://wasm/03565e26:wasm-function[980]:0x8fac1)
    at OPENSSL_init_ssl (wasm://wasm/03565e26:wasm-function[6882]:0x45e34f)
    at SSL_CTX_new (wasm://wasm/03565e26:wasm-function[4461]:0x2da5bb)
    at ossl_connect_common (wasm://wasm/03565e26:wasm-function[5909]:0x3ce9e8)
    at Curl_ossl_connect_nonblocking (wasm://wasm/03565e26:wasm-function[14817]:0x7069c2)

The current one we face. But if I replace this by -O0 I get this unreachable error :

      * __wrap_select
      * Curl_socket_check
      * Curl_is_connected
      * multi_runsingle
      * curl_multi_perform
      * curl_easy_perform
      * zif_curl_exec
      * ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER
      * execute_ex
      * zend_execute
      * zend_execute_scripts
      * php_execute_script
      * do_cli
      * main
      * run_cli
      
      RuntimeError: unreachable
    at asyncify_stop_unwind (wasm://wasm/04f5d2aa:wasm-function[19331]:0x8bc83e)
    at ret.<computed> (packages/php-wasm/node/index.cjs:33227:33)
    at runtime.asm.<computed> (packages/php-wasm/node/index.cjs:72081:18)
    at _asyncify_stop_unwind (packages/php-wasm/node/index.cjs:34065:76)
    at runAndAbortIfError (packages/php-wasm/node/index.cjs:33183:14)
    at Object.maybeStopUnwind (packages/php-wasm/node/index.cjs:33272:9)
    at ret.<computed> (packages/php-wasm/node/index.cjs:33232:28)
    at runtime.asm.<computed> (packages/php-wasm/node/index.cjs:72081:18)
    at Module._run_cli (packages/php-wasm/node/index.cjs:33801:71)

The OPTIMIZATION FLAGS are also influencing the called methods... ?

@adamziel @bgrgicak @brandonpayton At this point I probably need some insights from you wordpress-playground specialists. 😁

@adamziel
Copy link
Collaborator

Some functions probably get inlined with -O3 and don't show up in the stack trace. I'm confused, though, why it's so different.

@bgrgicak
Copy link
Collaborator

Some functions probably get inlined with -O3 and don't show up in the stack trace. I'm confused, though, why it's so different.

I remember that @adamziel and I experienced the same behavior while working on another task, but we couldn't find out why that was happening.

@adamziel
Copy link
Collaborator

@adamziel I tried to printf things in files from openssl by copying these files into the openssl lib before make libopenssl but nothing worked. It seems like nothing is printed.

We redirect Emscripten stdout stream to /internal/stdout and PHP reads the output from there, unless it crashes of course. It's a good question why that output was visible in the browser – I'd have to step through the code to answer as I'm not sure off the top of my head. One way to explain it would be that infof function still somehow wrote information to stdout, in which case Emscripten console.logs it.

There's two things you could do from here:

  1. Comment out the stdout redirection and inspect the script output in the console
  2. Debug in the browser, even though the target environment here is Node.js, and once it works there rebuild the Node.js version with the same Dockerfile – it should just work 🤞

@mho22
Copy link
Contributor Author

mho22 commented Apr 19, 2024

@adamziel I made some tests in browser, alongside the message Trying 172.29.1.0:443... I added methods like :

printf( "This should be printed using printf \n" );
fprintf( stdout, "This should be printed using fprintf in stdout \n" );
fprintf( stderr, "This should be printed using fprintf in stderr \n" );

The two first prints showed before Trying 172.29.1.0:443.... So, in browser we could simply use printf or fprintf to debug or investigate in files. On the other hand, these prints are not visible when using php-wasm:node. Nothing is displayed except Trying 172.29.1.0:433... .

The problem is, when I am on browser, I have a different error as in node.
This is the message I get when running curl.php within the browser :

* Trying 172.29.1.0:443... * Immediate connect fail for 172.29.1.0: Host is unreachable * Closing connection 0 bool(false) string(0) ""

No unreachable error indeed. Am I missing something ? Is there a way to debug node in browser that I am not aware of ? It sounds cristal clear when you talk about this but I am probably missing a lot of things here.

I also tried to comment out these lines earlier :

stdout_replacement = redirect_stream_to_file(stdout, "/internal/stdout");
stderr_replacement = redirect_stream_to_file(stderr, "/internal/stderr");
if (stdout_replacement == -1 || stderr_replacement == -1)
{
        return -1;
}

but nothing different happened in terminal. My main goal here is to find out how to print anything in the terminal when using php-wasm:node. What works in browser [ printf or fprintf ] looks like not working in node.

@adamziel
Copy link
Collaborator

The problem is, when I am on browser, I have a different error as in node.
This is the message I get when running curl.php within the browser :

  • Trying 172.29.1.0:443... * Immediate connect fail for 172.29.1.0: Host is unreachable * Closing connection 0 bool(false) string(0) ""
    No unreachable error indeed. Am I missing something

Just to be sure – are you reproducing the same setup as here? Or did that also work without the unreachable error? If yes, it's a curious case where the networking workaround we use in the browser changes something about the code execution flow as compared to node.

Is there a way to debug node in browser that I am not aware of ? It sounds cristal clear when you talk about this but I am probably missing a lot of things here.

I may have more historical context, but I'm far from being a WASM expert and am just as confused as you :D

@mho22
Copy link
Contributor Author

mho22 commented Apr 19, 2024

@adamziel I am sorry about that, I thought you digged in the files changed in this PR : I added the strict necessary in this PR to reproduce the error I had in the Explore curl support PR for node so I avoided every aspect of networking. You said it would not be necessary for node.

I wanted to have an environment as "light" as possible compared to the other PR. I suppose Host is unreachable on desktop is logical here. What I wanted to highlight is the fact that with kitchen-sink and node, the same PHP code will behave differently.

When on desktop everything seems to be working in ASYNCIFY_ONLY, some things are missing in node.

@adamziel
Copy link
Collaborator

I am sorry about that, I thought you digged in the files changed in this PR

I didn't get a chance to review in details at that time, sorry, I was just basing on the conversation here.

I added the strict necessary in this PR to reproduce the error I had in the Explore curl support PR for node so I avoided every aspect of networking. You said it would not be necessary for node.

👍 Node has its own networking support, it opens a WebSocket to a local server that translates the received traffic to a raw TCP socket.

I wanted to have an environment as "light" as possible compared to the other PR. I suppose Host is unreachable on desktop is logical here.

Yeah without that network handler it's very likely that curl figures out it cannot open the socket and short-circuits before the line where the crash happens. Debugging this in the browser without that HTTPS handler might not be possible.

What I wanted to highlight is the fact that with kitchen-sink and node, the same PHP code will behave differently.

This is weird and I don't have a good answer. The easiest way to debug this, then, might not be via prints but attaching a debugger and stepping through the execution.

When on desktop everything seems to be working in ASYNCIFY_ONLY, some things are missing in node.

I don't think the code execution gets to the point where it calls the same functions that crash on node.

@mho22
Copy link
Contributor Author

mho22 commented Apr 23, 2024

@adamziel I don't think node reaches the same steps than kitchen-sink, I think it throws the unreachableerror before. In fact, if it was working better than on kitchen-sink it would have called wasm_poll_socket() multiple times after connection. But it is not. It stops after this line :

* Connected to wordpress.org (172.29.1.0) port 443 (#0).

When kitchen-sink goes we further like on the first screenshot of this other PR's message.

But anyways. How, and thank you for your patience explaining these things to me but, how could I use that debugger and step through the methods used in C while using the node version [ based on my observation above, thinking that node has its own unreachable methods stack ] ?

@adamziel
Copy link
Collaborator

But anyways. How, and thank you for your patience explaining these things to me but, how could I use that debugger and step through the methods used in C while using the node version [ based on my observation above, thinking that node has its own unreachable methods stack ] ?

I had some luck with the VS Code debugger by running a test script via its debug configurations. I never got it to step through the actual C source code, though, it was always wasm bytecode. I know @brandonpayton got the source map and symbol names to work in the browser so I'm CCing him for insights. Perhaps you could build PHP with a separate DWARF, plug it in some VS Code panel, and have it "just work"?

@mho22
Copy link
Contributor Author

mho22 commented Apr 23, 2024

@adamziel I managed to generate the sourcemaps of php-wasm:node@8.0 simply by adding WITH_SOURCEMAPS: 'yes' and this is quite impressive :

Capture d’écran 2024-04-23 à 16 25 35

Unfortunately a lot of files are missing, and it seems like the files are also missing a lot of functions.

Looking at file php.wasm.map I can see that the word curl is only displayed here :

"php-src/ext/curl/multi.c","php-src/ext/curl/curl_private.h","php-src/ext/curl/share.c","php-src/ext/curl/curl_file.c","php-src/ext/curl/interface.c"

Meaning the only curl files available in sourcemaps are those used by php-src. So no propre curl extension files here.

I assume this is still interesting. @brandonpayton I hope you could help here.

@brandonpayton
Copy link
Member

brandonpayton commented Apr 23, 2024

I know @brandonpayton got the source map and symbol names to work in the browser so I'm CCing him for insights. Perhaps you could build PHP with a separate DWARF, plug it in some VS Code panel, and have it "just work"?

When I was debugging, I only enabled DWARF symbols and not source maps, but it seems like they could co-exist with no issues. Unfortunately, when I was looking, utilizing DWARF symbols only worked with Chrome Canary with the CPP/WASM extension installed.

I made a note to experiment with debugging in VS Code and will let you know if I find anything more.

Unfortunately a lot of files are missing, and it seems like the files are also missing a lot of functions.
...snip...
Meaning the only curl files available in sourcemaps are those used by php-src. So no propre curl extension files here.

I assume this is still interesting. @brandonpayton I hope you could help here.

@mho22 In this case, when libcurl.a is built with Emscripten, is debug info included? I'm not sure if Emscripten will leverage that or not, but if libcurl.a isn't built with debug info, then it does not seem Emscripten has any place to find debug info when building php-wasm:node.

🤔 @adamziel I wonder whether we could build all our pre-built libs with debug info and trust Emscripten to exclude it when linking normal php-wasm builds. Then, if debug info is desired and configured in the php-wasm build, maybe it would just be included from the pre-built libs automatically.

@adamziel
Copy link
Collaborator

🤔 @adamziel I wonder whether we could build all our pre-built libs with debug info and trust Emscripten to exclude it when linking normal php-wasm builds. Then, if debug info is desired and configured in the php-wasm build, maybe it would just be included from the pre-built libs automatically.

If the linker can do tree shaking, minification etc and get us the same bundle size regardless of the optimized/unoptimized inputs, I'm all for this! Otherwise we could ship "prod" and "dev" libraries in the repo.

@mho22
Copy link
Contributor Author

mho22 commented Apr 24, 2024

@brandonpayton @adamziel I followed these steps :

  1. I added WITH_SOURCEMAPS: 'yes', in compile/build.jsline135objectnode`.

-> This added the 2 files :

packages/php-wasm/node/public/8_0_30/php-src/
packages/php-wasm/node/public/8_0_30/php.wasm.map

But only 7 occurences of curl were found in php.wasm.map file.

"php-src/ext/curl/multi.c","php-src/ext/curl/curl_private.h","php-src/ext/curl/share.c","php-src/ext/curl/curl_file.c","php-src/ext/curl/interface.c"
  1. I added --enable-debug option in libcurl/Dockerfile line 24. And ran command make libcurl.

-> After php-wasm:node recompiled, this added 93 other occurences of the word curl in php.wasm.map file :

"curl-7.69.1/lib/version.c","curl-7.69.1/lib/escape.c","curl-7.69.1/lib/mprintf.c","curl-7.69.1/lib/sendf.c","curl-7.69.1/lib/strcase.c","curl-7.69.1/lib/cookie.c","curl-7.69.1/lib/getinfo.c","curl-7.69.1/lib/timeval.c","curl-7.69.1/lib/hostip.c","curl-7.69.1/lib/progress.c"...

But right now, when I run my script node --stack-trace-limit=100 cli.js curl.php I don't get unreachable error anymore but this :

Capture d’écran 2024-04-24 à 12 26 44
  1. When I read the php.wasm.map and the different occurences of curl, I see these things :
php-src/Zend/zend_string.c      // Returns the correct path for the file `zend_string` in the `php-src` folder
curl-7.69.1/lib/version.c        // There is no directory named `curl-7.69.1`  

So, I decided to build this directory. How ? By adding this line in compile/Makefile at line 97 and run make libcurl:

docker cp $$(docker create playground-php-wasm:libcurl):/root/curl-7.69.1 ./libcurl/dist/

Now I have libcurl/dist/root and libcurl/dist/curl-7.69.1

And these lines in compile/Dockerfile :

line 37 : COPY ./libcurl/dist/curl-7.69.1/ /libs/curl-7.69.1
line 1024 : cp -r /libs/curl-7.69.1 "/root/output/${PHP_VERSION_ESCAPED}/curl-7.69.1"; \

After recompiling and building node I get this :

Capture d’écran 2024-04-24 à 12 32 17

So I decide to debug in VS Code and add a marker on the first line of method multi_runsingle on line 1513 from file curl-7.69.1/lib/multi.c I now it passes through because of this previous error stack :

 * __wrap_select
 * Curl_socket_check
 * Curl_is_connected
 * multi_runsingle
 * curl_multi_perform
 * curl_easy_perform
 * zif_curl_exec
 * ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER
 * execute_ex
 * zend_execute
 * zend_execute_scripts
 * php_execute_script
 * do_cli
 * main
 * run_cli
Capture d’écran 2024-04-24 à 12 18 25

Nothing. But I still get two infos :

* STATE: INIT => CONNECT handle 0x1048a18; line 1619 (connection #-5000)

// line 1619 => multistate(data, CURLM_STATE_CONNECT);

* STATE: CONNECT => WAITCONNECT handle 0x1048a18; line 1675 (connection #0)

// line 1675 => multistate(data, CURLM_STATE_WAITCONNECT);

multistate calls in fact mstate from line 120 of multi.c

This indicates it goes logically through the steps but after step WAITCONNECT, invalid state: 1 is printed.

But what now? What can we do to make the debug work correctly in VSCode? Should I push my current code on this PR ?

@adamziel
Copy link
Collaborator

Invalid state looks like an Asyncify error so we may be getting somewhere:

abort(`invalid state: ${Asyncify.state}`);

1 is "Unwinding" so an unexpected handleSleep was called while the stack was being restored. It would be super useful to see what the JavaScript stack trace was at the time of the crash, or a little while before it.

@mho22
Copy link
Contributor Author

mho22 commented Apr 24, 2024

@adamziel It looks like it is the handleSleep() method called from emscripten_sleep() that is called from __wrap_select() causes this invalid state. I commented out emscripten_sleep(0) from wrap_select and got another invalid state provoked here by the wasm_poll_socket's handlesleep method.

I had to add if (crypticError.message === "unreachable" || crypticError.message === 'null function or function signature mismatch' ) { To get the stack trace and voila :

Capture d’écran 2024-04-24 à 15 37 46

Two new ASYNCIFY_ONLY methods found :

"easy_perform",\
"easy_transfer",\

Let's investigate further.

@mho22
Copy link
Contributor Author

mho22 commented Apr 24, 2024

@adamziel New step :

* STATE: INIT => CONNECT handle 0x1048a18; line 1619 (connection #-5000)
* Added connection 0. The cache now contains 1 members
*   Trying 172.29.1.0:443...
* STATE: CONNECT => WAITCONNECT handle 0x1048a18; line 1675 (connection #0)
* Connected to wordpress.org (172.29.1.0) port 443 (#0)
* STATE: WAITCONNECT => SENDPROTOCONNECT handle 0x1048a18; line 1795 (connection #0)
* Marked for [keep alive]: HTTP default

But we now have a new unreachableerror with a new stack trace :

      * __wrap_select
      * RAND_poll
      * rand_status
      * RAND_status
      * rand_enough
      * Curl_ossl_seed
      * ossl_connect_step1
      * ossl_connect_common
      * Curl_ossl_connect_nonblocking
      * Curl_ssl_connect_nonblocking
      * https_connecting
      * Curl_http_connect
      * protocol_connect
      * multi_runsingle
      * curl_multi_perform
      * easy_transfer
      * easy_perform
      * curl_easy_perform
      * zif_curl_exec
      * ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER
      * execute_ex
      * zend_execute
      * zend_execute_scripts
      * php_execute_script
      * do_cli
      * main
      * run_cli

I added the new elements from this stack trace :

rand_enough
ossl_connect_step1

But this won't change the new stack trace. Stuck again. What we can define is the step where we are :

* Marked for [keep alive]: HTTP default

Is called from connkeep(conn, "HTTP default"); on line 1466 in Curl_http_connect function from http.c file.

Next step in the stack trace is result = https_connecting(conn, done); on line 1495.

@mho22 mho22 requested a review from a team as a code owner April 28, 2024 20:12
@adamziel
Copy link
Collaborator

adamziel commented Apr 28, 2024

@mho22 I think PHP 8.0 unit tests will pass now. I'm happy to merge this PR as soon as it ships all the rebuilt PHP versions with green unit tests and a few tests to confirm curl works.

I found the missing functions that tripped up the multisite unit tests by doing this on the jspi branch (cc @brandonpayton – thank you for that excellent idea).

cd packages/playground/blueprints
node --experimental-wasm-stack-switching ../../../node_modules/.bin/vitest src/lib/steps/enable-multisite.spec.ts

And by adding this bit to the enable-multisite.spec.ts:

		for (const fn of global.asyncifyFunctions) {
			console.log(`"${fn}",`);
		}

@mho22
Copy link
Contributor Author

mho22 commented Apr 29, 2024

@adamziel I am trying to build Curl for 7.3 version and I get the following error [ I suppose you also tried it but I prefer being on the same page here ] :

#42 44.43 checking for cURL support... yes
#42 44.43 checking for libcurl.pc... not found
#42 44.43 configure: WARNING: Could not find libcurl.pc. Try without /root/lib or set PKG_CONFIG_PATH
#42 44.43 configure: WARNING: Fallback: search for curl headers and curl-config
#42 44.43 checking for cURL 7.15.5 or greater... ./configure: line 26682: curl-config: command not found
#42 44.44 configure: error: cURL version 7.15.5 or later is required to compile php with cURL support

While with 7.4 :

#42 39.49 checking for cURL support... yes
#42 39.49 checking for libcurl >= 7.15.5... yes

Something is probably missing in 7.3

@adamziel
Copy link
Collaborator

PHP 7.3 uses the following check:

    if $PKG_CONFIG --atleast-version 7.15.5 $PKNAME; then
      curl_version_full=`$PKG_CONFIG --modversion $PKNAME`
      { printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $curl_version_full" >&5
printf "%s\n" "$curl_version_full" >&6; }
    else
      as_fn_error $? "cURL version 7.15.5 or later is required to compile php with cURL support" "$LINENO" 5
    fi

PHP 7.4, however, doesn't seem to test for the --atleast-version. I'll try removing that check.

@mho22
Copy link
Contributor Author

mho22 commented Apr 29, 2024

@adamziel I don't think this will solve the issue. It seems on PHP 7.3 file config.m4, that it first check if there is a PKNAME in :

PKNAME=$PHP_CURL/$PHP_LIBDIR/pkgconfig/libcurl.pc
PKNAME=$PHP_CURL/lib/pkgconfig/libcurl.pc

if not, then it will avoid the code you pasted above, and there is no PKNAME because this message is printed : Could not find libcurl.pc. Try without $PHP_CURL or set PKG_CONFIG_PATH

The next message printed is Fallback: search for curl headers and curl-config

And the last message cURL version 7.15.5 or later is required to compile php with cURL support is printed because CURL_CONFIG is missing.

     if ${CURL_DIR}/bin/curl-config --libs > /dev/null 2>&1; then
      CURL_CONFIG=${CURL_DIR}/bin/curl-config
    else
      if ${CURL_DIR}/curl-config --libs > /dev/null 2>&1; then
        CURL_CONFIG=${CURL_DIR}/curl-config
      fi
    fi

While in PHP 7.4.33 this line seems to be enough and works as expected :

PKG_CHECK_MODULES([CURL], [libcurl >= 7.15.5])

We then should probably try to add libcurl.pc into pkgconfig or add curl-config in /root/lib/ or root/lib/bin. Or maybe comment a lot of lines in PHP 7.3.

@adamziel
Copy link
Collaborator

adamziel commented Apr 29, 2024

@mho22 I wrote the libcurl.pc by hand. It satisfied one check, but the version check kept failing. I removed that version check, too, and the build worked for all PHP versions! :) I found one more ASYNCIFY_ONLY entry, added a fee more unit tests, and I'm rebuilding everything right now.

@mho22
Copy link
Contributor Author

mho22 commented Apr 29, 2024

@adamziel Haha fantastic!

@adamziel adamziel changed the title php-wasm:node with Curl extension Curl extension for the Node.js build of PHP.wasm Apr 29, 2024
This was referenced Apr 29, 2024
@adamziel adamziel merged commit 3fd0315 into WordPress:trunk Apr 29, 2024
5 checks passed
@adamziel
Copy link
Collaborator

Let's ship! 🎉

@mho22 mho22 deleted the node-with-curl-extension-patch branch April 29, 2024 12:37
@brandonpayton
Copy link
Member

🌟 Awesome work, @mho22 and @adamziel!

@bgrgicak
Copy link
Collaborator

bgrgicak commented May 2, 2024

Wow 😮

Thank you @mho22 and @adamziel, great work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants