> -----Original Message-----
> From: Christoph M. Becker [mailto:cmbecke...@gmx.de]
> Sent: Friday, August 19, 2016 1:38 PM
> To: Anatol Belski <anatol....@belski.net>; php-...@lists.php.net;
> internals@lists.php.net
> Subject: [PHP-DEV] Re: [PHP-CVS] com php-src: Fix dba configuration for
> Windows: ext/dba/config.w32
> 
> Hi Anatol!
> 
> On 19.08.2016 at 13:17, Anatol Belski wrote:
> 
> > Hi Christoph,
> >
> >> -----Original Message-----
> >> From: Christoph Michael Becker [mailto:c...@php.net]
> >> Sent: Friday, August 19, 2016 11:42 AM
> >> To: php-...@lists.php.net
> >> Subject: [PHP-CVS] com php-src: Fix dba configuration for Windows:
> >> ext/dba/config.w32
> >>
> >> Commit:    ad76e8a529eabf150f17d313bb035b329bc68dec
> >> Author:    Christoph M. Becker <cmbecke...@gmx.de>         Fri, 19 Aug 2016
> >> 11:42:16 +0200
> >> Parents:   bc1214f25e7c9525336b34e09aec1f1db82b9894
> >> Branches:  PHP-5.6 PHP-7.0 PHP-7.1 master
> >>
> >> Link:       http://git.php.net/?p=php-
> >> src.git;a=commitdiff;h=ad76e8a529eabf150f17d313bb035b329bc68dec
> >>
> >> Log:
> >> Fix dba configuration for Windows
> >>
> >> To be able to build the dba extension on Windows, libdb was required.
> >> This is contrary to *nix where each handler can be configured
> >> individually. To avoid BC breaks, we only do minimal modifications,
> >> instead of adjusting the Windows configuration to match the *nix
> configuration, for now.
> >>
> >> Changed paths:
> >>   M  ext/dba/config.w32
> >>
> >>
> >> Diff:
> >> diff --git a/ext/dba/config.w32 b/ext/dba/config.w32 index
> >> 4f3514e..c747323
> >> 100644
> >> --- a/ext/dba/config.w32
> >> +++ b/ext/dba/config.w32
> >> @@ -4,15 +4,16 @@
> >>  ARG_WITH("dba", "DBA support", "no");
> >>
> >>  if (PHP_DBA != "no") {
> >> +  EXTENSION("dba", "dba.c dba_cdb.c dba_db1.c dba_db2.c dba_db3.c
> >> dba_dbm.c dba_flatfile.c dba_gdbm.c dba_ndbm.c dba_inifile.c");
> >> +  ADD_SOURCES("ext/dba/libcdb", "cdb.c cdb_make.c uint32.c", "dba");
> >> +  ADD_SOURCES("ext/dba/libflatfile", "flatfile.c", "dba");
> >> +  ADD_SOURCES("ext/dba/libinifile", "inifile.c", "dba");
> >> +  AC_DEFINE('HAVE_DBA', 1, 'DBA support');
> >> +  ADD_FLAG("CFLAGS_DBA", "/D DBA_FLATFILE=1 /D DBA_CDB=1 /D
> >> +DBA_CDB_MAKE=1 /D DBA_CDB_BUILTIN=1 /D DBA_INIFILE=1");
> >>    if (CHECK_LIB("libdb31s.lib", "dba", PHP_DBA) &&
> >>            CHECK_HEADER_ADD_INCLUDE("db.h", "CFLAGS_DBA")) {
> >> -          EXTENSION("dba", "dba.c dba_cdb.c dba_db1.c dba_db2.c
> >> dba_db3.c dba_dbm.c dba_flatfile.c dba_gdbm.c dba_ndbm.c dba_inifile.c");
> >> -          ADD_SOURCES("ext/dba/libcdb", "cdb.c cdb_make.c uint32.c",
> >> "dba");
> >> -          ADD_SOURCES("ext/dba/libflatfile", "flatfile.c", "dba");
> >> -          ADD_SOURCES("ext/dba/libinifile", "inifile.c", "dba");
> >> -          AC_DEFINE('HAVE_DBA', 1, 'DBA support');
> >> -          ADD_FLAG("CFLAGS_DBA", "/D DBA_DB1=0 /D
> >> DB1_VERSION=\"\\\"Berkeley DB 1.85 emulation in DB3\\\"\" /D
> >> DB1_INCLUDE_FILE=\"\\\"db_185.h\\\"\" /D DBA_DB3=1 /D
> >> DB3_INCLUDE_FILE=\"\\\"db.h\\\"\" /D DBA_FLATFILE=1 /D DBA_CDB=1 /D
> >> DBA_CDB_MAKE=1 /D DBA_CDB_BUILTIN=1 /D DBA_INIFILE=1");
> >> -  } else {
> >> -          WARNING("dba not enabled; libraries and headers not found");
> >> +          ADD_FLAG("CFLAGS_DBA", "/D DBA_DB1=0 /D
> >> DB1_VERSION=\"\\\"Berkeley DB 1.85 emulation in DB3\\\"\" /D
> >> DB1_INCLUDE_FILE=\"\\\"db_185.h\\\"\" /D DBA_DB3=1 /D
> >> DB3_INCLUDE_FILE=\"\\\"db.h\\\"\"");
> >> +  } else if (PHP_DBA != "yes") {
> >> +          WARNING("dba: db handlers not enabled; libraries and headers
> >> not
> >> +found");
> >>    }
> >>  }
> >
> > I'm not sure it's ok to suddenly change this for stable branches, and even 
> > not
> sure with 7.1. Effectively no builds for this were provided since 5.3, if I 
> don't err.
> 
> Yes, you're right.
> 
> > So from the QA perspective, it's quite a risky move.
> > It should go by master, so we have time to catch up with bugs, deps and 
> > users
> to start testing and using it again.
> 
> This appears to be a chicken-and-egg problem.  Currently, there are several
> known issues wrt. to the flatfile and inifile drivers on Windows.  These 
> drivers
> will be enabled as soon as libdb3.1s (or
> libdb6.1 as of PHP 7.0.0) are available.  Users may than experience these 
> issues.
> 
> However, it is not possible to test (and debug) with dba, if the libs are not
> available (unless one modifies config.w32), and it may be hard to get the
> respective libdb binaries.  There may, however, dba_php.dlls available from
> third-party sites.
> 
> Anyhow, if you still think the commit should only go to master, I can revert 
> the
> changes.  Or feel free to do so yourself. :-)
>
The ini parsing functionality is available in the core anyway. The bundled 
libcdb originates to year 2000, it needs to be reviewed and possibly updated 
with the latest patches. In between, very good alternatives like Redis, MongoDB 
or even upscaledb and others are available for key/value storage. So ext/dba 
doesn't look like something that needs to be ungently available today. 

The bins might be available as patching the config is easy, or even just with 
using the Berkeley DB libs. However, the bug tracker is silent, which is not 
good. There was no testing since over 5 years on Windows side, there was likely 
no testing in PHP 7 as it's always disabled. IMHO selling that state just as is 
for stable is dangerous. In addition, at least qdbm, not sure with tcadb, could 
be supported. And the bundled libcdb is questionable anyway.

It's an if/else question, as for me it's a decision that should be in favor of 
the quality. That would require some time to be guaranteed. I'd ask you to 
please keep it only for master, so there's enough time for all the complex 
work. Clear, some bugs are always there, there's no reason to omit the usual QA 
work. Please keep the incentive, really appreciated!

Thanks

Anatol


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to