SQLite3MultipleCiphers icon indicating copy to clipboard operation
SQLite3MultipleCiphers copied to clipboard

Add CMake support

Open lwttai opened this issue 5 years ago • 7 comments

Fixes #9

lwttai avatar Jul 27 '20 08:07 lwttai

First of all, thanks for your attempt to add CMake support.

However, before accepting the PR I have a few remarks.

  1. I don't know much about CMake, but looking at CMakeLists.txt, I get the impression that it is quite Windows specific. The goal for SQLite3MultipleCiphers is to be platform-independent. That is, I would certainly prefer if the CMake support would work on other platforms as Windows, too. Would you be able to address this aspect? If not now, maybe later in the future?

  2. Before making the first release I intend to remove the pthreads4w component (and most likely the test applications depending on it). Why? SQLite3MultipleCiphers itself does not depend on pthreads4w, and pthreads4w is Windows-only. I added it only for testing the multi-threading behaviour, especially shared cache mode (a feature which is deprecated and should be discouraged to be used according to the SQLite developers). That is, the file CMakeLists.txt will have to be adjusted before making the release.

utelle avatar Jul 27 '20 08:07 utelle

Sorry for the poor infomation for this PR. "CMake support" is not correct for this PR, maybe "a prototype of supporting cmake" is much better. Currently, it's just convertions of the current VS solutions and it need to be add more work on it for other platfrom & build tool supports. Curently, I just test it on windows for VS2010 and VS2019.

  1. I don't know much about CMake, but looking at CMakeLists.txt, I get the impression that it is quite Windows specific. The goal for SQLite3MultipleCiphers is to be platform-independent. That is, I would certainly prefer if the CMake support would work on other platforms as Windows, too. Would you be able to address this aspect? If not now, maybe later in the future?

CMake is designd for cross platfrom. Actualy I should combine both the makefiles and VS solution into the CMakeList.txt, but currently its much like only for generating MSVC solutions. Unfortunately, my poor Linux knowledge limiting me to finished that job. It should be a easy task, such as adding platfrom specified build tool args for certain platform.

  1. Before making the first release I intend to remove the pthreads4w component (and most likely the test applications depending on it). Why? SQLite3MultipleCiphers itself does not depend on pthreads4w, and pthreads4w is Windows-only. I added it only for testing the multi-threading behaviour, especially shared cache mode (a feature which is deprecated and should be discouraged to be used according to the SQLite developers). That is, the file CMakeLists.txt will have to be adjusted before making the release.

In my opinion, whatever changes is going to make to this project, moving to CMake always a good idea. Editing a single or couple cmake files is much easier than editing makefiles and tons of VS solutions for differnt VS version. If currently pthreads4w is windows only, we can just adding something like if(WIN32) ...pthreads4w stuff... end() to only add this project for windows.

lwttai avatar Jul 27 '20 10:07 lwttai

Sorry for the poor infomation for this PR. "CMake support" is not correct for this PR, maybe "a prototype of supporting cmake" is much better. Currently, it's just convertions of the current VS solutions and it need to be add more work on it for other platform & build tool supports.

Starting with support for Windows only, is fine with me. However, the final goal should be to support all relevant major platforms (i.e. Windows, Linux, OSX).

Currently, I just test it on windows for VS2010 and VS2019.

For development I use mainly VS2019, but I'm able to test with VS2010, VS2015, and VS2017 as well. Additionally, I test with MinGW64 occasionally.

CMake is designd for cross platfrom. Actually I should combine both the makefiles and VS solution into the CMakeList.txt, but currently its much like only for generating MSVC solutions. Unfortunately, my poor Linux knowledge limiting me to finished that job. It should be a easy task, such as adding platform specified build tool args for certain platform.

Well, since I'm not acquainted with CMake, I hope other developers will then become encouraged to contribute to the CMake build support.

In my opinion, whatever changes is going to make to this project, moving to CMake always a good idea. Editing a single or couple cmake files is much easier than editing makefiles and tons of VS solutions for differnt VS version.

Hm, I use premake5 to generate the build files for VS and MinGW/gcc. So regenerating the build files after some changes is typically a matter of a few seconds only.

If currently pthreads4w is windows only, we can just adding something like if(WIN32) ...pthreads4w stuff... end() to only add this project for windows.

As said I intend to remove pthreads4w from the repository. If there is a demand I might set up a separate repository for testing the component.

utelle avatar Jul 27 '20 10:07 utelle

Hm, I use premake5 to generate the build files for VS and MinGW/gcc. So regenerating the build files after some changes is typically a matter of a few seconds only.

Hm, not notice there already a build system. Maybe a build guide is more important. Users can easily to conver the project into what they need. However, maybe CMake is more famous? opencv, CEF, vcpkg are using cmake. I think a binaries with a CMakeList.txt witch use to manage the binaries for externel projects as a release is easier to use.

lwttai avatar Jul 28 '20 01:07 lwttai

Maybe a build guide is more important. Users can easily to conver the project into what they need.

Yes, a build guide has still to be written. Its lack is one of the reasons I haven't made an official release yet.

However, maybe CMake is more famous? opencv, CEF, vcpkg are using cmake.

I know that CMake is almost ubiquitous nowadays. However, when I started to develop open source some 15 years ago, it was extremely cumbersome to use, and I began to use bakefile (the build system of wxWidgets back then) and switched to premake later on.

Adding CMake support to this project is certainly a good idea. Most likely that would make integration of this component into the vcpkg world etc easier. However, that requires platform-independency.

I think a binaries with a CMakeList.txt witch use to manage the binaries for externel projects as a release is easier to use.

Again, I'm certainly not against adding CMake support. In the contrary. However, I would prefer that you eliminate at least most if not all of the add_compile_options section (because it is not only platform-specific but also compiler-specific), before I merge the PR. Thanks in advance for your efforts.

utelle avatar Jul 28 '20 07:07 utelle

FWIW, there are two ways to support CMake in a project:

  1. Build the project itself with CMake.
  2. Allow other CMake-based projects to consume easily your project, as a CMake dependency.

If you're happy with premake, there's no reason to change it, and only do #2. That's what Boost does for example: it builds using its own b2 build system, which generates as a build product the CMake files for upstream consumption.

ddevienne avatar Jul 28 '20 07:07 ddevienne

Thanks for your comments, @ddevienne.

FWIW, there are two ways to support CMake in a project:

  1. Build the project itself with CMake.
  2. Allow other CMake-based projects to consume easily your project, as a CMake dependency.

If you're happy with premake, there's no reason to change it, and only do (2). That's what Boost does for example: it builds using its own b2 build system, which generates as a build product the CMake files for upstream consumption.

At least at the moment I don't intend to replace the premake based build system. However, adding some sort of CMake support is certainly a good idea, since CMake is in widespread use. Within the premake project there are efforts to add CMake as a supported build file format. That is, maybe premake will support generating CMake files in the not too far future.

utelle avatar Jul 28 '20 09:07 utelle