catkin_lint and cleanup

[v4hn]

See #298.

This can not simply be cherry-picked to jade-devel,
so I'll open a new request for that once this one is merged.

[davetcoleman]

did you intend to remove this?

[v4hn]

yes, you might want to look through the different commits for this request.
resource_retriever is not referenced in moveit_core.

[davetcoleman]

Seems like big dependency changes like this maybe should be reserved for less stable releases (jade/kinetic) so as to prevent the risk of breaking other's builds who are incorrectly depending on included dependencies from this package. But I'm open to debating this.

Also you are switching to Eigen3 for indigo which we've already done for Jade. Maybe that shouldn't be in this commit? Especially since Travis is failing because of it

+1 to catkin linting the moveit codebase!

[v4hn]

I do not get why travis is failing because of Eigen3.
ubuntu 14.04 already provides /usr/share/cmake-2.8/Modules/FindEigen3.cmake in libeigen3-dev and this is referenced in the package.xml as eigen.
This is why I moved to find_package(Eigen3).
@130s do you have an explanation for that?

[v4hn]

On Tue, Jul 05, 2016 at 01:19:40PM -0700, Dave Coleman wrote:

Seems like big dependency changes like this maybe should be reserved for less stable releases (jade/kinetic) so as to prevent the risk of breaking other's builds who are incorrectly depending on included dependencies from this package. But I'm open to debating this.

Actually, although it looks like big changes, it is actually not.
It's mainly sorting entries, adding stuff that was missing to find_package(catkin COMPONENTS and catkin_package(CATKIN_DEPENDS
and correcting previously broken entries. The only relevant changes that might have to be discussed are
the removed dependencies on cmake_modules and actionlib_msgs.
The rest is actually additions of missing dependencies.

The line removing resource_retriever that you pointed out in find_package(catkin,
was not even present in the package.xml, so it was a plain error that travis did not pick up.

[v4hn]

One thing I'm still unsure about is whether C/C++-only ros-packages have to specify run_depends
on the *_msgs packages they use. For Python they are probably required, but for C++ all message definitions are compiled into the binaries, so as far as I can see these can/should be removed in this case. Any ideas?

[davetcoleman]

whether C/C++-only ros-packages have to specify run_depends on the *_msgs packages they use

I'm afraid I do not know

[130s]

https://travis-ci.org/ros-planning/moveit_core/jobs/142586748#L2018 for indigo (and for jade it looks the same).

Errors     << moveit_core:cmake /home/travis/ros/ws_moveit/logs/moveit_core/build.cmake.000.log
CMake Error at /home/travis/ros/ws_moveit/src/moveit_core/CMakeLists.txt:13 (find_package):
  By not providing "FindEigen3.cmake" in CMAKE_MODULE_PATH this project has
  asked CMake to find a package configuration file provided by "Eigen3", but
  CMake did not find one.
  Could not find a package configuration file provided by "Eigen3" with any
  of the following names:
    Eigen3Config.cmake
    eigen3-config.cmake

I don't have a clear answer, but we've already got stuck and chose not to dig deeper #293 (comment).
Do you use catkin_make locally when you test this PR? These Travis jobs use catkin_tools. I wonder if there's a case where catkin_tools does not set CMAKE_MODULE_PATH but I'm not sure.

[v4hn]

en Wed, Jul 06, 2016 at 04:50:34AM -0700, Isaac I.Y. Saito wrote:

Do you use catkin_make locally when you test this PR?
These Travis jobs use catkin_tools.

That's probably the difference.

I wonder if there's a case where catkin_tools does not set CMAKE_MODULE_PATH but I'm not sure.

Afaik cmake automatically includes its own Modules folder, so maybe catkin_tools specifies some
magic flag to disable that behavior (whyever anyone understanding build systems would do that...).

I removed the Eigen3 changes from this request as well as the one in geometric_shapes to avoid this bug/discussion.
I'm perfectly fine with having cmake_modules as a dependency in indigo, I was just trying to get rid of old overhead.

Anything else?

[davetcoleman]

+1 indigo onward

[davetcoleman]

don't forget to cherry-pick this complicated commit

[v4hn]

On Thu, Jul 07, 2016 at 11:49:37AM -0700, Dave Coleman wrote:

don't forget to cherry-pick this complicated commit

See my first message in this request :)