-
Notifications
You must be signed in to change notification settings - Fork 59
Feat: modernize CMake files #35
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
Changes from 2 commits
1f559d5
b6644ff
2d30a8f
6b0c445
2a0331f
8108686
58d8f7d
bba76de
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,39 +2,66 @@ | |
| # Distributed under the Boost Software License, Version 1.0. | ||
| # See accompanying file LICENSE_1_0.txt or copy at https://www.boost.org/LICENSE_1_0.txt | ||
|
|
||
| cmake_minimum_required( VERSION 3.5...3.31 ) | ||
| project( boost_any VERSION "${BOOST_SUPERPROJECT_VERSION}" LANGUAGES CXX ) | ||
| cmake_minimum_required(VERSION 3.21...4.2) | ||
| project(boost_any VERSION "${BOOST_SUPERPROJECT_VERSION}" LANGUAGES CXX) | ||
|
|
||
| if (BOOST_USE_MODULES) | ||
| if(BOOST_USE_MODULES) | ||
| add_library(boost_any) | ||
| target_sources(boost_any PUBLIC | ||
| FILE_SET modules_public TYPE CXX_MODULES FILES | ||
| ${CMAKE_CURRENT_LIST_DIR}/modules/boost_any.cppm | ||
| target_sources( | ||
| boost_any | ||
| PUBLIC | ||
| FILE_SET modules_public | ||
| TYPE CXX_MODULES | ||
| FILES ${CMAKE_CURRENT_LIST_DIR}/modules/boost_any.cppm | ||
| ) | ||
|
|
||
| target_compile_features(boost_any PUBLIC cxx_std_20) | ||
| target_compile_definitions(boost_any PUBLIC BOOST_USE_MODULES) | ||
| if (CMAKE_CXX_COMPILER_IMPORT_STD) | ||
| if(${CMAKE_CXX_STANDARD} IN_LIST CMAKE_CXX_COMPILER_IMPORT_STD) | ||
| set(CMAKE_CXX_MODULE_STD ON) | ||
| target_compile_definitions(boost_any PRIVATE BOOST_ANY_USE_STD_MODULE) | ||
| message(STATUS "Using `import std;`") | ||
| else() | ||
| message(STATUS "`import std;` is not available") | ||
| message(WARNING "`import std;` is not available") | ||
| endif() | ||
| set(__scope PUBLIC) | ||
|
|
||
| add_executable(module_sample modules/usage_sample.cpp) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please don't build examples from top-level cmakes
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
| target_link_libraries(module_sample PRIVATE boost_any) | ||
| else() | ||
| set(CMAKE_VERIFY_INTERFACE_HEADER_SETS ON) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CMake docs suggest to not set this directly in code (https://cmake.org/cmake/help/latest/variable/CMAKE_VERIFY_INTERFACE_HEADER_SETS.html)
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I changed it so it is only usable for standalone builds |
||
| add_library(boost_any INTERFACE) | ||
| set(__scope INTERFACE) | ||
| endif() | ||
|
|
||
| target_include_directories(boost_any ${__scope} include) | ||
| target_link_libraries( boost_any | ||
| if(CMAKE_SKIP_INSTALL_RULES OR PROJECT_IS_TOP_LEVEL) | ||
| target_sources( | ||
| boost_any | ||
| ${__scope} | ||
| FILE_SET headers_public | ||
| TYPE HEADERS | ||
| BASE_DIRS include | ||
| FILES | ||
| include/boost/any/bad_any_cast.hpp | ||
| include/boost/any/basic_any.hpp | ||
| include/boost/any/fwd.hpp | ||
| include/boost/any/unique_any.hpp | ||
| include/boost/any/detail/config.hpp | ||
| include/boost/any/detail/placeholder.hpp | ||
| ) | ||
| else() | ||
| target_include_directories(boost_any ${__scope} include) | ||
| endif() | ||
|
|
||
| target_link_libraries( | ||
| boost_any | ||
| ${__scope} | ||
| Boost::config | ||
| Boost::throw_exception | ||
| Boost::type_index | ||
| ) | ||
|
|
||
| add_library( Boost::any ALIAS boost_any ) | ||
| add_library(Boost::any ALIAS boost_any) | ||
|
|
||
| if(BUILD_TESTING) | ||
| add_subdirectory(test) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,7 +11,9 @@ module; | |
| #include <boost/assert.hpp> | ||
| #include <boost/config.hpp> | ||
| #include <boost/throw_exception.hpp> | ||
| #include <boost/type_index.hpp> | ||
|
|
||
| // FIXME(CK): #include <boost/type_index.hpp> | ||
| import boost.type_index; | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @codeowners any reason to not use import?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But direct import is right and clear? |
||
|
|
||
| #ifdef BOOST_ANY_USE_STD_MODULE | ||
| import std; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@codeowners what is the minimum version that must be supported?
import std;is usable starting withcmake v3.30AFAIKThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we want to bump the minimum version so much. These CMake files are invoked from the Boost superproject, so bumping a minimum version in one affects every other library. I think 3.8 is okay.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on every build host
cmake v4.2may be installed withpipx install cmake?I want to have
PROJECT_IS_TOP_LEVELavailable withcmake version 3.21anything older is not state of the art!