Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compile sqlite3 in compile step #137

Merged
merged 5 commits into from Jun 7, 2022
Merged

Conversation

aloisklink
Copy link
Contributor

@aloisklink aloisklink commented May 23, 2022

Use ExternalProject_Add() to compile SQLite3, if needed.

ExternalProject_Add() automatically downloads, configures, compiles, and runs make install during the cmake --build step of the parent project.

Other stuff:

  • Throws an error if BUILD_SQLITE_LIB=OFF, and SQLite3 cannot be found.

    BREAKING CHANGE: The minimum cmake version to compile EDGESec
    is now cmake v3.14.0, instead of v3.13.0, as it is the first version that adds support
    for FindSQLite3.cmake. This is a 3 year old version that came out in March 2019, so I don't think this should matter much.

  • Prevents installing sqlite3 if it's a static lib

Cmake now automatically includes LIB_SQLITE_DIR only when needed.
Compiles SQLITE3 in the `cmake --build` compile step,
instead of compiling during the `cmake` configure step.
Throws an error if BUILD_SQLITE_LIB=OFF, and SQLite3 cannot be found.

BREAKING CHANGE: The minimum `cmake` version to compile EDGESec
  is now cmake v3.14.0, as it is the first version that adds support
  for FindSQLite3.cmake
If sqlite3 is compiled/link statically, we avoid installing
it into lib/edgesec, as it is not needed.
@aloisklink aloisklink added refactor Refactoring code dependencies Pull requests that update a dependency file labels May 23, 2022
@aloisklink aloisklink requested a review from mereacre May 23, 2022 18:48
Comment on lines +49 to +51
add_library(supervisor_config INTERFACE)
set_target_properties(supervisor_config PROPERTIES PUBLIC_HEADER "supervisor_config.h")
target_link_libraries(supervisor_config INTERFACE SQLite::SQLite3)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this as an "Interface" aka header-only library, since many files depend on supervisor_config.h, but there is no supervisor_config.c.

That way, if we ever remove "sqlite.h` from this library (or add another depedency), we only need to change it in one place.

target_include_directories(sqlhook PRIVATE ${LIBSQLITE_INCLUDE_DIR})
target_link_libraries(sqlhook PRIVATE domain os SQLite::sqlite)
target_link_libraries(sqlhook PRIVATE domain os SQLite::SQLite3)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed all of the target_include_directories(... LIBSQLITE_INCLUDE_DIR), since they get automatically added when we use target_link_libraries(SQLite::SQLite3)

It looks like the toolchain file only gets run on configure.
As this sets our PATH env, we need to redeclare that environment
variable for the make and make install commands.

It's messy, but it looks like this is the best way of doing it:
https://stackoverflow.com/a/62437353/10149169
Copy link
Contributor

@mereacre mereacre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work.

@mereacre mereacre merged commit cf40806 into main Jun 7, 2022
@aloisklink aloisklink deleted the compile-sqlite3-in-compile-step branch June 7, 2022 20:31
@aloisklink aloisklink mentioned this pull request Jun 10, 2022
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file refactor Refactoring code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants