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

Setting CMAKE_CXX_STANDARD in the library build config is problematic #202

Closed
kamchatka-volcano opened this issue Oct 11, 2022 · 4 comments

Comments

@kamchatka-volcano
Copy link

kamchatka-volcano commented Oct 11, 2022

Hi there! I encountered a problem introduced by toml11's CMakeLists which wasn't easy to figure out.
I tried to configure a project which uses toml11 and Microsoft.GSL libraries and was getting the error GSL: Requires at least CXX standard 14, user provided 11 from the GSL CMake config which was produced by the following code

if (${CMAKE_CXX_STANDARD} VERSION_LESS ${min_cxx_standard})
            message(FATAL_ERROR "GSL: Requires at least CXX standard ${min_cxx_standard}, user provided ${CMAKE_CXX_STANDARD}")
        endif()

In my projects I use target_compile_features to set the required C++ standard, so the variable CMAKE_CXX_STANDARD is left undefined, so I was puzzled until I find out that toml11 CMake sets this variable to 11 standard unconditionally.
Please consider to check this variable and give an error if it's less than 11 standard instead of setting it by yourself.

Also there's somewhat popular CMake guidelines that agrees that this variable should only be set by the end user of the library.

@ToruNiina
Copy link
Owner

Hi,

Thank you for reporting. I replaced the code that sets CMAKE_CXX_STANDARD with if(NOT DEFINED CMAKE_CXX_STANDARD) message(FATAL_ERROR) endif().

P.S. Actually, you don't need to build toml11 to use because it is a header-only library (you need to build to run tests). So you don't need to add_subdirectory in your CMakeLists.txt but to set include path.

@kamchatka-volcano
Copy link
Author

I see, this way I still would get a problem because CMAKE_CXX_STANDARD is undefined in my projects, but at least now with error message the source of the problem is much more clear, thanks! I already found the workaround like setting CMAKE_CXX_STANDARD before adding toml11 and unsetting it after it, so this issue isn't critical for me.

So you don't need to add_subdirectory

I know, but I prefer working with targets instead of paths and link libraries with target_link_libraries even when they're header-only, that's why I add them with add_subdirectory or FetchContent_MakeAvailable.

@hansfn
Copy link

hansfn commented Oct 25, 2022

Why don't you wrap all the check compiler stuff in

if (toml11_BUILD_TEST)

endif()

The fatal error for missing CMAKE_CXX_STANDARD broke our build / install of this library. And we aren't building the tests (because they are disabled by default).

@Barsay
Copy link

Barsay commented Nov 30, 2022

This fix breaks our systems as well.
We used to install TOML as shared on our system using

mkdir build cd build cmake -Dtoml11_BUILD_TEST=OFF .. make make install

How can we make install it now?

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

No branches or pull requests

4 participants