Re: [boost] [cmake] Minimum viable cmakeification for Boost

Subject: Re: [boost] [cmake] Minimum viable cmakeification for Boost
From: paul (pfultz2_at_[hidden])
Date: 2017-06-20 16:37:33

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 $<BUILD_INTERFACE> and $<INSTALL_INTERFACE> 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.