-
Notifications
You must be signed in to change notification settings - Fork 265
Boost dependency handling in CMake #322
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
Open
francisduffy
wants to merge
18
commits into
OpenSourceRisk:master
Choose a base branch
from
francisduffy:build_fixes_boost_02
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
Boost dependency handling in CMake #322
francisduffy
wants to merge
18
commits into
OpenSourceRisk:master
from
francisduffy:build_fixes_boost_02
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
When find_package uses config search mode for Boost instead of the old
module search mode, there are Boost libraries missing in the link step:
- Boost::log_setup missing for QLE and ORED test suites.
- Boost::regex missing in ORE-SWIG build.
To build using the CMake config file provided by Boost, you do not
create the BOOST and BOOST_LIB64 environment variables and hence
BOOST_INCLUDEDIR and BOOST_LIBRARYDIR are not set in the presets.
You instead supply the path to BoostConfig.cmake in the variable
Boost_DIR. Also, if you are using CMake 3.30 and above, you should
set the variable CMAKE_POLICY_DEFAULT_CMP0167 to NEW to avoid
warnings about the policy not being set.
This issue is not apparent when you create the BOOST and BOOST_LIB64
environment variables because CMake uses the module mode search and the
CMake FindBoost.cmake module has entries of the form:
set(_Boost_LOG_DEPENDENCIES log_setup regex ...)
so the dependencies on log_setup and regex are pulled in.
Note: to see the difference in the link commands, you need to keep the
.rsp files that are used to store the .obj and .lib file arguments. If
building with Ninja, you can do this by passing `-d keeprsp` to the
build step. From command line, something like this for example:
cmake --build --preset=windows-ninja-x64-debug
--target quantext-test-suite.exe --verbose -- -d keeprsp
or if you want to add it to the preset file for VS to pick up:
"nativeToolOptions": [ "-d keeprsp" ]
As is done in QuantLib, if BOOST_ALL_NO_LIB is set, do not include any auto_link.hpp files. In other words, auto linking is turned off and the `/DEFAULTLIB:...` linker directives specifying library names to link against are not written to the .obj files during an MSVC build.
Remove Boost timer where not used and place inside include guard when it is not used outside of it.
Set BOOST_ALL_NO_LIB in cmake/commonSettings.cmake to turn off auto-linking as is done in QuantLib. Use specific Boost targets in target_link_libraries calls in QuantExt and QuantExt test suite. As per QuantLib, no linking to Boost unit_test_framework needed since the header only approach is being used i.e. #define BOOST_TEST_MODULE "QuantExtTestSuite" #include <boost/test/included/unit_test.hpp> Centralize the find_package call for Boost to cmake/commonSettings.cmake. Fix set_ql_library_name macro when USE_GLOBAL_ORE_BUILD is OFF.
Use specific Boost targets in target_link_libraries calls in OREData and OREData test suite. No need to use Boost_INCLUDE_DIRS and Boost_LIBRARIES as the usage requirements are passed via the Boost targets.
Will need an additional change later for the ORE_USE_ZLIB case.
With just Boost::iostreams target, Boost::zlib and Boost::bzip2 are pulled in as well.
Also use the imported target ZLIB::ZLIB rather than ZLIB_LIBRARIES.
Refactor by adding function to avoid duplication.
Prevent commonSettings.cmake getting included more than once. It was getting included by CMakeLists.txt and ORE-SWIG/CMakeLists.txt.
This is done a couple of other places:
OREData/ored/portfolio/builders/yoycapfloor.cpp
OREData/ored/portfolio/builders/capfloor.cpp
My gcc compiler version:
gcc (Ubuntu 13.3.0-6ubuntu2~24.04) 13.3.0
If you preprocess the file first and then remove the do {...}
while(false); surrounding the code, the compilation works even with the
break; statement.
Everywhere in QuantLib that we have a non-void function with returns
inside switch case blocks, the default case has QL_FAIL not followed by
a break;. I guess it is for this reason.
The order matters as Boost::iostreams depends on zlib.
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.
This PR is a proposal to improve / tidy up the Boost dependency handling in the CMake files. If this PR is accepted, there is no need for PR #321.
In summary, the main changes in the PR are listed below.
BOOST_ALL_NO_LIBis set, turn off all auto-linking. This is commit a941b21. Again similar to QuantLib, I add a compile definition to turn it off by default i.e.add_compile_definitions(BOOST_ALL_NO_LIB)in 808bc18.${Boost_LIBRARIES}and${Boost_INCLUDE_DIRS}variables. Thefind_package(Boost ...)invocation is only in thecmake/commonSettings.cmakefile and removed from all the other CMakeLists.txt files. The Boost targets are then used where they are needed via thetarget_link_libraries(...)call.USE_GLOBAL_ORE_BUILDisOFF, the QuantLib library name needs to be updated manually for the Debug build to work. This is in commit 808bc18 i.e.set(QL_LIB_NAME "${QL_LIB_NAME}$<$<CONFIG:Debug>:${CMAKE_DEBUG_POSTFIX}>").zlibpackage i.e.find_package(zlib ...)in one place and use thezlibtarget i.e.ZLIB::ZLIB.cmake/commonSettings.cmakefrom being included more than once during CMake configuration. It was being included fromCMakeLists.txtand thenORE-SWIG/CMakeLists.txt.There are a few things that I am not sure about. If they were cleared up, there may be potential for more changes to the CMake files.
cmake_minimum_required,project(...). Are these files e.g. QuantExt/CMakeLists.txt, OREData/CMakeLists.txt, etc. being used somewhere in isolation as top-level CMakeLists.txt project files? I can't see how they would be since they use macros defined incmake/commonSettings.cmakeand it is only included in the top-level CMakeLists.txt file. If we can assume they are always used under a top-level CMakeLists.txt file then I think there is potential to simplify them.USE_GLOBAL_ORE_BUILDsomething that is used? It appears that it is there so that you can point at a QuantLib library that is already built i.e. when it isOFF, the QuantLib library name is built as a string using the macroget_library_nameand it is used intarget_link_librariescalls instead of the CMake targetql_library. But, in the top level CMakeLists.txt project file, there isadd_subdirectory("QuantLib")which means that we always build the submodule QuantLib and have the targetql_libraryavailable.