find_package(catkin)

[gerkey]

If you have a directory with two files, Foo.msg and CMakeLists.txt, with the following content:

Foo.msg:

int16 foo

CMakeLists.txt:

cmake_minimum_required(VERSION 2.8.3)
#find_package(catkin REQUIRED)
find_package(genmsg REQUIRED)
add_message_files(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} FILES Foo.msg)

then running cmake yields this error about an undefined macro that's provided by catkin:

-- Using these message generators: 
CMake Error at /opt/ros/jade/share/genmsg/cmake/genmsg-extras.cmake:110 (assert_file_exists):
  Unknown CMake command "assert_file_exists".
Call Stack (most recent call first):
  CMakeLists.txt:4 (add_message_files)

The error can be avoided by uncommenting the find_package(catkin REQUIRED) line in the user's CMakeLists.txt, but that's not a good answer.

This PR pushes the find_package(catkin REQUIRED) call into genmsg, where the macro is being called. I'm not sure whether genmsg's package.xml dependencies should also be modified.

[dirk-thomas]

I don't think using this simple function justifies a run dependency on catkin. I have created #62 to get rid of the implicit dependency.

[gerkey]

Agreed, better to remove the dependency.

[dirk-thomas]

In order to make catkin available at runtime of this package the manifest needs a run dependency on catkin. Otherwise it looks good.

[dirk-thomas]

Please either remove the comment (since the mentioned function is not the only thing used from catkin) or extend to all parts of catkin which are used. Since the second option is likely getting outdated I would suggest the first one.

[gerkey]

Added a run dependency in c8d2295 and updated the comment in 169bf66.

[dirk-thomas]

I don't think there is a reason for specifying a minimum version of catkin. All the functions and variables used at runtime exist in catkin "forever".

[gerkey]

So the buildtool depend should have a minimum version, but the run depend should not?

[dirk-thomas]

Yes, the minimum version for the build dependency was introduced in 6bbac53 to ensure a new enough catkin version is being used when compiling the package. That is not necessary for the runtime of the package.

[gerkey]

Min version updated and commits squashed.

[dirk-thomas]

catkin variables are already being used in the lines above. Therefore I think this needs to be moved to line ~10.

[gerkey]

Moved earlier in e54d7e3

[dirk-thomas]

Thanks for the PR and iterating on it.