Skip to content

Add install rules and BoostConfig script #45

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Orphis wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add install rules and BoostConfig script #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions BoostConfig.cmake.in
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
set(_boost_deps @BOOST_FIND_PACKAGE@)

foreach(dep ${_boost_deps})
find_package(${dep} QUIET)

@rcdailey rcdailey Aug 31, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@Orphis Orphis Aug 31, 2017

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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}")

@rcdailey rcdailey Aug 31, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

unset(_boost_deps)
unset(_boost_deps_found)
unset(_boost_deps_missing)

include("${CMAKE_CURRENT_LIST_DIR}/BoostTargets.cmake")
14 changes: 8 additions & 6 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,16 @@ endforeach()
# TODO: Move those to option() calls in the right file
if(NOT BOOST_STANDALONE)
# Compilation options required by all platforms
set_property(TARGET Boost::boost APPEND PROPERTY INTERFACE_COMPILE_DEFINITIONS $<$<CONFIG:Release>:BOOST_DISABLE_ASSERT>)
set_property(TARGET Boost::boost APPEND PROPERTY INTERFACE_COMPILE_DEFINITIONS BOOST_SYSTEM_NO_DEPRECATED)
set_property(TARGET Boost::boost APPEND PROPERTY INTERFACE_COMPILE_DEFINITIONS BOOST_THREAD_VERSION=4)
set_property(TARGET Boost::boost APPEND PROPERTY INTERFACE_COMPILE_DEFINITIONS BOOST_THREAD_USES_CHRONO)
set_property(TARGET Boost::boost APPEND PROPERTY INTERFACE_COMPILE_DEFINITIONS BOOST_THREAD_PROVIDES_EXECUTORS)
set_property(TARGET Boost_boost APPEND PROPERTY INTERFACE_COMPILE_DEFINITIONS $<$<CONFIG:Release>:BOOST_DISABLE_ASSERT>)
set_property(TARGET Boost_boost APPEND PROPERTY INTERFACE_COMPILE_DEFINITIONS BOOST_SYSTEM_NO_DEPRECATED)
set_property(TARGET Boost_boost APPEND PROPERTY INTERFACE_COMPILE_DEFINITIONS BOOST_THREAD_VERSION=4)
set_property(TARGET Boost_boost APPEND PROPERTY INTERFACE_COMPILE_DEFINITIONS BOOST_THREAD_USES_CHRONO)
set_property(TARGET Boost_boost APPEND PROPERTY INTERFACE_COMPILE_DEFINITIONS BOOST_THREAD_PROVIDES_EXECUTORS)
endif()

if(USE_ANDROID)
# Android doesn't support thread local storage through compiler intrinsics
set_property(TARGET Boost::boost APPEND PROPERTY INTERFACE_COMPILE_DEFINITIONS BOOST_ASIO_DISABLE_THREAD_KEYWORD_EXTENSION)
set_property(TARGET Boost_boost APPEND PROPERTY INTERFACE_COMPILE_DEFINITIONS BOOST_ASIO_DISABLE_THREAD_KEYWORD_EXTENSION)
endif()

include(ExportLibraries)
2 changes: 2 additions & 0 deletions cmake/Modules/AddBoostLib.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ function(_add_boost_lib)
set_target_properties(Boost_${BOOSTLIB_NAME} PROPERTIES
OUTPUT_NAME "boost_${BOOSTLIB_NAME}"
FOLDER "Boost"
EXPORT_NAME "${BOOSTLIB_NAME}"
)
if(NOT BOOST_STANDALONE)
set_target_properties(Boost_${BOOSTLIB_NAME} PROPERTIES EXCLUDE_FROM_ALL 1)
endif()
target_link_libraries(Boost_${BOOSTLIB_NAME} PUBLIC Boost::boost)
install(TARGETS Boost_${BOOSTLIB_NAME} DESTINATION lib EXPORT boost-libs)
if(MSVC)
target_compile_options(Boost_${BOOSTLIB_NAME} PRIVATE /W0)
else()
Expand Down
25 changes: 25 additions & 0 deletions cmake/Modules/ExportLibraries.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
include(CMakePackageConfigHelpers)
write_basic_package_Version_file("BoostConfigVersion.cmake"
VERSION ${BOOST_VERSION}
COMPATIBILITY AnyNewerVersion)

get_property(BOOST_FIND_PACKAGE GLOBAL PROPERTY Boost_Find_Package)
if(BOOST_FIND_PACKAGE)
# The list may be empty
list(REMOVE_DUPLICATES BOOST_FIND_PACKAGE)
endif()
configure_file("BoostConfig.cmake.in" "${CMAKE_CURRENT_BINARY_DIR}/BoostConfig.cmake" @ONLY)

install(FILES
"${CMAKE_CURRENT_BINARY_DIR}/BoostConfig.cmake"
"${CMAKE_CURRENT_BINARY_DIR}/BoostConfigVersion.cmake"
DESTINATION lib/cmake/Boost
)
install(DIRECTORY ${BOOST_SOURCE}/boost
DESTINATION include
)
install(EXPORT boost-libs

@rcdailey rcdailey Aug 31, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@Orphis Orphis Aug 31, 2017

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@rcdailey rcdailey Sep 1, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@Orphis Orphis Sep 1, 2017

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@rcdailey rcdailey Sep 1, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@Orphis Orphis Sep 1, 2017

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@rcdailey rcdailey Sep 1, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@Orphis Orphis Sep 1, 2017

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@rcdailey rcdailey Sep 1, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

@rcdailey rcdailey Sep 1, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

DESTINATION lib/cmake/Boost
NAMESPACE Boost::
FILE BoostTargets.cmake
)
1 change: 1 addition & 0 deletions libs/chrono.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ _add_boost_lib(
if(NOT USE_WINDOWS)
find_package(Threads)
target_link_libraries(Boost_chrono PRIVATE Threads::Threads)
set_property(GLOBAL APPEND PROPERTY Boost_Find_Package Threads)

find_library(RT_LIBRARY
NAMES rt
Expand Down
19 changes: 14 additions & 5 deletions libs/header.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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)

@rcdailey rcdailey Aug 31, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@Orphis Orphis Aug 31, 2017

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

set_target_properties(Boost_boost PROPERTIES
EXPORT_NAME "boost"
)

if(CMAKE_GENERATOR MATCHES "Xcode")
# The CMake Xcode generator doesn't support system headers directly
set_target_properties(Boost::boost PROPERTIES INTERFACE_COMPILE_OPTIONS "-isystem;${BOOST_SOURCE}" )
set_target_properties(Boost_boost PROPERTIES INTERFACE_COMPILE_OPTIONS "-isystem;${BOOST_SOURCE}" )
else()
set_target_properties(Boost::boost PROPERTIES INTERFACE_SYSTEM_INCLUDE_DIRECTORIES ${BOOST_SOURCE} )
set_target_properties(Boost::boost PROPERTIES INTERFACE_INCLUDE_DIRECTORIES ${BOOST_SOURCE})
set_target_properties(Boost_boost PROPERTIES INTERFACE_SYSTEM_INCLUDE_DIRECTORIES ${BOOST_SOURCE} )
set_target_properties(Boost_boost PROPERTIES INTERFACE_INCLUDE_DIRECTORIES "$<BUILD_INTERFACE:${BOOST_SOURCE}>;$<INSTALL_INTERFACE:include>")
endif()
install(TARGETS Boost_boost DESTINATION lib EXPORT boost-libs)

# Disable autolink
set_property(TARGET Boost::boost APPEND PROPERTY INTERFACE_COMPILE_DEFINITIONS BOOST_ALL_NO_LIB=1)
set_property(TARGET Boost_boost APPEND PROPERTY INTERFACE_COMPILE_DEFINITIONS BOOST_ALL_NO_LIB=1)

add_library(Boost::boost_imported INTERFACE IMPORTED)
set_target_properties(Boost::boost_imported PROPERTIES INTERFACE_INCLUDE_DIRECTORIES "${BOOST_SOURCE}")
2 changes: 2 additions & 0 deletions libs/iostreams.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ if(BOOST_IOSTREAMS_ENABLE_BZIP2)
target_link_libraries(Boost_iostreams PRIVATE
BZip2::BZip2
)
set_property(GLOBAL APPEND PROPERTY Boost_Find_Package BZip2)
endif()
endif()

Expand All @@ -32,6 +33,7 @@ if(BOOST_IOSTREAMS_ENABLE_ZLIB)
target_link_libraries(Boost_iostreams PRIVATE
ZLIB::ZLIB
)
set_property(GLOBAL APPEND PROPERTY Boost_Find_Package ZLIB)
endif()
endif()

Expand Down
3 changes: 3 additions & 0 deletions libs/locale.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

@rcdailey rcdailey Aug 31, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

@Orphis Orphis Aug 31, 2017

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!


if(BOOST_LOCALE_ENABLE_ICU_BACKEND AND ICU_FOUND)
target_sources(Boost_locale PRIVATE
Expand All @@ -79,6 +80,7 @@ if(BOOST_LOCALE_ENABLE_ICU_BACKEND AND ICU_FOUND)
ICU::uc
)
target_compile_definitions(Boost_locale_deps INTERFACE BOOST_LOCALE_WITH_ICU=1)
set_property(GLOBAL APPEND PROPERTY Boost_Find_Package ICU)
endif()

if(BOOST_LOCALE_ENABLE_STD_BACKEND)
Expand All @@ -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)

@rcdailey rcdailey Aug 31, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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()!

@Orphis Orphis Aug 31, 2017

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@rcdailey rcdailey Sep 1, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

endif()

if(BOOST_LOCALE_ENABLE_WINAPI_BACKEND)
Expand Down
2 changes: 1 addition & 1 deletion libs/log.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ endif()
try_compile(HAVE_ATOMIC_INT32
"${CMAKE_CURRENT_BINARY_DIR}"
"${BOOST_SOURCE}/libs/log/config/atomic-int32/atomic_int32.cpp"
LINK_LIBRARIES Boost::boost
LINK_LIBRARIES Boost::boost_imported
)
if(NOT HAVE_ATOMIC_INT32)
target_compile_definitions(Boost_log PRIVATE BOOST_LOG_WITHOUT_IPC)
Expand Down
1 change: 1 addition & 0 deletions libs/mpi.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,4 @@ _add_boost_lib(
)
target_include_directories(Boost_mpi PUBLIC ${MPI_CXX_INCLUDE_PATH})
target_link_libraries(Boost_mpi PUBLIC ${MPI_CXX_LINK_FLAGS} ${MPI_CXX_LIBRARIES})
set_property(GLOBAL APPEND PROPERTY Boost_Find_Property MPI)
3 changes: 2 additions & 1 deletion libs/thread.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,11 @@ _add_boost_lib(
Boost::system
)
if(NOT USE_WINDOWS)
find_package(Threads REQUIRED)
find_package(Threads)

target_link_libraries(Boost_thread PUBLIC Threads::Threads)
target_compile_definitions(Boost_thread PRIVATE
BOOST_THREAD_POSIX
)
set_property(GLOBAL APPEND PROPERTY Boost_Find_Package "Threads")
endif()
7 changes: 7 additions & 0 deletions test/import-test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
cmake_minimum_required(VERSION 3.8.0)
project(import_test)

find_package(Boost CONFIG COMPONENTS unit_test_framework)

add_executable(import_test main.cpp)
target_link_libraries(import_test PRIVATE Boost::unit_test_framework Boost::iostreams)
3 changes: 3 additions & 0 deletions test/import-test/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
int main(int argc, char* argv[]) {
return 0;
}