Added support for external sqlite3

[emmenlau]

This PR adds support for an externally compiled SQLite3. There are two main changes:

1. Detect and use SQLite3 via cmake if requested by the user, and
2. Always link SQLiteCpp against cmake detected Threads::Threads and DL-libs.

The second change is a simplification and cleanup. SQLite3 always requires threads and DL-libraries. Therefore it seems not sensible to specify them individually for each target.

This PR also uses cmake standard functionality to detect the actual threads-implementation and DL-libraries for the current platform. This should improve portability to other (less common) platforms that may not use the name pthreads but another threading library name.

[SRombauts]

Hello @emmenlau

Thank you for all your pull requests!
I am don't know if you have noticed, but all CI tests failed on this one?

In the meantime, I believe that I have merged another pull request similar to this one.

Cheers!
Sébastien

[SRombauts]

Hello @emmenlau, looking back at this one, I see that there are still very valuable bits here!

Would you have the time to rebase it onto the new master branch?
If not, I will hopefully find some time to do it at a later time.

Cheers!

[emmenlau]

Dear @SRombauts , I've just reviewed the upstream changes and I think that some relevant parts have not been answered by the previous MR. I've therefore updated my PR accordingly.

This PR changes/improves multiple things:

• It transitively links SQLiteCpp against either the included sqlite3 or the system sqlite3
• It generalizes the linking against dl and thread libraries on Linux/Unix/Mac, so that this works for all targets and also is passed transitively to downstream projects
• Due to the transitive linking of sqlite3, dl and threads, the CMakeLists.txt can be simplified and cleaned a bit, and downstream projects work more comfortably

[emmenlau]

But one question: SQLiteCpp can be built against an internal sqlite3. However this internal sqlite3 is never installed. Is this intentional? It seems not sensible to allow building required dependencies but then leaving it to the user to resolve them for downstream projects?

Would it not be better if the internal sqlite3, if built, is installed alongside SQLiteCpp?

[SRombauts]

Oh yes, for sure, the thing is I never use this kind of "install" workflow

[emmenlau]

Ok I'll give it a shot!

[emmenlau]

Ok I've implemented installation instructions for the internal sqlite3. To add a cmake export for the internal sqlite, I've followed the guidelines given in https://stackoverflow.com/questions/36103012/cmake-dependency-management-in-a-multi-library-package-export. I've tested the solution with and without internal sqlite3 and it works for me. However I must admit that the size of the change is big enough that I can not 100% guarantee it covers all cases. I'm curious about the result from your CI testing.

[emmenlau]

Please ignore this comment, it is deprecated with the latest version.

[coveralls]

Coverage remained the same at 100.0% when pulling a166062 on BioDataAnalysis:emmenlau_support_external_sqlite into 8485bb7 on SRombauts:master.

[emmenlau]

Dear @SRombauts , I've continued to improve the cmake instructions in this PR. In order to support internal or external sqlite3 very cleanly, I've removed the generic line

include_directories("${PROJECT_SOURCE_DIR}/include")

and instead used specific includes with target_include_directories(PRIVATE for every target. This does not add much more code, but its the recommended clean code for "modern cmake". It can help in the future to discover targets dependencies and wrong/incorrect includes, because every target only has their minimal includes set.

Is this ok with you?

[SRombauts]

Hello, fun fact, I have already removed the line "include_directories("${PROJECT_SOURCE_DIR}/include")" yesterday evening on master ;)

[emmenlau]

Haha ok! I think all builds should now show green lights. If nothing else comes up, can this be merged?

[SRombauts]

Yes, it looks good too me... even though we don't actually test the installation itself, nor the use of it by a test project.

Adding an installation step and checking the result should be fairly easy I guess.
Testing the use of the installed library might be a bit more involving: I do have a separate SQLiteCpp_Example project, but perhaps could we even do that directly in here

[emmenlau]

For what its worth, I've checked the installation in our CI on the three major platforms Win, Lin and Mac, and could use the installed sqlite3 in downstream projects. And this PR also makes SQLiteCpp usable with a completely external sqlite3 for me, which previously caused some issues...

[SRombauts]

Very good, thank you for your contribution!

I'll see if I can automate some tests to avoid any regressions around these kind of sensitive features.

Cheers!

[emmenlau]

Great, thanks!