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

On Jun 20, 2017, at 7:17 AM, Niall Douglas via Boost wrote:

On 20/06/2017 07:27, P F wrote:

...

...

On Jun 19, 2017, at 6:42 PM, Niall Douglas via Boost wrote:

I've finished the mockup which can be studied at:

https://github.com/ned14/boost-bmv-cmake

Just Boost.System was cmakeified. Extensive comments are throughout to explain every single line of cmake and why it's there. A small sample program is in the root of the repo to show how this cmake would be used by end user programs.

Here some feedback, I have:

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

Moreover, cmake now supports header only libraries, and BUILD_SHARED_LIBS causes problems with eventual C++ Modules support. The ::hl suffix means "link against the header only library edition", the ::sl suffix means "link against the static library edition" and the ::dl suffix means "link against the dynamic library edition".

Some bikeshedding over those names occurred off list. They don't hugely matter, they could be boost::system::header, boost::system::static and boost::system::shared if you wanted.

Some bikeshedding also occurred over what boost::system with no suffix should mean. I recommend "whatever library edition the maintainer recommends as the best one" which is NOT necessarily the header only edition. For example, in the future with C++ Modules support the additional suffixs ::hlm, ::slm and ::dlm will be needed. Or longer named, boost::system::header_module, boost::system::static_module and boost::system::shared_module.

So if C++ Modules were available, the maintainer might recommend the ::dlm variety, but recommend the ::hl variety if they weren't available. I think that will hugely vary per library as C++ Modules won't necessarily be always a win. Also, end users may wish to assemble all of Boost into a single C++ Module. We don't want to get in the way.

...

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

...

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

...

- There is no installation of the targets

Not necessary right now, so out of scope.

It is necessary.

...

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

(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 `$` generator expression in order to make the installation relocatable. [1]: https://de.slideshare.net/DanielPfeifer1/cmake-48475415