CMake: exported ceres target does not have include propertes set #274

Open
crmoore opened this Issue Apr 21, 2017 · 2 comments

Comments

Projects
None yet
3 participants

crmoore commented Apr 21, 2017

CMakeLists.txt should use target_include_directories( [target] PUBLIC [directories] ) instead the directory-level include_directories. This way the proper include directories will be exported to projects depending on ceres.

Expected behavior in a downstream project: target_link_libraries(mytarget ceres) should be enough to use ceres. Instead, downstream projects need to also add ${CERES_INCLUDE_DIRS} manually.

sergiud commented Apr 22, 2017

I second this. Ideally, Ceres should be usable from the build directory as well which can be achieved using $<BUILD_INTERFACE:path> and $<INSTALL_INTERFACE:relpath> generator expressions.

Contributor

alexsmac commented Apr 30, 2017

Whilst we still support CMake 2.8 which does not support target_include_directories() I'm not keen on duplicating all the logic for handling our includes when the current solution works for all CMake versions and is still widely used. Currently this includes Eigen (requires SYSTEM to suppress internal Eigen warnings on some compilers/versions), MINIGLOG and Ceres config.h file (which requires BEFORE to ensure expected behaviour on systems which install to /usr/local when using an exported Ceres build directory when an installed version of Ceres already exists) amongst others.

Given that our current system works across CMake versions, and given the number of users who still run on OSs with CMake 2.8.x, and as the buildsystem should never be the impediment to building Ceres I imagine we wouldn't upgrade our required version until after 18.04 comes out and we solicited feedback on the change from users, as in general it would not provide them, as a client of Ceres, with any significant changes/improvements but might break their builds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment