-
Notifications
You must be signed in to change notification settings - Fork 10
Add boost dependency to CMakeLists #21
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
base: master
Are you sure you want to change the base?
Conversation
Please also add this dependency to the manifest.xml. |
done |
src/CMakeLists.txt
Outdated
rock_find_cmake(Boost) | ||
|
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, this should be a dependency of the rock_library
directly, making sure it ends up in the pkg-config file (since Combinatorics.hpp is including a boost header)
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 didn't found a way to add it directly to the rock_library
so far. For me, only rock_find_cmake(Boost)
is working correctly. If I add DEPS_CMAKE boost
it is found in configure phase but doesn't set the boost include directory correctly, either it is added to the numeric.pc
file.
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.
Ok, with a capital "b" it is working in the rock_library as cmake dependency. ;-)
I changed that, but it still doesn't end up in the pkg-config file. Since pkg-config cannot deal with non-pkg-config packages as dependencies it might make sense. How do you deal with such configurations?
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.
Not sure what is going on here ... I'll check it out.
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.
DEPS_CMAKE are not added by default to pkg-config and this is actually good, as CMake for Boost now exports targets, which cannot be used with pkg-config at all.
I think we can merge.
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.
DEPS_CMAKE are not added by default to pkg-config and this is actually good, as CMake for Boost now exports targets, which cannot be used with pkg-config at all.
Strongly disagree. While pkg-config does not have the concept of targets, targets are in the end just a glorified module system, which resolves to cflags and libs just like the rest.
I believe the main issue was that the .pc.in template was grossly outdated. I've pushed an updated version on top of these changes in the add_boost_dependency
branch. @malter could you give it a try ? I don't have a boost installation in a custom location to test.
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.
Works for me. Boost is still not listed as dependency but the include path is added to the Cflags. So it should also be fine for libs using base_numeric via pkg_config.
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.
Great. The pkg-config file cannot list Boost as a dependency as Boost does not provide a pkg-config file. Having the include / libs paths in was the goal.
Can you cherry-pick my commit in here ? Then I can let CI pass and merge.
No description provided.