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

CMakeLists.txt: use transitive compile definitions via cmake #228

Conversation

emmenlau
Copy link
Contributor

@emmenlau emmenlau commented Nov 8, 2019

This PR adds transitive compile definitions via cmake. When this PR is used, a slightly more "modern" syntax is employed for the compile defines SQLITE_ENABLE_COLUMN_METADATA, SQLITECPP_ENABLE_ASSERT_HANDLER, and SQLITE_USE_LEGACY_STRUCT. With this more modern syntax, its possible to specify for every flag a PUBLIC or PRIVATE visibility. With PUBLIC visibility (currently the default in this PR), downstream projects also have access to the flags.

This can be specifically helpful for SQLITE_ENABLE_COLUMN_METADATA, which downstream projects can use to tweak their code at compile time.

Please feel free to change the visibility of the individual flags. The previous default (before this PR) was PRIVATE visibility for all three flags.

@SRombauts SRombauts self-assigned this Nov 18, 2019
@SRombauts
Copy link
Owner

Thank you, this looks promising, but have you seen that all builds are failing?

libSQLiteCpp.a(Column.cpp.o): In function `SQLite::Column::getOriginName() const':
/home/travis/build/SRombauts/SQLiteCpp/src/Column.cpp:51: undefined reference to `sqlite3_column_origin_name'
libSQLiteCpp.a(Statement.cpp.o): In function `SQLite::Statement::getColumnOriginName(int) const':
/home/travis/build/SRombauts/SQLiteCpp/src/Statement.cpp:379: undefined reference to `sqlite3_column_origin_name'
collect2: error: ld returned 1 exit status
make[2]: *** [SQLiteCpp_example1] Error 1
make[1]: *** [CMakeFiles/SQLiteCpp_example1.dir/all] Error 2
SQLiteCpp.lib(Statement.obj) : error LNK2019: unresolved external symbol _sqlite3_column_origin_name referenced in function "public: char const * __thiscall SQLite::Statement::getColumnOriginName(int)const " (?getColumnOriginName@Statement@SQLite@@QBEPBDH@Z) [C:\projects\sqlitecpp\build\SQLiteCpp_example1.vcxproj]

@emmenlau
Copy link
Contributor Author

Oh, thanks, I did not realize! I'll take a look...

@emmenlau emmenlau force-pushed the emmenlau_cmake_add_transitive_compile_definitions branch 2 times, most recently from 5a6747e to 36181de Compare November 18, 2019 09:32
@emmenlau
Copy link
Contributor Author

Dear @SRombauts, can you help me shed some light on this? I am under the impression that the CMakeLists.txt is correct. The build error we keep on getting indicates that at least the travis build tries to link sqlite but the provided sqlite does not have support for _sqlite3_column_origin_name(). I fail to see how this may have worked before, because option SQLITE_ENABLE_COLUMN_METADATA defaulted to ON already before.

@emmenlau
Copy link
Contributor Author

Ah I see the issue now.

@emmenlau emmenlau force-pushed the emmenlau_cmake_add_transitive_compile_definitions branch from 8b83c6a to 24564cf Compare November 18, 2019 10:13
@emmenlau
Copy link
Contributor Author

emmenlau commented Nov 18, 2019

I've copied the define also to the supplied sqlite3/CMakeLists.txt, sorry I overlooked this before.

Actually, due to the transitiveness of the new cmake flags, it would be sufficient to only have this option in sqlite3/CMakeLists.txt, as long as users would use the supplied sqlite. But to be more flexible with external sqlite, I think its fine (and better) to duplicate this option.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 96.586% when pulling 24564cf on BioDataAnalysis:emmenlau_cmake_add_transitive_compile_definitions into 5b5ca14 on SRombauts:master.

@emmenlau
Copy link
Contributor Author

Now I see more green lights :-)

I hope the PR is fine like this?

@SRombauts SRombauts merged commit f732569 into SRombauts:master Nov 25, 2019
@SRombauts
Copy link
Owner

Yes, thanks a lot for this!

@emmenlau
Copy link
Contributor Author

Thanks to you!

@emmenlau emmenlau deleted the emmenlau_cmake_add_transitive_compile_definitions branch November 25, 2019 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants