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

modernize CMake for submodule #268

Merged
merged 3 commits into from
May 18, 2018

Conversation

geoffviola
Copy link

This should be friendlier for people who expect the project to work as a git submodule. With modern CMake, all the include directories come via target_link_libraries

@caselitz
Copy link

caselitz commented Apr 4, 2018

this issue might be of interest regarding this pr

@geoffviola
Copy link
Author

Yes, that looks related. That issue seems related to installation. This changeset is focused on using add_subdirectory(g2o) instead of find_package(g2o). I expect that both methods work in the modern way.

It looks like this changeset is currently incomplete because it is failing on the CI. I'll take a look later this week.

@sjulier
Copy link
Collaborator

sjulier commented Apr 4, 2018

Haven't had time to chase up on this for a while (teaching got in the way!) However, last time I checked it (January) everything worked okay on both macOS and Linux (although Ubuntu 16.04 ships with a surprisingly old version of eigen) but it needed more testing. Also I found very little documentaton on the "right way" to do it in cmake, and so I was hoping to check it's actually done right.

Unfortunately, I've got project review now and won't really be able to look at it for a few weeks. However if somebody else wanted to take ownership...

@geoffviola
Copy link
Author

@sjulier The right way to do CMake is evolving. There are some good talks. I've been trying to push ideas from https://docs.google.com/presentation/d/1LvpDU25sr7AF8RLArSrY3KpZLV2qfnml3XuN5MpeCkw/edit#slide=id.g2f12105e36_0_89.

I'm afraid I don't rely on this library enough to want to be a dedicated owner. I'm just trying to reduce pain points in and out of my organization.

I'm still experimenting with new versions of Eigen. There's some insanely tricky error at runtime with -march=native, but I haven't debugged it yet.

@RainerKuemmerle RainerKuemmerle merged commit 1238473 into RainerKuemmerle:master May 18, 2018
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

Successfully merging this pull request may close these issues.

5 participants