-
Notifications
You must be signed in to change notification settings - Fork 676
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
Guard linking tests against GEOS behind ENABLE_DATA_TOOLS #2901
Conversation
@PureTryOut i think the best place to add the GEOS dependency is in here: https://github.com/valhalla/valhalla/blob/master/src/mjolnir/CMakeLists.txt#L84 this is the only part of the library that makes use of GEOS except the tests that test this part of the library, but i think when DATA_TOOLS is on then that part of the library is compiled and that target dependency will trickle down to the tests. i could be wrong though i didnt try it! |
@kevinkreiser you seem to be right, that's a much better solution! Updated |
I can't see the results of those CircleCI builds. What is breaking currently? |
Here is the full trace
|
@kevinkreiser I couldn't get the CI to succeed when doing what you suggested originally, so I reverted the change to just guarding the linking. |
@PureTryOut no worries this is better than before at least. we can get to the bottom of the mystery at a later date! |
@PureTryOut It seems like you down-graded pybind from 2.6.2 to 2.5.0 in this PR. cc @nilsnolde |
ill revert that bit, apologies for not noticing it! |
done |
submodules can be a pita until changes are propagated to all branches.. thanks for noticing and fixing! |
Oh woops, sorry for that! |
Resolves #2899