Make the -DTHREAD_SAFE cmake option change the target#786
Open
janbraun wants to merge 1 commit intoUSCiLab:masterfrom
Open
Make the -DTHREAD_SAFE cmake option change the target#786janbraun wants to merge 1 commit intoUSCiLab:masterfrom
janbraun wants to merge 1 commit intoUSCiLab:masterfrom
Conversation
Issue: Configuring cereal with the -DTHREAD_SAFE=ON option will not lead to any change in the installation. Using target_compile_definitions and target_link_libraries, the cmake targets installed in PREFIX/cmake/cereal will be updated to include the definition and library for thread safety.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Issue: Configuring cereal with the -DTHREAD_SAFE=ON option will not lead to any change in the installation.
I would like to have a system install defaulting to thread safe code, but I am not sure how the best way to solve this issue looks like. One option would be to change the default value of CEREAL_THREAD_SAFE in macros.hpp; the other option would be to create cmake targets which set the relevant compiler flags. The first solution would still suffer from not having proper support for linking a threading library.
The implemented solution tries to adjust CMakeLists.txt in such a way that it does what I think it should have done all along:
Using target_compile_definitions and target_link_libraries, the cmake targets installed in PREFIX/cmake/cereal will be updated to include the definition and library for thread safety. This will result in projects using cmake to include cereal automatically using the thread safe code globally.
A third option could be to provide two different cmake targets, one for single threaded and for multi threaded targets.
Resolves #785