On Tue, 2017-06-20 at 16:35 +0100, Niall Douglas via Boost wrote:
- Having things like `::hl` or `::sl` is quite strange. In cmake, I set `BUILD_SHARED_LIBS=On` when I want a shared library or I set it to `BUILD_SHARED_LIBS=Off` when I want a static library. There should only be one target `boost::config` or `boost::system` that I use.
BUILD_SHARED_LIBS is a cmake2-ism and should not be present in modern cmake.
No, cmake best practice including cmake 3:
* Leave the control of `BUILD_SHARED_LIBS` to your clients[1]
That's exactly what I'm doing. A rootlevel CMakeLists
The clients aren't necessarily the rootlevel cmake. They are mainly the ones who are invoking cmake.
can choose to observe global variables and apply them by choosing only the targets which are static or shared for build. Non-rootlevel CMakeLists should have no business ever touching, using, or relying on any global variable or state. Declaration only.
- Each project do things like `include("${CMAKE_CURRENT_SOURCE_DIR}/../../cmake/BoostVersion.cmake”) ` which is broken when built on its own.
That was intentional. Some bikeshedding occurred over that too with respect to which Boost library guaranteed to be a dependency for all other libraries ought to host the version setting script. Boost.Config is most likely. So the above is a placeholder.
Yes, but this breaks cmake best practice that says:
* Make sure all your projects can be built as standalone and as a subproject of another project.
Let me repeat myself because you didn't read my words:
"So the above is a placeholder"
- Doing `if(NOT TARGET boost::assert::hl)` is wrong. A target does not have to exists to be able to link against it. Plus, it should call `find_package(boost_assert)` to bring in the target for when its not built in the superproject.
That part is there solely to enable modular build. If you don't want to bother with modular build as this is a bare minimum viable cmakeification, you can eliminate it.
What do you mean by modular build? There is only two ways it can be as standalone or as a modular build.
Ok, "exactly what is needed" build so. If I ask to link against Boost.System, only Boost.System and its dependencies get built by cmake. No unnecessary stuff.
That already happens when you use `EXCLUDE_FROM_ALL`, so cmake only builds the targets it needs.
You don't actually need that for minimum viable cmake of course. You could have b2.exe call cmake to build the whole of Boost and place it in staging exactly as at present.
But it seemed to me if one is to bother with cmakeification, you might as well formally specify the dependencies of each library and make possible "exactly what is needed build on demand". It's not much extra work, and it contributes much added value.
- There is no installation of the targets
Not necessary right now, so out of scope.
It is necessary.
No, it really isn't. If people want installation logic, it's trivially easy for them to write a rootlevel CMakeLists which implements that from what we've given them. No need for Boost to dictate what installation means or is by hard coding it into non-rootlevel CMakeLists.
Yes, but when we are building standalone the so called "non-rootlevel" is the rootlevel cmake.
- Doing `target_include_directories(boost_core-hl INTERFACE "include”)` is wrong as this will only add the include directories for the build. The include directories should be added for the build and for the installation.
As I've repeatedly explained now, installation logic is best implemented by a rootmost CMakeLists. Not in the per-library CMakeLists. We do tell cmake what targets are used by installation via install(TARGETS ...). That's the bare minimum needed for other cmake to implement the remaining installation logic.
Even if were implemented in the topmost, it would be broken when using `find_package` as the target is using the include directories in the build directory and not the install directory.
That's trivially easy to change at the rootlevel. Just iterate the targets and change the properties to whatever you feel they need to be.
I also deliberately left open a customisation point on this where headers.cmake is auto-generated by rootlevel cmake script. It could inject $
and $ there if it chose. I think it unwise to hard code that stuff. It's non-fixed configuration. End users reusing our cmake may have custom install paths and needs.
This is to support custom install paths, and it will support relocatbility as well.
We should not be dictating to them what those should be. Let rootlevel CMakeLists implement that stuff.
For standalone, it is the rootlevel.