search

utelle / SQLite3MultipleCiphers

SQLite3 encryption extension with support for multiple ciphers
https://utelle.github.io/SQLite3MultipleCiphers/
MIT License
390 stars 73 forks source link

Add CMake Support For Windows, Linux, Mac OS X and fix shell and shell_icu compile errors when using SQLITE_ENALE_SERIES and SQLITE_ENABLE_REGEXP #102

Closed jammerxd closed 1 year ago

jammerxd commented 1 year ago

This PR is simular to #101 except it fixes the compile-time issues with shell.c to allow the SQLITE_ENABLE_SERIES and SQLITE_ENABLE_REGEXP preprocessor flags to compile without errors.

This PR adds a proof-of-concept CMakeLists.txt - this CMakeLists.txt will work for Windows and Linux (I tested under Ubuntu 22.04 Desktop edition in a virtual machine)

To build with cmake on windows with icu support, specify -DWITH_ICU= on the command line. This should be the folder that has the include, lib, and bin folders or include, lib64, and bin64 folders when building x64.

The build.bat is included as an example of how to build 32-bit Debug/Release and 64-bit Debug/Release and have the build binaries/libs go to separate folders.

jammerxd commented 1 year ago

This is based on the work that @lwttai did and closes #9

utelle commented 1 year ago

First of all, thanks for your efforts. I have to admit that I haven't taken care of adding CMake support for quite a long time. However, I'm not yet happy with the CMake specifications provided by this PR. Of course, it is good that you added and tested build support for Linux (and macOS) systems.

IMHO still too many os and compiler specific options are defined within the CMake file. IMHO the CMake file should be based on the default compiler settings only as far as possible. If I'm not mistaken CMake allows to specify additional options from the outside (although I don't know the details). This would be the preferred approach. Only really necessary details should be part of the CMake file. This is also true for specifying the type of runtime libraries. You defined separate build targets for static and dynamic runtime libraries. Again, IMHO this is something that could and should defined outside of the CMake file. Doing it that way would give developers the greatest possible freedom without the need to modify the CMake file itself.

This PR is simular to #101 except it fixes the compile-time issues with shell.c to allow the SQLITE_ENABLE_SERIES and SQLITE_ENABLE_REGEXP preprocessor flags to compile without errors.

For unknown reasons the SQLite developers decided to include the sources of these 2 extensions in the shell code and not the library. I really don't want to tamper with the original SQLite sources more than absolutely necessary.

IMHO the better approach would be to define 3 build targets

1) Dynamic library 2) Static library 3) Shell application. For this target add sqlite3mc.c to the list of sources (instead of using the static library). This allows to use a different set of defines for the shell and the library.

This PR adds a proof-of-concept CMakeLists.txt - this CMakeLists.txt will work for Windows and Linux (I tested under Ubuntu 22.04 Desktop edition in a virtual machine)

To build with cmake on windows with icu support, specify -DWITH_ICU= on the command line. This should be the folder that has the include, lib, and bin folders or include, lib64, and bin64 folders when building x64.

Currently, I don't know which kind of support CMake offers to specify conditional dependencies.

The build.bat is included as an example of how to build 32-bit Debug/Release and 64-bit Debug/Release and have the build binaries/libs go to separate folders.

IMHO this information should be added to the documentation instead.

jammerxd commented 1 year ago

Compiler Flags

IMHO still too many os and compiler specific options are defined within the CMake file

There are certain flags that are set for MSVC that are not compatible with linux or darwin systems. All of the compiler-specific flags are flags that were set and taken from the visual studio project/solution files provided in the build folder. Can you point out the ones you are not happy with?

Visual Studio Project File for sqlite3mc_lib from build\sqlite3mc_vc17 (for example):

/ifcOutput "obj\vc17\Win64\Release\sqlite3mc_dll\" /GS /W3 /Gy /Zc:wchar_t /I"..\src" /Zi /Gm- /Ox /Fd"obj\vc17\Win64\Release\sqlite3mc_dll\vc143.pdb" /Zc:inline /fp:precise /D "_WINDOWS" /D "WIN32" /D "_CRT_SECURE_NO_WARNINGS" /D "_CRT_SECURE_NO_DEPRECATE" /D "_CRT_NONSTDC_NO_WARNINGS" /D "_CRT_NONSTDC_NO_DEPRECATE" /D "NDEBUG" /D "_USRDLL" /D "CODEC_TYPE=CODEC_TYPE_CHACHA20" /D "SQLITE_ENABLE_DEBUG=0" /D "SQLITE_THREADSAFE=1" /D "SQLITE_DQS=0" /D "SQLITE_MAX_ATTACHED=10" /D "SQLITE_SOUNDEX=1" /D "SQLITE_ENABLE_COLUMN_METADATA=1" /D "SQLITE_SECURE_DELETE=1" /D "SQLITE_ENABLE_DESERIALIZE=1" /D "SQLITE_ENABLE_FTS3=1" /D "SQLITE_ENABLE_FTS3_PARENTHESIS=1" /D "SQLITE_ENABLE_FTS4=1" /D "SQLITE_ENABLE_FTS5=1" /D "SQLITE_ENABLE_RTREE=1" /D "SQLITE_ENABLE_GEOPOLY=1" /D "SQLITE_ENABLE_PREUPDATE_HOOK=1" /D "SQLITE_CORE=1" /D "SQLITE_ENABLE_EXTFUNC=1" /D "SQLITE_ENABLE_MATH_FUNCTIONS=1" /D "SQLITE_ENABLE_CSV=1" /D "SQLITE_ENABLE_VSV=1" /D "SQLITE_ENABLE_SHA3=1" /D "SQLITE_ENABLE_CARRAY=1" /D "SQLITE_ENABLE_FILEIO=1" /D "SQLITE_ENABLE_SERIES=1" /D "SQLITE_ENABLE_UUID=1" /D "SQLITE_ENABLE_REGEXP=1" /D "SQLITE_TEMP_STORE=2" /D "SQLITE_USE_URI=1" /D "SQLITE_USER_AUTHENTICATION=1" /D "_WINDLL" /D "_UNICODE" /D "UNICODE" /errorReport:prompt /GF /WX- /Zc:forScope /Gd /Oi /MT /FC /Fa"obj\vc17\Win64\Release\sqlite3mc_dll\" /EHsc /nologo /Fo"obj\vc17\Win64\Release\sqlite3mc_dll\" /Fp"obj\vc17\Win64\Release\sqlite3mc_dll\sqlite3mc_x64.pch" /diagnostics:column

Notice how /Zi, /GF, /EHsc, /Gy, /Oi, and /Ox are specified on the command line and in the project properties of visual studio? That's because these are not the default for MSVC.

CMake File:

# Debug Information Format: Program Database
/Zi
# Enable String Pooling
/GF
# Enable C++ Exceptions
/EHsc
# Enable Function-Level Linking
/Gy
# Full Optimize for Release
$<$<CONFIG:Release>:/Ox>
# Enable Intrinsic Functions for Release
$<$<CONFIG:Release>:/Oi>

The ones above are wrapped in the if(MSVC) block because they are specific to the MSVC compiler and won't work on linux compilers - there's no way around this and this is generally how CMake files structure themselves to be cross-compiler compatible.

Similarly, in order to compile successfully on linux/osx, the compiler flag -march=native needs to be specified, but this does not need to be specified on windows and will cause the MSVC compiler to fail. So this flag is wrapped in a if Linux and if Darwin block.

Static vs Dynamic

You defined separate build targets for static and dynamic runtime libraries. Again, IMHO this is something that could and should defined outside of the CMake file - yes because in CMAKE, they are different targets, and so are /MT, /MD, /MTd, and /MDd.

To define a static library in cmake - you need to change the call to add_library. Other projects also define separate targets for static versions - OpenSSL, zlib, curl, boost.

So in combination there's the following: Multithreaded Release (/MT) Multithreaded Debug (/MTd) Multithreaded DLL Release (/MD) Multithreaded DLL Debug (/MDd)

There is no 'standard' and every CMake library is done diffrerently.

Sqlite3 shell

I really don't want to tamper with the original SQLite sources more than absolutely necessary.

I agree. The only change I made was to comment out the body of the 2 conflicting functions and mark them as extern to allow the linker to pull the functions from the compiled library and to actually compile without complaining. These flags should be supported and this should be reported as a bug in whatever tool generated the shell.c script, however I don't have the time or skills to go that far up the chain. This solves this problem with minimal impact.

sqlite3mc.c - Sure I can add that as the source file.

Conditional Dependencies

Again, there is no 'Standard' for doing this. Most of the time these options are specified on the command line using variables. To specify a variable on the CMake build command line, use the syntax cmake -D<variable>=<value> <sourceDir>. So if I wanted to compile with ICU library support and include it that's where the WITH_ICU flag comes in. To specify it - the command would look like cmake -DWITH_ICU=<path> but there's a catch:

ICU on Windows

For windows, there are premade binaries available to download and structured in the following tree for 32-bit: include bin lib For 64-bit - the folders are different: include bin64 lib64

Thus we need another if(MSVC) block that specifies to add the lib64 or lib folder to the library search path for the linker depending on if we're compiling for 64-bit or 32-bit. We also need to add the include path to the include search path on windows.

So we'll need to download and extract the binaries and then on the command line specify that folder, for example: cmake -DWITH_ICU=C:\Users\Developer\Git\3rdparty\icu .

ICU on Linux

For linux, namely ubuntu, there is a package already made available in apt, calling sudo apt install libicu-devel (package details here, linux will install the include files to /usr/include and the lib files to /usr/lib/x86-x64-gnu-linux. Thus for linux, we only need to link the associated libraries and can leave the variable empty and just define it: cmake -DWITH_ICU .

Build.bat

Sure I can add that to the docs instead if that is preferred. I just found it more convenient to include something usable to get people up and running out of the box.

A lot of this same logic exists in the make files and autoconf files as well. If you run premake5 and autoconf and reconf, and then actually stop to go examine the entire makefile chain, you will see a lot of the same compiler-specific logic applies.

jammerxd commented 1 year ago

All in all, CMake uses variables with the -D flag -e.g. cmake -D<variable>=<value>.... <sourceDir>. These do not go to be set as preprocessor flags but rather CMake variables. Which is why there's going to need to be a lot more logic if you want the ability to just have all the SQLITE_ flags carry through.

These variables are then referenced via ${VARIABLE} in the cmake script.

jammerxd commented 1 year ago

I can attempt to make some further enhancements and build on the logic, but the more compatibility and options you want, the more complex and compiler-specific the file is going to look.

utelle commented 1 year ago

Compiler Flags

IMHO still too many os and compiler specific options are defined within the CMake file

There are certain flags that are set for MSVC that are not compatible with linux or darwin systems. All of the compiler-specific flags are flags that were set and taken from the visual studio project/solution files provided in the build folder. Can you point out the ones you are not happy with?

To be frank: all of them. However, I think I have to explain this a bit (see below).

Visual Studio Project File for sqlite3mc_lib from build\sqlite3mc_vc17 [...] Notice how /Zi, /GF, /EHsc, /Gy, /Oi, and /Ox are specified on the command line and in the project properties of visual studio? That's because these are not the default for MSVC.

I use premake5 to generate all build files (VC++ solutions, gcc makefiles). If you take a look at the premake5 build description file premake5.lua (and wx_config.lua) in the root folder you will see that no compiler options are directly defined (with the exception of an option to enable AES hardware support for gcc). Only properties are defined like kind (StaticLib or SharedLib) or characterset or staticruntime and so on. premake5 knows which compiler options to generate from those properties for which compiler.

Unfortunately, I don't know much about CMake, but I would assume that such a popular tool like CMake has a similar mechanism.

CMake File: ` # Debug Information Format: Program Database /Zi # Enable String Pooling /GF # Enable C++ Exceptions /EHsc # Enable Function-Level Linking /Gy # Full Optimize for Release $<$CONFIG:Release:/Ox> # Enable Intrinsic Functions for Release $<$CONFIG:Release:/Oi>

The ones above are wrapped in the if(MSVC) block because they are specific to the MSVC compiler and won't work on linux compilers - there's no way around this and this is generally how CMake files structure themselves to be cross-compiler compatible.

Again, this is something premake5 simply knows, based on the selected toolset. It would surprise me, if CMake doesn't offer such features.

Similarly, in order to compile successfully on linux/osx, the compiler flag -march=native needs to be specified, but this does not need to be specified on windows and will cause the MSVC compiler to fail. So this flag is wrapped in a if Linux and if Darwin block.

I learned that the flag -march=native should not be used. Instead the flags -msse4.2 -maes are specified for gcc on x86 platforms.

I'm aware of that this is compiler and platform specific and has to be defined explicitly (using if blocks if necessary.

Static vs Dynamic

You defined separate build targets for static and dynamic runtime libraries. Again, IMHO this is something that could and should defined outside of the CMake file - yes because in CMAKE, they are different targets, and so are /MT, /MD, /MTd, and /MDd.

To define a static library in cmake - you need to change the call to add_library. Other projects also define separate targets for static versions - OpenSSL, zlib, curl, boost.

Sorry, this is a misunderstanding. I didn't meant static vs dynamic library targets. I meant whether static or dynamic runtime libraries (for example, standard C library) are used. In your current CMakeList file you have separate targets to differentiate between the 2 options.

I'm pretty sure that that can be solved using CMake variables.

So in combination there's the following: Multithreaded Release (/MT) Multithreaded Debug (/MTd) Multithreaded DLL Release (/MD) Multithreaded DLL Debug (/MDd)

Usually, a developer doesn't need all variants of a library. So, selecting between (/MT,/MTd) and (/MD,/MDd) could be done via a variable.

Sqlite3 shell

I really don't want to tamper with the original SQLite sources more than absolutely necessary.

I agree. The only change I made was to comment out the body of the 2 conflicting functions and mark them as extern to allow the linker to pull the functions from the compiled library and to actually compile without complaining.

Well, this works if you use CMake, but it breaks the other build options (see the results of the CI runs).

shell.c should be left alone. And that's easy to accomplish by adding the source file sqlite3mc.c to the shell target.

These flags should be supported and this should be reported as a bug in whatever tool generated the shell.c script, however I don't have the time or skills to go that far up the chain. This solves this problem with minimal impact.

Yes, these flags should be supported. And I have to admit that it is a deficiency of the build files coming with the project that the flags can't be defined for the static library target. Probably I will address this issue.

Actually, we have no influence on which extensions are included in the shell source and which not. Therefore the source shell.c should not be modified regarding the extensions.

Conditional Dependencies

Again, there is no 'Standard' for doing this. Most of the time these options are specified on the command line using variables. To specify a variable on the CMake build command line, use the syntax cmake -D<variable>=<value> <sourceDir>. So if I wanted to compile with ICU library support and include it that's where the WITH_ICU flag comes in. To specify it - the command would look like cmake -DWITH_ICU=<path> but there's a catch:

I know that building on Windows is often a challenge, because there is no system package manager.

However, AFAIK CMake has a mechanism find_module (or similar) to search for dependencies. If possible, that mechanism should be used.

Build.bat

Sure I can add that to the docs instead if that is preferred.

Yes. Build information could be added to readme.md (and later on to the online documentation).

I just found it more convenient to include something usable to get people up and running out of the box.

Well, the .bat file creates all configurations unconditionally. Most developers only need a few of the configurations.

A lot of this same logic exists in the make files and autoconf files as well. If you run premake5 and autoconf and reconf, and then actually stop to go examine the entire makefile chain, you will see a lot of the same compiler-specific logic applies.

Not sure what you intend to tell me by this.

premake5 is different from CMake in that aspect that it is usually run only by me to generate build files that work out of the box. Only if a user is not happy with the pre-configured options he may need to rerun premake5.

The build files generated by premake5 are for Windows platforms only. For Unix-like platforms the autoconf/automake toolchain is supported. Here, the user needs to execute autoreconf, configure and make.

utelle commented 1 year ago

All in all, CMake uses variables with the -D flag -e.g. cmake -D<variable>=<value>.... <sourceDir>. These do not go to be set as preprocessor flags but rather CMake variables. Which is why there's going to need to be a lot more logic if you want the ability to just have all the SQLITE_ flags carry through.

Again, I don't know much about CMake, but AFAIK you can specify variables with default values that can be overwritten via the command line (or the CMake GUI tool). IMHO using that mechanism for defining which SQLite extensions should be included and which not, would an advantage of CMake over premake5.

utelle commented 1 year ago

I can attempt to make some further enhancements and build on the logic, but the more compatibility and options you want, the more complex and compiler-specific the file is going to look.

Compiler and platform specifics should be avoided as far as possible (I know it's not always possible). Generic complexity (like options to select which SQLite extension to enable) are not such a problem.

jammerxd commented 1 year ago

Compiler Flags

Okay I think I got it worked out - CMake does set the same flags in the visual studio solutions that are generated - so we're good there.

I'll still need to go through and add specify an OPTION for each of the SQLITE_XXXX preprocessor flags so that CMake just adds them to the target_compile_definitions.

I learned that the flag -march=native should not be used. Instead the flags -msse4.2 -maes are specified for gcc on x86 platforms.

Got it - I made that change as well. Note this only applies for linux and osx.

Essentially, CMake is the same as your premake in that it generates make files and visual studio project/solution files.

Static vs Dynamic

Gotcha - makes sense. I updated the CMakeLists to use SQLITE3MC_STATIC_RUNTIME_LINK and SQLITE3MC_STATIC over the command line. By default both of these are off.

SQLITE3MC_STATIC=ON will build sqlite3mc as a static library with a target name of sqlite3mc_static

SQLITE3MC_STATIC_RUNTIME_LINK=ON will specify the /MT, and /MTd flags. Otherwise the CMake defaults of /MD and /MDd will be used.

Sqlite3 shell

This one is still troublesome as I seem to only need to include sqlite3mc.c in the sources on windows. Not sure why. If I include sqlite3mc.c as a source file on linux, I get all kinds of "redefinition" issues.

See the latest commit. I managed to fix this.

ICU and conditional dependency in CMake:

However, AFAIK CMake has a mechanism find_module (or similar) to search for dependencies. If possible, that mechanism should be used.

Didn't think about this one - but in this case, yes, ICU is supported by the find_module function in cmake. link. I went and switched to that. You will still need to specify the SQLITE3MC_WITH_ICU variable over the command line to include it.

Build.bat

I'll work on potential resources for this, but in reality those using the cmake build system should know how to tell cmake to build debug or release, 32-bit or 64-bit.

utelle commented 1 year ago

Compiler Flags

Okay I think I got it worked out - CMake does set the same flags in the visual studio solutions that are generated - so we're good there.

Thanks a lot for your work on this. The CMake build file looks cleaner now and is more user friendly.

I'll still need to go through and add specify an OPTION for each of the SQLITE_XXXX preprocessor flags so that CMake just adds them to the target_compile_definitions.

Essentially, CMake is the same as your premake in that it generates make files and visual studio project/solution files.

When I made my decision to use premake many years ago, CMake was not able to generate a combined VC++ solution with support for Debug/Release and 32-bit/64-bit architecture. I don't know whether recent CMake versions are now able to do that. However, probably this doesn't matter much for CMake users.

Static vs Dynamic

Gotcha - makes sense. I updated the CMakeLists to use SQLITE3MC_STATIC_RUNTIME_LINK and SQLITE3MC_STATIC over the command line. By default both of these are off.

Great.

SQLITE3MC_STATIC=ON will build sqlite3mc as a static library with a target name of sqlite3mc_static

SQLITE3MC_STATIC_RUNTIME_LINK=ON will specify the /MT, and /MTd flags. Otherwise the CMake defaults of /MD and /MDd will be used.

Fantastic.

Sqlite3 shell

This one is still troublesome as I seem to only need to include sqlite3mc.c in the sources on windows. Not sure why. If I include sqlite3mc.c as a source file on linux, I get all kinds of "redefinition" issues.

You only have to specify 2 source files - shell.c and sqlite3mc.c. sqlite3mc.c includes all other sourece file via #include directive. This is true for all platforms.

Regarding redefinition issues: most likely you have defined symbols SQLITE_ENABLE_* for extensions that are already part of shell.c. Please check the list of options for the static library in premake5.lua. After removing the symbols

for the shell target the redefinition issues should vanish.

ICU and conditional dependency in CMake:

However, AFAIK CMake has a mechanism find_module (or similar) to search for dependencies. If possible, that mechanism should be used.

Didn't think about this one - but in this case, yes, ICU is supported by the find_module function in cmake. link. I went and switched to that. You will still need to specify the SQLITE3MC_WITH_ICU variable over the command line to include it.

Build.bat

I'll work on potential resources for this, but in reality those using the cmake build system should know how to tell cmake to build debug or release, 32-bit or 64-bit.

Exactly. Therefore I think we really don't need this file.

jammerxd commented 1 year ago

So at this point the CMakeLists.txt will build the combinations of: Debug/Release - native CMake (cmake --build . --config Release or cmake --build . --config Debug) Win32, x86, or x64 - native CMake (cmake -A Win32 . - before calling --build) Static or Dynamic Library - cmake -DSQLITE3MC_STATIC=ON . (before calling --build) Static runtime linking - cmake -SQLITE3MC_STATIC_RUNTIME_LINK=ON . (before calling --build)

And has no outstanding issues with any modules to include - the issue with SQLITE_ENABLE_REGEXP and SQLITE_ENABLE_SERIES is resolved and these flags are supported and build successfully.

This will now generate build files for windows, linux, and osx as well as compile them appropriately.

To generate the build files: cmake <Defines> <sourceDir>

To specify 32-bit or 64-bit: cmake -A Win32 <Defines> <sourceDir> cmake -A x64 <Defines> <sourceDir> cmake -A x86 <Defines> <sourceDir>

To build using the generated build files: cmake --build <sourceDir>

To build using the generated build files in Release mode: cmake --build <sourceDir> --config Release

jammerxd commented 1 year ago

Shell

You only have to specify 2 source files - shell.c and sqlite3mc.c. sqlite3mc.c includes all other sourece file via #include directive. This is true for all platforms.

Regarding redefinition issues: most likely you have defined symbols SQLITEENABLE* for extensions that are already part of shell.c. Please check the list of options for the static library in premake5.lua. After removing the symbols

SQLITE_ENABLE_SHA3=1
SQLITE_ENABLE_FILEIO=1
SQLITE_ENABLE_SERIES=1
SQLITE_ENABLE_REGEXP=1

for the shell target the redefinition issues should vanish.

Actually I ended up resolving it completely without having to remove those flags. It was because sqlite3.h was being included as a source or (.c) file instead of an include file. I removed it from the sources files list and that resolved the issues completely.

jammerxd commented 1 year ago

When I made my decision to use premake many years ago, CMake was not able to generate a combined VC++ solution with support for Debug/Release and 32-bit/64-bit architecture. I don't know whether recent CMake versions are now able to do that. However, probably this doesn't matter much for CMake users.

Correct they still don't but there are command line options to tell CMake which versions to build. Debug/Release are included in the same one, but 32-bit and 64-bit are still controlled by the -A parameter at the time of generating the build files.

jammerxd commented 1 year ago

So the last thing I'll do is add an OPTION in the CMakeLists for all of the various SQLITE_XXXXX flags that are currently hard coded. The only one that might be troublesome is the CODEC.

jammerxd commented 1 year ago

I'll also preface this by saying I have no idea if this will work on mac osx as I don't own one, nor do I own the proper hardware to test this on. I can only test on linux - there's only an attempt to add osx compatibility here.

jammerxd commented 1 year ago

I think this should be it!

I think I've tackled pretty much everything except documentation.

utelle commented 1 year ago

So the last thing I'll do is add an OPTION in the CMakeLists for all of the various SQLITE_XXXXX flags that are currently hard coded. The only one that might be troublesome is the CODEC.

In this area we still need a bit of fine-tuning. Some flags should not be exposed at all, some flags are not boolean, but numeric. Additionally, we have to take into account a subtlety of SQLite's use of preprocessor symbols.

Now, a word about SQLite's use of preprocessor symbols. The SQLite code checks most of the time only whether a symbol is defined or not without taking the defined value into account. For example, if one defines SQLITE_ENABLE_CARRAY=0 it will have the same effect as defining SQLITE_ENABLE_CARRAY=1 - in both cases the preprocessor defines the symbol, and only the definition status is checked by the preprocessor statement #ifdef. That is, to disable a feature, the symbol must not be defined at all. Nevertheless, if a boolean flag is true, the symbol should be defined with the value 1, because sometimes the value matters, too (is checked with #if instead of #ifdef).

So, all the following boolean symbols

should only be defined for the preprocessor, if their value is 1 (true). Otherwise, the symbol should not be defined for the preprocessor.

Hopefully, it is not too difficult to implement this behaviour in CMake.

I'll also preface this by saying I have no idea if this will work on mac osx as I don't own one, nor do I own the proper hardware to test this on. I can only test on linux - there's only an attempt to add osx compatibility here.

I will test it for Mac OS over the weekend.

I think this should be it!

Yes, we are almost there - except for the handling of the option flags (see above). As soon as the proper handling of options is established I intend to merge your PR. Thanks a lot for your efforts.

I think I've tackled pretty much everything except documentation.

Yes. A bit of documentation can be added later after merging. I don't think that a lot of documentation is needed, because most likely CMake users already know how to invoke CMake and how to specify options on the command line or in the CMake GUI tool.

jammerxd commented 1 year ago

Gotcha I'll work on that - and this is where a quirk of CMake comes in - on the CMake command line, the command line variables being defined as true can be set as 1, ON, YES or TRUE. 0, OFF, NO, FALSE for false flags. Using $<BOOL:${VARIABLE}> is a generator expression and outputs a 1 or 0 in CMake (you can see this in the visual studio project files generated).

utelle commented 1 year ago

Gotcha I'll work on that - and this is where a quirk of CMake comes in - on the CMake command line, the command line variables being defined as true can be set as 1, ON, YES or TRUE. 0, OFF, NO, FALSE for false flags. Using $<BOOL:${VARIABLE}> is a generator expression and outputs a 1 or 0 in CMake (you can see this in the visual studio project files generated).

At least for the boolean options something like this might work:

# Define option
OPTION(_SQLITE_ENABLE_GEOPOLY "" ON)
# Define preprocessor symbol if option is selected (true)
$<$<BOOL:${_SQLITE_ENABLE_GEOPOLY}>:SQLITE_ENABLE_GEOPOLY=1>

or alternatively

# Define option
OPTION(_SQLITE_ENABLE_GEOPOLY "" ON)
# Define preprocessor symbol if option is selected (true)
if(_SQLITE_ENABLE_GEOPOLY)
  SQLITE_ENABLE_GEOPOLY=1
endif()
jammerxd commented 1 year ago

You're close and that's how I'm planning on doing that - in CMake, there is a difference between a variable and a preprocessor flag. The variable does not equate to a preprocessor flag - so in your example I'd be doing (psuedo code)

set (SQLITE3MC_BASE_DEFINITIONS 
$<$<BOOL:${SQLITE_ENABLE_GEOPOLY}>:SQLITE_ENABLE_GEOPOLY=1>
)

${SQLITE_ENABLE_GEOPOLY} is the variable specified over the command line, SQLITE_ENABLE_GOPOLY=1 is added to the variable SQLITE3MC_BASE_DEFINITIONS only if the variable SQLITE_ENABLE_GEOPOLY is specified as true on the command line (true being any of 1, YES, ON, TRUE) - and if it's not true then nothing is added to SQLITE3MC_BASE_DEFINITIONS

There's also different ways of accessing variables in CMake - in an if block, no extra parenthesis or syntax is needed, in a generator expression, $<VARIABLE> is used, when using the variable for output or in a function, it's accessed as ${VARIABLE}. CMake is just weird like that

jammerxd commented 1 year ago

Do you think you could provide me a full list of options that should be exposed? This is the list I have from scanning the sqlite3.c and sqlite3mc.c files for #ifdef SQLITE_*

SQLITE_4_BYTE_ALIGNED_MALLOC
SQLITE_64BIT_STATS
SQLITE_ALLOW_COVERING_INDEX_SCAN
SQLITE_ALLOW_ROWID_IN_VIEW
SQLITE_ALLOW_URI_AUTHORITY
SQLITE_AMALGAMATION
SQLITE_ASCII
SQLITE_ASSERT_NO_FILES
SQLITE_ATOMIC_INTRINSICS
SQLITE_BITMASK_TYPE
SQLITE_BUG_COMPATIBLE_20160819
SQLITE_CASE_SENSITIVE_LIKE
SQLITE_CHECK_PAGES
SQLITE_COUNTOFVIEW_OPTIMIZATION
SQLITE_COVERAGE_TEST
SQLITE_CUSTOM_INCLUDE
SQLITE_DEBUG
SQLITE_DEBUG_SORTER_THREADS
SQLITE_DEFAULT_AUTOMATIC_INDEX
SQLITE_DEFAULT_AUTOVACUUM
SQLITE_DEFAULT_CACHE_SIZE
SQLITE_DEFAULT_CKPTFULLFSYNC
SQLITE_DEFAULT_FILE_FORMAT
SQLITE_DEFAULT_FILE_PERMISSIONS
SQLITE_DEFAULT_FOREIGN_KEYS
SQLITE_DEFAULT_JOURNAL_SIZE_LIMIT
SQLITE_DEFAULT_LOCKING_MODE
SQLITE_DEFAULT_LOOKASIDE
SQLITE_DEFAULT_MEMSTATUS
SQLITE_DEFAULT_MMAP_SIZE
SQLITE_DEFAULT_PAGE_SIZE
SQLITE_DEFAULT_PCACHE_INITSZ
SQLITE_DEFAULT_PROXYDIR_PERMISSIONS
SQLITE_DEFAULT_RECURSIVE_TRIGGERS
SQLITE_DEFAULT_ROWEST
SQLITE_DEFAULT_SECTOR_SIZE
SQLITE_DEFAULT_SYNCHRONOUS
SQLITE_DEFAULT_UNIX_VFS
SQLITE_DEFAULT_WAL_AUTOCHECKPOINT
SQLITE_DEFAULT_WAL_SYNCHRONOUS
SQLITE_DEFAULT_WORKER_THREADS
SQLITE_DIRECT_OVERFLOW_READ
SQLITE_DISABLE_DIRSYNC
SQLITE_DISABLE_FTS3_UNICODE
SQLITE_DISABLE_FTS4_DEFERRED
SQLITE_DISABLE_INTRINSIC
SQLITE_DISABLE_LFS
SQLITE_DISABLE_PAGECACHE_OVERFLOW_STATS
SQLITE_DISABLE_SKIPAHEAD_DISTINCT
SQLITE_DQS
SQLITE_EBCDIC
SQLITE_ENABLE_8_3_NAMES
SQLITE_ENABLE_API_ARMOR
SQLITE_ENABLE_ATOMIC_WRITE
SQLITE_ENABLE_BATCH_ATOMIC_WRITE
SQLITE_ENABLE_BYTECODE_VTAB
SQLITE_ENABLE_CARRAY
SQLITE_ENABLE_CEROD
SQLITE_ENABLE_COLUMN_METADATA
SQLITE_ENABLE_COLUMN_USED_MASK
SQLITE_ENABLE_COMPRESS
SQLITE_ENABLE_COSTMULT
SQLITE_ENABLE_CSV
SQLITE_ENABLE_CURSOR_HINTS
SQLITE_ENABLE_DBPAGE_VTAB
SQLITE_ENABLE_DBSTAT_VTAB
SQLITE_ENABLE_EXTFUNC
SQLITE_ENABLE_EXPENSIVE_ASSERT
SQLITE_ENABLE_EXPLAIN_COMMENTS
SQLITE_ENABLE_FILEIO
SQLITE_ENABLE_FTS1
SQLITE_ENABLE_FTS2
SQLITE_ENABLE_FTS3
SQLITE_ENABLE_FTS3_PARENTHESIS
SQLITE_ENABLE_FTS3_TOKENIZER
SQLITE_ENABLE_FTS4
SQLITE_ENABLE_FTS5
SQLITE_ENABLE_GEOPOLY
SQLITE_ENABLE_HIDDEN_COLUMNS
SQLITE_ENABLE_ICU
SQLITE_ENABLE_INTERNAL_FUNCTIONS
SQLITE_ENABLE_IOTRACE
SQLITE_ENABLE_LOAD_EXTENSION
SQLITE_ENABLE_LOCKING_STYLE
SQLITE_ENABLE_MATH_FUNCTIONS
SQLITE_ENABLE_MEMORY_MANAGEMENT
SQLITE_ENABLE_MEMSYS3
SQLITE_ENABLE_MEMSYS5
SQLITE_ENABLE_MULTIPLEX
SQLITE_ENABLE_MULTITHREADED_CHECKS
SQLITE_ENABLE_NORMALIZE
SQLITE_ENABLE_NULL_TRIM
SQLITE_ENABLE_OFFSET_SQL_FUNC
SQLITE_ENABLE_OVERSIZE_CELL_CHECK
SQLITE_ENABLE_PREUPDATE_HOOK
SQLITE_ENABLE_QPSG
SQLITE_ENABLE_RBU
SQLITE_ENABLE_REGEXP
SQLITE_ENABLE_RTREE
SQLITE_ENABLE_SERIES
SQLITE_ENABLE_SESSION
SQLITE_ENABLE_SETLK_TIMEOUT
SQLITE_ENABLE_SHA3
SQLITE_ENABLE_SNAPSHOT
SQLITE_ENABLE_SORTER_MMAP
SQLITE_ENABLE_SORTER_REFERENCES
SQLITE_ENABLE_SQLAR
SQLITE_ENABLE_SQLLOG
SQLITE_ENABLE_STAT4
SQLITE_ENABLE_STMT_SCANSTATUS
SQLITE_ENABLE_STMTVTAB
SQLITE_ENABLE_TCL
SQLITE_ENABLE_TREETRACE
SQLITE_ENABLE_UNKNOWN_SQL_FUNCTION
SQLITE_ENABLE_UNLOCK_NOTIFY
SQLITE_ENABLE_UPDATE_DELETE_LIMIT
SQLITE_ENABLE_URI_00_ERROR
SQLITE_ENABLE_UUID
SQLITE_ENABLE_VFSTRACE
SQLITE_ENABLE_VSV
SQLITE_ENABLE_WHERETRACE
SQLITE_ENABLE_ZIPFILE
SQLITE_ENABLE_ZIPVFS
SQLITE_EXPLAIN_ESTIMATED_ROWS
SQLITE_EXTRA_DURABLE
SQLITE_EXTRA_IFNULLROW
SQLITE_EXTRA_INIT
SQLITE_EXTRA_SHUTDOWN
SQLITE_FP_PRECISION_LIMIT
SQLITE_FTS3_MAX_EXPR_DEPTH
SQLITE_FTS5_ENABLE_TEST_MI
SQLITE_FTS5_NO_WITHOUT_ROWID
SQLITE_HAVE_OS_TRACE
SQLITE_HOMEGROWN_RECURSIVE_MUTEX
SQLITE_IGNORE_AFP_LOCK_ERRORS
SQLITE_IGNORE_FLOCK_LOCK_ERRORS
SQLITE_INLINE_MEMCPY
SQLITE_INT64_TYPE
SQLITE_INTEGRITY_CHECK_ERROR_MAX
SQLITE_LIKE_DOESNT_MATCH_BLOBS
SQLITE_LOCK_TRACE
SQLITE_LOG_CACHE_SPILL
SQLITE_MALLOC_SOFT_LIMIT
SQLITE_MALLOCSIZE
SQLITE_MAX_ATTACHED
SQLITE_MAX_COLUMN
SQLITE_MAX_COMPOUND_SELECT
SQLITE_MAX_DEFAULT_PAGE_SIZE
SQLITE_MAX_EXPR_DEPTH
SQLITE_MAX_FUNCTION_ARG
SQLITE_MAX_LENGTH
SQLITE_MAX_LIKE_PATTERN_LENGTH
SQLITE_MAX_MEMORY
SQLITE_MAX_MMAP_SIZE
SQLITE_MAX_MMAP_SIZE_
SQLITE_MAX_PAGE_COUNT
SQLITE_MAX_PAGE_SIZE
SQLITE_MAX_SCHEMA_RETRY
SQLITE_MAX_SQL_LENGTH
SQLITE_MAX_TRIGGER_DEPTH
SQLITE_MAX_VARIABLE_NUMBER
SQLITE_MAX_VDBE_OP
SQLITE_MAX_WORKER_THREADS
SQLITE_MEMDEBUG
SQLITE_MIGHT_BE_SINGLE_CORE
SQLITE_MIXED_ENDIAN_64BIT_FLOAT
SQLITE_MMAP_READWRITE
SQLITE_MUTEX_NOOP
SQLITE_MUTEX_OMIT
SQLITE_MUTEX_PTHREADS
SQLITE_MUTEX_W32
SQLITE_NEED_ERR_NAME
SQLITE_NO_SYNC
SQLITE_NOINLINE
SQLITE_OMIT_ALTERTABLE
SQLITE_OMIT_ANALYZE
SQLITE_OMIT_ATTACH
SQLITE_OMIT_AUTHORIZATION
SQLITE_OMIT_AUTOINCREMENT
SQLITE_OMIT_AUTOINIT
SQLITE_OMIT_AUTOMATIC_INDEX
SQLITE_OMIT_AUTORESET
SQLITE_OMIT_AUTOVACUUM
SQLITE_OMIT_BETWEEN_OPTIMIZATION
SQLITE_OMIT_BLOB_LITERAL
SQLITE_OMIT_CAST
SQLITE_OMIT_CHECK
SQLITE_OMIT_COMPLETE
SQLITE_OMIT_COMPOUND_SELECT
SQLITE_OMIT_CONFLICT_CLAUSE
SQLITE_OMIT_CTE
SQLITE_OMIT_DATETIME_FUNCS
SQLITE_OMIT_DECLTYPE
SQLITE_OMIT_DEPRECATED
SQLITE_OMIT_DESERIALIZE
SQLITE_OMIT_DISKIO
SQLITE_OMIT_EXPLAIN
SQLITE_OMIT_FLAG_PRAGMAS
SQLITE_OMIT_FLOATING_POINT
SQLITE_OMIT_FOREIGN_KEY
SQLITE_OMIT_GENERATED_COLUMNS
SQLITE_OMIT_GET_TABLE
SQLITE_OMIT_HEX_INTEGER
SQLITE_OMIT_INCRBLOB
SQLITE_OMIT_INTEGRITY_CHECK
SQLITE_OMIT_INTROSPECTION_PRAGMAS
SQLITE_OMIT_JSON
SQLITE_OMIT_LIKE_OPTIMIZATION
SQLITE_OMIT_LOAD_EXTENSION
SQLITE_OMIT_LOCALTIME
SQLITE_OMIT_LOOKASIDE
SQLITE_OMIT_MEMORYDB
SQLITE_OMIT_OR_OPTIMIZATION
SQLITE_OMIT_PAGER_PRAGMAS
SQLITE_OMIT_PARSER_TRACE
SQLITE_OMIT_POPEN
SQLITE_OMIT_PRAGMA
SQLITE_OMIT_PROGRESS_CALLBACK
SQLITE_OMIT_QUICKBALANCE
SQLITE_OMIT_REINDEX
SQLITE_OMIT_SCHEMA_PRAGMAS
SQLITE_OMIT_SCHEMA_VERSION_PRAGMAS
SQLITE_OMIT_SHARED_CACHE
SQLITE_OMIT_SHUTDOWN_DIRECTORIES
SQLITE_OMIT_SUBQUERY
SQLITE_OMIT_TCL_VARIABLE
SQLITE_OMIT_TEMPDB
SQLITE_OMIT_TEST_CONTROL
SQLITE_OMIT_TRACE
SQLITE_OMIT_TRIGGER
SQLITE_OMIT_TRUNCATE_OPTIMIZATION
SQLITE_OMIT_TWOSIZE_LOOKASIDE
SQLITE_OMIT_UTF16
SQLITE_OMIT_VACUUM
SQLITE_OMIT_VIEW
SQLITE_OMIT_VIRTUALTABLE
SQLITE_OMIT_WAL
SQLITE_OMIT_WINDOWFUNC
SQLITE_OMIT_WSD
SQLITE_OMIT_XFER_OPT
SQLITE_OS_KV_ALWAYS_LOCAL
SQLITE_OS_KV_OPTIONAL
SQLITE_OS_TRACE_PROC
SQLITE_PERFORMANCE_TRACE
SQLITE_POWERSAFE_OVERWRITE
SQLITE_PREFER_PROXY_LOCKING
SQLITE_PRINTF_PRECISION_LIMIT
SQLITE_PROXY_DEBUG
SQLITE_REPLACE_INVALID_UTF
SQLITE_REVERSE_UNORDERED_SELECTS
SQLITE_RTREE_INT_ONLY
SQLITE_SECURE_DELETE
SQLITE_SHM_DIRECTORY
SQLITE_SMALL_STACK
SQLITE_SORTER_PMASZ
SQLITE_SOUNDEX
SQLITE_STAT4_SAMPLES
SQLITE_STMTJRNL_SPILL
SQLITE_SUBSTR_COMPATIBILITY
SQLITE_SYSTEM_MALLOC
SQLITE_TCL
SQLITE_TEMP_STORE
SQLITE_TEST
SQLITE_TEST_REALLOC_STRESS
SQLITE_TEXT
SQLITE_TRACE_SIZE_LIMIT
SQLITE_UNLINK_AFTER_CLOSE
SQLITE_UNTESTABLE
SQLITE_USE_ALLOCA
SQLITE_USE_FCNTL_TRACE
SQLITE_USE_URI
SQLITE_USER_AUTHENTICATION
SQLITE_VDBE_COVERAGE
SQLITE_VERSION
SQLITE_VERSION_NUMBER
SQLITE_WIN32_HAS_ANSI
SQLITE_WIN32_MALLOC
SQLITE_WIN32_MUTEX_TRACE_DYNAMIC
SQLITE_WIN32_MUTEX_TRACE_STATIC
SQLITE_ZERO_MALLOC

Sorted the list alphabetically.

utelle commented 1 year ago

Do you think you could provide me a full list of options

A more or less complete list of options can be found here: SQLite compile time options.

that should be exposed?

From your list of options you extracted from the sources you already see that the number of options is overwhelming. IMHO only a (relatively small) subset needs to be exposed.

The problem with many options is that they can't be applied easily to the SQLite amalgamation source code on which SQLite3MC is based. For example, using any of the SQLITE_OMIT_ options often requires to regenerate the amalgamation to get a functional library. This is not in the scope of this project. If a developer really needs to do that, he/she can regenerate SQLite itself and adapt it to SQLite3MC on his/her own.

The next big group are the SQLITE_ENABLE_* options. Many of them are related to (optional) extensions. Usually additional sources have to be added to actually enable an extension. Many extension are included in the core SQLite library (and they are already among the options you added for CMake).

So, enabling extensions typically requires changes to the source code. That is, simply exposing the option is not enough.

If there is a demand by users additional extensions can be made available later (and then related options can be added to CMake.

The group of SQLITE_DEFAULT_* options allows to change certain default values. However, in most cases these options can be set at runtime via SQL pragma commands. Therefore it is usually not necessary to expose thes options at compile time.

Several options are system specific or for debugging.

This is the list I have from scanning the sqlite3.c and sqlite3mc.c files for #ifdef SQLITE_*

From my point of view the number of exposed options is already high and gives a really high degree of freedom to configure the library. We shouldn't add more at the moment. If the need arises the list of exposed options can be modified in the future.

jammerxd commented 1 year ago

Try this and let me know what you think. I've made some changes to handle the scenarios mentioned with some flags being integers.

jammerxd commented 1 year ago

Actually hold before merging, there's one more thing I need to look at.

utelle commented 1 year ago

Try this and let me know what you think. I've made some changes to handle the scenarios mentioned with some flags being integers.

Unfortunately I was busy today with other things, so that I hadn't time to check the new version yet.

Actually hold before merging, there's one more thing I need to look at.

Ok. Take your time. There is absolutely no hurry.

utelle commented 1 year ago

In the meantime I inspected the file CMakeLists.txt. Please find an updated version of the file here: CMakeLists.txt (I didn't commit to your branch, because I didn't want to interfer with your work.)

I made the following modifications:

There are some open issues:

jammerxd commented 1 year ago

Gotcha I'll take a look. I ended up getting busy with a few other things - including a mac purchase. So maybe I can try compiling this on windows, linux, and osx.

jammerxd commented 1 year ago

I actually may have found a better way to handle the options that have multiple options besides on/off

jammerxd commented 1 year ago

Oh you did that -excellent!

jammerxd commented 1 year ago

Okay I got what I needed worked out. Sorry about the wait.

utelle commented 1 year ago

Okay I got what I needed worked out. Sorry about the wait.

No problem.

There are still a few issues:

  1. In line 106 the wrong symbol is referenced: 
    $<$<BOOL:${SQLITE3MC_USE_MINIZ}>:SQLITE_USE_TCL=1>

    should be changed to

    $<$<BOOL:${SQLITE3MC_ENABLE_TCL}>:SQLITE_USE_TCL=1>
  2. Line 308 
     ${SQLITE3MC_TARGET}

    should be removed for the shell target. Since the source file sqlite3mc.cpp was added to the shell target, referencing the library is not necessary and referencing it can lead to compile problems.

  3. Symbols defined for the shell target Since the source file sqlite3mc.cpp was added to the shell target, it is necessary to add the symbols related to the library itself as well. However, the symbols for the extensions that are part of the shell source code need to be excluded.
  4. The ZLIB dependency is only required, if SQLITE3MC_USE_MINIZ is not enabled.
  5. The right definitions for the symbols SQLITE_ENABLE_COMPRESS, SQLITE_ENABLE_SQLAR, and SQLITE_ENABLE_ZIPFILE are a bit tricky. Again, only if SQLITE3MC_USE_MINIZ is not enabled, ZLIB is required. For compiling the shell these symbols must not be defined, because the shell enables them if the symbol SQLITE_HAVE_ZLIB is defined. If SQLITE3MC_USE_MINIZ is enabled, the symbol SQLITE_HAVE_ZLIB will be defined, too (but that does not require the ZLIB dependency).
utelle commented 1 year ago

I committed a new version of CMakeLists.txt to address the items mentioned in my previous post.

I tested only on Windows. Tests on Linux and macOS have still to be done.

Unfortunately, there are still (at least) 2 issues that need to be addressed before merging:

  1. The compiler flags -msse4.2 -maes need to be specified for the gcc compiler on Windows platforms, too. However, these flags should only be specified for x64 or x64 platforms, but not for ARM platforms.
  2. If the TCL option is enabled the build depends on the availability of the TCL library. Typically, a TCL enabled SQLite build depends on further defines. However, I have absolutely no experience with the TCL enabled build. Therefore I tend to remove the TCL option from CMakeLists.txt for now.
utelle commented 1 year ago

Unfortunately, there are still (at least) 2 issues that need to be addressed before merging:

  1. The compiler flags -msse4.2 -maes need to be specified for the gcc compiler on Windows platforms, too. However, these flags should only be specified for x64 or x64 platforms, but not for ARM platforms.

AFAICS CMake does not provide means to reliably detect the architecture under all circumstances. For now, I intend to check for non-MSVC compilers under Windows to apply the compiler flags.

  1. If the TCL option is enabled the build depends on the availability of the TCL library. Typically, a TCL enabled SQLite build depends on further defines. However, I have absolutely no experience with the TCL enabled build. Therefore I tend to remove the TCL option from CMakeLists.txt for now.

For now, I will remove the TCL option.

I will apply the changes later today.

A new version of SQLite3 was released yesterday, and I plan to merge the CMake support before making a new release.