bpo-45774: Autoconfiscate SQLite detection (GH-29507)

[erlend-aasland]

https://bugs.python.org/issue45774

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @erlend-aasland for commit 9bc5d7b 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

[erlend-aasland]

SQLite was not detected on buildbot AMD64 FreeBSD Shared PR.

[tiran]

SQLite was not detected on buildbot AMD64 FreeBSD Shared PR.

@koobs where is sqlite installed on your FreeBSD buildbot? Is it in /usr/local by any chance? Would the solution https://bugs.python.org/issue45774#msg406076 work for FreeBSD?

[koobs]

SQLite was not detected on buildbot AMD64 FreeBSD Shared PR.

@koobs where is sqlite installed on your FreeBSD buildbot? Is it in /usr/local by any chance? Would the solution https://bugs.python.org/issue45774#msg406076 work for FreeBSD?

sqlite3-3.35.5 is installed on both FreeBSD buildbot workers in LOCALBASE (/usr/local by default). This is the standard location for third party libraries installed via FreeBSD ports and official packages.

I haven't deeply reviewed the implementation in issue 45774, but I don't see a reference to --with(out)-sqlite3 to explicitly disable/enable, but otherwise and generally speaking, standard autoconf/configure checks for dependencies is fine.

I would also caution that wherever possible when doing dependency detection and configuration, we should include the ability to pass dependency specific include/library paths and *FLAGS, because Pythons build system is particularly inconsistent and flaky with respect to passing *FLAGS globally, which downstreams often have to utilise.

This can causes include ordering issues, as well as build/link time conflicts when a dependency is installed in more than one location, and both are picked up.

Other than at the configure level --with-foo-{inc,lib}=<path> (or similar), this also means Modules/Setup, if support for it is not there already (like SSL).

Further Sqlite3 Information

Installed libraries and header names:

# ls -la /usr/local/lib/libsqlite3.so*
lrwxr-xr-x  1 root  wheel       19 Oct 30 01:09 /usr/local/lib/libsqlite3.so@ -> libsqlite3.so.0.8.6
lrwxr-xr-x  1 root  wheel       19 Oct 30 01:09 /usr/local/lib/libsqlite3.so.0@ -> libsqlite3.so.0.8.6
-rwxr-xr-x  1 root  wheel  1422160 Oct 30 01:09 /usr/local/lib/libsqlite3.so.0.8.6*

# ls -la /usr/local/include/sql*
-rw-r--r--  1 root  wheel     838 Oct  4 08:15 /usr/local/include/sql3types.h
-rw-r--r--  1 root  wheel    1283 Oct  4 08:15 /usr/local/include/sqlca.h
-rw-r--r--  1 root  wheel    1586 Oct  4 08:15 /usr/local/include/sqlda-compat.h
-rw-r--r--  1 root  wheel     824 Oct  4 08:15 /usr/local/include/sqlda-native.h
-rw-r--r--  1 root  wheel     321 Oct  4 08:15 /usr/local/include/sqlda.h
-rw-r--r--  1 root  wheel  584223 Oct 30 01:09 /usr/local/include/sqlite3.h
-rw-r--r--  1 root  wheel   35437 Oct 30 01:09 /usr/local/include/sqlite3ext.h

sqlite also installed a pkg-config file:

# pkgconf --list-all |grep sql
# sqlite3                        SQLite - SQL database engine

# pkgconf sqlite3 --cflags --libs
# -I/usr/local/include -L/usr/local/lib -lsqlite3

[tiran]

Thanks @koobs !

Yes, I agree. setup.py does some problematic things in regards of default include and linker path. We are working on a series of improvements for the build system. One goal is to move header and linker checks out of setup.py and improve Modules/Setup.local.

We don't use pkg-config for sqlite3 yet. Is /usr/local/include on the default search path?

[koobs]

We should assume it is (and not hardcode paths, unless there's no better alternative). Right now most if not all downstreams will be passing -isystem and/or custom build *FLAGS (along with CC et al if needed) at the global environment, and/or configure environment, and/or make environment and or Modules/Setup level to make the build work, given the current state of the Python build system. It's very tough to make things work without regressing something else. Adding another unique or special/different case would be another step backward IMO.

Sorry for the non-binary answer :)

[tiran]

FYI, I summarized my plan in https://bugs.python.org/issue45573#msg406084

[erlend-aasland]

FYI: Holding this until GH-29164 has landed.

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @erlend-aasland for commit 0c36ebac224953603971079cbe706a77da248d5f 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @tiran for commit d7c4180 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

[tiran]

Looks good! Great job! Let's do another buildbot run. I'll merge the PR tomorrow.

[erlend-aasland]

FreeBSD still emits a warning:

checking sqlite3.h usability... yes
checking sqlite3.h presence... no
configure: WARNING: sqlite3.h: accepted by the compiler, rejected by the preprocessor!
configure: WARNING: sqlite3.h: proceeding with the compiler's result
checking for sqlite3.h... yes
checking for sqlite3_open_v2 in -lsqlite3... yes
checking for sqlite3_load_extension in -lsqlite3... yes

It also failed the configure step, but that was bco. getaddrinfo:

Fatal: You must get working getaddrinfo() function.
       or you can specify "--disable-ipv6".

UPDATE 1
How about patching CPPFLAGS in addition to instead of CFLAGS before running the AC checks?

UPDATE 2
I was previously unable to reproduce this on my FreeBSD, but that was because I had GCC installed. The FreeBSD buildbot does not have GCC installed. After pkg remove gcc, I was able to reproduce this issue, and I was also able to verify that the CPPFLAGS workaround works.

[erlend-aasland]

The ARM64 macOS buildbot failed at AC_CHECK_LIB([sqlite3], [sqlite3_open_v2], ..., leaving sqlite3 disabled. From previous runs, I see that it has SQLite 3.32.3 installed; there should be no reason for it to fail.

(The x86-64 macOS buildbot is fine, though.)

UPDATE
Suspecting the following might fix it:

diff --git a/configure.ac b/configure.ac
index 88b5a18ae9..1d87235a94 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3176,8 +3176,8 @@ AS_VAR_APPEND([LIBSQLITE3_CFLAGS], [' -I$(srcdir)/Modules/_sqlite'])
 
 AS_VAR_COPY([save_CFLAGS], [CFLAGS])
 AS_VAR_COPY([save_LDFLAGS], [LDFLAGS])
-AS_VAR_COPY([CFLAGS], [LIBSQLITE3_CFLAGS])
-AS_VAR_COPY([LDFLAGS], [LIBSQLITE3_LIBS])
+CFLAGS="$LIBSQLITE3_CFLAGS $CFLAGS"
+LDFLAGS="$LIBSQLITE3_LIBS $LDFLAGS"
 
 AC_CHECK_HEADER([sqlite3.h], [
   AC_CHECK_LIB([sqlite3], [sqlite3_open_v2], [

[tiran]

You figured it out already!

FreeBSD has 3rd party headers in /usr/local/include. The pre-processor does not use CFLAGS. You need CPPFLAGS here. I concur that it's safe to use CPPFLAGS instead of CFLAGS for sqlite. Most pgkconf files set -I, -D, -L, and -l. Other rare cases may need both CFLAGS and CPPFLAGS, though.

[erlend-aasland]

If 41a78b5 looks ok for you (as a fix for both the FreeBSD and macOS arm issues), we'll run it through the buildbots again.

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @tiran for commit 41a78b5 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

[erlend-aasland]

SQLite is now detected without warnings on FreeBSD. It does however still fail bco. getaddrinfo. It also fails like that on main.

Fatal: You must get working getaddrinfo() function.
       or you can specify "--disable-ipv6".

UPDATE: This fails because LIBS is touched by AC_CHECK_LIB. After the SQLite check, LIBS contains -lsqlite3, so when we run the getaddrinfo check a little bit later, it fails because it cannot find libsqlite3. Fixed by 0173b66.

[erlend-aasland]

ARM64 macOS is also fine now. Nice.

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @erlend-aasland for commit 0173b66 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

[tiran]

Tests are passing!

[erlend-aasland]

Tests are passing!

Finally 😆

[koobs]

Great job!