Skip to content

Conversation

@francisduffy
Copy link

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.

  • Similar to QuantLib, if BOOST_ALL_NO_LIB is 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.
  • Commits 808bc18 through 4929e59 use Boost targets rather than using the ${Boost_LIBRARIES} and ${Boost_INCLUDE_DIRS} variables. The find_package(Boost ...) invocation is only in the cmake/commonSettings.cmake file and removed from all the other CMakeLists.txt files. The Boost targets are then used where they are needed via the target_link_libraries(...) call.
  • When USE_GLOBAL_ORE_BUILD is OFF, 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}>").
  • Commit 27cd1b6 applies a similar approach to the zlib package i.e. find_package(zlib ...) in one place and use the zlib target i.e. ZLIB::ZLIB.
  • Commit 3a2d0fa is a minor change that adds a function to avoid duplicate code for Doxygen docs generation.
  • Commit c50820f prevents cmake/commonSettings.cmake from being included more than once during CMake configuration. It was being included from CMakeLists.txt and then ORE-SWIG/CMakeLists.txt.
  • My build on Linux was failing without commit 314b2b2. I have seen it in other places in the code and in QuantLib. It seems to be a potential bug in the gcc compiler.

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.

  1. There are a number of items repeated in sub-directory CMakeLists.txt files e.g. 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 in cmake/commonSettings.cmake and 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.
  2. Is USE_GLOBAL_ORE_BUILD something 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 is OFF, the QuantLib library name is built as a string using the macro get_library_name and it is used in target_link_libraries calls instead of the CMake target ql_library. But, in the top level CMakeLists.txt project file, there is add_subdirectory("QuantLib") which means that we always build the submodule QuantLib and have the target ql_library available.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant