Re: New package: mysql_xdevapi

From: Date: Mon, 05 Dec 2016 17:00:17 +0000
Subject: Re: New package: mysql_xdevapi
References: 1 2  Groups: php.pecl.dev 
Request: Send a blank email to pecl-dev+get-14303@lists.php.net to get a copy of this message
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_PARSERS", "/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! > 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. > 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. > With this mentioned, the local build is ok. Great! > A question to the testing - is the following the only required step? > > INSTALL PLUGIN mysqlx SONAME 'mysqlx.dll'; > > As described here > https://dev.mysql.com/doc/refman/5.7/en/document-store-setting-up.html Yes, that loads the server plugin which adds support for the new protbuf based protocol. > 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? > Could you please clear out the protobuf and boost usage, mentioned earlier? I'd go setup > it on the buildhost then. > > Thanks. Thank you for testing! johannes

« previous php.pecl.dev (#14303) next »