laurentkneip / opengv

OpenGV is a collection of computer vision methods for solving geometric vision problems. It is hosted and maintained by the Mobile Perception Lab of ShanghaiTech.
Other
1.02k stars 353 forks source link

Some cmake improvements #45

Closed simogasp closed 5 years ago

simogasp commented 7 years ago

Hi Laurent,

this PR add some improvements (I hope..) to the cmake building system, but it does not break any compatibility with the previous version (ie building, installation and use as 3rd party is the same as before, hopefully more robust). In particular:

I hope it helps.

Cheers

Simone

NikolausDemmel commented 5 years ago

Do you happen to know what would be required to make the opengvConfig also work in the build directory? I.e. such that in a project where I want to use a specific local build of opengv I could find_package after setting opengv_DIR to the build directory?

I tried with your changes, but looks like while opengvConfig.cmake and opengvConfigVersion.cmake are now generated in the build directory, they are not useable without installing.

CMake Error at /Users/demmeln/work/slam/opengv/build/opengvConfig.cmake:27 (include):
  include could not find load file:

    /Users/demmeln/work/slam/opengv/build/opengvTargets.cmake
Call Stack (most recent call first):
  CMakeLists.txt:6 (find_package)

-- Configuring incomplete, errors occurred!

Other libraries like ceres and Pangolin support this.

simogasp commented 5 years ago

that's true, opengvTargets.cmake is not generated for the build directory. You have two options: 1) try to add this before install(TARGETS opengv... (line 356)

export(TARGETS opengv FILE "${CMAKE_CURRENT_BINARY_DIR}/${targets_export_name}.cmake")

that should generate the missing file for the build directory. (sorry no time to test it...)

2) you can install locally: run cmake with the option -DCMAKE_INSTALL_PREFIX:PATH=/Users/demmeln/work/slam/opengv/build/install (or whichever other path you want) and then make install. That will install the lib in a non-system directory and you'ill be able to use it with opengv_DIR

Hope it helps

S.

NikolausDemmel commented 5 years ago

Thanks for the quick reply.

Option 1 works for me, very nice! It would probably be good to include this in this PR, but no rush.

Option 2 would also work, but has the downside that when you use this for development and do some changes in OpenGV headers, you don't notice in git, since they are done in the install folder.

simogasp commented 5 years ago

Option 2 would also work, but has the downside that when you use this for development and do some changes in OpenGV headers, you don't notice in git, since they are done in the install folder.

Well, actually that's a very bad idea! :-) You should never change the headers in the installed part, you should make your changes in the repository as you usually do and just use make install instead of the usual make.

I'll add it to the PR soon, hoping it will be merged one day...

NikolausDemmel commented 5 years ago

Well, actually that's a very bad idea! :-) You should never change the headers in the installed part, you should make your changes in the repository as you usually do and just use make install instead of the usual make.

Yes exactly, but this is what happens if you use option 2) and are not super careful and your IDE opens the installed headers, since it doesn't know about the source directory.

NikolausDemmel commented 5 years ago

I'll add it to the PR soon, hoping it will be merged one day...

+1 from me, works for my use case nicely (with the additional line).