Conversation
|
Using ext-grpc to compile all grpc dependencies looks not good for our build procedure. First of all, it's too slow for building static php binary every time. Building separately could reduce a lot of time when debugging and publishing. And another potential issue is building openssl, libcares and zlib twice. And also, although we can make a patch for boringssl and openssl built by grpc source to solve the CI error above, it often leads to misleading errors when building in-tree ext-grpc with all dependencies. |
|
I know, but the extension is just shit. There's nothing we can do about it, short of completely replacing the extension build logic. We built the library standalone before, but the extension build doesn't use prebuilt libraries, it builds every time no matter what. |
| $config_m4 = <<<'M4' | ||
| PHP_ARG_ENABLE(grpc, [whether to enable grpc support], [AS_HELP_STRING([--enable-grpc], [Enable grpc support])]) | ||
|
|
||
| if test "$PHP_GRPC" != "no"; then | ||
| PHP_ADD_INCLUDE(PHP_EXT_SRCDIR()/include) | ||
| PHP_ADD_INCLUDE(PHP_EXT_SRCDIR()/src/php/ext/grpc) | ||
| GRPC_LIBDIR=@@build_lib_path@@ | ||
| PHP_ADD_LIBPATH($GRPC_LIBDIR) | ||
| PHP_ADD_LIBRARY(grpc,,GRPC_SHARED_LIBADD) | ||
| LIBS="-lpthread $LIBS" | ||
| PHP_ADD_LIBRARY(pthread) | ||
|
|
||
| case $host in | ||
| *darwin*) | ||
| PHP_ADD_LIBRARY(c++,1,GRPC_SHARED_LIBADD) | ||
| ;; | ||
| *) | ||
| PHP_ADD_LIBRARY(stdc++,1,GRPC_SHARED_LIBADD) | ||
| PHP_ADD_LIBRARY(rt,,GRPC_SHARED_LIBADD) | ||
| PHP_ADD_LIBRARY(rt) | ||
| ;; | ||
| esac | ||
|
|
||
| PHP_NEW_EXTENSION(grpc, @grpc_c_files@, $ext_shared, , -DGRPC_POSIX_FORK_ALLOW_PTHREAD_ATFORK=1) | ||
| PHP_SUBST(GRPC_SHARED_LIBADD) | ||
| PHP_INSTALL_HEADERS([ext/grpc], [php_grpc.h]) | ||
| fi | ||
| M4; |
There was a problem hiding this comment.
Very ugly, but the best we can do at this point. We will need to keep it in sync with version changes, though.
There was a problem hiding this comment.
Yes, and for now, unless it changes the interface, we can tolerate it adding new files. It's ugly, but much simpler than patching the original files.
| // '8.2', | ||
| // '8.3', |
There was a problem hiding this comment.
did you test for 8.3 too? 8.2 can be ignored.
There was a problem hiding this comment.
I even tested it locally on version 8.0, and this m4 basically has maximum compatibility.
2 participants
What does this PR do?
Closes #1054
Checklist before merging
*.phpor*.json, run them locally to ensure your changes are valid:composer cs-fixcomposer analysecomposer testbin/spc dev:sort-configsrc/globals/test-extensions.php.extension testortest extensionsto trigger full test suite.