Add install rules and BoostConfig script #45

Orphis · 2017-08-14T15:28:56Z

No description provided.

This will also generate a BoostConfig.cmake file ready to be used by "find_package(Boost CONFIG)".

rcdailey

Let me know if you need help with any of the suggestions I made. I actively need this, and I plan to test what you have here (I haven't yet). But I'm also in a good position to help code it as well.

rcdailey · 2017-08-31T18:35:29Z

BoostConfig.cmake.in

+set(_boost_deps @BOOST_FIND_PACKAGE@)
+
+foreach(dep ${_boost_deps})
+  find_package(${dep} QUIET)


You're just trying to find dependencies across boost components right? (For example how filesystem probably requires system?). In that case, shouldn't you be using the find_dependency() macro? According to the docs, it's the job of the downstream CMake scripts to find dependencies:

https://cmake.org/cmake/help/v3.6/module/CMakeFindDependencyMacro.html#module:CMakeFindDependencyMacro

It's for third party dependencies as you guessed later. Things like zlib, bzip2 or mpi.

rcdailey · 2017-08-31T18:36:25Z

BoostConfig.cmake.in

+  endif()
+endforeach()
+
+message(STATUS "Boost dependencies found: ${_boost_deps_found}")


find_package already has mechanisms to handle config package dependencies, I think you should follow those procedures (if I understand what you're doing):

https://cmake.org/cmake/help/v3.6/manual/cmake-packages.7.html#id17

rcdailey · 2017-08-31T18:38:41Z

cmake/Modules/ExportLibraries.cmake

+install(DIRECTORY ${BOOST_SOURCE}/boost
+  DESTINATION include
+)
+install(EXPORT boost-libs


In order to get dependency searching working per idiomatic CMake config package methodology, you're going to have to generate an export target per boost component. This means, for example, having:

BoostFilesystemTargets.cmake

BoostThreadTargets.cmake

etc.

I haven't found any documentation for this, not sure if that's the way to go.

If you look at this page (a bit towards the bottom of that linked section), you will see an example of how they handle components in config packages:

include(CMakeFindDependencyMacro) find_dependency(Stats 2.6.4) include("${CMAKE_CURRENT_LIST_DIR}/ClimbingStatsTargets.cmake") include("${CMAKE_CURRENT_LIST_DIR}/ClimbingStatsMacros.cmake") set(_supported_components Plot Table) foreach(_comp ${ClimbingStats_FIND_COMPONENTS}) if (NOT ";${_supported_components};" MATCHES _comp) set(ClimbingStats_FOUND False) set(ClimbingStats_NOTFOUND_MESSAGE "Unsupported component: ${_comp}") endif() include("${CMAKE_CURRENT_LIST_DIR}/ClimbingStats${_comp}Targets.cmake") endforeach()

This is what I'm basing my suggestion on. Would this make sense for boost?

Hmm... That's just extra complexity for the sake of it, I don't really see the point of adding that.
It could be possible to add easily though.

You're the boss, I'm just sharing my thoughts. Honestly I don't have much experience with configure packages so I'm not sure what is right, wrong, or ideal

I don't have that much experience either, to be honest!

So let me ask then: If I do this:

find_package( boost CONFIG COMPONENTS filesystem )

Will I also have targets like Boost::thread? If so, I think that's why separate target.cmake files are output, so that you only define the import targets that the user asked for. If it's all in one big file I think no matter what components they specify, you'll get all of them.

Am I correct on this? I'm having some work machine problems, but once I get past that I'll be able to test that theory for you.

Yes, but I believe that's relevant for Find*.cmake modules that do a lot of extra work to find various components.
In this case, you have a pre-made list of libraries, so the impact is really minimal.

Not sure if it's practical or not, I'm just making a point in terms of design: Having targets defined that you didn't explicitly ask for might go against what the CMake developers originally intended. However, they don't seem to document this. I'll ask on the mailing list, not because I'm particularly concerned about how you've chosen to do this, but more for my own understanding since lately I've been trying to learn more about config packages. I'll let you know what they say, if you're curious.

Thanks for discussing!

Already got a response:

Hi Robert,

On Fri, Sep 1, 2017 at 9:21 PM, Robert Dailey rcdailey.lists@gmail.com wrote:
One problem I thought of with the former (one big target.cmake with
all import targets in there) is that if you only ask for a subset of
components in find_package(), you will still get all of them since all
imports are defined in a single file.

In my project I have a bunch of components and do one exported target per component
exactly by the mentioned reason -- user didn't ask for others...

Does this go against any design
principles?

As far as I know, there are no clear design principles :) (yet, at least nowadays) -- at least doing
a lot of CMake projects since 2009, I've never seen an explicit list of them %)
IMHO, there is a lack of "official guildelines" (or it is really hard to search for 'em)

Assuming this really happens, are there any negative side
effects?

I could see the impact on build time only in this case... and for me the most obvious is increasing
time to process the lists (which is for some reasons really slow on Windows, at least in our
build farm which uses vargant and VirtualBox images)
(but I don't have any particular numbers, cuz never implemented the first approach)

rcdailey · 2017-08-31T18:39:36Z

libs/header.cmake

@@ -1,12 +1,21 @@
 # Define the header-only Boost target
-add_library(Boost::boost INTERFACE IMPORTED GLOBAL)
+add_library(Boost_boost INTERFACE)
+add_library(Boost::boost ALIAS Boost_boost)


To eliminate confusion, I think if you're going to adopt the Boost_ convention you should probably convert everything, but maybe there's a functional reason for this. Just food for thought.

The real targets are already named Boost_*. Rerefences between libraries are done using Boost::.
This change was made to allow the target with the header only libraries to be exported (since it was IMPORTED, nothing would happen).

rcdailey · 2017-08-31T18:43:42Z

libs/locale.cmake

@@ -59,6 +59,7 @@ _add_boost_lib(
 # Convenience interface library to link deps to both main library and tests
 add_library(Boost_locale_deps INTERFACE)
 target_link_libraries(Boost_locale PRIVATE Boost_locale_deps)
+install(TARGETS Boost_locale_deps DESTINATION lib EXPORT boost-libs)


Per one of my last comments, as I was considering implementing config packages for your projects myself, the way I would have done it is by adding a custom _install_boost_component() function that does something like:

function( _install_boost_component component ) set( export_name Boost${component}Targets ) install( TARGETS Boost_${component} EXPORT ${export_name} RUNTIME DESTINATION bin LIBRARY DESTINATION lib ARCHIVE DESTINATION lib INCLUDES DESTINATION include ) install(EXPORT ${export_name} DESTINATION share/boost-${BOOST_VERSION}/cmake ) # TODO: # install(FILES) for the header files since `INCLUDES DESTINATION` just maps include path and doesn't actually install any files endfunction()

I think this keeps things more modular, and allows you to use the COMPONENTS section of find_package() more reliably (i.e. find_dependency() macro)

In my limited testing from weeks ago, the COMPONENTS "worked". But it might just be that everything was just loaded regardless of the value. I'll do more investigation!

rcdailey · 2017-08-31T18:46:56Z

libs/locale.cmake

@@ -98,6 +100,7 @@ if(BOOST_LOCALE_ENABLE_ICONV_BACKEND AND ICONV_FOUND)
    Iconv::Iconv
  )
  target_compile_definitions(Boost_locale_deps INTERFACE BOOST_LOCALE_WITH_ICONV=1)
+  set_property(GLOBAL APPEND PROPERTY Boost_Find_Package Iconv)


Ah, I see now. This means you aren't searching for boost components but actual third party dependencies. In that case yes, you should most definitely be using find_dependency()!

The problem with find_dependency() is that it will abort the whole find_package() call if any of those is not met. I can easily imagine people providing binaries built with MPI enabled and later on users without MPI.
Using this, the other libraries can still be used.

I think the point of it is to make sure that EXACT is respected, but I've never personally used it. For example, if a boost component is listed under OPTIONAL_COMPONENTS, if it finds that boost component but not its dependency, that should be a "soft" failure, i.e. treat that component as not found, but keep going. Not sure if this case is handled here, but it's something worth testing (if you haven't already). Honestly it's pretty complicated, I'm not sure what the de facto behavior should be.

rcdailey · 2017-08-31T23:26:49Z

Thanks for responding to my comments. I am in the middle of testing your branch in my workplace right now. It's not quite set up yet, but I'll get you more feedback based on how that turns out. Stay tuned.

rcdailey · 2017-09-01T18:57:56Z

So I was able to finally test your changes, and so far it seems to be working great. Here is my code:

set( boost_components
    chrono
    context
    date_time
    exception
    filesystem
    regex
    signals
    system
    thread
)

find_or_build_third_party_library( boost 1.64.0
    COMPONENTS ${boost_components}
    USER_ARGS
        -D BOOST_DOWNLOAD_TO_BINARY_DIR=ON
        -D BOOST_ARCHIVE_DIRECTORY=${CMAKE_CURRENT_SOURCE_DIR}
        -D BOOST_DISABLE_TESTS=ON
)

set( definitions
    # Do not rely on deprecated interfaces in boost::filesystem.
    # Namely, in C++/CLI code, the member function `generic()` conflicts
    # with built-in symbols.
    BOOST_FILESYSTEM_NO_DEPRECATED

    # Do not use boost::bind placeholders, they conflict with std::placeholders
    BOOST_BIND_NO_PLACEHOLDERS
)

set( project_name Boost )

add_library( ${project_name} INTERFACE )
target_compile_definitions( ${project_name} INTERFACE ${definitions} )

# Boost::boost is not a real component but specifies include dirs for header-only libs
target_link_libraries( ${project_name} INTERFACE Boost::boost ${qualified_components} )

This is kinda complex, but my goals are:

Build boost in real-time as part of my parent repository's configure process, if it hasn't been built already
Find that version of boost, and only the components I specify
Create an interface target named Boost that pulls in all of the import targets for each of the components found, so that my other targets need only link to 'Boost' to pull in everything (easier maintenance for me).

Don't worry too much about what find_or_build_third_party_library is doing, basically it just runs an execute_process() to invoke a CMake child process to build & install your repository (similar to ExternalProject_Add() but less complex and supports find_package() after building).

So other than the polish we talked about (one target cmake script per component, which I recommend based on feedback I've been getting so far), I found one other bit of feedback. When you install your configuration scripts, you currently place them in a path like:

${CMAKE_INSTALL_PREFIX}/lib/cmake/Boost/*.cmake

I'd recommend two things:

Install to share instead of lib. While both are valid, IMHO share makes a little more sense, since you're not really installing libs (This is just my personal opinion)
More objectively: You should prepend the version to your path, such as: share/Boost-1.64.0/cmake/*.cmake. This allows users to install multiple versions of boost on a system, and using the VERSION parameter of find_package(), be able to use multiple side-by-side installations.

rcdailey · 2017-09-03T00:48:16Z

Status update: Friday I started working on a modification of your branch that does 1 export script per component. I'm not sure if this is the best route yet, I'm just testing it out mainly to learn more about this style as well as learn a little more about how components work. Secondly, this will give you the opportunity to see this working as well so you can decide which you like better.

I'm still not done yet but will resume work on Tuesday. I'll keep you updated!

Orphis · 2017-09-03T10:47:21Z

Thanks!

Btw, I'm flying tomorrow, and for some reason, I can do a lot of work in planes. So if you have other feature requests, do them today and I may have a look at those :)

rcdailey · 2017-09-06T14:53:21Z

So, I did some more digging on this but I'm running into issues with dependencies. This approach (per-component exports) requires managing inter-dependencies in the same package (components depending on other components). Not sure of the right way of handling this. Invoking find_package() for the package currently being found seems like it would cause infinite recursion easily. I'm waiting on some feedback from the cmake list for this.

At the moment, however, I think we should move forward with your way of doing this. If for whatever reason later on this ends up being the better approach, I think what you have could be modified without breaking backward compatibility.

That being said, I am making further refinements to your changes that I will let you review by EOD today. Before, that, though I wanted to ask you about your dependency finding logic in your Config.cmake script:

set(_boost_deps @BOOST_FIND_PACKAGE@)

foreach(dep ${_boost_deps})
  find_package(${dep} QUIET)
  if(${dep}_FOUND)
    list(APPEND _boost_deps_found ${dep})
  else()
    list(APPEND _boost_deps_missing ${dep})
  endif()
endforeach()

message(STATUS "Boost dependencies found: ${_boost_deps_found}")
if(_boost_deps_missing)
  message(STATUS "Boost dependencies missing: ${_boost_deps_missing}")
endif()

There are <package>_FOUND and <package>_NOT_FOUND_MESSAGE variables that can be set if the package cannot be found. In your case, do you want any failures to occur if your config script can't find these dependencies? Or is it your intent to require the user to add find_package() themselves for missing dependencies?

If the former, I am happy to incorporate some failure logic into the coding I already did on your branch. I just wanted to understand your approach before I made any assumptions.

Thanks in advance.

rcdailey · 2017-09-06T16:30:01Z

More food for thought:

Concerning parallel installations of boost, I install config and target export scripts to share/boost-${BOOST_VERSION}/cmake now, but the libs and headers are not versioned. I took care of the libs by setting the output name like:

OUTPUT_NAME boost_${BOOSTLIB_NAME}-${BOOST_VERSION}

And prior to that I set (in the root CMakeLists.txt):

if(NOT CMAKE_DEBUG_POSTFIX)
    set(CMAKE_DEBUG_POSTFIX -debug)
endif()

You get installed libs like this (for both debug and release on windows as an example):

lib/boost_container-1.64.0.lib
lib/boost_container-1.64.0-debug.lib
lib/boost_atomic-1.64.0.lib
lib/boost_atomic-1.64.0-debug.lib
lib/boost_chrono-1.64.0.lib
lib/boost_chrono-1.64.0-debug.lib

The only thing left is the includes, which I'm not sure how to version. One solution is to install headers to include/boost-${BOOST_VERSION}/include, and set INCLUDE DESTINATION to that when exporting each target. However, I don't know if this follows linux-best-practices, which I assume most installation logic should follow, even on Windows.

rcdailey · 2017-09-06T18:37:55Z

Here is what I have so far, it seems to be working good. Be sure to read over my commit messages, I addressed multiple things. Let me know what you think. The only outstanding issue is how to version the include directories, as I mentioned in my last post, I'm not sure how to best handle that.

install_rules...Ziosk:install_rules

helmesjo · 2017-09-13T21:05:53Z

Regarding install-dirs, check out this tutorial and the use of GNUInstallDirs.

rcdailey · 2017-09-13T21:11:55Z

@helmesjo that's interesting, but doesn't seem to have any special behavior for Windows. And I don't think we should use something that ignores that platform. Well, I'm assuming it is ignored, because I do not see Windows mentioned explicitly in the official documentation page for that module.

Can you explain why you recommend using that module versus just hard-coding the subdirectories? I'm not super experienced with Linux, so just keep that in mind.

helmesjo · 2017-09-24T16:06:04Z

@rcdailey Woop, sorry for late answer!

I'm not super experienced in either platform (in the context of install-paths and conventions), and yes as you say GNUInstallDirs focuses solely on Linux, but in the tutorial I mentioned he explicitly handles the Windows-case (see install(TARGETS ...).

More of a "could this be of interest?" than something I can argue about... :)

rcdailey · 2017-10-06T13:45:41Z

Just wondering how everyone feels about my modification to @Orphis's original branch. Would be nice to get this in at some point. I probably need to rebase it and clean it up a bit, and maybe investigate using GNUInstallDirs per recommendation from @helmesjo. Thoughts?

helmesjo · 2017-10-10T10:06:24Z

I'm a little confused... At what part does the generation of the export target for the build-tree occur? Is this what BoostConfig.cmake.in is used for?

If I'm not missing something, consider using the built-in export-command (instead of hand rolling it). Assuming each target is set up correctly, cmake can generate it all for us (the same way install(export ...) does).

export: Export targets from the build tree for use by outside projects.

helmesjo · 2017-10-10T10:18:53Z

Also, throw an eye on BCM (Boost CMake Modules) and that boost-cmake account in general. Seems to have gotten some momentum, with (not only) Daniel Pfeifer behind it who's been very prominent around the whole boost+cmake idea since... way back.

Orphis · 2017-10-10T12:22:48Z

The problem with BCM is that it doesn't reduce the boilerplate that much. It's also untested, whereas those build scripts are used to ship millions of apps around the world on the 5 major platforms.

export doesn't work for us, I don't believe it will generate the boilerplate to find the dependencies required by some packages.

rcdailey · 2017-10-10T14:39:03Z

export() is for import targets but only within the context of that project's CMAKE_BINARY_DIR. It is not designed for exporting import target scripts that should be prefix-path agnostic or for permanent installation on a system. That's what install(EXPORT) is for (I am using this; just dig around in the scripts some more).

helmesjo · 2017-10-10T15:10:52Z

@rcdailey Yes exactly, I was only talking about the export targets for the build-tree (I only saw install(EXPORT) which, as you say, is for the install-tree). Is the former not part of this pull request? That's what I need first and foremost... :)

@Orphis: Ok!

rcdailey · 2017-10-10T15:13:19Z

To be honest, I never saw the value in built-tree import targets. I always install my third party dependencies before using them, even if I have them as git submodules. What are the main use cases for build-tree import targets, as opposed to just doing add_subdirectory()?

helmesjo · 2017-10-10T15:25:34Z

Usually:
For a fully version controlled & cross-platform repository. All dependencies included = no setup/install required when cloning to a clean workstation, and target platforms are not predetermined.
Just clone & build (well, we need a compiler present but that's more trivial)!

Alternative would probably be to keep binaries for all supported platforms in the repo, which isn't very space-efficient or flexible. Obviously quicker build-times though...
Or solve it with docker.

rcdailey · 2017-10-10T15:42:57Z

@helmesjo I'm sorry but nothing you said is making sense to me. Can you elaborate? What does version control have to do with it? What do you mean by "all dependencies included"? What do you mean "cloning to a clean workstation" (again, not sure what VCS has to do with it).

helmesjo · 2017-10-10T16:59:23Z

Wow, I misinterpreted your question and digressed heavily. Sorry! :) Scratch the part about pre-built binaries. Just imagine all third-party dependencies being version controlled in the same repo, and built from source.

Now, regarding the last part I didn't read:
Sure, add_subdirectory( A ) can be used and A can just expect the dependee B to be present. But A might be a third-party library (again, built from source), and thus probably uses find_package(B REQUIRED) to "make sure" B is available.

Now in my ideal world A shouldn't have to care whether target B is in the build-tree or the install-tree, it just "finds the exported target" and links it. (caveat: this presumes find_package(B REQUIRED) works for both build- & install-targets, but sadly in the former case we have to specify the CONFIG flag... So imagine the convention that A uses both find_package(B) & find_package(B CONFIG) and fails only if neither is found). Sometimes B is installed, sometimes B is part of the same build: A should be agnostic.
In above I was confused & incorrect about how module-mode works**

Using build-tree targets for all dependencies, I can have one "uniform" build for the whole project (that is: clone & build. Two commands and all is done. No separate steps to "first install then build"). With install-targets, I first have to build & install all dependency-targets before kicking of the "real" build. In other words first cmake build & install dependencies, then cmake build & install my own lib(s) (Right? *).

I might be trying to run unnecessary miles with this. Maybe building & installing dependencies as a separate step before building my own source is the way to go.

* I might be lacking some cmake-knowledge here, or just be confused about it all in general (I'm by no means a CMake expert, barely been using it sporadically for a few months). In that case, please fill me in! :)

** I was wrong regarding find_package, it first searches in module mode, and, if non found, switches to config mode:

If no module is found and the MODULE option is not given the command proceeds to Config mode.

rcdailey · 2018-02-06T15:39:12Z

@Orphis: I propose we merge this into mainline so that it can be used in conjunction with your boost-1.66.0 branch. There's been very little movement in the past few months on this PR, I think it should be merged and further improvements made later on as needed.

rcdailey · 2018-02-06T15:50:12Z

Also @Orphis: I mentioned this before but please look at some refinements I added on top of your changes here: rcdailey@cd79b8e

Weiming-Hu · 2020-03-12T21:30:54Z

Hi. Just would like to ask whether there is any interest in carrying this issue further? Thank you.

… build it. WARNING: results in unusable BTAS in install tree, to be fixed by Orphis/boost-cmake#45

This was referenced Aug 14, 2017

Optional install rules? #25

Open

Added install rules #43

Closed

Orphis force-pushed the install_rules branch 2 times, most recently from 533dc71 to 722244f Compare August 15, 2017 07:58

Add experimental install rules

fe2fad0

This will also generate a BoostConfig.cmake file ready to be used by "find_package(Boost CONFIG)".

Orphis force-pushed the install_rules branch from 722244f to fe2fad0 Compare August 15, 2017 08:40

rcdailey reviewed Aug 31, 2017

View reviewed changes

evaleev added a commit to ValeevGroup/BTAS that referenced this pull request Dec 10, 2020

if Boost is missing will use https://github.com/Orphis/boost-cmake to…

7fc2bb6

… build it. WARNING: results in unusable BTAS in install tree, to be fixed by Orphis/boost-cmake#45

Add install rules and BoostConfig script #45

Are you sure you want to change the base?

Add install rules and BoostConfig script #45

Conversation

Orphis commented Aug 14, 2017

rcdailey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rcdailey Sep 1, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rcdailey Sep 1, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rcdailey commented Aug 31, 2017

rcdailey commented Sep 1, 2017

rcdailey commented Sep 3, 2017 • edited Loading

Orphis commented Sep 3, 2017

rcdailey commented Sep 6, 2017 • edited Loading

rcdailey commented Sep 6, 2017

rcdailey commented Sep 6, 2017

helmesjo commented Sep 13, 2017

rcdailey commented Sep 13, 2017

helmesjo commented Sep 24, 2017 • edited Loading

rcdailey commented Oct 6, 2017

helmesjo commented Oct 10, 2017

helmesjo commented Oct 10, 2017

Orphis commented Oct 10, 2017

rcdailey commented Oct 10, 2017 • edited Loading

helmesjo commented Oct 10, 2017 • edited Loading

rcdailey commented Oct 10, 2017

helmesjo commented Oct 10, 2017

rcdailey commented Oct 10, 2017

helmesjo commented Oct 10, 2017 • edited Loading

rcdailey commented Feb 6, 2018

rcdailey commented Feb 6, 2018

Weiming-Hu commented Mar 12, 2020

rcdailey Sep 1, 2017 •

edited

Loading

rcdailey Sep 1, 2017 •

edited

Loading

rcdailey commented Sep 3, 2017 •

edited

Loading

rcdailey commented Sep 6, 2017 •

edited

Loading

helmesjo commented Sep 24, 2017 •

edited

Loading

rcdailey commented Oct 10, 2017 •

edited

Loading

helmesjo commented Oct 10, 2017 •

edited

Loading

helmesjo commented Oct 10, 2017 •

edited

Loading