REP 136 for releasing third party packages

[wjwwood]

Looking for comments.

Pending links to examples and tutorials for bloom.

[wjwwood]

@dirk-thomas @tfoote @mirzashah @ablasdel

Can you guys review this while I work up the documentation and examples?

[wjwwood]

@ablasdel fixed.

[wjwwood]

I was going to leave that to the tutorials, but I can mention the process I guess. It doesn't make sense to describe where it goes unless you describe why it goes there and what happens with it. (I thought this might be out of the scope of the REP)

[wjwwood]

That's a good pro to that approach, I'll add that.

[wjwwood]

We also need to determine the type of the REP, I am leaning toward Informational, thoughts?

[tfoote]

Informational sounds right to me.

[tfoote]

I assigned this number 136

[wjwwood]

I have added new documentation:

http://ros.org/wiki/bloom_0.3

Still todo on documentation is regenerating the RST documentation and filling out the FAQ's.

[wjwwood]

@tfoote I think the REP is ready for community review, thoughts?

[wjwwood]

@dirk-thomas Can you vette the examples for correctness?

https://github.com/ros-infrastructure/rep/blob/release_third_party/rep-0136.rst#resources

[wjwwood]

Moved http://ros.org/wiki/bloom_0.3 -> http://ros.org/wiki/bloom

The old documentation there was useless anyways.

[tfoote]

It's ready to email. I think that a few more definitions in the intro and motivation would be valuable for people who are not as familiar with what it means to be 3rdparty vs catkin. And why would we wrap 3rdparty libraries, and what are good candidates vs bad ones.

[jack-oquin]

The clarifications look good to me. Thanks, William.

[vrabaud]

There is still a big thing missing: the addition of a package_name.pc in the pkgconfig folder.
rosbuild packages need pkgconfig and 3rd party packages do not necessarily install that file or might give it a different name (e.g., OpenCV's is opencv.pc, not opencv2.pc, bfl is orocos-bcl.pc and not bfl).

[wjwwood]

@vrabaud, I talked with @tfoote and @dirk-thomas and we agree that creating a <pkg-name>.pc file is not something that should be recommended unless there are legacy rosbuild packages which depend on your third party library as if it were a ROS package.

For example, if you had a rosbuild package called opencv_proc, which had a manifest.xml like this:

<package>
  <description brief="opencv_proc">

     does image processing with opencv

  </description>
  <author>Foo Bar</author>
  <license>BSD</license>
  <url>http://ros.org/wiki/opencv_proc_node</url>
  <depend package="roscpp"/>
  <depend package="sensor_msgs"/>
  <depend package="opencv2"/>

</package>

And a CMakeLists.txt like this:

cmake_minimum_required(VERSION 2.4.6)
include($ENV{ROS_ROOT}/core/rosbuild/rosbuild.cmake)

rosbuild_init()

set(EXECUTABLE_OUTPUT_PATH ${PROJECT_SOURCE_DIR}/bin)
set(LIBRARY_OUTPUT_PATH ${PROJECT_SOURCE_DIR}/lib)

rosbuild_add_executable(opencv_proc_node src/opencv_proc_node.cpp)

And we will assume that the opencv_proc_node.cpp file includes the opencv headers and that it needs to be linked against opencv.

The above is an example of the legacy way to find and use opencv in ROS. This REP encourages users to use third party packages as they were intended to by the third party package developers, not via a custom ROS mechanism. So in this case the correct thing to do is for opencv to not have a opencv2.pc file for rosbuild to find, but instead for the opencv_proc package to actually find_package(OpenCV REQUIRED) like people outside of ROS would.

So in that case the manifest.xml would become:

<package>
  <description brief="opencv_proc">

     does image processing with opencv

  </description>
  <author>Foo Bar</author>
  <license>BSD</license>
  <url>http://ros.org/wiki/opencv_proc_node</url>
  <depend package="roscpp"/>
  <depend package="sensor_msgs"/>

  <rosdep name="opencv2"/>

</package>

And the CMakeLists.txt would become:

cmake_minimum_required(VERSION 2.4.6)
include($ENV{ROS_ROOT}/core/rosbuild/rosbuild.cmake)

rosbuild_init()

set(EXECUTABLE_OUTPUT_PATH ${PROJECT_SOURCE_DIR}/bin)
set(LIBRARY_OUTPUT_PATH ${PROJECT_SOURCE_DIR}/lib)

find_package(OpenCV REQUIRED)

include_directories(${OpenCV_INCLUDE_DIRS})

rosbuild_add_executable(opencv_proc_node src/opencv_proc_node.cpp)
target_link_libraries(opencv_proc_node ${OpenCV_LIBRARIES})

All of that being said, I can put a section in the REP about backwards compatibility stating that to prevent breaking existing rosbuild packages, which previously depended on this third party package, you should ensure that rospack can get build flags about your package by ensuring that .pc is installed.

[vrabaud]

totally agree with that decision, but the REP should at least mention what to do with Groovy: break backward compatibility and not install that *.pc file ? or do it but only for Groovy as the REP will be applied only in Hydro and above.

Also, I read it quickly but it seems the REP does not mention that the third party package has to install a FindFoo.cmake and/or a proper .pc file (or both I don't know) as otherwise, they are unfindable by ROS.

[tfoote]

totally agree with that decision, but the REP should at least mention what
to do with Groovy: break backward compatibility and not install that *.pc
file ? or do it but only for Groovy as the REP will be applied only in
Hydro and above.

This seems like the choice of the package maintainer and not an element for
the REP. The most common solution when converting a dependency is to keep
an empty ROS package which provides the old .pc files and is marked as
deprecated for the next cycle. Thus dependent packages have one cycle to
switch to the new name.

Also, I read it quickly but it seems the REP does not mention that the
third party package has to install a FindFoo.cmake and/or a proper .pc file
(or both I don't know) as otherwise, they are unfindable by ROS.

Although CMake Find rules or .pc files should really be there, I think
that's it's better to say we will use whatever mechanism upstream
recommends. Otherwise we'll do things like patch in an additional .pc or
.cmake file to make it "ROS" compliant. And when in the future they add
their own .cmake file suddenly ours might be incompatible and there may be
no simple transition. We've been bit by this sort of thing in the past
where we effectively fork a 3rdparty library and it causes a lot of pain.

[wjwwood]

On Sun, Mar 3, 2013 at 4:55 PM, Vincent Rabaud notifications@github.comwrote:

totally agree with that decision, but the REP should at least mention what
to do with Groovy: break backward compatibility and not install that *.pc
file ? or do it but only for Groovy as the REP will be applied only in
Hydro and above.

You should install it, as to not break backwards compatibility. New packing
of third party packages, or packages with little to no dependents that
would need to be changed (i.e. foo is only depended on by foo_ros and
everyone else depends on foo_ros) should not install it.

Also, I read it quickly but it seems the REP does not mention that the
third party package has to install a FindFoo.cmake and/or a proper .pc file
(or both I don't know) as otherwise, they are unfindable by ROS.

ROS doesn't find anything, the user finds them, we simply package them and
make them available. So the method of finding a third party package is not
for us to determine, a third party package should instruct its users on how
to be find found, if it provides no mechanism, then the users should use
find_library and find_path to find the library.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/23#issuecomment-14359712
.

[tkruse]

Isn't build_depend on catkin missing here, given our current (strange) requirement that catkin packages need to explicitly state that indeed they depend on catkin for build?

[tkruse]

I mean buildtool_depend

[wjwwood]

This package does not need catkin at build time to build because catkin is not invoked from the CMakeLists.txt, and so the missing build*_depend on catkin is intentional. The run_depend on catkin is to ensure that setup.*sh files are installed when the packages are installed from debs, otherwise you might end up with something like only opencv in /opt/ros/groovy without the setup.*sh files need to add that prefix to the path's.

[wjwwood]

@vrabaud Can you review the Backwards Compatibility section to ensure it addresses the concerns you raised about the pkg-config files for rosbuild?

[vrabaud]

+1 to me. Addresses all the issues I had. Works on the farm too, thx.