Improve CMake correctness and handling #1165
Merged
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
The more general CMake way to handle library suffixing is to set CMAKE_<CONFIG>_POSTFIX, so setting the Debug output suffix name should be more correctly done by the caller or CMake configurer by setting the desired value in CMAKE_DEBUG_POSTFIX.
As of CMake 3.0 with CMP0042, MACOSX_RPATH is enabled by default. Since the validated version used by jsoncpp is later than 3.0, this is already covered.
Since CMake has subdirectory variable scope, unilaterally set the CMAKE_CXX_STANDARD variable to use C++11. This covers cases with the library being included externally, both in cases of only C++98 being specified, as well as later versions being specified (since the CXX_STANDARD itself isn't a library dependency, only the PUBLIC target_compile_features on jsoncpp_lib). The previous direct check for C++98 is handled by requiring C++11 on this library; should the compiler being used not support C++11 then CMake will issue an error.
Since the introduction of CMAKE_COMPILER_IS_GNUCXX CMake has suggested using CMAKE_CXX_COMPILER_ID for more general checks.
EXTRA_CXX_FLAGS is never defined, making this a noop. Further, COMPILE_OPTIONS is invalid to set as a DIRECTORY property.
Commit aebc7fa added version checks for CMake compatibility. In reality, only the add_compile_definitions need the check - add_compile_options itself has been supported since 3.0. Tested and confirmed built successfully with CMake 3.8.0.
This is already covered by the toplevel CMake, which also serves to provide a consistent minimum version.
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.
Update the CMake processing to improve interoperability with being included as CMake from a larger project using jsoncpp.
This is a follow-on replacement to #1143