m6w6 / ext-http

Extended HTTP Support
BSD 2-Clause "Simplified" License
79 stars 22 forks source link

Windows PECL DLL builds are failing #97

Closed cmb69 closed 3 years ago

cmb69 commented 5 years ago

This is a follow up to m6w6/ext-raphf#7.

nmake fails to build pecl_http 3.2.3 with the following error:

ext\http\src\php_http_client_curl_event.c(18): fatal error C1083: Cannot open include file: 'event.h': No such file or directory

I think this line should be:

#include <event2/event.h>

or config.w32 would have to be adjusted:

        if (CHECK_HEADER_ADD_INCLUDE("event2/event.h", "CFLAGS_HTTP", null, null, true) &&
Jan-E commented 5 years ago

Libevent comes with 2 times 'event.h': https://github.com/libevent/libevent/tree/master/include https://github.com/libevent/libevent/tree/master/include/event2

I tried to include only the second one by removing the one in my main include dir and checking on

CHECK_HEADER_ADD_INCLUDE("event.h", "CFLAGS_HTTP", PHP_PHP_BUILD + "\\include\\event2;" + PHP_HTTP

but this failed:

ext\http\src\php_http_client_curl_event.c(30): error C2079: 'evnt' uses undefined struct 'event'
ext\http\src\php_http_client_curl_event.c(264): error C2027: use of undefined type 'event'
NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.16.27023\bin\HostX64\x64\cl.exe"' : return code '0x2'

Both headers have to be there. At configure time it checks if libevent2 is installed and at compile time it includes the event.h in the main include directory.

Jan-E commented 5 years ago

@cmb69 Can you check if the latest commit fixes this issue?

Jan-E commented 5 years ago

Ugly typo in the commit description: https://github.com/m6w6/ext-http/commit/85cce2121de3ab8993ee35c1f8d8b8f7c696fd3d

cmb69 commented 5 years ago

Ah, we're coming closer, thanks! The problem is that our libevent packages don't include this header, probably because this header is deprecated as of libevent 2. I'll add the missing headers and give it try, but probably this should be resolved in ext-http (I wonder if the include/event.h is used by the Linux builds).

cmb69 commented 5 years ago

Little by little, the bird builds its nest…

The builds are ready now, but they're not linked on the PECL package page, possibly due to the package name (pecl_http vs. http). I'll have a look at this issue when I'm back on the keyboard.

cmb69 commented 5 years ago

Indeed, just an issue with rmtools. After manual renaming, the binary builds are available at https://pecl.php.net/package/pecl_http/3.2.3/windows.

Jan-E commented 5 years ago

Did you build it with ot without https://github.com/m6w6/ext-http/commit/85cce2121de3ab8993ee35c1f8d8b8f7c696fd3d ?

cmb69 commented 5 years ago

I built an unmodified 3.2.3 (otherwise I would not have published this as release).

To summarize, there have been two issues:

So everything is fine from your side; of course, 85cce21 makes sense, though.

Sorry for the inconvenience; I'll try to fix the outstanding issues soon.

m6w6 commented 3 years ago

I was about to release 4.0 stable, but noticed there's no 4.0b1 windows release on pecl, so I guess this is still a problem?

cmb69 commented 3 years ago

It seems that build never had been attempted. I'm having a look.

cmb69 commented 3 years ago

Apparently 4.0b1 requires PHP 8.0.0, but we do not do these builds yet; mass rebuilds will be done in a few weeks. I'll trigger a snapshot of 4.0b1, to see whether there are any issues.

cmb69 commented 3 years ago

"Error: At least PHP '8.0.0' required, got '8.0.0-dev'" – d'oh. Need to update PHP 8.0 first, so has to wait till evening.

m6w6 commented 3 years ago

NP, thanks a lot Christoph!

Jan-E commented 3 years ago

@m6w6 Both the dependent ext-propro as ext-http build without modifications on Windows, so that should not be a reason to delay a 4.0 stable release. It looks ext-propro did not have any essential changes since the last release, but maybe an extra release to indicate that it is PHP 8-ready might be useful.

m6w6 commented 3 years ago

propro is dead, though, for PHP-8, the ZE API it used is gone now...

m6w6 commented 3 years ago

as per https://github.com/m6w6/ext-propro/pull/5

Jan-E commented 3 years ago

Oops. I meant the dependent ext-raphf. For that extension an extra release would be welcome.

m6w6 commented 3 years ago

Hmmmm, why? Seriously curious 🤷

Jan-E commented 3 years ago

Maybe just to trigger a PHP 8 build of the DLL's. Unless they are automatically added in the mass rebuild, announced by @cmb69

Jan-E commented 3 years ago

BTW: I dropped PHP_HTTP_HAVE_EXT_PROPRO in config.w32: https://github.com/m6w6/ext-http/commit/ed4c349227aaeb0a850ac641dfde43f54a04d504

m6w6 commented 3 years ago

Yep, I saw, thanks a lot!

cmb69 commented 3 years ago

@Jan-E, currently, no PHP 8.0 builds are done anyway. The mass rebuild is supposed to rebuild everything on https://windows.php.net/downloads/pecl/releases/ (always latest releases).

Jan-E commented 3 years ago

OK. With respect to ext-http 4.0 see https://phpdev.toolsforresearch.com/php-8.0.0-nts-Win32-vs16-x64.htm

m6w6 commented 3 years ago

Bear with me, but I looked through most of extensions I noticed I kind of maintain or have to look at:

Sidenote: xmlrpc? 😁

cmb69 commented 3 years ago

I'll gladly transfer xmlrpc lead maintainership to you. :p

Jan-E commented 3 years ago

I always build from git head. For instance xmlrpc is built from https://github.com/php/pecl-networking-xmlrpc/commits/master

geoip does not look to be dead:

=====================================================================
Number of tests :   17                17
Tests skipped   :    0 (  0.0%) --------
Tests warned    :    0 (  0.0%) (  0.0%)
Tests failed    :    1 (  5.9%) (  5.9%)
Tests passed    :   16 ( 94.1%) ( 94.1%)
---------------------------------------------------------------------
Time taken      :    2 seconds
=====================================================================

=====================================================================
FAILED TEST SUMMARY
---------------------------------------------------------------------
Checking Country (Free) DB availability [N:\php-sdk\php80dev\ext\geoip\tests\001.phpt]
=====================================================================
Jan-E commented 3 years ago

@remicollet added the arginfo to geoip: https://svn.php.net/viewvc?view=revision&revision=351082 That extension will need a new release to survive the mass rebuild for PHP8.

cmb69 commented 3 years ago

propro is dead, though, for PHP-8, the ZE API it used is gone now...

That appears to be an issue, due to configuration of the build machine. If I remove propro there, no further pecl_http 3 version could be built. Or wouldn't that be an issue?

Jan-E commented 3 years ago

propro is not needed anymore for PHP8, only raphf

Jan-E commented 3 years ago

So, we should try to avoid: | propro | shared | That can be achieved by adding if (PHP_VERSION <= 7) to the config.w32 of ext-propro:

ARG_ENABLE("propro", "for propro support", "no");

if (PHP_PROPRO == "yes" && PHP_VERSION <= 7) {
    var PHP_PROPRO_HEADERS=glob(configure_module_dirname + "/src/*.h");
    var PHP_PROPRO_SRC_ARRAY=glob(configure_module_dirname + "/src/*.c");
    var PHP_PROPRO_SOURCES="";
    for (var i=0; i<PHP_PROPRO_SRC_ARRAY.length; ++i) {
        var basename = FSO.GetFileName(PHP_PROPRO_SRC_ARRAY[i]);
        PHP_PROPRO_SOURCES = PHP_PROPRO_SOURCES + " " + basename;
    }

    //EXTENSION("propro");
    ADD_SOURCES(configure_module_dirname + "/src", PHP_PROPRO_SOURCES, "propro");
    EXTENSION("propro", "src/php_propro_api.c");
    PHP_INSTALL_HEADERS("ext/propro", "php_propro.h");
    for (var i=0; i<PHP_PROPRO_HEADERS.length; ++i) {
        var basename = FSO.GetFileName(PHP_PROPRO_HEADERS[i]);
        copy_and_subst("src/" + basename, basename, []);
        PHP_INSTALL_HEADERS(configure_module_dirname, basename);
    }
    ADD_FLAG("CFLAGS_PROPRO", "/I" + configure_module_dirname + " ");

    AC_DEFINE("HAVE_PROPRO", 1);
}

Shall I make this change?

Jan-E commented 3 years ago

I checked if this broke anything. It did not, even when building PHP 5.3. So I made the change: https://github.com/m6w6/ext-propro/commit/1bf4a3379b530ca6c10940e9e16eca5b382342f2

cmb69 commented 3 years ago

@Jan-E, thanks, this change certainly makes sense on it's own – why even try to build an extension with some PHP version where the build is guaranteed to fail? However, I'm not sure whether that would be sufficent for the PECL builder, and I actually found out that it is well possible to have different ext configurations for different PHP versions. x64 snapshots are already available. I'm going to update the x86 tree, and also have a look how to fix libevent support.

Jan-E commented 3 years ago

I tried it with propro, raphf and http in the ext path. It starts with snapshot: forcing propro on but ends with

| pgsql              | shared |
| phar               | static |
| phpdbg_webhelper   | shared |
| pspell             | shared |
| radius             | shared |
| raphf              | shared |

It does not try to build propro at all, so it does not bork like it did before (from your old logs):

EXT pgsql build complete
NMAKE : warning U4004: too many rules for target 'C:\php-snap-build\obj\8.0-ts-windows-vs16-x64\Release_TS\ext\propro\src\php_propro_api.obj'
    "cl.exe" /D COMPILE_DL_PROPRO /D PROPRO_EXPORTS=1 /Iext\propro /nologo /I . /I main /I Zend /I TSRM /I ext /D _WINDOWS /D WINDOWS=1 /D ZEND_WIN32=1 /D PHP_WIN32=1 /D WIN32 /D _MBCS /D _USE_MATH_DEFINES /FD /wd4996 /Qspectre /guard:cf /Zc:inline /Gw /Zc:__cplusplus /d2FuncCache1 /Zc:wchar_t /MP /Zi /LD /MD /Ox /D NDebug /D NDEBUG /D ZEND_WIN32_FORCE_INLINE /GF /D ZEND_DEBUG=0 /D ZTS=1 /I "C:\php-snap-build\php-src\php80\x64\deps\include" /I "C:\php-snap-build\dep-aux\vs16\x64\libevent\include" /D FD_SETSIZE=256 /DHAVE_ARGON2ID_HASH_RAW=1 /FoC:\php-snap-build\obj\8.0-ts-windows-vs16-x64\Release_TS\ext\propro\src\ /FpC:\php-snap-build\obj\8.0-ts-windows-vs16-x64\Release_TS\ext\propro\src\ /FRC:\php-snap-build\obj\8.0-ts-windows-vs16-x64\Release_TS\ext\propro\src\ /FdC:\php-snap-build\obj\8.0-ts-windows-vs16-x64\Release_TS\ext\propro\src\ /c ext\propro\src\php_propro_api.c
php_propro_api.c
ext\propro\src\php_propro_api.c(368): warning C4133: 'function': incompatible types - from 'zval *' to 'zend_object *'
ext\propro\src\php_propro_api.c(421): warning C4133: 'function': incompatible types - from 'zval *' to 'zend_object *'
ext\propro\src\php_propro_api.c(680): warning C4028: formal parameter 1 different from declaration
ext\propro\src\php_propro_api.c(681): warning C4028: formal parameter 1 different from declaration
ext\propro\src\php_propro_api.c(682): error C2039: 'set': is not a member of '_zend_object_handlers'
C:\php-snap-build\php-src\php80\x64\php-src\Zend\zend_object_handlers.h(147): note: see declaration of '_zend_object_handlers'
ext\propro\src\php_propro_api.c(683): error C2039: 'get': is not a member of '_zend_object_handlers'
C:\php-snap-build\php-src\php80\x64\php-src\Zend\zend_object_handlers.h(147): note: see declaration of '_zend_object_handlers'
ext\propro\src\php_propro_api.c(684): warning C4028: formal parameter 1 different from declaration
ext\propro\src\php_propro_api.c(684): warning C4133: '=': incompatible types - from 'ZEND_RESULT_CODE (__cdecl *)(zval *,zval *,int)' to 'zend_object_cast_t'
ext\propro\src\php_propro_api.c(685): warning C4028: formal parameter 1 different from declaration
ext\propro\src\php_propro_api.c(686): warning C4028: formal parameter 1 different from declaration
ext\propro\src\php_propro_api.c(687): warning C4028: formal parameter 1 different from declaration
ext\propro\src\php_propro_api.c(688): warning C4028: formal parameter 1 different from declaration
NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.28.29333\bin\HostX64\x64\cl.exe"' : return code '0x2'
Stop.

But I see that you already succeeded: https://windows.php.net/downloads/pecl/snaps/http/4.0.0beta1/

Jan-E commented 3 years ago

I double-checked. After https://github.com/m6w6/ext-propro/commit/1bf4a3379b530ca6c10940e9e16eca5b382342f2 and a snapshot build the only references in the complete build environment to propro were some lines of js-code in configure.js (that do not get executed) and the 3 test-files that get added to x64/Release/php-test-pack-8.0.0.zip. Nothing in the Makefile or wherever else.

The configure command is at the top of https://phpdev.toolsforresearch.com/php-8.0.0-nts-Win32-vs16-x64.htm

So the commit to ext-propro effectively avoids build failures under PHP8 when propro is in the ext-path.

m6w6 commented 3 years ago

So, we're awesome now?

cmb69 commented 3 years ago

I think so.

Jan-E commented 3 years ago

Maybe it would be wise to make a final release of ext-propro, including the commit that prevents build failures under PHP8. If you agree, I will make a tiny change to the config.w32 of propro with a warning that the propro extension has been discontinued under PHP8. That will be later on today. ATM I am travelling by public transport between Amsterdam and Rotterdam, with a face mask.

m6w6 commented 3 years ago

Alright. I'll add max/exclude PHP versions to the package.xml

Jan-E commented 3 years ago

https://github.com/m6w6/ext-propro/commit/921871c048332818704008ce7b67f8f2e32d3cf4 in the train near Gouda

Jan-E commented 3 years ago

@cmb69 Is there any reason why https://windows.php.net/downloads/pecl/releases/http/3.2.4/ aren't linked on https://pecl.php.net/package/pecl_http ?

I can imagine that you do not link https://windows.php.net/downloads/pecl/releases/http/4.0.0/ yet. Any idea when that will be the case?

cmb69 commented 3 years ago

One problem is the naming issue: the PECL package is called pecl_http, but it is build as http. I have copied the missing builds to https://windows.php.net/downloads/pecl/releases/pecl_http/ about an hour ago, but for some reason they are still not linked.

That might be related to the caching mechanism, but for some (potentially related) reason, requesting https://pecl.php.net/package/pecl_http takes a very long time to respond (at least for me).

I'm keeping an eye on this, and will investigate further if the DLLs are not listed later today or tomorrow.

PS: ah, I need to fix the names as well. @m6w6, could that naming mess be resolved at some point in the future? :)

m6w6 commented 3 years ago

That might be related to the caching mechanism, but for some (potentially related) reason, requesting https://pecl.php.net/package/pecl_http takes a very long time to respond (at least for me).

Doesn't it do that for every package for years now? (since the PECL server mirated?)

m6w6 commented 3 years ago

PS: ah, I need to fix the names as well. @m6w6, could that naming mess be resolved at some point in the future? :)

As explained, it's that way because there's a PEAR HTTP package, and there's formal support in tooling and package.xml in form of "package>providesextension" vs "package>name". So, granted, it's clunky, but not a mess. :grin:

What I personally consider a mess, is the possibility of creating mixed case package names :sob: :slightly_smiling_face:

cmb69 commented 3 years ago

Requesting the package pages often takes quite some time, but for pecl_http it was even way longer. I shall have a look at that caching mechanism; it may not work as expected (perhaps due to the move to CDN).

Anyhow, the DLLs are now listed. \o/

The naming issue is addressed for the builds, but unfortunately not for the uploads. If it's not possible to change names, I shall have a look at this as well.

Jan-E commented 3 years ago

Closing. If any problem still exists it has to be addressed separately.