lballabio / QuantLib

The QuantLib C++ library
http://quantlib.org
Other
5.43k stars 1.8k forks source link

cmake policy CMP0167 #2088

Open jongbongan opened 1 month ago

jongbongan commented 1 month ago

Actually, it may not be a big deal.

I am currently using CMake >= 3.30 to build the project. It outputs a warning :

CMake Warning (dev) at CMakeLists.txt:164 (find_package):Policy CMP0167 is not set: The FindBoost module is removed.  Run "cmake --help-policy CMP0167" for policy details.  Use the cmake_policy command to set the policy and suppress this warning.

In CMake document, (cmake --help-policy CMP0167) it says

.. versionadded:: 3.30

The ``FindBoost`` module is removed.

CMake 3.29 and below provide a ``FindBoost`` module, but it needs constant
updates to keep up with upstream Boost releases.  Upstream Boost 1.70
and above provide a ``BoostConfig.cmake`` package configuration file.
``find_package(Boost CONFIG)`` finds the upstream package directly,
without the find module.

CMake 3.30 and above prefer to not provide the ``FindBoost`` module
so that ``find_package(Boost)`` calls, without the ``CONFIG`` or
``NO_MODULE`` options, find the upstream ``BoostConfig.cmake`` directly.
This policy provides compatibility for projects that have not been ported
to use the upstream Boost package.

The ``OLD`` behavior of this policy is for ``find_package(Boost)`` to
load CMake's ``FindBoost`` module.  The ``NEW`` behavior is for
``find_package(Boost)`` to search for the upstream ``BoostConfig.cmake``.

This policy was introduced in CMake version 3.30.
It may be set by ``cmake_policy()`` or ``cmake_minimum_required()``.
If it is not set, CMake warns, and uses ``OLD`` behavior.

.. note::
  The ``OLD`` behavior of a policy is
  ``deprecated by definition``
  and may be removed in a future version of CMake.

I think it is better to suppress such a warning until we require the minimum version of Boost >= 1.70.

boring-cyborg[bot] commented 1 month ago

Thanks for posting! It might take a while before we look at your issue, so don't worry if there seems to be no feedback. We'll get to it.

sweemer commented 1 month ago

I think we should use the config based find_package when the CMake version >= 3.30 and FindBoost when < 3.30. If users can use the newest versions of CMake then they can probably upgrade to the newest versions of boost as well.

jongbongan commented 1 month ago

Because boost supports its cmake configuration for version >= 1.70, I will add a config argument in find_package(boost ... ) as soon as the minimum required version of boost changes to >= 1.70.

(now, {QL_BOOST_VERSION} would be 1.58.0)

sweemer commented 1 month ago

I was thinking more like the following:

if (CMAKE_VERSION GREATER_EQUAL "3.30")
    set(QL_BOOST_VERSION 1.70.0)
    find_package(Boost ${QL_BOOST_VERSION} CONFIG REQUIRED)
else()
    find_package(Boost ${QL_BOOST_VERSION} REQUIRED)
endif()
jongbongan commented 1 month ago

I will make a PR for config based find_package(boost... ) asap then.

ralfkonrad commented 1 month ago

Another possibility would be to check if the POLICY CMP0167 is available


if (POLICY CMP0167)
    set(QL_BOOST_VERSION 1.70.0)
    # The FindBoost module is removed in cmake 3.30, finding the upstream BoostConfig.cmake
    find_package(Boost ${QL_BOOST_VERSION} CONFIG REQUIRED)
else()
    find_package(Boost ${QL_BOOST_VERSION} REQUIRED)
endif()

It gives a clearer picture why we distinguish between the two find_package(Boost ...) methods and where to find information about it.


However, be aware that (almost?) all GitHub runners have updated to cmake v3.30 already.

For the macos and ubuntu runners it doesn't make a difference as the new CONFIG file BoostConfig.cmake is found automatically using the default methods (like apt and brew) to setup boost.

For the windows runners it is different when you download and extract the boost binaries. You need to provide the environment variable Boost_DIR with the path to the BoostConfig.cmake file. Have a look here how this can be solved.

jongbongan commented 1 month ago

I will check it. Thank you.