Skip to content
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

[cmake] fix for Boost 1.70 #65

Merged
merged 11 commits into from
Feb 5, 2020
Merged

[cmake] fix for Boost 1.70 #65

merged 11 commits into from
Feb 5, 2020

Conversation

simogasp
Copy link
Member

@simogasp simogasp commented Dec 9, 2019

Boost 1.70 uses only the modern CMake way of find_package() and it imports both the targets and set the Boost_LIBRARIES variable with the found components targets (as opposed to the path to the libs).
This leads to a linking error for the examples as the variables are not set again with the proper targets (maybe a bug on boost side).
Here the variables are first voided before the find_package() and this seems to restore the correct behavior.

In the future, it would be better to completely switch to the modern CMake way (which requires to raise the CMake version at least to 3.5)

@griwodz
Copy link
Member

griwodz commented Dec 18, 2019

It would be nice to upgrade CMake version to 3.7 because that gives the GREATER_EQUAL macro, which makes it easier to read conditional statements.

@@ -15,6 +15,10 @@ endif()
find_package(Boost 1.53.0 REQUIRED COMPONENTS program_options system filesystem)
Copy link
Member

Choose a reason for hiding this comment

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

@simogasp Do we need find_package(Boost ...) twice for this fix to work?

Copy link
Member Author

@simogasp simogasp Dec 18, 2019

Choose a reason for hiding this comment

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

yes because the applications can be an independent cmake project (you can use popsift installed somewhere else to build, so you can test the use as 3rd pary) and requires a different set of components from boost.

Copy link
Member Author

Choose a reason for hiding this comment

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

oh no, sorry i didn't see that, i thought you were referring to the one in the main cmakefile. Obviously this is unnecessary, i'll fix it

@simogasp
Copy link
Member Author

It would be nice to upgrade CMake version to 3.7 because that gives the GREATER_EQUAL macro, which makes it easier to read conditional statements.

If it were to me, at this point I'd move all projects to 3.15 or 3.16, like that for at least a year we won't have to deal with many different versions of cmake. It's a bit rude but it would simplify a lot everything. Cmake comes with pre-built binaries for every platform, so it is not a big deal to download the latest version.
@fabiencastan

@fabiencastan
Copy link
Member

Not sure if we really need 3.15 as minimal.
But no problem for me to update the minimal version of cmake needed.

@griwodz
Copy link
Member

griwodz commented Dec 18, 2019

I'm also happy with a push to 3.15.

@simogasp simogasp added this to the v1.0.0 milestone Jan 17, 2020
@fabiencastan fabiencastan merged commit 1416604 into develop Feb 5, 2020
@fabiencastan fabiencastan deleted the cmake/fixBoost1.70 branch February 5, 2020 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants