Hi, > -----Original Message----- > From: Darek Slusarczyk [mailto:[email protected]] > Sent: Tuesday, December 6, 2016 1:08 AM > To: Johannes Schlüter <[email protected]>; Anatol Belski > <[email protected]> > Cc: 'pecl-dev' <[email protected]>; 'Filip Janiszewski' > <[email protected]>; 'Andrey Hristov' <[email protected]> > Subject: Re: [PECL-DEV] New package: mysql_xdevapi > > Hi, > > > On 2016-12-05 18:00, Johannes Schlüter wrote: > > Hi > > > > thanks for taking a look, some comments below. > > > > On Mon, 2016-12-05 at 17:53 +0100, Anatol Belski wrote: > >> Thanks for the ping. I've just checked the build locally, some question > >> arise. > >> > >> I had to add the following three lines to config.w32 > >> > >> ADD_FLAG("CFLAGS_BD_EXT_MYSQL_XDEVAPI_MESSAGES", > "/D ZEND_WIN32_KEEP_INLINE=1 /U ZEND_WIN32_FORCE_INLINE"); > >> > ADD_FLAG("CFLAGS_BD_EXT_MYSQL_XDEVAPI_XMYSQLND_CRUD_PAR > SERS", "/D ZEND_WIN32_KEEP_INLINE=1 /U ZEND_WIN32_FORCE_INLINE"); > >> ADD_FLAG("CFLAGS_BD_EXT_MYSQL_XDEVAPI_XMYSQLND", > "/D > >> ZEND_WIN32_KEEP_INLINE=1 /U ZEND_WIN32_FORCE_INLINE"); > >> > >> The issue here with vc14 is, that otherwise the default option in PHP > >> is ZEND_WIN32_FORCE_INLINE which replaces "inline" with > >> "__forceinline". While it is ok for C oriented code, it generates > >> wrong code for "inline namespace" in C++. Please add these three > >> lines to the config.w32. It's probably only an issue with C++11, but > >> the extension is only 7.1 compatible and we use vc14 for that. AFM, > >> while this issue is seldom, the solution looks more than suboptimal, > >> I'll look to fix it in master, if not in 7.1 already. > > Darek, can you test on your system and add those to config.w32 in our > > repo? Thanks! > > For release build there appear some messages, but they rather don't hurt. > cl : Command line warning D9025 : overriding '/DZEND_WIN32_FORCE_INLINE' > with '/UZEND_WIN32_FORCE_INLINE' > cl : Command line warning D9025 : overriding '/DZEND_WIN32_FORCE_INLINE' > with '/UZEND_WIN32_FORCE_INLINE' > cl : Command line warning D9025 : overriding '/DZEND_WIN32_FORCE_INLINE' > with '/UZEND_WIN32_FORCE_INLINE' > > the reason is - in single cl.exe cmdline these macros appear more than once, > i.e. > are defined/undefined. I assume ZEND_WIN32_FORCE_INLINE is by default > enabled for release build, but is suppressed with Anatol's patch. > Yeah, these warnings can be ignored, that just undo the previous /D ZEND_WIN32_FORCE_INLINE. It's activated automatically, but it's obviously not correct for C++ code, see http://lxr.php.net/xref/PHP-7.0/Zend/zend_config.w32.h#64 . I'll be checking for a better solution to put into the core, as I guess it was already a breach with vc12, just we didn't use it for PHP builds. Maybe just excluding this for C++11 mode will suffice, need to do a bit of home work yet.
> In the end tests pass (except mentioned below 001/011) for debug/release > build. > > >> It otherwise turned out, that some parts unconditionally depend on > >> Boost. It was OK, when I've pointed to headers from 1.61.0, but > >> strangely that was it. Is it, that no actual Boost libs are required > >> for linking? Otherwise, which Boost version should be used? I could > >> try first to use bins > >> https://sourceforge.net/projects/boost/files/boost-binaries/ , or > >> prepare own compatible builds otherwise. > > We only need header-only libs from boost. 1.61.0 is fine, no boost > > binaries required. > > > So far I was building it with boost 1.59.0, but today also tested with > 1.61.0 and 1.62.0, and they all pass. > > > >> It otherwise succeeded with protobuf 3.0.2, while seems there are some > >> symbol issues with 3.0.0. And the latest 3.1.0 is broken on Windows > >> 64-bit, so couldn't tell how it would be. Any version known to fit > >> best here? > > We had some issues with protobuf 3, I myself mostly use 2.6 but 3.0.2 > > should be fine, too. > > I had some problems with one of protobuf 3.x.y on Linux, but don't > remember the exact version number :-} > On Win 2.6 should work, the same for 3.0.2. Recently I was also using > 3.1.0 for x86 builds. > Ok, so I keep protobuf 3.0.2 and boost 1.62.0 for now. Please let me know, if something changed in between. FYI protobuf 3.1.0 has an issue with x64 build, could be hot patched, though. > >> Locally I've these only tests failing, default insecure server setup > >> > >> ========DIFF======== > >> 001+ '[HY000] php_network_getaddresses: getaddrinfo failed: No such host > is known. ' != '[HY000] php_network_getaddresses: getaddrinfo failed: Name or > service not known' > >> 002+ Some expectation were not meet! > >> 003+ Expected: 1111111111, result: 0111111111 > >> ========DONE======== > >> FAIL mysqlx connection (success/fail) [C:\php-sdk\php71\vc14\x64\php- > src\ext\mysql_xdevapi\tests\001.phpt] > >> > >> ========DIFF======== > >> 001+ Warning: mysql_xdevapi\NodeTableSelect::execute(): [1365] Division by > 0 in C:\php-sdk\php71\vc14\x64\php-src\ext\mysql_xdevapi\tests\011.php on > line 16 > >> 002+ > >> 003+ Warning: mysql_xdevapi\NodeTableSelect::execute(): [1365] Division by > 0 in C:\php-sdk\php71\vc14\x64\php-src\ext\mysql_xdevapi\tests\011.php on > line 16 > >> 004+ > >> 005+ Warning: mysql_xdevapi\NodeTableSelect::execute(): [1365] Division by > 0 in C:\php-sdk\php71\vc14\x64\php-src\ext\mysql_xdevapi\tests\011.php on > line 16 > >> 006+ '' != '3' > >> 007+ Some expectation were not meet! > >> 008+ Expected: 1, result: 0 > >> ========DONE======== > >> FAIL mysqlx warnings [C:\php-sdk\php71\vc14\x64\php- > src\ext\mysql_xdevapi\tests\011.phpt] > > Darek/Filip could you take a look at those tests? > > Regarding 001 - the message in pattern matches Unix, but is different on > Win > 011 - it doesn't pass neither on Win, nor on Unixes (AFAIR it never did) > So 001 can be just forked, or modified to ignore the exact warning text. With 011 seems more investigation needed, if it's supposed to pass. Otherwise, probably a test bug as well. > >> Could you please clear out the protobuf and boost usage, mentioned earlier? > I'd go setup it on the buildhost then. > >> > > Generally I build binaries according to > https://wiki.php.net/internals/windows/stepbystepbuild > then copy boost headers (e.g. boost_1_62_0\boost\*) and protobuf > install files > into deps (for boost into deps/include) > in my case it is > W:\mysql.php\phpdev\vc14\x86\deps\ > Normally, one doesn't need to pollute the deps directory. There are several methods, AFM I use --with-extra-libs and --with-extra-includes to give separate dependency dirs. > > > So far I was using prebuilt deps-7.1-vc14-* from > http://windows.php.net/downloads/php-sdk/ > but I see there is newer version (Nov 17th), so tested with it too. And > tests pass. > I was wondering about OpenSSL that Remi mentioned, but it is not yet a dependency for the Windows build, so currently not linked. Other than that, looks like the ext own deps are the only what matters for now. Thanks Anatol -- PECL development discussion Mailing List (http://pecl.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
