CMake config scripts broken in 1.70
Hi all, I discovered a severe logic flaw in the generated CMake Config files for the new Boost 1.70 which make them almost unusable. As an example the static variant contains: if(NOT"${Boost_USE_STATIC_LIBS}"STREQUAL""ANDNOTBoost_USE_STATIC_LIBS) _BOOST_SKIPPED("libboost_filesystem.a""static, Boost_USE_STATIC_LIBS=${Boost_USE_STATIC_LIBS}") return() endif() if(BUILD_SHARED_LIBS) _BOOST_SKIPPED("libboost_filesystem.a""static, BUILD_SHARED_LIBS=${BUILD_SHARED_LIBS}, set Boost_USE_STATIC_LIBS=ON to override") return() endif() The shared variant contains: if(NOT"${Boost_USE_STATIC_LIBS}"STREQUAL""ANDBoost_USE_STATIC_LIBS) _BOOST_SKIPPED("libboost_filesystem.so.1.70.0""shared, Boost_USE_STATIC_LIBS=${Boost_USE_STATIC_LIBS}") return() endif() if(NOTBUILD_SHARED_LIBS) _BOOST_SKIPPED("libboost_filesystem.so.1.70.0""shared, BUILD_SHARED_LIBS not ON, set Boost_USE_STATIC_LIBS=OFF to override") return() endif() Although the message text says otherwise the 2nd return in both cases depends solely on BUILD_SHARED_LIBS. I'm pretty sure the intention was too check for BUILD_SHARED_LIBS *only* if Boost_USE_STATIC_LIBS is not set (see thread https://lists.boost.org/Archives/boost/2019/02/245030.php). As can be seen this is not the case. I haven't found the repo which contains the code that generates this so I can't report it there or fix it myself. As the issue here makes using Boost 1.70 (almost) impossible (in Scenarios like BUILD_SHARED_LIBS=ON (a CMake variable influencing the project build) and Boost_USE_STATIC_LIBS=ON) I'd recommend fixing this and pushing out a bugfix release. Regards, Alexander Grund
Alexander Grund wrote:
Hi all,
I discovered a severe logic flaw in the generated CMake Config files for the new Boost 1.70 which make them almost unusable.
This bug should be fixed in https://github.com/boostorg/boost_install/commit/160c7cb2b2c720e74463865ef04... If you discover anything else wrong with the current master/develop, do let me know; it would be nice if we could get 1.71 as correct as possible. Unfortunately, most mistakes are only discovered after the release goes out, and by then it's too late.
Hi Peter, thanks, yes this fixes the issue, although I'd used an "elseif" to reduce the nesting. Another possible issue is pretty much every use of `if(NOT Boost_*`, e.g. `if(NOT Boost_USE_STATIC_RUNTIME)`. The CMake FindModule documents this as "Set to ON or OFF to specify whether to use libraries linked statically to the C++ runtime ('s' tag). Default is platform dependent.". The current implementation is "Default is OFF". This should be checked for all other variables too. E.g. "Boost_USE_DEBUG_RUNTIME" should default to "ON". Currently it seems to be kinda random: Depending on which variant file gets sourced last, that's the one that wins. Not sure how to solve this other than ordering the variant includes appropriately. Finally: All uses of `message` should be reevaluated and either removed or guarded by `Boost_DEBUG` or `*_FIND_QUIETLY`. There is to much noise especially when you have multiple calls to FindPackage(Boost...) with different components over the project. Stuff like "Found x" is handled by the final "find_package_handle_standard_args" (at least in recent CMake Find module) and "Adding xxx dependencies: " is a Debug output at best. Some possible improvements: - Adding the _BOOST_INCLUDEDIR is not required. All targets depend on Boost::headers which has them. Same for BOOST_ALL_NO_LIB definition - Regarding: "find_dependency doesn't forward arguments until 3.9, so we have to roll our own". This does not seem correct: https://cmake.org/cmake/help/v3.0/module/CMakeFindDependencyMacro.html Replacing this would reduce the amount of code Regards, Alex Am 06.06.19 um 23:48 schrieb Peter Dimov via Boost:
Alexander Grund wrote:
Hi all,
I discovered a severe logic flaw in the generated CMake Config files for the new Boost 1.70 which make them almost unusable.
This bug should be fixed in
https://github.com/boostorg/boost_install/commit/160c7cb2b2c720e74463865ef04...
If you discover anything else wrong with the current master/develop, do let me know; it would be nice if we could get 1.71 as correct as possible. Unfortunately, most mistakes are only discovered after the release goes out, and by then it's too late.
Alexander Grund wrote:
Hi Peter,
thanks, yes this fixes the issue, although I'd used an "elseif" to reduce the nesting.
Another possible issue is pretty much every use of `if(NOT Boost_*`, e.g. `if(NOT Boost_USE_STATIC_RUNTIME)`. The CMake FindModule documents this as "Set to ON or OFF to specify whether to use libraries linked statically to the C++ runtime ('s' tag). Default is platform dependent.". The current implementation is "Default is OFF".
Yes, this is intentional. https://github.com/boostorg/boost_install/blob/develop/BoostConfig.cmake#L30... Both `b2 install` and autolink default to shared runtime under Windows.
This should be checked for all other variables too. E.g. "Boost_USE_DEBUG_RUNTIME" should default to "ON". Currently it seems to be kinda random: Depending on which variant file gets sourced last, that's the one that wins. Not sure how to solve this other than ordering the variant includes appropriately.
It's very rare for there to be two variants that only differ by the debugness / debugity of the runtime; the typical case is a debug build using the debug runtime, and a release build using the release runtime. I'm not even sure if it's possible to mix debug/release with the "other" runtime. So this shouldn't be a problem in practice.
Finally: All uses of `message` should be reevaluated and either removed or guarded by `Boost_DEBUG` or `*_FIND_QUIETLY`.
This was one of the first things to be fixed; the current master/develop should not issue any messages at all unless Boost_DEBUG or Boost_VERBOSE are set, per established CMake practices (of which I wasn't aware previously).
- Adding the _BOOST_INCLUDEDIR is not required. All targets depend on Boost::headers which has them. Same for BOOST_ALL_NO_LIB definition
Shouldn't hurt.
- Regarding: "find_dependency doesn't forward arguments until 3.9, so we have to roll our own". This does not seem correct: https://cmake.org/cmake/help/v3.0/module/CMakeFindDependencyMacro.html
The comment is no longer correct, so I've already removed it. It used to be correct though; CMake 3.9+ forward additional arguments (${ARGN}) from find_dependency, such as `HINTS ...`.
Hi again, just a ping that I opened a PR at https://github.com/boostorg/boost_install/pull/10 that aims to improve and simplify the code. I'd appreciate any feedback there. Besides the one "bug" mentioned I did not find any bugs so far. So good work! Thanks, Alex Am 08.06.19 um 19:29 schrieb Peter Dimov via Boost:
Alexander Grund wrote:
Hi Peter,
thanks, yes this fixes the issue, although I'd used an "elseif" to reduce the nesting.
Another possible issue is pretty much every use of `if(NOT Boost_*`, e.g. `if(NOT Boost_USE_STATIC_RUNTIME)`. The CMake FindModule documents this as "Set to ON or OFF to specify whether to use libraries linked statically to the C++ runtime ('s' tag). Default is platform dependent.". The current implementation is "Default is OFF".
Yes, this is intentional.
https://github.com/boostorg/boost_install/blob/develop/BoostConfig.cmake#L30...
Both `b2 install` and autolink default to shared runtime under Windows.
This should be checked for all other variables too. E.g. "Boost_USE_DEBUG_RUNTIME" should default to "ON". Currently it seems to be kinda random: Depending on which variant file gets sourced last, that's the one that wins. Not sure how to solve this other than ordering the variant includes appropriately.
It's very rare for there to be two variants that only differ by the debugness / debugity of the runtime; the typical case is a debug build using the debug runtime, and a release build using the release runtime. I'm not even sure if it's possible to mix debug/release with the "other" runtime. So this shouldn't be a problem in practice.
Finally: All uses of `message` should be reevaluated and either removed or guarded by `Boost_DEBUG` or `*_FIND_QUIETLY`.
This was one of the first things to be fixed; the current master/develop should not issue any messages at all unless Boost_DEBUG or Boost_VERBOSE are set, per established CMake practices (of which I wasn't aware previously).
- Adding the _BOOST_INCLUDEDIR is not required. All targets depend on Boost::headers which has them. Same for BOOST_ALL_NO_LIB definition
Shouldn't hurt.
- Regarding: "find_dependency doesn't forward arguments until 3.9, so we have to roll our own". This does not seem correct: https://cmake.org/cmake/help/v3.0/module/CMakeFindDependencyMacro.html
The comment is no longer correct, so I've already removed it. It used to be correct though; CMake 3.9+ forward additional arguments (${ARGN}) from find_dependency, such as `HINTS ...`.
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Alexander Grund Interdisziplinäre Anwendungsunterstützung und Koordination (IAK) Technische Universität Dresden Zentrum für Informationsdienste und Hochleistungsrechnen (ZIH) Würzburger Str.35/Chemnitzer Str.50, Raum 010 01062 Dresden Tel.: +49 (351) 463-35982 E-Mail: alexander.grund@tu-dresden.de ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
participants (2)
-
Alexander Grund -
Peter Dimov