Modernize CMakeLists.txt #5
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Use CMAKE_CXX_STANDARD - Use PROJET_NAME - Set PROJECT_VERSION - Use CMake search paths to find ExternalProject
The recommended method is to explicitly list all sources files for targets. This way build configuration only needs to change if CMakeFiles.txt is explicitly modified. There's no confusion about what files are part of the build.
Each user configurable setting should have a corresponding option() for booleans or typed cache variable for strings and paths.
The original library type was unclear, but POSITION_INDEPENDENT_CODE would hint that shared library was intended. CMake automatically enables PIC for shared libraries and modules.
The package creates an imported target Threads::Threads that configures the correct library.
The test directory doesn't need to be a project, in fact it SHOULDN'T be a project. All necessary flags etc. should be set by the library target the tests link against.
ExternalProject used the most recent version of gtest which will eventually lead to failing tests. Pin the version using a submodule.
Adding SOVERSION property to the target makes CMake generate symlinks
during installation. The RESOURCE property is supported by
install(TARGETS) and can be used to automatically install *.pc
files. The install(TARGETS) call specifies an EXPORT target name,
initialized to PROJECT_NAME, that can be used when adding this
repository as a submodule:
set(EXPORT_TARGET_NAME myproj)
add_subdirectory(tacopie)
install(TARGETS myexe EXPORT myproj
RUNTIME)
install(EXPORT myproj DESTINATION lib/myproj)
The last line will generate ${PREFIX}/lib/myproj/myproj.cmake that
contains necessary CMake spells to include the installed files in
another projects as CMake targets. Somewhat like pkg-config, but nicer
for Windows people.
|
Just chiming in, this would be a huge help |
|
Thank for PR, works like charm... |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactor the build file to use a more modern set of CMake features than the current master branch. Since
cpp_redishas merged similar PR the main build is broken:This PR fixes that by setting tacopie target public include directories so that consuming projects can just link against the target and get all the configuration goodies without manually having to specify them.
This PR fixes cpp-redis/cpp_redis#105 once the cpp_redis tacopie submodule has been updated.