On Mon, Aug 12, 2024 at 11:39 PM Daniel Sahlberg <daniel.l.sahlb...@gmail.com> wrote: > > Hi, > > One question on the SQLiteAmalgamation code. > > There is currently a configuration option to request to use an amalgamation > for SQLite (SVN_SQLITE_USE_AMALGAMATION), defaulting to ON. If you don't have > an amalgamation downloaded (in the sqlite-amalgamation directory), > configuration will fail unless you set the option to OFF. > > The old ./configure script had the logic to use the amalgamation if available > or else check for a library. > > Is there a reason for defaulting to the amalgamation and requiring an > explicit option to NOT use it? > > With something like below, it seems the build would check for the > amalgamation and use it if found and otherwise check for a library. > > [[[ > Index: CMakeLists.txt > =================================================================== > --- CMakeLists.txt (revision 1919836) > +++ CMakeLists.txt (working copy) > @@ -228,8 +228,8 @@ > > ### SQLite3 > > -if(SVN_SQLITE_USE_AMALGAMATION) > - find_package(SQLiteAmalgamation REQUIRED) > +find_package(SQLiteAmalgamation) > +if(SQLiteAmalgamation_FOUND) > add_library(external-sqlite ALIAS SQLite::SQLite3Amalgamation) > else() > find_package(SQLite3 REQUIRED) > ]]] > > WDYT?
Hi, I also met this problem, especially on Linux, because I use the amalgamation on Windows and it works without any modifications or additional options. The current logic has existed since the initial version of the CMake and I didn't change anything there, because it was simpler to implement at the beginning. I would agree with the logic suggested, but my idea was to make less implicit options for build to be more repeatable, clear, and understandable. For example, to enable ra-serf, the user would have to explicitly pass the -DSVN_ENABLE_RA_SERF=ON option, even if it is installed on the system. However the case with SQLite differs a bit from the Serf one. There is also a similar situation with lz4 and utf8proc libraries, which could be used from internal and as external libraries. The other approach would be to change the default of the SVN_SQLITE_USE_AMALGAMATION to OFF to require the installed SQLite version. This should be okay, because usually only the advanced users or packagers use the amalgamation. However the amalgamation is simpler to use on WIndows than the installed one, because it doesn't require any compilation, so the other users could use it and it might be better to check both versions in this case for sure. -- Timofei Zhakov
