spkg-configure.m4 for sqlite

[orlitzky]

SQLite is another package that many people will have installed (firefox uses it, for example) and that has few dependencies in Sage (only readline).

libsqlite3 is used by Python during its build. Some Python packages need sufficiently new versions. We set the lower bound slightly lower than the current version available on XCode.

We detect it using AC_RUN_IFELSE because on macOS without homebrew, there is no pkgconfig file.

CC: @embray @dimpase @isuruf

Component: build: configure

Author: Michael Orlitzky, Matthias Koeppe

Branch: b261b31

Reviewer: Dima Pasechnik, Matthias Koeppe

Issue created by migration from https://trac.sagemath.org/ticket/29002

[orlitzky]

comment:1

I don't think we need such a new version, but 3.29.0 (from 2019-07-10) is the oldest one in Gentoo that I can test against. Should we accept versions back to 3.27 or 3.16 (or further) for Debian?

---

New commits:

e5e79c9

Trac #29002: new spkg-configure.m4 for sqlite.

[orlitzky]

Branch: u/mjo/ticket/29002

[orlitzky]

Commit: e5e79c9

[dimpase]

comment:2

I'm testing this with sqlite 3.20 on Fedora 26 now. (My Debian system has 3.27).

[dimpase]

comment:3

could you make the minimal version 3.20? Otherwise, all good. (I modified the version being 3.20 and tested with such sqlite)

On Fedora the packages are named sqlite and sqlite-devel - these ought to be listed in the corresponding docs - perhaps on another ticket.

[dimpase]

Reviewer: Dima Pasechnik

[sagetrac-git]

Changed commit from e5e79c9 to 61d7822

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

61d7822

Trac #29002: lower the version bound on the system sqlite check.

[orlitzky]

comment:6

Done, but the commit hook set this back to needs_review.

[fchapoton]

comment:8

missing author full name please

[orlitzky]

comment:9

Sorry, I'm new at this.

[orlitzky]

Author: Michael Orlitzky

[mkoeppe]

comment:10

This does not accept the system sqlite3 on macOS because we are installing readline -- but that is irrelevant because the system sqlite3 is linked against libedit, not readline.

[mkoeppe]

comment:11

egret:~/s/sage/sage-rebasing/worktree-venv (t/27824/public/27824-python3-spkg-configure *$)$ otool -L /usr/lib/libsqlite3.dylib 
/usr/lib/libsqlite3.dylib:
	/usr/lib/libsqlite3.dylib (compatibility version 9.0.0, current version 308.4.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1281.0.0)
egret:~/s/sage/sage-rebasing/worktree-venv (t/27824/public/27824-python3-spkg-configure *$)$ otool -L /usr/bin/sqlite3 
/usr/bin/sqlite3:
	/usr/lib/libncurses.5.4.dylib (compatibility version 5.4.0, current version 5.4.0)
	/usr/lib/libedit.3.dylib (compatibility version 2.0.0, current version 3.0.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1281.0.0)

[mkoeppe]

comment:12

Is the executable sqlite3 used at all in Sage? That is the only thing that depends on readline on Linux.
On Debian, libsqlite3.so and /usr/bin/sqlite3 are provided by different packages.

[dimpase]

comment:13

how does pkg-config even know about MacOS's sqlite3?

[mkoeppe]

comment:14

Replying to @mkoeppe:

Is the executable sqlite3 used at all in Sage?

Apart from providing the shortcut sage -sqlite, apparently not.

Should the spkg-configure check for the availability of the binary?

[orlitzky]

comment:71

Replying to @mkoeppe:

They changed that code so that older versions of sqlite will work.

Which versions do?

Python doesn't do a version check, and doesn't document any version bounds, so presumably all of them.

Put another way: if we find a version that makes it past our version-less check but that breaks python... then that's a python bug.

[mkoeppe]

comment:72

But it is not the goal of sage-the-distribution to provoke python bugs.

We vendor a version of sqlite so that the vendored python can build.

[orlitzky]

comment:73

Replying to @mkoeppe:

But it is not the goal of sage-the-distribution to provoke python bugs.

You make it sound so negative =)

If there's a version of sqlite that doesn't work with python, and if someone tries to build sage with it (both unlikely), then we report the bug upstream. At that point -- depending on the upstream fix -- we either do nothing or amend this spkg-configure with a lower bound.

Otherwise, we're just making things complicated for no ostensible reason. Since nobody is going to test every 3.x version of sqlite, we're still ultimately guessing at the lower version bound. My guess just happens to be 3.0.0.

If it's wrong, we can change it, but 3.0.0 is more likely to be correct than any other untested bound, and simplifies this check as a bonus.

[dimpase]

comment:74

I think one reason for these checks is to make supporting sage(lib) a bearable job. We don't want to bog down in corner cases on museum-grade OSes, just cause they might carry a museum-grade version of sqlite3.

So I am for keeping a version check.

One other thing - for better of worse, spkg-configure.m4 in other packages undefine local m4 variables they happen to define, whereas here it does not happen.
They use m4_pushdef/m4_popdef pair of macros for these purposes.
Can we keep to the same style here?

[orlitzky]

comment:75

Replying to @dimpase:

I think one reason for these checks is to make supporting sage(lib) a bearable job. We don't want to bog down in corner cases on museum-grade OSes, just cause they might carry a museum-grade version of sqlite3.

So I am for keeping a version check.

That's a bit ironic considering that four people have wasted hours replacing

SAGE_SPKG_CONFIGURE([sqlite], [
  PKG_CHECK_MODULES([SQLITE],
                    [sqlite3 >= 3.8.7],
                    [],
                    [sage_spkg_install_sqlite=yes])
])

with

SAGE_SPKG_CONFIGURE([sqlite], [
  m4_define([sqlite3_min_version_major], [3])
  m4_define([sqlite3_min_version_minor], [8])
  m4_define([sqlite3_min_version_micro], [7])
  m4_define([sqlite3_min_version], [sqlite3_min_version_major.sqlite3_min_version_minor.sqlite3_min_version_micro])
  AC_MSG_CHECKING([libsqlite3 >= sqlite3_min_version])
                     dnl https://www.sqlite.org/c3ref/libversion.html
                     dnl https://www.sqlite.org/c3ref/c_source_id.html
                     SQLITE_SAVED_LIBS="$LIBS"
                     LIBS="$LIBS -lsqlite3"
                     AC_RUN_IFELSE([
                       AC_LANG_PROGRAM([[
                                         #include <sqlite3.h>
                                         #include <assert.h>
                                         #include <stdlib.h>
                                         #include <string.h>
                                       ]],
                                       [[
                                         assert( strcmp(sqlite3_libversion(),SQLITE_VERSION)==0 );
                                         if (SQLITE_VERSION_NUMBER < ]]sqlite3_min_version_major[[*1000000 + ]]sqlite3_min_version_minor[[*1000 + ]]sqlite3_min_version_micro[[)
                                           exit(1);
                                         else
                                           exit(0);
                                       ]])
                       ],
                       [AC_MSG_RESULT([yes])],
                       [AC_MSG_RESULT([no])
                        LIBS="$SQLITE_SAVED_LIBS"
                        sage_spkg_install_sqlite=yes])
])

to support one version of sqlite on one proprietary OS whose maintainers went out of their way to delete an important file from the package.

sagelib doesn't even use sqlite, it's a dependency of a dependency

[mkoeppe]

comment:76

Replying to @orlitzky:

considering that four people have wasted hours

Excuse me, m4 and autotools are my hobby.

[mkoeppe]

comment:77

Replying to @dimpase:

spkg-configure.m4 in other packages undefine local m4 variables they happen to define, whereas here it does not happen.
They use m4_pushdef/m4_popdef pair of macros for these purposes.
Can we keep to the same style here?

Good idea, will do.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

b261b31

build/pkgs/sqlite/spkg-configure.m4: Use m4_push/popdef and match style of other spkg-configure.m4

[sagetrac-git]

Changed commit from 18f32a9 to b261b31

[mkoeppe]

comment:79

Ready for review.

[dimpase]

comment:82

lgtm

[mkoeppe]

comment:83

Thank you!

[embray]

comment:84

Replying to @mkoeppe:

Replying to @orlitzky:

considering that four people have wasted hours

Excuse me, m4 and autotools are my hobby.

HAHA :)

I'm also +1 for keeping the version check; personally I would lower the minimum version 3.0.0 until/unless a stronger minimum is actually determined. But 3.8.7 is still old-enough that it should be satisfied on most system so I'm okay with it.

[mkoeppe]

comment:85

Replying to @embray:

I would lower the minimum version 3.0.0 until/unless a stronger minimum is actually determined. But 3.8.7 is still old-enough that it should be satisfied on most system so I'm okay with it.

3.0.0 was released in 2004. https://www.sqlite.org/chronology.html

[vbraun]

Changed branch from u/mkoeppe/ticket/29002 to b261b31

[embray]

comment:87

It turns out there are some tests that fail if the sqlite3 executable is not installed:

sage -t --long --warn-long 78.2 src/sage/tests/cmdline.py
**********************************************************************
File "src/sage/tests/cmdline.py", line 608, in sage.tests.cmdline.test_executable
Failed example:
    out.startswith("3.")
Expected:
    True
Got:
    False
**********************************************************************
File "src/sage/tests/cmdline.py", line 610, in sage.tests.cmdline.test_executable
Failed example:
    err
Expected:
    ''
Got:
    '/home/sage/sage/src/bin/sage: line 546: exec: sqlite3: not found\n'
**********************************************************************
File "src/sage/tests/cmdline.py", line 612, in sage.tests.cmdline.test_executable
Failed example:
    ret
Expected:
    0
Got:
    127
**********************************************************************
1 item had failures:
   3 of 236 in sage.tests.cmdline.test_executable
    [235 tests, 3 failures, 45.53 s]
----------------------------------------------------------------------
sage -t --long --warn-long 78.2 src/sage/tests/cmdline.py  # 3 doctests failed
----------------------------------------------------------------------

So, technically it is a regression of sage -sqlite3 does not work. I don't think this is very important functionality and it would probably be better if it were slowly deprecated (I think the same should be the case for most sage -<program name>--rather, that most of those should be undocumented, and that there should be a generic interface for running executables that might be installed in the Sage distribution).

In the meantime, I'm leaning towards we need to check for it...

[embray]

Changed commit from b261b31 to none

[EmmanuelCharpentier]

comment:88

Replying to @embray:

It turns out there are some tests that fail if the sqlite3 executable is not installed:

[ Snip... ]

So, technically it is a regression of sage -sqlite3 does not work.

Indeed. I just got bitten by this (trying to ebuild 9.1.beta2 from scratch rather than upgrading).

I don't think this is very important functionality and it would probably be better if it were slowly deprecated (I think the same should be the case for most sage -<program name>--rather, that most of those should be undocumented, and that there should be a generic interface for running executables that might be installed in the Sage distribution).

I tend to disagree : an unsuspecting user might have a hard time to understand why "our" version replaces the one he/she installed in his/her system if he/she runs out utility that installs system shortcuts for Sage-installed programs ...

In the meantime, I'm leaning towards we need to check for it...

Indeed. But since sqlite is so widely used, we might alsoi depend on the relevant packages (easy for Unices, possibly not so easy for Windows (Madison ?), and possible Apple shenanigans...).

BTW, our move to reduce the size of Sage-the-distribution should reopen the question of what are our minimal dependencies. I would have no problem to depend on sqlite, more on Maxima (Lisp interpreters !) and even more on R (size, dependencies on OpenSSL, etc...)

Should that be disciussed on sage-devel ?

In the meantime, should we open a new ticket ?

[dimpase]

comment:89

Replying to @EmmanuelCharpentier:

Replying to @embray:

It turns out there are some tests that fail if the sqlite3 executable is not installed:

[ Snip... ]

So, technically it is a regression of sage -sqlite3 does not work.

Indeed. I just got bitten by this (trying to ebuild 9.1.beta2 from scratch rather than upgrading).

I don't think this is very important functionality and it would probably be better if it were slowly deprecated (I think the same should be the case for most sage -<program name>--rather, that most of those should be undocumented, and that there should be a generic interface for running executables that might be installed in the Sage distribution).

I tend to disagree : an unsuspecting user might have a hard time to understand why "our" version replaces the one he/she installed in his/her system if he/she runs out utility that installs system shortcuts for Sage-installed programs ...

In the long run there should be no sqlite or libsqlite bundled into Sage.

In the meantime, I'm leaning towards we need to check for it...

Indeed. But since sqlite is so widely used, we might alsoi depend on the relevant packages (easy for Unices, possibly not so easy for Windows (Madison ?), and possible Apple shenanigans...).

BTW, our move to reduce the size of Sage-the-distribution should reopen the question of what are our minimal dependencies. I would have no problem to depend on sqlite, more on Maxima (Lisp interpreters !) and even more on R (size, dependencies on OpenSSL, etc...)

I won't worry too much about R, as it's already more or less sorted.
Maxima/ECL is harder, as it's tigher integrated into Sage, and as ECL is moving very slowly nowadays (and with significant changes to come).

Should that be disciussed on sage-devel ?

In the meantime, should we open a new ticket ?

yes, please, for deprecating/removing sage -FOO functionality for FOO=sqlite and other values like this, right?

[mkoeppe]

comment:90

@embray Thanks for spotting this and for opening the follow-up ticket:

• sqlite3 CLI executable should be checked for at configure time #29092 sqlite3 CLI executable should be checked for at configure time
  I agree that this needs to be fixed.

[mkoeppe]

comment:91

I have opened:

• src/bin/sage: Delegate handling of options of sage-the-distribution, specifying a subset that is supported by sagelib proper #29111 Specify a subset of sage command line options that are supported by sagelib - rather than sage-the-distribution