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

Subject: Re: [boost] [cmake] Minimum viable cmakeification for Boost
From: Niall Douglas (s_sourceforge_at_[hidden])
Date: 2017-06-20 15:35:38

>>> - 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 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.

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.

>>> - 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. We
should not be dictating to them what those should be. Let rootlevel
CMakeLists implement that stuff.

>> (On wider picture stuff, install paths need to be absolute I've found,
>> whereas build paths can usually be relative. Relative paths are much
>> nicer during build. In a rootmost CMakeLists which implements some
>> installation logic, you'd iterate the targets for the Boost libraries
>> and configure each programmatically for the appropriate install for that
>> library and that type of library. All the metadata you need to make
>> decisions is available)
> 
> I dont what you are saying here. Install paths should be relative or
> they should use the `$<INSTALL_PREFIX>` generator expression in order to
> make the installation relocatable.

For the output yes. I was referring to the input.

Niall

-- 
ned Productions Limited Consulting
http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/