forked from Cylix/tacopie
-
Notifications
You must be signed in to change notification settings - Fork 10
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
Modernize CMakeLists.txt #5
Open
lindblandro
wants to merge
23
commits into
cpp-redis:master
Choose a base branch
from
lindblandro:cmake-refactor
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains 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 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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_redis
has 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.