The indigo_devel branch's CMakeLists.txt include a DEPENDS eigen. Is that right this way?

[HannesSommer]

See https://github.com/ethz-asl/ethzasl_icp_mapping/blob/indigo_devel/ethzasl_gridmap_2d/CMakeLists.txt#L14. I think this should at least be a capital Eigen. See http://docs.ros.org/jade/api/catkin/html/dev_guide/generated_cmake_api.html#catkin_package.

Here https://github.com/ethz-asl/ethzasl_icp_mapping/blob/indigo_devel/ethzasl_extrinsic_calibration/CMakeLists.txt#L6 it even comes before the find_package(Eigen REQUIRED), which seems to be explicitly wrong according to the link before.

This isn't urgent. Just to not forget.

[pomerlef]

Humm I'm confuse with all that. So to recap, we use:

find_package(Eigen REQUIRED)

and

catkin_package(
    ...<stuff>...
    DEPENDS eigen3
)

and

catkin_package(
    ...<stuff>...
    DEPENDS eigen
)

In that example, they use lower case and in that example they use Capital letter. In none of them, they specify the number (i.e., eigen3).

From this migration guide:

Additionally, the FindEigen.cmake CMake module used to reside in the catkin package, it now lives in the cmake_modules package

So I assume that find_package(Eigen3 REQUIRED) is the way to go for the Indigo branch (and beyond) and that

catkin_package(
    ...<stuff>...
    DEPENDS eigen3
)

should be use for the Hydro branch. In all cases, {E,e}igen should be changed to {E,e}igen3.

Any comments/concerns before I do the changes?

[HannesSommer]

I think that is wrong. We need both: find_package and DEPENDS.

The former to find it (the cmake way) and the second to tell catkin that this module is a cmake dependency. This is at least true for indigo, as the following extract from the catkin_package macro shows (from /opt/ros/indigo/share/catkin/cmake/catkin_package.cmake) :

  foreach(depend ${PROJECT_DEPENDS})
    string(REPLACE " " ";" depend_list ${depend})
    # check if the second argument is the COMPONENTS keyword
    list(LENGTH depend_list count)
    set(second_item "")
    if(${count} GREATER 1)
      list(GET depend_list 1 second_item)
    endif()
    if("${second_item}" STREQUAL "COMPONENTS")
      list(GET depend_list 0 depend_name)
      if(NOT ${${depend_name}_FOUND})
        message(FATAL_ERROR "catkin_package() DEPENDS on '${depend}' which must be find_package()-ed before")
      endif()
      message(WARNING "catkin_package() DEPENDS on '${depend}' which is deprecated. find_package() it before and only DEPENDS on '${depend_name}' instead")
      list(APPEND PROJECT_DEPENDENCIES_INCLUDE_DIRS ${${depend_name}_INCLUDE_DIRS})
      list(APPEND PROJECT_DEPENDENCIES_LIBRARIES ${${depend_name}_LIBRARIES})
    else()
      # split multiple names (without COMPONENTS) into separate dependencies
      foreach(depend_name ${depend_list})
        if(${depend_name}_FOUND_CATKIN_PROJECT)
          #message(WARNING "catkin_package() DEPENDS on catkin package '${depend_name}' which is deprecated. Use CATKIN_DEPENDS for catkin packages instead.")
          list(APPEND _PROJECT_CATKIN_DEPENDS ${depend_name})
        else()
          if(NOT ${${depend_name}_FOUND})
            message(FATAL_ERROR "catkin_package() DEPENDS on '${depend_name}' which must be find_package()-ed before. If it is a catkin package it can be declared as CATKIN_DEPENDS instead without find_package()-ing it.")
          endif()
          list(APPEND PROJECT_DEPENDENCIES_INCLUDE_DIRS ${${depend_name}_INCLUDE_DIRS})
          list(APPEND PROJECT_DEPENDENCIES_LIBRARIES ${${depend_name}_LIBRARIES})
        endif()
      endforeach()
    endif()
  endforeach()

The lines containing ${depend_name}_FOUND tell you about the precise spelling of the DEPENDS argument: It should be exactly as the variable prefix as defined in the cmake find script. So in case of trusty lib-eigen3-dev it is EIGEN3, as the following extract from /usr/share/cmake-2.8/Modules/FindEigen3.cmake shows:

if (EIGEN3_INCLUDE_DIR)

  # in cache already
  _eigen3_check_version()
  set(EIGEN3_FOUND ${EIGEN3_VERSION_OK})

else (EIGEN3_INCLUDE_DIR)
...

Maybe you want to do the same research for hydro? My state of mind is: If it is a catkin dependency in hydro then you don't need find_package and use CATKIN_DEPENDS instead of DEPENDS.

[HannesSommer]

Btw. the case sensitivity of cmake for variable names is pretty opaque to me. But I'm sure that I witnessed cases of being sensitive. So it is probably a good idea to respect the case as much as possible.

[pomerlef]

Chapter 1: catkin_package

ok, I understand a bit more what catkin_package is doing...

You can literally write anything within the DEPENDS field and not check will be done as long as no other packages link to it. So, if there is a mistake there, it won't show up until somebody link to it (and I'm to lazy to do that). So you're right saying that it should be:

catkin_package(
    ...<stuff>...
    DEPENDS EIGEN
)

Now, that half of the problem since it's not clear how to standardize EIGEN or EIGEN3 there.

Chapter 2: Eigen vs Eigen3

I did multiple tests and I cannot use find_package(Eigen3 REQUIRED) on Ubuntu 12.04 as the file /usr/share/cmake-2.8/Modules/FindEigen3.cmake is not installed. The discussion here describes well the problem. They suggest to use the following to avoid compatibility issues:

find_package(Eigen3)
if(NOT EIGEN3_FOUND)
  # Fallback to cmake_modules
  find_package(Eigen REQUIRED)
  set(EIGEN3_INCLUDE_DIRS ${EIGEN_INCLUDE_DIRS})
endif()

I have the feeling that find_package(Eigen) work on 14.04 ( @HannesSommer ??), but I'm not sure why.

Chapter 3: Fuerte vs Hydro vs Indigo vs Jade

Do we need to keep all of those branches if it's only the CMakeLists.txt that will change. Can we just have switches in it to take care of that. I'm not sure how to propagate changes in sources without touching the cmake stuff easily...

[pomerlef]

Hummm not sure if I need to add that, but this work...

find_package(EiGeN REQUIRED)

so find_package() is more relax on the casing.

[HannesSommer]

Chapter 1:

The funny thing is here, that actually there is a bug in the catkin_package macro at exactly that check, that prevents cmake to complain about a unsatisfied depend (which it is with eigen3 after find_package(Eigen). The following ERROR will not be triggered :

if(NOT ${${depend_name}_FOUND})
 message(FATAL_ERROR "catkin_package() DEPENDS on '${depend_name}' which must be find_package()-ed before. If it is a catkin package it can be declared as CATKIN_DEPENDS instead without find_package()-ing it.")
          endif()

The ting is that NOT ${${depend_name}_FOUND} is always false. At least with cmake version 2.8.12.2 (trusty). NOT ${depend_name}_FOUND would work as apparently intended. I hope to find the time to report this.

Chapter 2:

Yes, find_package(Eigen REQUIRED) does work with 14.04 but only via the FindEigen.cmake provided by cmake_modules. So if we rely on that we should make sure that there is a <build_depend>cmake_modules</build_depend> in the corresponding package.xml. Together with the cmake code

find_package(Eigen3)
if(NOT EIGEN3_FOUND)
  # Fallback to cmake_modules
  find_package(Eigen REQUIRED)
  set(EIGEN3_INCLUDE_DIRS ${EIGEN_INCLUDE_DIRS})
endif()

we are in a good shape I guess. Maybe we should add a comment from when on we can remove the extra complication (for the future maintenance).

Chapter 3:

If it is really just the CMakeLists.txt code I'd strongly vote for having one branch with some if statements. It gets more difficult if the package.xml needs to be different. But we should solve this! If we decide to stick with multiple branches, we should discuss how to propagate master changes to the branches and what master should actually target at. A good work-flow can save much effort there.

[HannesSommer]

About find_package(EiGeN REQUIRED) :). Yes, I guess when it comes to finding files (and EiGeN will be used as part of filenames) cmake is case insensitive because it is also for mac and windows, where case insensitive file naming is quite common. But for cmake variable names such as EIGEN_FOUND it is different.

[pomerlef]

Final solution:

find_package(Eigen3 QUIET)

# Eigen3 will not be found on Ubuntu 12.04, so we moke it
if(NOT EIGEN3_FOUND)
    # Fallback to cmake_modules
    find_package(Eigen REQUIRED)
    set(EIGEN3_INCLUDE_DIRS ${EIGEN_INCLUDE_DIRS})
    set(EIGEN_NAME "EIGEN")
else()
    set(EIGEN_NAME "EIGEN3")
endif()

catkin_package(
    CATKIN_DEPENDS roscpp rospy tf 
    DEPENDS ${EIGEN_NAME}
)

I will open a different issue for merging the different catkin branches.

[HannesSommer]

Looks great. Quick question: why set(EIGEN3_INCLUDE_DIRS ${EIGEN_INCLUDE_DIRS}) together with include_directories(${EIGEN3_INCLUDE_DIRS})?

I would expect the list(APPEND PROJECT_DEPENDENCIES_INCLUDE_DIRS ${${depend_name}_INCLUDE_DIRS}) up there to this job. No? Did you try without?

[pomerlef]

If I comment the line:

include_directories(${EIGEN3_INCLUDE_DIRS})

The result is:

fatal error: Eigen/Eigen: No such file or directory

[HannesSommer]

Hm, then something is going wrong there, I suppose. Thanks for trying. No further questions ;).

[skohlbr]

Building using catkin_make_isolated I (and everybody else using the same setup) get the following with latest indigo_devel:

==> Processing catkin package: 'ethzasl_extrinsic_calibration'
==> Building with env: '/home/kohlbrecher/argo_repo2/devel_isolated/env.sh'
Makefile exists, skipping explicit cmake invocation...
==> make cmake_check_build_system in '/home/kohlbrecher/argo_repo2/build_isolated/ethzasl_extrinsic_calibration'
==> make -j8 -l8 in '/home/kohlbrecher/argo_repo2/build_isolated/ethzasl_extrinsic_calibration'
[100%] [100%] Built target tf_logger
Building CXX object CMakeFiles/optimize.dir/src/optimize.cpp.o
/home/kohlbrecher/argo_repo2/src/external/ethzasl_icp_mapping/ethzasl_extrinsic_calibration/src/optimize.cpp:6:23: fatal error: Eigen/Eigen: No such file or directory
 #include 
                       ^
compilation terminated.
make[2]: *** [CMakeFiles/optimize.dir/src/optimize.cpp.o] Error 1
make[1]: *** [CMakeFiles/optimize.dir/all] Error 2
make: *** [all] Error 2
<== Failed to process package 'ethzasl_extrinsic_calibration': 
  Command '['/home/kohlbrecher/argo_repo2/devel_isolated/env.sh', 'make', '-j8', '-l8']' returned non-zero exit status 2
Reproduce this error by running:
==> cd /home/kohlbrecher/argo_repo2/build_isolated/ethzasl_extrinsic_calibration && /home/kohlbrecher/argo_repo2/devel_isolated/env.sh make -j8 -l8

For me, CATKIN_IGNORE fixes things as I don't need the package in question, but it still would be nicer to not get the error. Timing-wise I think we started seeing this after the changes described above.

[skohlbr]

This is on 14.04/64Bit/ROS Indigo, forgot stating system specs before.

[HannesSommer]

Oh, my thanks for the hint. This is such a mess. I've just had a closer look. In fact FindEigen from cmake_modules produces Eigen_INCLUDE_DIRS with lower case (currently also all upper case, but this is not part of the specification. BUT FindEigen3 from the Eigen authors provides EIGEN3_INCLUDE_DIR (all upper case but no trailing S). I'll open a PR fixing that for indigo. @pomerlef , could you check the files /usr/share/cmake-2.8/Modules/FindEigen3.cmake and /usr/share/cmake-2.8/Modules/FindEigen3.cmake in for 12.04?

[HannesSommer]

PR #42 does apply the essence of my findings. Jenkins complains but I guess the indigo branch was failing before. Some important changes in master are not included. The problem behind this is the pending decision how to continue with the multiple catkin based branches.

[pomerlef]

On Ubuntu 12.04, from the package manager of libeigen3-dev, here what is installed in /usr/share/:

/usr/share/doc/libeigen3-dev/changelog.Debian.gz
/usr/share/doc/libeigen3-dev/copyright
/usr/share/pkgconfig/eigen3.pc

The file FindEigen.cmake seems to come from within ROS Hydro, but it's not really clear which one is used...

I'll check PR #42 on my side and fix the problems reported by Jenkins.

[skohlbr]

Confirmed working also from my side. Thanks for the fast reponse!