Unbundle libsqlite3 #3548

cmb69 · 2018-09-23T12:32:10Z

Since there is no need to patch libsqlite3 for our purposes, and since
libsqlite3 ≥ 3.3.9 (which is our current requirement) is widely
available on distros, there is no reason anymore to bundle the library.

Besides removing the bundled libsqlite, and adapting the configuration
respectively, we also fix the use of the SQLITE_ENABLE_COLUMN_METADATA
compile time constant to detect whether sqlite3_column_table_name() is
available by a working feature detection (otherwise bug_42589.phpt
would fail).

We also completely drop support for the obscure pdo_sqlite_external
extension (which could have been enabled on Windows only by passing
--pdo-sqlite-external to configure), since it is not needed anymore.

staabm · 2018-09-23T12:36:01Z

Side question: will unbundling of this huge libs also speedup CI run as a lot of code will be less to compile?

cmb69 · 2018-09-23T12:42:25Z

will unbundling of this huge libs also speedup CI run as a lot of code will be less to compile?

I guess that the performance improvement would not really be noticable.

cmb69 · 2018-09-23T13:32:25Z

The earlier Travis build failure has been caused by Xenial's libsqlite 3.11.0-1ubuntu1, which apparently exhibits bug https://sqlite.org/src/info/ef360601. I've added a respective SKIPIF clause to work around that. It might be appropriate to backport this fix to older PHP branches as well.

KalleZ · 2018-09-23T14:08:15Z

ext/pdo_sqlite/config.w32

+		CHECK_HEADER_ADD_INCLUDE("sqlite3.h", "CFLAGS_PDO_SQLITE") &&
+		CHECK_HEADER_ADD_INCLUDE("sqlite3ext.h", "CFLAGS_PDO_SQLITE")
+	) {
+		EXTENSION("pdo_sqlite", "pdo_sqlite.c sqlite_driver.c sqlite_statement.c");


Just making a note here, are we sure that we don't need the old CFLAGS like SQLITE_ENABLE_FTS3, SQLITE_ENABLE_FTS4, SQLITE_ENABLE_FTS5, SQLITE_ENABLE_JSON1 & SQLITE_CORE here?

I've grepped for these, and they are not used anywhere. However, if they were, that wouldn't work anyway, since the external libsqlite might have been compiled differently.

Noted, then we should be good to go then as I don't see any other obvious things at a glance

Yep, those are library flags, not extension flags. Pushed a repo now https://github.com/winlibs/sqlite3.

KalleZ · 2018-09-23T14:08:54Z

ext/sqlite3/config.w32

+		CHECK_HEADER_ADD_INCLUDE("sqlite3.h", "CFLAGS_SQLITE3") &&
+		CHECK_HEADER_ADD_INCLUDE("sqlite3ext.h", "CFLAGS_SQLITE3")
+	) {
+		EXTENSION("sqlite3", "sqlite3.c", null, "/DZEND_ENABLE_STATIC_TSRMLS_CACHE=1");


Same as for the pdo_sqlite/config.w32 note here

weltling

Some notes could be addressed, but actually looks good.

Thanks.

weltling · 2018-09-23T16:25:43Z

ext/sqlite3/config0.m4

-
-    if test "$ZEND_DEBUG" = "yes"; then
-      debug_flags="-DSQLITE_DEBUG=1"
+  AC_MSG_CHECKING([for sqlite3 files in default path])


I'd suggest to prefer pkg-config and fallback to the below otherwise.

Thanks. I've already thought about that, but would prefer if someone more knowledgable regarding the Linux build system to do that.

weltling · 2018-09-23T18:04:35Z

ext/pdo_sqlite/config.w32

+		CHECK_HEADER_ADD_INCLUDE("sqlite3.h", "CFLAGS_PDO_SQLITE") &&
+		CHECK_HEADER_ADD_INCLUDE("sqlite3ext.h", "CFLAGS_PDO_SQLITE")
+	) {
+		EXTENSION("pdo_sqlite", "pdo_sqlite.c sqlite_driver.c sqlite_statement.c");


Yep, those are library flags, not extension flags. Pushed a repo now https://github.com/winlibs/sqlite3.

weltling · 2018-09-23T18:04:52Z

ext/pdo_sqlite/config.w32

-}
+	if (CHECK_LIB("libsqlite3_a.lib;libsqlite3.lib", "pdo_sqlite", PHP_PDO_SQLITE) &&
+		CHECK_HEADER_ADD_INCLUDE("sqlite3.h", "CFLAGS_PDO_SQLITE") &&
+		CHECK_HEADER_ADD_INCLUDE("sqlite3ext.h", "CFLAGS_PDO_SQLITE")


This could be generalized into a function like it is done for OpenSSL. Not critical for now.

Also shared might be a better choice to prefer for the shared extension build.

Makes sense. I'll see to it.

carusogabriel

We should also remove the lib from the "PHP source code directory structure" CONTRIBUTING section

cmb69 · 2018-09-25T09:43:56Z

@carusogabriel Good catch!

petk

Also maybe worth updating in this patch or separately, file README.REDIST.BINS contains a mention of the bundled sqlite library.

Since there is no need to patch libsqlite3 for our purposes, and since libsqlite3 ≥ 3.3.9 (which is our current requirement) is widely available on distros, there is no reason anymore to bundle the library. Besides removing the bundled libsqlite, and adapting the configuration respectively, we also fix the use of the SQLITE_ENABLE_COLUMN_METADATA compile time constant to detect whether sqlite3_column_table_name() is available by a working feature detection (otherwise bug_42589.phpt would fail). We also completely drop support for the obscure pdo_sqlite_external extension (which could have been enabled on Windows only by passing `--pdo-sqlite-external` to configure), since it is not needed anymore.

We skip this test for libsqlite 3.11.0 to 3.14.1, since they have bug https://sqlite.org/src/info/ef360601.

We also extract the SETUP_SQLITE3() function for code re-use.

cmb69 · 2018-09-29T22:09:53Z

@petk Thanks! Done.

If there are no objections, I'm going to apply this PR on the next weekend.

carusogabriel

We should also remove the library from Makefile.gcov exclusion list:

php-src/Makefile.gcov

Line 60 in 257bec3

    
           for dir in ext/bcmath/libbcmath ext/date/lib ext/fileinfo/libmagic ext/gd/libgd ext/mbstring/libmbfl ext/mbstring/oniguruma ext/pcre/pcre2lib ext/sqlite3/libsqlite ext/xmlrpc/libxmlrpc ext/zip/lib; do \

cmb69 · 2018-09-30T10:56:54Z

Thanks, @carusogabriel!

php-pulls · 2018-10-06T10:37:54Z

Comment on behalf of cmb at php.net:

Applied via 6083a38.

cmb69 · 2018-10-06T11:19:42Z

Documented via http://svn.php.net/viewvc?view=revision&revision=345781.

Cf. <php/php-src#3548>. git-svn-id: https://svn.php.net/repository/phpdoc/en/trunk@345781 c90b9560-bf6c-de11-be94-00142212c4b1

Cf. <php/php-src#3548>. git-svn-id: http://svn.php.net/repository/phpdoc/en@345781 c90b9560-bf6c-de11-be94-00142212c4b1

Cf. <php/php-src#3548>. git-svn-id: https://svn.php.net/repository/phpdoc/en/trunk@345781 c90b9560-bf6c-de11-be94-00142212c4b1

KalleZ reviewed Sep 23, 2018

View reviewed changes

cmb69 mentioned this pull request Sep 23, 2018

Require SQLite ≥ 3.5.0 #3543

Closed

weltling reviewed Sep 23, 2018

View reviewed changes

carusogabriel suggested changes Sep 24, 2018

View reviewed changes

petk reviewed Sep 26, 2018

View reviewed changes

cmb69 added 5 commits September 29, 2018 23:47

Fix bug73068.phpt

6a1d775

We skip this test for libsqlite 3.11.0 to 3.14.1, since they have bug https://sqlite.org/src/info/ef360601.

[ci skip] Remove sqlite from the source code directory structure docs

798309b

Prefer shared libsqlite3 for shared ext builds

9806741

We also extract the SETUP_SQLITE3() function for code re-use.

Update README.REDIST.BINS

3474cbd

cmb69 force-pushed the unbundle-libsqlite branch from 756480e to 3474cbd Compare September 29, 2018 22:08

carusogabriel reviewed Sep 30, 2018

View reviewed changes

Remove removed dir from Makefile.gcov exclusion list

e3fe383

php-pulls closed this Oct 6, 2018

cmb69 deleted the unbundle-libsqlite branch October 6, 2018 10:39

salathe pushed a commit to salathe/phpdoc-en that referenced this pull request Oct 6, 2018

Document unbundling of libsqlite

280b27d

Cf. <php/php-src#3548>. git-svn-id: https://svn.php.net/repository/phpdoc/en/trunk@345781 c90b9560-bf6c-de11-be94-00142212c4b1

svn2github pushed a commit to svn2github/phpdoc_en that referenced this pull request Oct 7, 2018

Document unbundling of libsqlite

c7d96a2

Cf. <php/php-src#3548>. git-svn-id: http://svn.php.net/repository/phpdoc/en@345781 c90b9560-bf6c-de11-be94-00142212c4b1

heiglandreas pushed a commit to phpdoctest/en that referenced this pull request Feb 4, 2020

Document unbundling of libsqlite

17d4866

Cf. <php/php-src#3548>. git-svn-id: https://svn.php.net/repository/phpdoc/en/trunk@345781 c90b9560-bf6c-de11-be94-00142212c4b1

salathe pushed a commit to salathe/phpdoc-en that referenced this pull request Sep 3, 2020

Document unbundling of libsqlite

765e43a

Cf. <php/php-src#3548>. git-svn-id: https://svn.php.net/repository/phpdoc/en/trunk@345781 c90b9560-bf6c-de11-be94-00142212c4b1

Unbundle libsqlite3 #3548

Unbundle libsqlite3 #3548

Uh oh!

Conversation

cmb69 commented Sep 23, 2018

Uh oh!

staabm commented Sep 23, 2018

Uh oh!

cmb69 commented Sep 23, 2018

Uh oh!

cmb69 commented Sep 23, 2018

Uh oh!

KalleZ Sep 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weltling left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carusogabriel left a comment

Choose a reason for hiding this comment

Uh oh!

cmb69 commented Sep 25, 2018

Uh oh!

petk left a comment

Choose a reason for hiding this comment

Uh oh!

cmb69 commented Sep 29, 2018

Uh oh!

carusogabriel left a comment

Choose a reason for hiding this comment

Uh oh!

cmb69 commented Sep 30, 2018

Uh oh!

php-pulls commented Oct 6, 2018

Uh oh!

cmb69 commented Oct 6, 2018

Uh oh!

Uh oh!

KalleZ Sep 23, 2018 •

edited

Loading