Skip to content

Boost library link dependencies are incorrect #141

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

Open
BenWhetton opened this issue Jan 20, 2020 · 32 comments
Open

Boost library link dependencies are incorrect #141

BenWhetton opened this issue Jan 20, 2020 · 32 comments

Comments

@BenWhetton
Copy link

  • I've read Brief overview section and do understand basic concepts. [Yes]
  • I've read F.A.Q. section and there is no solution to my problem there. [Yes]
  • I've read Code of Conduct, I promise to be polite and will do my best at being constructive. [Yes]
  • I've read Reporting bugs section carefully. [Yes]
  • I've checked that all the hunter_add_package/find_package API used by me in the example is the same as in documentation. [Yes]
  • I'm using latest Hunter URL/SHA1. [Yes]
  • I've created SSCCE reproducing the issue:
# CMakeLists.txt
cmake_minimum_required(VERSION 3.16.2)
include(CMakePrintHelpers)
include("cmake/HunterGate.cmake")

HunterGate(URL "https://github.com/cpp-pm/hunter/archive/v0.23.244.tar.gz"
           SHA1 "2c0f491fd0b80f7b09e3d21adb97237161ef9835"
           LOCAL)

project(test_hunter_boost_log)

hunter_add_package(Boost COMPONENTS log)
find_package(Boost CONFIG REQUIRED log)

cmake_print_properties(
        TARGETS Boost::log
        PROPERTIES INTERFACE_LINK_LIBRARIES
)

(cmake/Hunter/config.cmake contains hunter_config(Boost VERSION 1.71.0-p0) to avoid issues with boost 1.72)

Here is the log until first error reported by Hunter, option HUNTER_STATUS_DEBUG is OFF (it doesn't add anything):

-- [hunter] Calculating Toolchain-SHA1
-- [hunter] Calculating Config-SHA1
-- [hunter] HUNTER_ROOT: /export/home/ben.whetton/.hunter
-- [hunter] [ Hunter-ID: 2c0f491 | Toolchain-ID: 511a137 | Config-ID: 3aaac7f ]
-- [hunter] BOOST_ROOT: /export/home/ben.whetton/.hunter/_Base/2c0f491/511a137/3aaac7f/Install (ver.: 1.71.0-p0)
-- [hunter] BOOST_ROOT: /export/home/ben.whetton/.hunter/_Base/2c0f491/511a137/3aaac7f/Install (ver.: 1.71.0-p0)
-- Boost version: 1.71.0
-- Found the following Boost libraries:
--   log
-- 
 Properties for TARGET Boost::log:
   Boost::log.INTERFACE_LINK_LIBRARIES = "Boost::boost"

-- Configuring done
-- Generating done
-- Build files have been written to: /export/home/ben.whetton/project/cpp/test_hunter_boost_log/cmake-build-debug-neptune

[Finished]

As you can see, Boost::log only links to Boost::boost, which is not correct.

  • I'm building on [Linux].
  • [I'm using official CMake release]
  • CMake version: <3.16.2>
  • I'm not using toolchain (I don't think it's relevant)

I'm using the next command line on generate step:

cmake -DCMAKE_BUILD_TYPE=Debug -G "CodeBlocks - Unix Makefiles"

As you can see, hunter doesn't find and link dependent libraries.

On the other hand, if we link to system boost without using Hunter, it finds all the correct link dependencies:

cmake_minimum_required(VERSION 3.16.2)
project(test_boost_log)
include(CMakePrintHelpers)

find_package(Boost REQUIRED log)

cmake_print_variables(Boost_VERSION_STRING)
cmake_print_properties(
        TARGETS Boost::log
        PROPERTIES INTERFACE_LINK_LIBRARIES
)
-- Found Boost: /usr/include (found version "1.65.1") found components: log date_time log_setup system filesystem thread regex chrono atomic 
-- Boost_VERSION_STRING="1.65.1"
-- 
 Properties for TARGET Boost::log:
   Boost::log.INTERFACE_LINK_LIBRARIES = "Boost::date_time;Boost::log_setup;Boost::system;Boost::filesystem;Boost::thread;Boost::regex;Boost::chrono;Boost::atomic"

-- Configuring done
-- Generating done
-- Build files have been written to: /export/home/ben.whetton/project/cpp/test_boost_log/cmake-build-debug

This is a major issue because Boost libraries must be linked in specific orders to avoid linking errors and CMake may optimise the linking order and break the build in the process if there are no explicit link dependencies between libraries.

It looks like CMake's FindBoost.cmake file contains logic for setting the dependencies up correctly. I think Boost are also implementing find CONFIG mode support from v1.70.

@daminetreg
Copy link

Could you try deleting your hunter folder and with :

include(HunterGate)
HunterGate(
    URL "https://github.com/nxxm/hunter/archive/feature/fix-boost-1.72-cmake-package-configs.zip"
    SHA1 "039b7abd220e4368d3fcbf7df520279678089cca"
)

Just to check if the fix I did for 1.72.0 is not fixing this one as well, because normally it should as far as I remeber then we use the FindBoost support.

@daminetreg
Copy link

I checked it and it does work as you expect it with my fixes. You've been fooled by the Boost package config files that I remove in my PR #140 I think.

See run on my machine :

-- Found Threads: TRUE
-- Found Boost: /Users/daminetreg/.hunter/_Base/039b7ab/b8cde86/0bf84ab/Install/include (found version "1.72.0") found components: log missing components: date_time log_setup filesystem thread regex chrono atomic
--
 Properties for TARGET Boost::log:
   Boost::log.INTERFACE_LINK_LIBRARIES = "Boost::date_time;Boost::log_setup;Boost::filesystem;Boost::thread;Boost::regex;Boost::chrono;Boost::atomic"

@BenWhetton
Copy link
Author

Looks like you're right! Awesome! I hope your fix gets merged in without many issues.

@daminetreg
Copy link

So far all tests passed and the changes are small, so I hope it get merged soon 😄.

Your report here means we are concerned by #133 for a longer time.

@daminetreg
Copy link

It actually even is somehow a duplicate of #133 I think.

@madmongo1
Copy link

madmongo1 commented Feb 3, 2020

Hi fellas,

Is there any chance you could let me know when the fix to this problem will be merged to master and released?

It's holding me back from upgrading to boost-1.72 in my hunter demos of Boost.

I'm a maintainer of Boost.Beast, and I use Hunter to demonstrate code to people would have issues or queries with the Boost.Beast library.

It would be great to have a solution.

Please note that using the patch mentioned above did not work for me (Fedora Linux 31, gcc9.2, CLion)

The SHA1 for the patch download on linux is 3092ec022af99e70bb4b11973ec48f1818e3bc51 (different line endings?)

With patch:

  Found package configuration file:

    /home/rhodges/.hunter/_Base/3092ec0/3e66e81/c9a6829/Install/lib/cmake/boost_system-1.72.0/boost_system-config.cmake

  but it set boost_system_FOUND to FALSE so package "boost_system" is
  considered to be NOT FOUND.  Reason given by package:

  No suitable build variant has been found.

  The following variants have been tried and rejected:

  * libboost_system-mt-d-x64.a (static, default is shared, set
  Boost_USE_STATIC_LIBS=ON to override)

  * libboost_system-mt-x64.a (static, default is shared, set
  Boost_USE_STATIC_LIBS=ON to override)

Demo repo:

https://github.com/test-scenarios/latest-hunter

Regards,

Richard
[email protected]

@Bjoe
Copy link

Bjoe commented Feb 4, 2020

Hi @madmongo1

it is great to see that someone from the "somehow official" ;-) is also using hunter :-) … I have the feeling that when I every year on the Meeting C++ conference and met people like Jon K. or when I hear Cpp-Chat or Cpp-Cast that everyone uses or only know the "other" package manager approach :-)
Anyway ...

There is a PR #140 (I think you saw this). Unfortunately it is not merged yet because there are some findings.
@daminetreg Have you seen the comment from @rbsheth ?

But you write this patch is not working for you? You mean with patch that PR #140 ?

I try to reproduced the error with your demo repo.

I use my C++ build environment in my ubuntu docker, image (from Dockerhub ).
with following toolchain: polly/clang-ninja-cxx17.cmake
I get following ID's.
[ Hunter-ID: 36e7f44 | Toolchain-ID: f49d2cd | Config-ID: 8d03466 ]

This works for me!

I will try with different toolchains ....

In the meanwhile, can you try following, add following line in your CMakeLists.txt:

set(Boost_USE_STATIC_LIBS ON)

And can you enable the "boost debugging" with:

set(Boost_DEBUG ON)

and post the output here.

Thanks.

@Bjoe
Copy link

Bjoe commented Feb 4, 2020

@madmongo1 Btw. you should know that boost is providing a "Boost Config" modul for cmake since 1.70. This is unfortunately not yet used in hunter. Hunter provides it's own "Boost Config" modul. That's why you must add manually and also in the right order all link depended library on target_link_libraries.
So don't create a bug report in boost if you think there is something wrong in the Boost Config module.
I'm already on the task to use the boost "boost config" module from boost.

@madmongo1
Copy link

madmongo1 commented Feb 4, 2020

@Bjoe I'm aware of the effort to cmake-ify boost. Peter Dimov is leading the work.

I know that he's been trying to ensure that the boost-created cmake packages are close enough to the behaviour of the kitware FindBoost.cmake script to be compatible, but I also know that boost cmake scripts create a target for each boost sub-library, whether header-only or not.

...Well that's odd. I've just nuked my ~/.hunter directory and rebuilt. All seems to be working with the feature/fix-boost-1.72-cmake-package-configs patch:

rm -rf ~/.hunter
mkdir ~/github/test-scenarios
mkdir ~/github/ruslo
cd ~/github/ruslo && git clone [email protected]:ruslo/polly.git
cd ~/github/test-scenarios
git clone -b patch1 [email protected]:test-scenarios/latest-hunter.git 
cd latest-hunter
mkdir cmake-build-release
cmake -H. -Bcmake-build-release -DCMAKE_TOOLCHAIN_FILE=${HOME}/github/ruslo/polly/cxx17.cmake -DBoost_DEBUG=ON

Thanks for looking into this.

By the way, I often bore my colleagues in the #boost and #beast channels of Cpplang Slack http://slack.cpp.al with how good Hunter is.

Please do continue the good work. Hunter is the package manager that c++ always needed.

@Bjoe
Copy link

Bjoe commented Feb 4, 2020

@madmongo1 ok you are faster then me :-)

I'm aware of the effort to cmake-ify boost. Peter Dimov is leading the work.

Hm Peter Dimov ... I think I saw/met/know him ....

Well that's odd. I've just nuked my ~/.hunter directory and rebuilt.

Should not happen that you had to nuked your ~/.hunter directory. But I think that has something to-do with the PACKAGE_INTERNAL_DEPS_ID ... that`s what I mentions in my review comment

Btw. I also use your example and add/change following:

HunterGate(
        URL "https://github.com/Bjoe/hunter/archive/730d52651180c953e53bd176bdc6312594d5381a.zip"
        SHA1 "eedc1bbe41cccaf19b996d7fb37378c0af403390"
)
set(Boost_USE_STATIC_LIBS ON)

to test my solution and it also works :-)
Unfortunately at the moment you must add Boost_USE_STATIC_LIBS. I'm working on that.
This solution uses now the Boost config module from Boost (verify with Boost_DEBUG) and you can then give Peter Dimov a "statement" about his work :-)

Last thing, this solution file(GLOB_RECURSE ... I think you know that this has a number of drawbacks and is actively discouraged by the CMake documentation (See Note in: https://cmake.org/cmake/help/latest/command/file.html#glob) or?
Sorry that I mention it, but I see project with that and they blame that cmake is not proper working ...

By the way, I often bore my colleagues in the #boost and #beast channels of Cpplang Slack http://slack.cpp.al with how good Hunter is.

Hm I should more reading my Slack channels :-) ... thanks for pointing to hunter 👍

Please do continue the good work. Hunter is the package manager that c++ always needed.

Yep ... @rbsheth and @bkotzz 👍

@madmongo1
Copy link

madmongo1 commented Feb 4, 2020

@Bjoe

Points noted, thank you.

However...

Last thing, this solution file(GLOB_RECURSE ... I think you know that this has a number of drawbacks

This has been fixed in cmake 3.16+ with the addition of the CONFIGURE_DEPENDS option. There are now no drawbacks if you add that option (which I do).

Also, I've made sure Hunter gets a mention in my C++ Alliance bio (see bottom of page):

https://cppalliance.org/people/richard

@rbsheth
Copy link
Member

rbsheth commented Feb 4, 2020

@madmongo1 Thanks for the report and praise. 😄

As @Bjoe correctly pointed out, the PR for Boost does not yet update PACKAGE_INTERNAL_DEPS_ID for all Boost packages, hence your need to nuke the ~/.hunter directory and rebuild. That was one of the two comments I had on the PR that are blocking its merge (the other being iOS support).

@Bjoe
Copy link

Bjoe commented Feb 13, 2020

I created a PR to enable via

hunter_config(Boost
    VERSION ${HUNTER_Boost_VERSION}
    CMAKE_ARGS USE_CONFIG_FROM_BOOST=ON
)

original cmake config modul from boost .... #154 needs tester/reviewer, thanks :-)

@rbsheth
Copy link
Member

rbsheth commented Feb 15, 2020

Solved by #140 I think

@rbsheth rbsheth closed this as completed Feb 15, 2020
@BenWhetton
Copy link
Author

Did you test this before closing it? I don't think it's solved. It doesn't work for me at least.

Running the test CMakelists.txt from the description with the latest Hunter release produces the same results as before: Boost::boost is the only value in Boost log's INTERFACE_LINK_LIBRARIES when there should be a bunch of other libraries. Can somebody else reproduce this to make sure I haven't messed something up?

One of the changes between the solution provided in @daminetreg's original comment and the merged PR seems to have undone the fix. (The artifact from the comment has since been updated so I can't repeat the test)

@daminetreg: Do you know what might have caused this?

@daminetreg
Copy link

Normally the same should just work, that's pretty strange that you experience this regression. But indeed it might then be due to the FindBoost.cmake inside the hunter project.

@daminetreg
Copy link

I'll check to solve it definitely.

@daminetreg
Copy link

daminetreg commented Feb 18, 2020

Just to be sure did you update the link to huntergate in the description comment ?

@daminetreg
Copy link

Indeed this one doesn't work anymore. I'll dig in it tonight.

@BenWhetton
Copy link
Author

Thanks @daminetreg . I'm not familiar enough with the details to know what might have changed here.

@rbsheth could you please re-open the issue?

@rbsheth
Copy link
Member

rbsheth commented Feb 18, 2020

Re-opening. I tried the newest Boost on my laptop and it worked so I'm unsure as well.

@rbsheth rbsheth reopened this Feb 18, 2020
@Bjoe
Copy link

Bjoe commented Feb 19, 2020

I'm confused .... it should not surprise you guys! ... that's what I mean with my comment #133 (comment)

The fix from @daminetreg is absolutly fine ... hunter is now working like before !

The real problem is, I can repeat it again: Hunter is introduced a not well maintained FindBoost in cmake/find

Now you should ask me, why is hunter using FindBoost.cmake even if you say CONFIG in find_package() .... because, as I comment, the BoostConfig.cmake from hunter call find_package(Boost MODULE ...)

I introduced in the hunter boost-log test build a debug output:

This means, you should add the dependecies your self!

See also the boost examples in hunter like example/Boost-log-shared/CMakeList.txt .... all dependencies are defined. Also you should be careful about the order of the dependencies ! See for example in Boost-chrono

If you should use my PR #154 then you have all dependencies well defined. See https://travis-ci.org/Bjoe/hunter/jobs/652334970#L6178

Now you should ask me, why example/Boost-log/CMakeLists.txt have not introduced the dependencies .... because foo.cpp is not using any boost log libs. It is only include a boost log header file and there is nothing todo for the linker :-). This example is bad and should be fixed.

Again, try my PR #154 and it should solve your problem ... if not, let me know ... or you should add the dependency your self.

Somebody should find out, why cmake/find/FindBoost.cmake is introduced? Maybe it can be removed? Or we should update FindBoost.cmake and maintained the dependencies ...

@daminetreg
Copy link

I'm confused .... it should not surprise you guys! ... that's what I mean with my comment #133 (comment)

Dear @Bjoe the problem is that it did work with the version I had made at some point. After diffing the findboost yesterday, I suspect that sometimes, for some odd reason cmake uses the official CMake FindBoost instead of the packaged one.

I will try to reproduce it to fully understand it.

But apart from that the FindBoost.cmake we have in hunter and the one in cmake is almost identical, there are some hunter paths and msvc platform specific fixes though, so I will backport them.

I'll also add the test we have here in this description to the hunter project, to ensure we don't fail again.

Somebody should find out, why cmake/find/FindBoost.cmake is introduced? Maybe it can be removed? Or we should update FindBoost.cmake and maintained the dependencies ...

I think at some point when your PR is done and we have feedback on big existing codebase we could remove fully the FindBoost, but in the meantime I'll try to update it, as it sounds like a 1h work.

1 similar comment
@daminetreg
Copy link

I'm confused .... it should not surprise you guys! ... that's what I mean with my comment #133 (comment)

Dear @Bjoe the problem is that it did work with the version I had made at some point. After diffing the findboost yesterday, I suspect that sometimes, for some odd reason cmake uses the official CMake FindBoost instead of the packaged one.

I will try to reproduce it to fully understand it.

But apart from that the FindBoost.cmake we have in hunter and the one in cmake is almost identical, there are some hunter paths and msvc platform specific fixes though, so I will backport them.

I'll also add the test we have here in this description to the hunter project, to ensure we don't fail again.

Somebody should find out, why cmake/find/FindBoost.cmake is introduced? Maybe it can be removed? Or we should update FindBoost.cmake and maintained the dependencies ...

I think at some point when your PR is done and we have feedback on big existing codebase we could remove fully the FindBoost, but in the meantime I'll try to update it, as it sounds like a 1h work.

@Bjoe
Copy link

Bjoe commented Feb 19, 2020

Dear @Bjoe

Sorry for the inconvenience if I express my self in a "wrong way" ... maybe I'm to hysteric but I'm bit afraid that we do the same error like the cmake <-> android/google guys ... ;-)

the problem is that it did work with the version I had made at some point. After diffing the findboost yesterday, I suspect that sometimes, for some odd reason cmake uses the official CMake FindBoost instead of the packaged one.

Yep, I think so because I use hunter since one year. Since that I added the dependencies manually like the examples in hunter.
It is implemented to add the dependencies manually see for example commit c18cf832c56f687a8a2afd16e00370d9604ffc27 or 0fd3c6f87b55eed2e8e6154c1e516ad665a53ded

But apart from that the FindBoost.cmake we have in hunter and the one in cmake is almost identical, there are some hunter paths and msvc platform specific fixes though, so I will backport them.

Have you find out from which cmake version FindBoost is copied into hunter for four years ago ?

I think at some point when your PR is done and we have feedback on big existing codebase we could remove fully the FindBoost, but in the meantime I'll try to update it, as it sounds like a 1h work.

That sounds like a plan ;-)

@BenWhetton
Copy link
Author

One thing to keep in mind is that the solution should work for older versions of Boost as well. There are users, myself included, who cannot use the latest version for various reasons.

@NeroBurner
Copy link

#154 is merged, can this Issue be closed?

@BenWhetton
Copy link
Author

I believe #154 only helps for the latest versions of boost. The problem still exists for all previous versions.

@Bjoe
Copy link

Bjoe commented Mar 28, 2020

Yep @BenWhetton is right. I'm not sure since when Boost is introduced the cmake package configuration. I verified it with Boost 1.72 but maybe it is also possible to use an earlier version.

@BenWhetton You need Boost 1.71 ?

@shoreadmin
Copy link

shoreadmin commented Aug 18, 2020

I'm trying to use Boost 1.67 and link dependencies don't work there either.

As @Bjoe mentioned, the problem is that the call to find_package(Boost MODULE REQUIRED ${Boost_FIND_COMPONENTS}) in the BoostConfig.cmake provided by Hunter calls FindBoost.cmake provided by Hunter (.../0.23.262/eb51e63/Unpacked/cmake/find/FindBoost.cmake:551) which doesn't support import targets.

I'm only evaluating Hunter, but the fact that it is "subsuming" the standard CMake FindBoost with it's own (ancient?) version is a little concerning... That might've been required prior to the introduction of find_package(CONFIG), but shouldn't be necessary now. Surely a better (long-term?) strategy would be to fix up the offending package-script to make it appropriately configurable and push the fixes upstream to CMake? As it is, it would seem that the current approach of back-porting all fixes/changes applied to every Hunter-supported FindPackage script is not all that practical...

Deleting Hunter's custom FindBoost.cmake and using the one provided by CMake itself solves the issue for me.

@Bjoe
Copy link

Bjoe commented Dec 6, 2020

@shoreadmin Which CMake version are you using?

Maybe hunter should decide which "FindBoost.cmake" they used based on cmake version?

@shoreadmin
Copy link

@Bjoe - My comment was from a while ago, but I'm pretty sure we were (and still are) using CMake 3.16.4. I haven't looked into Hunter any further recently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants