Skip to content

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.

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
malter wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add boost dependency to CMakeLists #21

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f53a0a7
Add boost dependency to CMakeLists
malter Nov 15, 2021
4d04f04
Add boost dependency to manifest, too
malter Nov 16, 2021
eb0e1ea
Add boost dependency to rock_library
malter Nov 23, 2021
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
1 change: 1 addition & 0 deletions manifest.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@
<license>LGPL-2.0-or-later</license>
<depend package="base/types" />
<depend package="base/logging" />
<depend package="boost" />
<tags>needs_opt</tags>
</package>
2 changes: 2 additions & 0 deletions src/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
rock_find_cmake(Boost)

Copy link
Copy Markdown
Member

@doudou doudou Nov 19, 2021

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)

Copy link
Copy Markdown
Author

@malter malter Nov 22, 2021 •
edited
Loading

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.

Copy link
Copy Markdown
Author

@malter malter Nov 23, 2021

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?

Copy link
Copy Markdown
Member

@doudou doudou Dec 3, 2021

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.

Copy link
Copy Markdown
Member

@planthaber planthaber Mar 9, 2022

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.

Copy link
Copy Markdown
Member

@doudou doudou Apr 13, 2022

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.

Copy link
Copy Markdown
Author

@malter malter May 23, 2022

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.

Copy link
Copy Markdown
Member

@doudou doudou Jun 8, 2022

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.

rock_library(numeric
HEADERS
Combinatorics.hpp
Expand Down