Sma/rework cmake module

[tSed]

Hi,

Here is a short series reworking CMake module files.

It now uses CMake primitives instead of hard-coded (absolute) paths, which was badly breaking cross-compilation.

Regards,
Samuel

[ahornung]

Looks like this breaks compiling the combined package, at least the CI testing is not happy. Does the CMAKE_INSTALL_PREFIX not set the absolute paths correctly for crosscompilation?

[tSed]

Arf! :(

The "combined package" is building the whole repository, not just one of the 3 sub-projects, right?

Note that I was not able to build dynamicEDT3D without having octomap installed in the sysroot beforehand (even before these patches).

I suspect what is breaking the CI is the configure_file cleanup.

I can resubmit a PR without renaming the *.cmake.in, and keeping the configure_file() calls.

[ahornung]

Yes, the combined package should compile like that:
https://github.com/OctoMap/octomap#overview

Also, in the end the CMakeConfigs should behave in the same way as
before for other libs depending on octomap.

[tSed]

Ok, here is the 2nd round ;)

I keep the existing template for the build, this should not break the combined package build anymore (tested in native and cross-compilation here).

[ahornung]

Sorry to say, but this now breaks the behavior of the installed CMake config. Essentially you are reverting Fix #14 for the install target. Each generated octomap-config.cmake needs to point to its own compiled version with absolute paths, otherwise the compiler and linker will pick up the version of the library it finds first (and not the one specified). This will create all kinds of problems when multiple versions are installed in parallel (common e.g. with ROS, or when overlaying a newer version).

In the commit you replace the referenced library and include paths with a dynamic lookup via find_library. If that is desired in code that relies in OctoMap then it can be done in the client library (or a FindOctomap.cmake module), as opposed to the exported config.

To test, you can compile and install OctoMap into /tmp/octomap-test. In a new directory, create the following CMakeLists.txt:

cmake_minimum_required(VERSION 2.8)
project(testoctomap)

FIND_PACKAGE(octomap REQUIRED)
message(STATUS "Found OctoMap")
message(STATUS "Libs: " ${OCTOMAP_LIBRARIES})
message(STATUS "Include: " ${OCTOMAP_INCLUDE_DIRS})

Then run with cmake .. This will not find OctoMap (or pick up a globally installed version). Explicitly pointing to the CMake config with your branch installed did not work: octomap_DIR=/tmp/octomap-test/share/octomap/ cmake .

Whereas installing the current devel or master version of OctoMap into tmp will return the correct paths:

-- Found OctoMap
-- Libs: /tmp/octomap-test/lib/liboctomap.so/tmp/octomap-test/lib/liboctomath.so
-- Include: /tmp/octomap-test/include

And I can also explicitly switch to another version octomap_DIR=/opt/ros/hydro/share/octomap/ cmake .

-- Found OctoMap
-- Libs: /opt/ros/hydro/lib/liboctomap.so/opt/ros/hydro/lib/liboctomath.so
-- Include: /opt/ros/hydro/include

[tSed]

Hi,

Then run with cmake .. This will not find OctoMap (or pick up a globally installed version). > Explicitly pointing to the CMake config with your branch installed did not work:
octomap_DIR=/tmp/octomap-test/share/octomap/ cmake .

I'm not sure this is the right way of using CMake. I mean, AFAI understand the CMake doc, when one wants to use something from a location different from the defaults, one should add -DCMAKE_FIND_ROOT_PATH=/my/custom/location;/somewhere/else on the cmake command line.

In the above use-case, if you do:
cmake . -DCMAKE_FIND_ROOT_PATH=/tmp/octomap-test
it works fine.

[tSed]

hi,

are you still interested in this change?
do you think this patch has a chance to end up in octomap?

regards,

[ahornung]

Hi tSed,

I can accept the patch only if it does not break the existing behavior, which requires the fixed absolute paths to remain in the CMake configs. Pointing to the config with libraryname_DIR is the preferred mechanism in CMake, whereas the FIND_ROOT_PATH is used for dynamically finding libs from a client library, e.g. with a FindLibrary.cmake (which is typically used when there is no proper config available / exported). For the current PR to work in the use case I outlined above, users would have to export both octomap_DIR and the FIND_ROOT_PATH (and thus breaks in the default case).

Instead, what about adding a flag such as NO_ABSOLUTE_PATHS to be specifically used for crosscompiling? If set, the absolute paths are omitted from the CMake configs (defaults to the current behaviour).

[tSed]

Hi,

For the use case you want to keep (and for which, I still not convinced), how do you do to get the /tmp/octomap-test hard-coded in the CMake modules?
Do you set it through CMAKE_INSTALL_PREFIX?
With make install DESTDIR=/tmp/octomap-test, it does not work at all.

Anyway, I think I'll do a new PR implementing the NO_ABSOLUTE_PATH soon.

Regards,

[ahornung]

Yes, CMAKE_INSTALL_PREFIX defines the installation prefix and it needs to be defined when configuring the project.

Using CMakePackageConfigHelpers may be another option that could solve the problem in a more portable way.