| Message ID | 1471971288-4564-1-git-send-email-bos@je-eigen-domein.nl |
|---|---|
| State | Accepted |
| Headers | show |
Hello, On Tue, 23 Aug 2016 18:54:48 +0200, Floris Bos wrote: > The Mysql Native Driver has been the default mysql driver since > PHP 5.4, but buildroot was still using libmysqlclient. > > Mysqlnd has several advantages such as improved memory management > and the more favorable PHP licensing terms. > (can combine it with proprietary PHP extensions like Ioncube > loader, while libmysqlclient requires commercial licensing if you > link to it and do not fall under their GPL/FOSS license exception) > > Signed-off-by: Floris Bos <bos@je-eigen-domein.nl> I see that you are changing the *existing* options to use the native driver rather than the libmysqlclient based one. Would there be any reason for someone to prefer the libmysqlclient based one over the native driver? I'm simply wondering if we should change the semantic of the existing options, or add new ones for the native driver. I very much prefer *less* options, so if the native driver is fine and a drop-in replacement for the libmysqlclient based one, I'm all for your patch, just wanted to confirm. Thanks, Thomas
Hi, On 08/23/2016 09:52 PM, Thomas Petazzoni wrote: > On Tue, 23 Aug 2016 18:54:48 +0200, Floris Bos wrote: >> The Mysql Native Driver has been the default mysql driver since >> PHP 5.4, but buildroot was still using libmysqlclient. >> >> Mysqlnd has several advantages such as improved memory management >> and the more favorable PHP licensing terms. >> (can combine it with proprietary PHP extensions like Ioncube >> loader, while libmysqlclient requires commercial licensing if you >> link to it and do not fall under their GPL/FOSS license exception) >> >> Signed-off-by: Floris Bos <bos@je-eigen-domein.nl> > I see that you are changing the *existing* options to use the native > driver rather than the libmysqlclient based one. > > Would there be any reason for someone to prefer the libmysqlclient > based one over the native driver? > > I'm simply wondering if we should change the semantic of the existing > options, or add new ones for the native driver. I very much prefer > *less* options, so if the native driver is fine and a drop-in > replacement for the libmysqlclient based one, I'm all for your patch, > just wanted to confirm. The upstream project changed the existing option as well. Years ago, if you specified --with-pdo-mysql (without exact path) it defaulted to searching for libmysqlclient, even when mysqlnd was already an alternative. Since the release of 5.4.0 (in 2012) it defaults to mysqlnd. Not sure if anyone still has a reason to prefer libmysqlclient, mysqlnd is indeed a drop-in replacement. I kinda have the expectation to get the same defaults when compiling in buildroot, as I would get if I compiled the upstream project manually. Yours sincerely, Floris Bos
On 23-08-16 18:54, Floris Bos wrote: > The Mysql Native Driver has been the default mysql driver since > PHP 5.4, but buildroot was still using libmysqlclient. > > Mysqlnd has several advantages such as improved memory management > and the more favorable PHP licensing terms. > (can combine it with proprietary PHP extensions like Ioncube > loader, while libmysqlclient requires commercial licensing if you > link to it and do not fall under their GPL/FOSS license exception) I may be too tired and could be missing something, but this commit message looks like it doesn't match the actual change... I _guess_ that the php config script will use mysqlnd when the plain --with-mysqli option is passed, and will use libmysqlclient when --with-mysqli has a parameter. If this is the case, please specify it explicitly in the commit message. If this is not the case, it should _definitely_ be explained in the commit message :-) With that, you can add my Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> Regards, Arnout > > Signed-off-by: Floris Bos <bos@je-eigen-domein.nl> > --- > package/php/Config.ext | 12 ------------ > package/php/php.mk | 6 ++---- > 2 files changed, 2 insertions(+), 16 deletions(-) > > diff --git a/package/php/Config.ext b/package/php/Config.ext > index 82aaf41..7c3ba7e 100644 > --- a/package/php/Config.ext > +++ b/package/php/Config.ext > @@ -113,10 +113,6 @@ endif > > config BR2_PACKAGE_PHP_EXT_MYSQLI > bool "Mysqli" > - depends on BR2_INSTALL_LIBSTDCPP > - depends on BR2_USE_MMU # mysql > - depends on BR2_TOOLCHAIN_HAS_THREADS # mysql > - select BR2_PACKAGE_MYSQL > help > MySQL Improved extension support > > @@ -135,17 +131,9 @@ if BR2_PACKAGE_PHP_EXT_PDO > > config BR2_PACKAGE_PHP_EXT_PDO_MYSQL > bool "MySQL" > - depends on BR2_INSTALL_LIBSTDCPP > - depends on BR2_USE_MMU # mysql > - depends on BR2_TOOLCHAIN_HAS_THREADS # mysql > - select BR2_PACKAGE_MYSQL > help > PDO driver for MySQL > > -comment "MySQL drivers need a toolchain w/ C++, threads" > - depends on BR2_USE_MMU > - depends on !BR2_INSTALL_LIBSTDCPP || !BR2_TOOLCHAIN_HAS_THREADS > - > config BR2_PACKAGE_PHP_EXT_PDO_POSTGRESQL > bool "PostgreSQL" > select BR2_PACKAGE_POSTGRESQL > diff --git a/package/php/php.mk b/package/php/php.mk > index d7e27a1..deaf56e 100644 > --- a/package/php/php.mk > +++ b/package/php/php.mk > @@ -181,8 +181,7 @@ endif > > ### Native SQL extensions > ifeq ($(BR2_PACKAGE_PHP_EXT_MYSQLI),y) > -PHP_CONF_OPTS += --with-mysqli=$(STAGING_DIR)/usr/bin/mysql_config > -PHP_DEPENDENCIES += mysql > +PHP_CONF_OPTS += --with-mysqli > endif > ifeq ($(BR2_PACKAGE_PHP_EXT_SQLITE),y) > PHP_CONF_OPTS += --with-sqlite3=$(STAGING_DIR)/usr > @@ -199,8 +198,7 @@ PHP_DEPENDENCIES += sqlite > PHP_CFLAGS += -DSQLITE_OMIT_LOAD_EXTENSION > endif > ifeq ($(BR2_PACKAGE_PHP_EXT_PDO_MYSQL),y) > -PHP_CONF_OPTS += --with-pdo-mysql=$(STAGING_DIR)/usr > -PHP_DEPENDENCIES += mysql > +PHP_CONF_OPTS += --with-pdo-mysql > endif > ifeq ($(BR2_PACKAGE_PHP_EXT_PDO_POSTGRESQL),y) > PHP_CONF_OPTS += --with-pdo-pgsql=$(STAGING_DIR)/usr >
Hello, On Wed, 24 Aug 2016 02:26:51 +0200, Arnout Vandecappelle wrote: > On 23-08-16 18:54, Floris Bos wrote: > > The Mysql Native Driver has been the default mysql driver since > > PHP 5.4, but buildroot was still using libmysqlclient. > > > > Mysqlnd has several advantages such as improved memory management > > and the more favorable PHP licensing terms. > > (can combine it with proprietary PHP extensions like Ioncube > > loader, while libmysqlclient requires commercial licensing if you > > link to it and do not fall under their GPL/FOSS license exception) > > I may be too tired and could be missing something, but this commit message > looks like it doesn't match the actual change... > > I _guess_ that the php config script will use mysqlnd when the plain > --with-mysqli option is passed, and will use libmysqlclient when --with-mysqli > has a parameter. If this is the case, please specify it explicitly in the commit > message. If this is not the case, it should _definitely_ be explained in the > commit message :-) Where do you see anything in the commit message that contradicts this? Yes, the commit message doesn't explain at all that passing --with-mysqli/--with-pdo-mysql with no arguments tell PHP to use its native MySQL driver. But it also doesn't state the opposite. Thomas
On 24-08-16 19:09, Thomas Petazzoni wrote: > Hello, > > On Wed, 24 Aug 2016 02:26:51 +0200, Arnout Vandecappelle wrote: >> On 23-08-16 18:54, Floris Bos wrote: >>> The Mysql Native Driver has been the default mysql driver since >>> PHP 5.4, but buildroot was still using libmysqlclient. >>> >>> Mysqlnd has several advantages such as improved memory management >>> and the more favorable PHP licensing terms. >>> (can combine it with proprietary PHP extensions like Ioncube >>> loader, while libmysqlclient requires commercial licensing if you >>> link to it and do not fall under their GPL/FOSS license exception) >> >> I may be too tired and could be missing something, but this commit message >> looks like it doesn't match the actual change... >> >> I _guess_ that the php config script will use mysqlnd when the plain >> --with-mysqli option is passed, and will use libmysqlclient when --with-mysqli >> has a parameter. If this is the case, please specify it explicitly in the commit >> message. If this is not the case, it should _definitely_ be explained in the >> commit message :-) > > Where do you see anything in the commit message that contradicts this? Not exactly contradicts. I read the commit message and expected a change like changing --with-mysqli into --with-mysqlnd. But instead it just removed all the dependencies on mysql, which looked weird. But as I said, maybe I was just too tired :-) Regards, Arnout > > Yes, the commit message doesn't explain at all that passing > --with-mysqli/--with-pdo-mysql with no arguments tell PHP to use its > native MySQL driver. But it also doesn't state the opposite. > > Thomas >
On 08/24/2016 10:41 PM, Arnout Vandecappelle wrote: > > On 24-08-16 19:09, Thomas Petazzoni wrote: >> Hello, >> >> On Wed, 24 Aug 2016 02:26:51 +0200, Arnout Vandecappelle wrote: >>> On 23-08-16 18:54, Floris Bos wrote: >>>> The Mysql Native Driver has been the default mysql driver since >>>> PHP 5.4, but buildroot was still using libmysqlclient. >>>> >>>> Mysqlnd has several advantages such as improved memory management >>>> and the more favorable PHP licensing terms. >>>> (can combine it with proprietary PHP extensions like Ioncube >>>> loader, while libmysqlclient requires commercial licensing if you >>>> link to it and do not fall under their GPL/FOSS license exception) >>> I may be too tired and could be missing something, but this commit message >>> looks like it doesn't match the actual change... >>> >>> I _guess_ that the php config script will use mysqlnd when the plain >>> --with-mysqli option is passed, and will use libmysqlclient when --with-mysqli >>> has a parameter. If this is the case, please specify it explicitly in the commit >>> message. If this is not the case, it should _definitely_ be explained in the >>> commit message :-) >> Where do you see anything in the commit message that contradicts this? > Not exactly contradicts. I read the commit message and expected a change like > changing --with-mysqli into --with-mysqlnd. But instead it just removed all the > dependencies on mysql, which looked weird. Well, could have also changed it to --with-mysqli=mysqlnd --with-pdo-mysql=mysqlnd That's another way to say that you want the mysqli and pdo_mysql PHP extensions build, and that those in turn should use the mysqlnd driver to talk to a Mysql server. Maybe that would have made the change better to understand. However then it would like we are deviating from the PHP default by explicity setting a parameter. Since mysqlnd is the default nowadays --with-mysqli --with-pdo-mysql is sufficient. Should the meaning of individual configure options really be spelled out in commit messages? What the change achieves is already in the first line "switch from libmysqlclient to mysqlnd" And if anyone wants to know the finer details, he could look it up in the upstream docs: http://php.net/manual/en/mysqlinfo.library.choosing.php Yours sincerely, Floris Bos
On 24-08-16 23:45, Floris Bos wrote: > On 08/24/2016 10:41 PM, Arnout Vandecappelle wrote: >> >> On 24-08-16 19:09, Thomas Petazzoni wrote: >>> Hello, >>> >>> On Wed, 24 Aug 2016 02:26:51 +0200, Arnout Vandecappelle wrote: >>>> On 23-08-16 18:54, Floris Bos wrote: >>>>> The Mysql Native Driver has been the default mysql driver since >>>>> PHP 5.4, but buildroot was still using libmysqlclient. >>>>> >>>>> Mysqlnd has several advantages such as improved memory management >>>>> and the more favorable PHP licensing terms. >>>>> (can combine it with proprietary PHP extensions like Ioncube >>>>> loader, while libmysqlclient requires commercial licensing if you >>>>> link to it and do not fall under their GPL/FOSS license exception) >>>> I may be too tired and could be missing something, but this commit message >>>> looks like it doesn't match the actual change... >>>> >>>> I _guess_ that the php config script will use mysqlnd when the plain >>>> --with-mysqli option is passed, and will use libmysqlclient when --with-mysqli >>>> has a parameter. If this is the case, please specify it explicitly in the >>>> commit >>>> message. If this is not the case, it should _definitely_ be explained in the >>>> commit message :-) >>> Where do you see anything in the commit message that contradicts this? >> Not exactly contradicts. I read the commit message and expected a change like >> changing --with-mysqli into --with-mysqlnd. But instead it just removed all the >> dependencies on mysql, which looked weird. > > Well, could have also changed it to --with-mysqli=mysqlnd --with-pdo-mysql=mysqlnd > That's another way to say that you want the mysqli and pdo_mysql PHP extensions > build, and that those in turn should use the mysqlnd driver to talk to a Mysql > server. > > Maybe that would have made the change better to understand. > However then it would like we are deviating from the PHP default by explicity > setting a parameter. > Since mysqlnd is the default nowadays --with-mysqli --with-pdo-mysql is sufficient. > > > Should the meaning of individual configure options really be spelled out in > commit messages? > What the change achieves is already in the first line "switch from > libmysqlclient to mysqlnd" > And if anyone wants to know the finer details, he could look it up in the > upstream docs: http://php.net/manual/en/mysqlinfo.library.choosing.php Reading the commit message and patch again with a slightly less tired head, it is indeed quite obvious. Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> Regards, Arnout > > > Yours sincerely, > > Floris Bos > >
Hello, On Tue, 23 Aug 2016 18:54:48 +0200, Floris Bos wrote: > The Mysql Native Driver has been the default mysql driver since > PHP 5.4, but buildroot was still using libmysqlclient. > > Mysqlnd has several advantages such as improved memory management > and the more favorable PHP licensing terms. > (can combine it with proprietary PHP extensions like Ioncube > loader, while libmysqlclient requires commercial licensing if you > link to it and do not fall under their GPL/FOSS license exception) > > Signed-off-by: Floris Bos <bos@je-eigen-domein.nl> > --- > package/php/Config.ext | 12 ------------ > package/php/php.mk | 6 ++---- > 2 files changed, 2 insertions(+), 16 deletions(-) Applied to master, thanks. Thomas
diff --git a/package/php/Config.ext b/package/php/Config.ext index 82aaf41..7c3ba7e 100644 --- a/package/php/Config.ext +++ b/package/php/Config.ext @@ -113,10 +113,6 @@ endif config BR2_PACKAGE_PHP_EXT_MYSQLI bool "Mysqli" - depends on BR2_INSTALL_LIBSTDCPP - depends on BR2_USE_MMU # mysql - depends on BR2_TOOLCHAIN_HAS_THREADS # mysql - select BR2_PACKAGE_MYSQL help MySQL Improved extension support @@ -135,17 +131,9 @@ if BR2_PACKAGE_PHP_EXT_PDO config BR2_PACKAGE_PHP_EXT_PDO_MYSQL bool "MySQL" - depends on BR2_INSTALL_LIBSTDCPP - depends on BR2_USE_MMU # mysql - depends on BR2_TOOLCHAIN_HAS_THREADS # mysql - select BR2_PACKAGE_MYSQL help PDO driver for MySQL -comment "MySQL drivers need a toolchain w/ C++, threads" - depends on BR2_USE_MMU - depends on !BR2_INSTALL_LIBSTDCPP || !BR2_TOOLCHAIN_HAS_THREADS - config BR2_PACKAGE_PHP_EXT_PDO_POSTGRESQL bool "PostgreSQL" select BR2_PACKAGE_POSTGRESQL diff --git a/package/php/php.mk b/package/php/php.mk index d7e27a1..deaf56e 100644 --- a/package/php/php.mk +++ b/package/php/php.mk @@ -181,8 +181,7 @@ endif ### Native SQL extensions ifeq ($(BR2_PACKAGE_PHP_EXT_MYSQLI),y) -PHP_CONF_OPTS += --with-mysqli=$(STAGING_DIR)/usr/bin/mysql_config -PHP_DEPENDENCIES += mysql +PHP_CONF_OPTS += --with-mysqli endif ifeq ($(BR2_PACKAGE_PHP_EXT_SQLITE),y) PHP_CONF_OPTS += --with-sqlite3=$(STAGING_DIR)/usr @@ -199,8 +198,7 @@ PHP_DEPENDENCIES += sqlite PHP_CFLAGS += -DSQLITE_OMIT_LOAD_EXTENSION endif ifeq ($(BR2_PACKAGE_PHP_EXT_PDO_MYSQL),y) -PHP_CONF_OPTS += --with-pdo-mysql=$(STAGING_DIR)/usr -PHP_DEPENDENCIES += mysql +PHP_CONF_OPTS += --with-pdo-mysql endif ifeq ($(BR2_PACKAGE_PHP_EXT_PDO_POSTGRESQL),y) PHP_CONF_OPTS += --with-pdo-pgsql=$(STAGING_DIR)/usr
The Mysql Native Driver has been the default mysql driver since PHP 5.4, but buildroot was still using libmysqlclient. Mysqlnd has several advantages such as improved memory management and the more favorable PHP licensing terms. (can combine it with proprietary PHP extensions like Ioncube loader, while libmysqlclient requires commercial licensing if you link to it and do not fall under their GPL/FOSS license exception) Signed-off-by: Floris Bos <bos@je-eigen-domein.nl> --- package/php/Config.ext | 12 ------------ package/php/php.mk | 6 ++---- 2 files changed, 2 insertions(+), 16 deletions(-)