-
Notifications
You must be signed in to change notification settings - Fork 189
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.
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
add how to add boost #575
add how to add boost #575
Conversation
In order to get a passing review I would like to see a defensible description at a minimum. |
Is now ok? |
examples/boost/CMakeLists.txt
Outdated
NAME Boost | ||
VERSION 1.84.0 | ||
URL https://github.com/boostorg/boost/releases/download/boost-1.84.0/boost-1.84.0.tar.xz | ||
URL_HASH SHA256=2e64e5d79a738d0fa6fb546c6e5c2bd28f88d268a2a080546f74e5ff98f29d0e | ||
OPTIONS "BOOST_ENABLE_CMAKE ON" "BOOST_INCLUDE_LIBRARIES container\\\;asio" # Note the escapes! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a simple, secure, working example that demonstrates including container
and asio
. For me security and simplicity are important.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that this is important.
examples/boost/CMakeLists.txt
Outdated
NAME AddBoost.CMake | ||
GIT_TAG main | ||
GITHUB_REPOSITORY Arniiiii/AddBoost.cmake |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example lacks security and a specific version of boost. When developing a commercial application, there's often a need to call out a specific revision of boost.
These lines are neither equivalent nor better than the removed lines for my purposes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security? Not sure. I admit the problem with specific version. I'll try to solve it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've thought a little bit about version handling. If to think about AddBoost.cmake not as wrapper for Boost but just another cmake script, In the end it's still find_package(Boost ${TRY_BOOST_VERSION} COMPONENTS ${BOOST_NOT_HEADER_ONLY_COMPONENTS})
or CPMAddPackage(NAME Boost VERSION ${TRY_BOOST_VERSION} PATCHES ... OPTIONS __CMAKE_BOOST_INSTALL_OR__BOOST_ENABLE_INSTALL)
.
If to think about specific revision of boost there's two options:
- Use specific boost version for which there's tag and patch it
- If to think about specifying SOURCE_DIR for CPM to get a separate boost.
It's easy to add the ability to do the second one to the AddBoost.cmake
README.md
Outdated
Boost is a large project and will take a while to download. Using | ||
`CPM_SOURCE_CACHE` is strongly recommended. Cloning moves much more | ||
data than a source archive, so this sample will use a compressed | ||
source archive (tar.xz) release from Boost's github page. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This removed bit is a very important part of this example. It coaches the user in best practices.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right. I'll try to save these lines of notes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess if add them to ## Tips
that would be ok
README.md
Outdated
``` | ||
|
||
For a working example of using CPM to download and configure the Boost C++ Libraries see [here](examples/boost). | ||
Also, to get install target if you use `add_subdirectory(dir_with_boost_source)`, you need to apply a patch for 1.80.0, and another patch if you want a version of Boost from 1.81.0 upto 1.84.0. To solve such problems, there's a script [AddBoost.CMake](https://github.com/Arniiiii/AddBoost.cmake) example usage of which you can see here: [here](examples/boost) or [here](https://github.com/Arniiiii/ModernCppStarterExampleBoostCmake). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this comment mean and how is it realted to nominal CPM usage?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a problem with cmake for boost from versions from 1.80.0 upto 1.84.0 that doesn't allow using it via add_subdirectory
in terms of not generating install targets. And for version 1.80.0 it's a little bit different patch than for other versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are using CPM, why would we call add_subdirectory
? I don't in any of my projects...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are using CPM, why would we call
add_subdirectory
? I don't in any of my projects...
I'm using CPM. Though the logic behind CPM is actually a kinda smart add_subdirectory. I don't use add_subdirectory manually in my projects too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I understand.
README.md
Outdated
`CPM_SOURCE_CACHE` is strongly recommended. Cloning moves much more | ||
data than a source archive, so this sample will use a compressed | ||
source archive (tar.xz) release from Boost's github page. | ||
Boost has incompatible targets: for boost installed via b2 (or via CMake upto version boost_1.84.0 or via CMake scripts from `boost/tools/boost_install`) there's no targets for header-only Boost's libraries. Starting from boost_1.85.0 there's b2 version **and CMake version** of *install* targets, and Boost CMake version install CMake target even for header-only libraries, which allows installing and using only necessary boost libraries. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this related to CPM? And why worry about installation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because assume there's no boost on system. What should cmake install when you asked to install your project that uses a boost library, when there's no boost on the specific system? Right, it installs the specific boost libraries too. That's why thinking about installation targets in this context is important.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm... In my use cases, I compile boost as a static library so there's no boost installation. In the case of a library, this gets more complicated. But I think the fix to patch Boost instead of add another dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm... In my use cases, I compile boost as a static library so there's no boost installation. In the case of a library, this gets more complicated. But I think the fix to patch Boost instead of add another dependency.
I support your idea. I'll try realize it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually the script allows more than patching, but also easy linking, easy installing and so on, no matter is boost installed on the system or not, which is great.
Overall, I don't feel like this would be a good change for my projects and I'm not certain it's in the spirit of CPM or Boost. I think what I would rather see you provide is a set of patches to improve older versions of Boost for use with CPM and work on getting those changes merged into Boost proper. One possible example: include(../../cmake/CPM.cmake)
# Git the patches and additional targets!
CPMAddPackage(
NAME AddBoost
GITHUB_REPOSITORY Arniiiii/AddBoost.cmake
GIT_TAG main
OPTIONS "ADD_BOOST_BOOST_VERSION 1.84.0"
)
CPMAddPackage(
NAME Boost
VERSION 1.84.0
URL https://github.com/boostorg/boost/releases/download/boost-1.84.0/boost-1.84.0.tar.xz
URL_HASH SHA256=2e64e5d79a738d0fa6fb546c6e5c2bd28f88d268a2a080546f74e5ff98f29d0e
OPTIONS "BOOST_ENABLE_CMAKE ON" "BOOST_INCLUDE_LIBRARIES container\\\;asio" # Note the escapes!
PATCHES ${ADD_BOOST_PATCHES}
)
target_link_libraries(CPMExampleBoost PRIVATE Boost::asio Boost::container)
add_boost( <I'm not sure what options go here since it's outside my use cases> ) |
You are right about version handling. That's the problem. The main problem for me is that it looks like CPM doesn't set a variable about is it local or not, only setting ${Boost_FOUND} no matter is it installed or downloaded. Also idk how to explain the moment that boost installed via b2 (let's say local boost), and boost downloaded and used via cmake have different install targets like boost_${name_of_a_header_only_boost_lib} boost_${name_of_NOT_header_only_lib} , and local boost has Boost::boost and Boost::${a_NOT_header_only_components} . That's why the script appeared in the first place. |
If this is the case, maybe you can add a patch to improve CPM's to behavior for your use case? That would help everybody! |
I agree, I'll try to add such behavior and test it together with boost. |
I'll try to add such behaviour to make the AddBoost.cmake using only CPMAddPackage and not a lot of my stuff with if(${IS_BOOST_LOCAL}) ... |
I've figured out: actually in CPM there's already mechanism for "is locally found" stuff:
I'm going to try push updated AddBoost.cmake and try to make example here up-to-date. After this, @ScottBailey please, let me understand what I should make to AddBoost.cmake to be good enough or how it should be solved. I would like to hear any complains. I would still use AddBoost.cmake as reference way, because the script there contain 150 LC and it's kinda long for snippets section and idk if it's ok for examples. |
Let me applaud you for trying to help and encourage you! But let's take a step back. First, I'm not a maintainer of this project, just an interested party. CPM caught my attention as an aesthetically pleasing way to add boost (and some other libraries, too!) to projects I do maintain. I am not a maintainer here, but I'm going to argue that the example as written is good enough. That means the bar to replace it is HIGH. For my projects, the example in master works. And it's simple and direct. For my projects, this PR adds another layer without providing ANYTHING useful. Instead, it slows down the build process and requires me to review an additional download. As the PR stands, and with This is a reduction in value. Rather than focus your efforts on getting AddBoost into this example, why don't you update the example to use boost version 1.85.0 and explain why. You can then add a note indicating add boost has patches for older versions and provide links to the AddBoost patch directory and an example of using CPM with AddBoost inside AddBoost. That would be value added and I could recommend a merge. |
Should be notes about how to deal with installation targets? |
I'm not sure. I think in most cases for average users, installation of Boost from a CPM target is a bad idea. There is too much risk of collision and partial installation between libraries. Professional package builders are going to do something completely different. And maintainers building a complicated library or application that need to install Boost as shared libraries will already have most of that knowledge. It's probably out of scope for and likely needs more supporting documentation than belong in this simple example. If it's something you are really familiar with, a discussion of it in the README file or some other document could be helpful. Actually, advice on using CPM for "professional" package builders could be very useful somewhere. Anyway, here's an idea to start you off: # Boost libraries
CPMAddPackage(
NAME Boost
URL https://github.com/boostorg/boost/releases/download/boost-1.85.0/boost-1.85.0-cmake.tar.xz
URL_HASH SHA256=0a9cc56ceae46986f5f4d43fe0311d90cf6d2fa9028258a95cab49ffdacf92ad
SYSTEM True
OPTIONS
"BOOST_ENABLE_CMAKE ON" # Required ONLY in version < 1.85.0
"BOOST_SKIP_INSTALL_RULES ON" # For installation, set this to `OFF` and apply a patch in versions < 1.85.0
"BUILD_SHARED_LIBS Off"
"BOOST_INCLUDE_LIBRARIES container\\\;asio" # Note the escapes!
)
#[Patches](https:://path to add boost patches) and additional [installation discussion](path to install docs) can be found at path to addboost main. |
What do you think now, @ScottBailey ? |
I don't see value in referencing AddBoost because there is a simpler, cleaner way to do everything. AddBoost may help an inexperienced user get started, but I think it does too much. Think about the Unix way: simple is better. Also, I believe using Maybe the BEST fix is the simplest: update to 1.85 and add a header only library so a little more of Boost's syntax is exposed. We should try to provide the simplest example to allow a new user to begin development of a simple Boost dependent project. CPMAddPackage(
NAME Boost
VERSION 1.85.0
URL https://github.com/boostorg/boost/releases/download/boost-1.85.0/boost-1.85.0-cmake.tar.xz
URL_HASH SHA256=0a9cc56ceae46986f5f4d43fe0311d90cf6d2fa9028258a95cab49ffdacf92ad
OPTIONS
"BOOST_ENABLE_CMAKE ON"
"BOOST_SKIP_INSTALL_RULES ON" # Set `OFF` for installation
"BUILD_SHARED_LIBS OFF"
"BOOST_INCLUDE_LIBRARIES container\\\;asio" # Note the escapes!
)
target_link_libraries(CPMExampleBoost PRIVATE Boost::asio Boost::container) A beginning user needs a simple example. This example gives us that and it's a really good starting point for someone building a more complex project. The expert user doesn't need this at all, other than to learn that it is possible. See #583 |
Boost has incompatible targets: for boost installed via b2 (or via CMake upto version boost_1.84.0 or via CMake scripts from
boost/tools/boost_install
) there's no targets for header-only Boost's libraries. Starting from boost_1.85.0 there's b2 version and CMake version of install targets, and Boost CMake version install CMake target even for header-only libraries, which allows installing and using only necessary boost libraries.Also, to get install target if you use
add_subdirectory(dir_with_boost_source)
, you need to apply a patch for 1.80.0, and another patch if you want a version of Boost from 1.81.0 upto 1.84.0. To solve such problems, there's a script AddBoost.CMake example usage of which you can see here: here or here.