nginx / unit

NGINX Unit - universal web app server - a lightweight and versatile open source server that simplifies the application stack by natively executing application code across eight different programming language runtimes.
https://unit.nginx.org
Apache License 2.0
5.37k stars 323 forks source link

./configure: error: no PHP embed SAPI found when there is no libphp.so #1163

Closed vt-alt closed 7 months ago

vt-alt commented 7 months ago

On ALT Linux (which allows multiple php versions, currently 8.1, 8.2, and 8.3):

00:34:40 configuring PHP module
00:34:40 checking for PHP ... found
00:34:40  + PHP SAPI: [cli info phpdbg]
00:34:40 checking for PHP version ... not found
00:34:40 checking for PHP embed SAPI ... not found
00:34:40
00:34:40 ./configure: error: no PHP embed SAPI found.

This is because of this heuristic in auto/modules/php:

        if [ $NXT_PHP_MAJOR_VERSION -ge 8 ]; then
            NXT_PHP_LIB="-lphp"
        else
            NXT_PHP_LIB="-lphp${NXT_PHP_VERSION%%.*}"
        fi

We have NXT_PHP_MAJOR_VERSION 8 but library is /usr/lib64/libphp-8.1.27.so so -lphp-8.1.27 should be used.

Can you please add parameter to override that value from configure? Like for example --libphp-ldflags=-lphp-8.1.27

thresheek commented 7 months ago

Hi @vt-alt !

It looks like php-embed is specifically disabled in the php-8.3 ALT Linux build:

https://packages.altlinux.org/en/sisyphus/srpms/php8.3/specfiles/#line-246

You'd need to enable it for Unit to be able to pick up - enabling it will create another shared library to link with. I didnt do a deep dive into patches you have on top of PHP 8.3, and maybe if you change the sonames here and there we might indeed need to have some fixes within auto/, but --enable-embed is a hard prereq.

(and with my ALT Linux developer hat) It's probably best to package it simiarly to php8.3-fpm-fcgi, with a separate embed sapi: https://packages.altlinux.org/en/sisyphus/srpms/php8.3-fpm-fcgi/specfiles/

vt-alt commented 7 months ago

IC. I am not expert in PHP intricacies, but I don't see your code calling PHP embed API, such as php_embed_init.

Our previous build of Unit with libphp (was patching auto/modules/php to add hardcoded NXT_PHP_LIB=-lphp-PHP_VERSION) was successful but I did not test it to actually work (I wonder what php_sapi_name it will return). If this was incorrect approach we should first disable building PHP Unit module until our php have --enable-embed, am I right?

thresheek commented 7 months ago

Ah!

Technically, I think we expect the embed SAPI since in the upstream PHP only this SAPI will provide a shared library to link with. I take it ALT heavily modifies PHP build system to produce it even for e.g. cli SAPI and then links extensions and an actual CLI with it.

I think none of the major distros do it this way though, which explains we it just works everywhere else with the php-embed package or a similar one.

With that, I don't think we need to support it with Unit, but it might be a nice touch. I tend to think if distro maintainers modify their packages heavily (as they have a full right to do so), they should expect those patches to be not-upstreamable since they don't really apply for other situations - meaning just keep a simple patch in ALT's Unit package.

vt-alt commented 7 months ago

Our PHP does not have Embed SAPI but we have multiple libphp named libphp-VERSION.so.

Can you please clarify if we should not link Unit with libphp without embed SAPI (which you don't even call)? In that case I will remove PHP module support from our Unit build.

If you only need libphp but wont patch auto/modules/php to support it I will take it, and we will continue to build with it (as before). (Maybe rethink packaging similar to php8.3-fpm-fcgi.)

ac000 commented 7 months ago

We do this check

nxt_feature="PHP embed SAPI"
nxt_feature_name=""
nxt_feature_run=no
nxt_feature_incs="${NXT_PHP_INCLUDE}"
nxt_feature_libs="${NXT_PHP_LIB} ${NXT_PHP_LDFLAGS}"
nxt_feature_test="
    #include <php.h>
    #include <php_main.h>

    int main(void) {
    #if (PHP_VERSION_ID < 80200)
        php_module_startup(NULL, NULL, 0);
    #else
        php_module_startup(NULL, NULL);
    #endif
        return 0;
    }"

. auto/feature

if [ $nxt_found = no ]; then
    $echo
    $echo $0: error: no PHP embed SAPI found.
    $echo
    exit 1;
fi

Our PHP language module calls that function.

vt-alt commented 7 months ago

But not php_embed_init nor PHP_EMBED_START_BLOCK. See for example https://github.com/php/php-src/blob/master/sapi/embed/README.md how embed SAPI is supposed to be used. Basically your code never initializes embed SAPI and just plays with PHP internals directly. With this I believe it does not require Embed SAPI and only needs shared library.

Thanks for your answers!

ac000 commented 7 months ago

Whether it needs whatever is called "Embedded SAPI" or not, it needs the php_module_startup() function which you don't seem to have.

vt-alt commented 7 months ago

Yes, we have it.

sisyphus x86_64 php8.1-libs-8.1.27-alt1.x86_64.rpm /usr/lib64/libphp-8.1.27.so php_module_startup:T
sisyphus x86_64 php8.3-libs-8.3.2-alt1.x86_64.rpm /usr/lib64/libphp-8.3.2.so php_module_startup:T
sisyphus x86_64 php8.2-libs-8.2.15-alt1.x86_64.rpm /usr/lib64/libphp-8.2.15.so php_module_startup:T

Otherwise, we would not be able to build a Unit with PHP support.

Our previous build of Unit with libphp (was patching auto/modules/php to add hardcoded NXT_PHP_LIB=-lphp-PHP_VERSION) was successful

ps. This issue was opened not to allow building Unit with PHP on ALT, but just to make it without patching auto/modules/php, but I see this is not feasible.

ac000 commented 7 months ago

Your simplest solution is to just symlink libphp.so to whichever version you have the headers installed for. You can get more more creative from there...

Although this check (that you pointed out)

else
        if [ $NXT_PHP_MAJOR_VERSION -ge 8 ]; then
            NXT_PHP_LIB="-lphp"
        else
            NXT_PHP_LIB="-lphp${NXT_PHP_VERSION%%.*}"
        fi

introduced by

commit 140b81208e83569913aa81f964eb64e15940d897
Author: Remi Collet <remi@remirepo.net>
Date:   Wed May 20 11:18:03 2020 +0300

    PHP: building with PHP 8 (development version).

diff --git a/auto/modules/php b/auto/modules/php
index e2e5498a..2cec2f44 100644
--- a/auto/modules/php
+++ b/auto/modules/php
@@ -100,7 +100,11 @@ if /bin/sh -c "${NXT_PHP_CONFIG} --version" >> $NXT_AUTOCONF_ERR 2>&1; then
                          `${NXT_PHP_CONFIG} --libs`"

     else
-        NXT_PHP_LIB="-lphp${NXT_PHP_VERSION%%.*}"
+        if [ $NXT_PHP_MAJOR_VERSION -ge 8 ]; then
+            NXT_PHP_LIB="-lphp"
+        else
+            NXT_PHP_LIB="-lphp${NXT_PHP_VERSION%%.*}"
+        fi

         if [ "$NXT_PHP_LIB_PATH" != "" ]; then
             # "php-config --ldflags" does not contain path to libphp, but
diff --git a/src/nxt_php_sapi.c b/src/nxt_php_sapi.c
index ccbdd475..d3c23c31 100644
--- a/src/nxt_php_sapi.c
+++ b/src/nxt_php_sapi.c
@@ -29,6 +29,14 @@
 #define NXT_PHP7 1
 #endif

+/* PHP 8 */
+#ifndef TSRMLS_CC
+#define TSRMLS_CC
+#define TSRMLS_DC
+#define TSRMLS_D  void
+#define TSRMLS_C
+#endif
+

 typedef struct {
     nxt_str_t  root;

seems a little dubious to me. It seems to be making some assumptions and seems more tailored to some particular setup.

I'd be tempted to remove that check, not that that will help your case.

However I think you may be able to hack around the problem like

$ ./configure php --lib-path="/usr/lib64 -l:libphp-8.3.2.so -Wl,-w"

if it can find the right header files...

For example

$ ls -l /usr/lib64/libphp*
-rwxr-xr-x 1 root root 5512584 Jan 16 00:00 /usr/lib64/libphp-8.2.so
lrwxrwxrwx 1 root root      13 Jan 16 00:00 /usr/lib64/libphp.so -> libphp-8.2.so
# I'll remove the symlink so -lphp won't work...
$ sudo rm /usr/lib64/libphp.so
$ ./configure php
configuring PHP module
checking for PHP ... found
 + PHP SAPI: [apache2handler litespeed fpm phpdbg cli embed cgi]
checking for PHP version ... not found
checking for PHP embed SAPI ... not found

./configure: error: no PHP embed SAPI found.
# Now lets try the above hack...
$ ./configure php --lib-path="/usr/lib64 -l:libphp-8.2.so -Wl,-w"
configuring PHP module
checking for PHP ... found
 + PHP SAPI: [apache2handler litespeed fpm phpdbg cli embed cgi]
checking for PHP version ... 8.2.15
checking for PHP embed SAPI ... found
checking for PHP Zend Thread Safety ... not found
checking for PHP zend_signal_startup() ... found
 + PHP module: php.unit.so
$ ldd build/lib/unit/modules/php.unit.so 
        linux-vdso.so.1 (0x00007ffc045a5000)
        libphp-8.2.so => /usr/lib64/libphp-8.2.so (0x00007f0bec200000)
        ...
$ readelf -d build/lib/unit/modules/php.unit.so

Dynamic section at offset 0x18d90 contains 26 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libphp-8.2.so]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
 0x000000000000001d (RUNPATH)            Library runpath: [/usr/lib64]
...
vt-alt commented 7 months ago

Your simplest solution is to just symlink libphp.so to whichever version you have the headers installed for.

Thank you for the ideas but this is not easily possible because build is performed under user. Also, --lib-path= will set rpath to a standard library directory which is considered a bad practice for ALT.

I think there no much reason to go to such extents to fool auto/modules/php instead of just patching it (which we already do).

if it can find the right header files...

Everything else (except libphp's own ldflags) is detected well via php-config.

Thanks,